From dfb20a3da79dbf5838efbbef2a829aa92e367151 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Wed, 10 Mar 2021 11:42:32 +0100 Subject: [PATCH] Cleanup ec2_vpc_route_table(_info) (#442) * Move ec2_vpc_route_table tests into group 4 * ec2_vpc_route_table - Use retries more consistently. * ec2_vpc_route_table_info - boto3 migration * changelog * Add return value documentation * catch WaiterError for cleaner error messages --- .../442-ec2_vpc_route_table-stability.yml | 4 + plugins/modules/ec2_vpc_route_table.py | 103 +++++--- plugins/modules/ec2_vpc_route_table_info.py | 243 ++++++++++++++---- .../targets/ec2_vpc_route_table/aliases | 8 +- 4 files changed, 270 insertions(+), 88 deletions(-) create mode 100644 changelogs/fragments/442-ec2_vpc_route_table-stability.yml diff --git a/changelogs/fragments/442-ec2_vpc_route_table-stability.yml b/changelogs/fragments/442-ec2_vpc_route_table-stability.yml new file mode 100644 index 00000000000..a91dbb6a378 --- /dev/null +++ b/changelogs/fragments/442-ec2_vpc_route_table-stability.yml @@ -0,0 +1,4 @@ +minor_changes: +- ec2_vpc_route_table_info - migrate to boto3 (https://github.com/ansible-collections/community.aws/pull/442). +- ec2_vpc_route_table - add AWSRetry decorators to improve reliability (https://github.com/ansible-collections/community.aws/pull/442). +- ec2_vpc_route_table - add boto3 pagination for some searches (https://github.com/ansible-collections/community.aws/pull/442). diff --git a/plugins/modules/ec2_vpc_route_table.py b/plugins/modules/ec2_vpc_route_table.py index cdab10a9d79..1ef10e89ceb 100644 --- a/plugins/modules/ec2_vpc_route_table.py +++ b/plugins/modules/ec2_vpc_route_table.py @@ -250,9 +250,33 @@ ROUTE_TABLE_RE = re.compile(r'^rtb-[A-z0-9]+$') -@AWSRetry.exponential_backoff() +@AWSRetry.jittered_backoff() def describe_subnets_with_backoff(connection, **params): - return connection.describe_subnets(**params)['Subnets'] + paginator = connection.get_paginator('describe_subnets') + return paginator.paginate(**params).build_full_result()['Subnets'] + + +@AWSRetry.jittered_backoff() +def describe_igws_with_backoff(connection, **params): + paginator = connection.get_paginator('describe_internet_gateways') + return paginator.paginate(**params).build_full_result()['InternetGateways'] + + +@AWSRetry.jittered_backoff() +def describe_tags_with_backoff(connection, resource_id): + filters = ansible_dict_to_boto3_filter_list({'resource-id': resource_id}) + paginator = connection.get_paginator('describe_tags') + tags = paginator.paginate(Filters=filters).build_full_result()['Tags'] + return boto3_tag_list_to_ansible_dict(tags) + + +@AWSRetry.jittered_backoff() +def describe_route_tables_with_backoff(connection, **params): + try: + paginator = connection.get_paginator('describe_route_tables') + return paginator.paginate(**params).build_full_result()['RouteTables'] + except is_boto3_error_code('InvalidRouteTableID.NotFound'): + return None def find_subnets(connection, module, vpc_id, identified_subnets): @@ -314,7 +338,7 @@ def find_igw(connection, module, vpc_id): """ filters = ansible_dict_to_boto3_filter_list({'attachment.vpc-id': vpc_id}) try: - igw = connection.describe_internet_gateways(Filters=filters)['InternetGateways'] + igw = describe_igws_with_backoff(connection, Filters=filters) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg='No IGW found for VPC {0}'.format(vpc_id)) if len(igw) == 1: @@ -325,14 +349,6 @@ def find_igw(connection, module, vpc_id): module.fail_json(msg='Multiple IGWs found for VPC {0}'.format(vpc_id)) -@AWSRetry.exponential_backoff() -def describe_tags_with_backoff(connection, resource_id): - filters = ansible_dict_to_boto3_filter_list({'resource-id': resource_id}) - paginator = connection.get_paginator('describe_tags') - tags = paginator.paginate(Filters=filters).build_full_result()['Tags'] - return boto3_tag_list_to_ansible_dict(tags) - - def tags_match(match_tags, candidate_tags): return all((k in candidate_tags and candidate_tags[k] == v for k, v in match_tags.items())) @@ -355,12 +371,18 @@ def ensure_tags(connection=None, module=None, resource_id=None, tags=None, purge if to_delete: try: - connection.delete_tags(Resources=[resource_id], Tags=[{'Key': k} for k in to_delete]) + connection.delete_tags( + aws_retry=True, + Resources=[resource_id], + Tags=[{'Key': k} for k in to_delete]) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Couldn't delete tags") if to_add: try: - connection.create_tags(Resources=[resource_id], Tags=ansible_dict_to_boto3_tag_list(to_add)) + connection.create_tags( + aws_retry=True, + Resources=[resource_id], + Tags=ansible_dict_to_boto3_tag_list(to_add)) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Couldn't create tags") @@ -371,14 +393,6 @@ def ensure_tags(connection=None, module=None, resource_id=None, tags=None, purge return {'changed': True, 'tags': latest_tags} -@AWSRetry.exponential_backoff() -def describe_route_tables_with_backoff(connection, **params): - try: - return connection.describe_route_tables(**params)['RouteTables'] - except is_boto3_error_code('InvalidRouteTableID.NotFound'): - return None - - def get_route_table_by_id(connection, module, route_table_id): route_table = None @@ -474,21 +488,28 @@ def ensure_routes(connection=None, module=None, route_table=None, route_specs=No if changed and not check_mode: for route in routes_to_delete: try: - connection.delete_route(RouteTableId=route_table['RouteTableId'], DestinationCidrBlock=route['DestinationCidrBlock']) + connection.delete_route( + aws_retry=True, + RouteTableId=route_table['RouteTableId'], + DestinationCidrBlock=route['DestinationCidrBlock']) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Couldn't delete route") for route_spec in route_specs_to_recreate: try: - connection.replace_route(RouteTableId=route_table['RouteTableId'], - **route_spec) + connection.replace_route( + aws_retry=True, + RouteTableId=route_table['RouteTableId'], + **route_spec) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Couldn't recreate route") for route_spec in route_specs_to_create: try: - connection.create_route(RouteTableId=route_table['RouteTableId'], - **route_spec) + connection.create_route( + aws_retry=True, + RouteTableId=route_table['RouteTableId'], + **route_spec) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Couldn't create route") @@ -515,12 +536,15 @@ def ensure_subnet_association(connection=None, module=None, vpc_id=None, route_t if check_mode: return {'changed': True} try: - connection.disassociate_route_table(AssociationId=a['RouteTableAssociationId']) + connection.disassociate_route_table( + aws_retry=True, AssociationId=a['RouteTableAssociationId']) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Couldn't disassociate subnet from route table") try: - association_id = connection.associate_route_table(RouteTableId=route_table_id, SubnetId=subnet_id) + association_id = connection.associate_route_table(aws_retry=True, + RouteTableId=route_table_id, + SubnetId=subnet_id) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Couldn't associate subnet with route table") return {'changed': True, 'association_id': association_id} @@ -532,8 +556,10 @@ def ensure_subnet_associations(connection=None, module=None, route_table=None, s new_association_ids = [] changed = False for subnet in subnets: - result = ensure_subnet_association(connection=connection, module=module, vpc_id=route_table['VpcId'], - route_table_id=route_table['RouteTableId'], subnet_id=subnet['SubnetId'], check_mode=check_mode) + result = ensure_subnet_association( + connection=connection, module=module, vpc_id=route_table['VpcId'], + route_table_id=route_table['RouteTableId'], subnet_id=subnet['SubnetId'], + check_mode=check_mode) changed = changed or result['changed'] if changed and check_mode: return {'changed': True} @@ -547,7 +573,7 @@ def ensure_subnet_associations(connection=None, module=None, route_table=None, s changed = True if not check_mode: try: - connection.disassociate_route_table(AssociationId=a_id) + connection.disassociate_route_table(aws_retry=True, AssociationId=a_id) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Couldn't disassociate subnet from route table") @@ -564,8 +590,10 @@ def ensure_propagation(connection=None, module=None, route_table=None, propagati if not check_mode: for vgw_id in to_add: try: - connection.enable_vgw_route_propagation(RouteTableId=route_table['RouteTableId'], - GatewayId=vgw_id) + connection.enable_vgw_route_propagation( + aws_retry=True, + RouteTableId=route_table['RouteTableId'], + GatewayId=vgw_id) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Couldn't enable route propagation") @@ -596,7 +624,7 @@ def ensure_route_table_absent(connection, module): ensure_subnet_associations(connection=connection, module=module, route_table=route_table, subnets=[], check_mode=False, purge_subnets=purge_subnets) try: - connection.delete_route_table(RouteTableId=route_table['RouteTableId']) + connection.delete_route_table(aws_retry=True, RouteTableId=route_table['RouteTableId']) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Error deleting route table") @@ -665,13 +693,15 @@ def ensure_route_table_present(connection, module): changed = True if not module.check_mode: try: - route_table = connection.create_route_table(VpcId=vpc_id)['RouteTable'] + route_table = connection.create_route_table(aws_retry=True, VpcId=vpc_id)['RouteTable'] # try to wait for route table to be present before moving on get_waiter( connection, 'route_table_exists' ).wait( RouteTableIds=[route_table['RouteTableId']], ) + except botocore.exceptions.WaiterError as e: + module.fail_json_aws(e, msg='Timeout waiting for route table creation') except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Error creating route table") else: @@ -730,7 +760,8 @@ def main(): ['state', 'present', ['vpc_id']]], supports_check_mode=True) - connection = module.client('ec2') + retry_decorator = AWSRetry.jittered_backoff(retries=10) + connection = module.client('ec2', retry_decorator=retry_decorator) state = module.params.get('state') diff --git a/plugins/modules/ec2_vpc_route_table_info.py b/plugins/modules/ec2_vpc_route_table_info.py index 9ff9959c271..2e4dd384930 100644 --- a/plugins/modules/ec2_vpc_route_table_info.py +++ b/plugins/modules/ec2_vpc_route_table_info.py @@ -6,7 +6,7 @@ __metaclass__ = type -DOCUMENTATION = ''' +DOCUMENTATION = r''' --- module: ec2_vpc_route_table_info version_added: 1.0.0 @@ -14,7 +14,9 @@ description: - Gather information about ec2 VPC route tables in AWS - This module was called C(ec2_vpc_route_table_facts) before Ansible 2.9. The usage did not change. -author: "Rob White (@wimnat)" +author: +- "Rob White (@wimnat)" +- "Mark Chappell (@tremble)" options: filters: description: @@ -27,7 +29,7 @@ ''' -EXAMPLES = ''' +EXAMPLES = r''' # Note: These examples do not set authentication details, see the AWS Guide for details. - name: Gather information about all VPC route tables @@ -47,56 +49,218 @@ community.aws.ec2_vpc_route_table_info: filters: vpc-id: vpc-abcdef00 +''' +RETURN = r''' +route_tables: + description: + - A list of dictionarys describing route tables + - See also U(https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/ec2.html#EC2.Client.describe_route_tables) + returned: always + type: complex + contains: + associations: + description: List of subnets associated with the route table + returned: always + type: complex + contains: + main: + description: Whether this is the main route table + returned: always + type: bool + sample: false + id: + description: ID of association between route table and subnet + returned: always + type: str + sample: rtbassoc-ab47cfc3 + route_table_association_id: + description: ID of association between route table and subnet + returned: always + type: str + sample: rtbassoc-ab47cfc3 + route_table_id: + description: ID of the route table + returned: always + type: str + sample: rtb-bf779ed7 + subnet_id: + description: ID of the subnet + returned: always + type: str + sample: subnet-82055af9 + association_state: + description: The state of the association + returned: always + type: complex + contains: + state: + description: The state of the association + returned: always + type: str + sample: associated + state_message: + description: Additional information about the state of the association + returned: when available + type: str + sample: 'Creating association' + id: + description: ID of the route table (same as route_table_id for backwards compatibility) + returned: always + type: str + sample: rtb-bf779ed7 + owner_id: + description: ID of the account which owns the route table + returned: always + type: str + sample: '012345678912' + propagating_vgws: + description: List of Virtual Private Gateways propagating routes + returned: always + type: list + sample: [] + route_table_id: + description: ID of the route table + returned: always + type: str + sample: rtb-bf779ed7 + routes: + description: List of routes in the route table + returned: always + type: complex + contains: + destination_cidr_block: + description: CIDR block of destination + returned: always + type: str + sample: 10.228.228.0/22 + gateway_id: + description: ID of the gateway + returned: when gateway is local or internet gateway + type: str + sample: local + instance_id: + description: + - ID of a NAT instance. + - Empty unless the route is via an EC2 instance + returned: always + type: str + sample: i-abcd123456789 + instance_owner_id: + description: + - AWS account owning the NAT instance + - Empty unless the route is via an EC2 instance + returned: always + type: str + sample: 123456789012 + network_interface_id: + description: + - The ID of the network interface + - Empty unless the route is via an EC2 instance + returned: always + type: str + sample: 123456789012 + nat_gateway_id: + description: ID of the NAT gateway + returned: when the route is via a NAT gateway + type: str + sample: local + origin: + description: mechanism through which the route is in the table + returned: always + type: str + sample: CreateRouteTable + state: + description: state of the route + returned: always + type: str + sample: active + tags: + description: Tags applied to the route table + returned: always + type: dict + sample: + Name: Public route table + Public: 'true' + vpc_id: + description: ID for the VPC in which the route lives + returned: always + type: str + sample: vpc-6e2d2407 ''' try: - import boto.vpc - from boto.exception import BotoServerError + import botocore except ImportError: - pass # Handled by HAS_BOTO + pass # Handled by AnsibleAWSModule + +from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AnsibleAWSError -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import connect_to_aws -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import get_aws_connection_info -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import HAS_BOTO +from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AWSRetry +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ansible_dict_to_boto3_filter_list +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto3_tag_list_to_ansible_dict + + +@AWSRetry.jittered_backoff() +def describe_route_tables_with_backoff(connection, **params): + try: + paginator = connection.get_paginator('describe_route_tables') + return paginator.paginate(**params).build_full_result() + except is_boto3_error_code('InvalidRouteTableID.NotFound'): + return None + +def normalize_route(route): + # Historically these were all there, but set to null when empty' + for legacy_key in ['DestinationCidrBlock', 'GatewayId', 'InstanceId', + 'Origin', 'State', 'NetworkInterfaceId']: + if legacy_key not in route: + route[legacy_key] = None + route['InterfaceId'] = route['NetworkInterfaceId'] + return route -def get_route_table_info(route_table): - # Add any routes to array - routes = [] - associations = [] - for route in route_table.routes: - routes.append(route.__dict__) - for association in route_table.associations: - associations.append(association.__dict__) +def normalize_association(assoc): + # Name change between boto v2 and boto v3, return both + assoc['Id'] = assoc['RouteTableAssociationId'] + return assoc - route_table_info = {'id': route_table.id, - 'routes': routes, - 'associations': associations, - 'tags': route_table.tags, - 'vpc_id': route_table.vpc_id - } - return route_table_info +def normalize_route_table(table): + table['tags'] = boto3_tag_list_to_ansible_dict(table['Tags']) + table['Associations'] = [normalize_association(assoc) for assoc in table['Associations']] + table['Routes'] = [normalize_route(route) for route in table['Routes']] + table['Id'] = table['RouteTableId'] + del table['Tags'] + return camel_dict_to_snake_dict(table, ignore_list=['tags']) + + +def normalize_results(results): + """ + We used to be a boto v2 module, make sure that the old return values are + maintained and the shape of the return values are what people expect + """ + + routes = [normalize_route_table(route) for route in results['RouteTables']] + del results['RouteTables'] + results = camel_dict_to_snake_dict(results) + results['route_tables'] = routes + return results def list_ec2_vpc_route_tables(connection, module): - filters = module.params.get("filters") - route_table_dict_array = [] + filters = ansible_dict_to_boto3_filter_list(module.params.get("filters")) try: - all_route_tables = connection.get_all_route_tables(filters=filters) - except BotoServerError as e: + results = describe_route_tables_with_backoff(connection, Filters=filters) + except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: module.fail_json_aws(e, msg="Failed to get route tables") - for route_table in all_route_tables: - route_table_dict_array.append(get_route_table_info(route_table)) - - module.exit_json(route_tables=route_table_dict_array) + results = normalize_results(results) + module.exit_json(changed=False, **results) def main(): @@ -110,18 +274,7 @@ def main(): module.deprecate("The 'ec2_vpc_route_table_facts' module has been renamed to 'ec2_vpc_route_table_info'", date='2021-12-01', collection_name='community.aws') - if not HAS_BOTO: - module.fail_json(msg='boto required for this module') - - region, ec2_url, aws_connect_params = get_aws_connection_info(module) - - if region: - try: - connection = connect_to_aws(boto.vpc, region, **aws_connect_params) - except (boto.exception.NoAuthHandlerFound, AnsibleAWSError) as e: - module.fail_json(msg=str(e)) - else: - module.fail_json(msg="region must be specified") + connection = module.client('ec2', retry_decorator=AWSRetry.jittered_backoff(retries=10)) list_ec2_vpc_route_tables(connection, module) diff --git a/tests/integration/targets/ec2_vpc_route_table/aliases b/tests/integration/targets/ec2_vpc_route_table/aliases index a7a2e37f611..eb74d33251c 100644 --- a/tests/integration/targets/ec2_vpc_route_table/aliases +++ b/tests/integration/targets/ec2_vpc_route_table/aliases @@ -1,10 +1,4 @@ -# reason: boto2 -# ec2_vpc_route_table_info is currently boto v2 based which means it can't handle API rate limiting -# https://github.com/ansible-collections/community.aws/issues/277 with AWSRetry -# would likely fix the issue. -unstable - cloud/aws -shippable/aws/group2 +shippable/aws/group4 ec2_vpc_route_table_info