[ec2_vpc_vgw] [ec2_vpc_vpn] stabilize modules for PR 35983 (#38666)

* Stabilize ec2_vpc_vgw and ec2_vpc_vpn so tests for ec2_vpc_vpn_facts in PR 35983 can be run in CI

* Add updated placebo recordings

* ensure find_vgw uses the virtual gateway id if available

Add AWSRetry.jittered_backoff to attach_vpn_gateway to deal with errors when attaching a new VPC directly after detaching

Add integrations tests for ec2_vpc_vgw

* Sort VPN Gateways by ID

lib/ansible/module_utils/aws/waiters.py

@@ -115,6 +115,24 @@ ec2_data = {
                 },
             ]
         },
+        "VpnGatewayExists": {
+            "delay": 5,
+            "maxAttempts": 40,
+            "operation": "DescribeVpnGateways",
+            "acceptors": [
+                {
+                    "matcher": "path",
+                    "expected": True,
+                    "argument": "length(VpnGateways[]) > `0`",
+                    "state": "success"
+                },
+                {
+                    "matcher": "error",
+                    "expected": "InvalidVpnGatewayID.NotFound",
+                    "state": "retry"
+                },
+            ]
+        },
     }
 }
 
@@ -197,6 +215,12 @@ waiters_by_name = {
         core_waiter.NormalizedOperationMethod(
             ec2.describe_subnets
         )),
+    ('EC2', 'vpn_gateway_exists'): lambda ec2: core_waiter.Waiter(
+        'vpn_gateway_exists',
+        ec2_model('VpnGatewayExists'),
+        core_waiter.NormalizedOperationMethod(
+            ec2.describe_vpn_gateways
+        )),
     ('WAF', 'change_token_in_sync'): lambda waf: core_waiter.Waiter(
         'change_token_in_sync',
         waf_model('ChangeTokenInSync'),

lib/ansible/modules/cloud/amazon/ec2_vpc_vgw.py

@@ -112,8 +112,9 @@ try:
 except ImportError:
     HAS_BOTO3 = False
 
+from ansible.module_utils.aws.waiters import get_waiter
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.ec2 import HAS_BOTO3, boto3_conn, ec2_argument_spec, get_aws_connection_info
+from ansible.module_utils.ec2 import HAS_BOTO3, boto3_conn, ec2_argument_spec, get_aws_connection_info, AWSRetry
 from ansible.module_utils._text import to_native
 
 
@@ -164,7 +165,7 @@ def attach_vgw(client, module, vpn_gateway_id):
     params['VpcId'] = module.params.get('vpc_id')
 
     try:
-        response = client.attach_vpn_gateway(VpnGatewayId=vpn_gateway_id, VpcId=params['VpcId'])
+        response = AWSRetry.jittered_backoff()(client.attach_vpn_gateway)(VpnGatewayId=vpn_gateway_id, VpcId=params['VpcId'])
     except botocore.exceptions.ClientError as e:
         module.fail_json(msg=to_native(e), exception=traceback.format_exc())
 
@@ -205,6 +206,14 @@ def create_vgw(client, module):
 
     try:
         response = client.create_vpn_gateway(Type=params['Type'])
+        get_waiter(
+            client, 'vpn_gateway_exists'
+        ).wait(
+            VpnGatewayIds=[response['VpnGateway']['VpnGatewayId']]
+        )
+    except botocore.exceptions.WaiterError as e:
+        module.fail_json(msg="Failed to wait for Vpn Gateway {0} to be available".format(response['VpnGateway']['VpnGatewayId']),
+                         exception=traceback.format_exc())
     except botocore.exceptions.ClientError as e:
         module.fail_json(msg=to_native(e), exception=traceback.format_exc())
 
@@ -329,37 +338,21 @@ def find_vpc(client, module):
 
 def find_vgw(client, module, vpn_gateway_id=None):
     params = dict()
-    params['Name'] = module.params.get('name')
-    params['Type'] = module.params.get('type')
-    params['State'] = module.params.get('state')
-
-    if params['State'] == 'present':
-        try:
-            response = client.describe_vpn_gateways(Filters=[
-                {'Name': 'type', 'Values': [params['Type']]},
-                {'Name': 'tag:Name', 'Values': [params['Name']]}
-            ])
-        except botocore.exceptions.ClientError as e:
-            module.fail_json(msg=to_native(e), exception=traceback.format_exc())
-
+    if vpn_gateway_id:
+        params['VpnGatewayIds'] = vpn_gateway_id
     else:
-        if vpn_gateway_id:
-            try:
-                response = client.describe_vpn_gateways(VpnGatewayIds=vpn_gateway_id)
-            except botocore.exceptions.ClientError as e:
-                module.fail_json(msg=to_native(e), exception=traceback.format_exc())
+        params['Filters'] = [
+            {'Name': 'type', 'Values': [module.params.get('type')]},
+            {'Name': 'tag:Name', 'Values': [module.params.get('name')]},
+        ]
+        if module.params.get('state') == 'present':
+            params['Filters'].append({'Name': 'state', 'Values': ['pending', 'available']})
+    try:
+        response = client.describe_vpn_gateways(**params)
+    except botocore.exceptions.ClientError as e:
+        module.fail_json(msg=to_native(e), exception=traceback.format_exc())
 
-        else:
-            try:
-                response = client.describe_vpn_gateways(Filters=[
-                    {'Name': 'type', 'Values': [params['Type']]},
-                    {'Name': 'tag:Name', 'Values': [params['Name']]}
-                ])
-            except botocore.exceptions.ClientError as e:
-                module.fail_json(msg=to_native(e), exception=traceback.format_exc())
-
-    result = response['VpnGateways']
-    return result
+    return sorted(response['VpnGateways'], key=lambda k: k['VpnGatewayId'])
 
 
 def ensure_vgw_present(client, module):
@@ -376,40 +369,33 @@ def ensure_vgw_present(client, module):
     params['Tags'] = module.params.get('tags')
     params['VpnGatewayIds'] = module.params.get('vpn_gateway_id')
 
-    # Check that a name argument has been supplied.
-    if not module.params.get('name'):
-        module.fail_json(msg='A name is required when a status of \'present\' is suppled')
+    # check that the vpc_id exists. If not, an exception is thrown
+    if params['VpcId']:
+        vpc = find_vpc(client, module)
 
     # check if a gateway matching our module args already exists
     existing_vgw = find_vgw(client, module)
 
-    if existing_vgw != [] and existing_vgw[0]['State'] != 'deleted':
+    if existing_vgw != []:
         vpn_gateway_id = existing_vgw[0]['VpnGatewayId']
         vgw, changed = check_tags(client, module, existing_vgw, vpn_gateway_id)
 
         # if a vpc_id was provided, check if it exists and if it's attached
         if params['VpcId']:
 
-            # check that the vpc_id exists. If not, an exception is thrown
-            vpc = find_vpc(client, module)
             current_vpc_attachments = existing_vgw[0]['VpcAttachments']
 
             if current_vpc_attachments != [] and current_vpc_attachments[0]['State'] == 'attached':
-                if current_vpc_attachments[0]['VpcId'] == params['VpcId'] and current_vpc_attachments[0]['State'] == 'attached':
-                    changed = False
-                else:
-
+                if current_vpc_attachments[0]['VpcId'] != params['VpcId'] or current_vpc_attachments[0]['State'] != 'attached':
                     # detach the existing vpc from the virtual gateway
                     vpc_to_detach = current_vpc_attachments[0]['VpcId']
                     detach_vgw(client, module, vpn_gateway_id, vpc_to_detach)
                     time.sleep(5)
                     attached_vgw = attach_vgw(client, module, vpn_gateway_id)
-                    vgw = find_vgw(client, module, [vpn_gateway_id])
                     changed = True
             else:
                 # attach the vgw to the supplied vpc
                 attached_vgw = attach_vgw(client, module, vpn_gateway_id)
-                vgw = find_vgw(client, module, [vpn_gateway_id])
                 changed = True
 
         # if params['VpcId'] is not provided, check the vgw is attached to a vpc. if so, detach it.
@@ -423,8 +409,6 @@ def ensure_vgw_present(client, module):
                     detach_vgw(client, module, vpn_gateway_id, vpc_to_detach)
                     changed = True
 
-                vgw = find_vgw(client, module, [vpn_gateway_id])
-
     else:
         # create a new vgw
         new_vgw = create_vgw(client, module)
@@ -434,15 +418,13 @@ def ensure_vgw_present(client, module):
         # tag the new virtual gateway
         create_tags(client, module, vpn_gateway_id)
 
-        # return current state of the vgw
-        vgw = find_vgw(client, module, [vpn_gateway_id])
-
         # if a vpc-id was supplied, attempt to attach it to the vgw
         if params['VpcId']:
             attached_vgw = attach_vgw(client, module, vpn_gateway_id)
             changed = True
-            vgw = find_vgw(client, module, [vpn_gateway_id])
 
+    # return current state of the vgw
+    vgw = find_vgw(client, module, [vpn_gateway_id])
     result = get_vgw_info(vgw)
     return changed, result
 
@@ -549,7 +531,8 @@ def main():
         tags=dict(default=None, required=False, type='dict', aliases=['resource_tags']),
     )
     )
-    module = AnsibleModule(argument_spec=argument_spec)
+    module = AnsibleModule(argument_spec=argument_spec,
+                           required_if=[['state', 'present', ['name']]])
 
     if not HAS_BOTO3:
         module.fail_json(msg='json and boto3 is required.')

lib/ansible/modules/cloud/amazon/ec2_vpc_vpn.py

@@ -266,24 +266,25 @@ vpn_connection_id:
     vpn_connection_id: vpn-781e0e19
 """
 
-from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.aws.core import AnsibleAWSModule
 from ansible.module_utils._text import to_text
-from ansible.module_utils import ec2 as ec2_utils
-import traceback
+from ansible.module_utils.ec2 import (
+    camel_dict_to_snake_dict,
+    boto3_tag_list_to_ansible_dict,
+    compare_aws_tags,
+    ansible_dict_to_boto3_tag_list,
+)
 
 try:
-    import boto3
-    import botocore
+    from botocore.exceptions import BotoCoreError, ClientError, WaiterError
 except ImportError:
-    # will be caught in main
-    pass
+    pass  # Handled by AnsibleAWSModule
 
 
 class VPNConnectionException(Exception):
-    def __init__(self, msg, exception=None, response=None):
+    def __init__(self, msg, exception=None):
         self.msg = msg
-        self.error_traceback = exception
-        self.response = response
+        self.exception = exception
 
 
 def find_connection(connection, module_params, vpn_connection_id=None):
@@ -309,16 +310,13 @@ def find_connection(connection, module_params, vpn_connection_id=None):
     # see if there is a unique matching connection
     try:
         if vpn_connection_id:
-            existing_conn = connection.describe_vpn_connections(DryRun=False,
-                                                                VpnConnectionIds=vpn_connection_id,
+            existing_conn = connection.describe_vpn_connections(VpnConnectionIds=vpn_connection_id,
                                                                 Filters=formatted_filter)
         else:
-            existing_conn = connection.describe_vpn_connections(DryRun=False,
-                                                                Filters=formatted_filter)
-    except botocore.exceptions.ClientError as e:
+            existing_conn = connection.describe_vpn_connections(Filters=formatted_filter)
+    except (BotoCoreError, ClientError) as e:
         raise VPNConnectionException(msg="Failed while describing VPN connection.",
-                                     exception=traceback.format_exc(),
-                                     **ec2_utils.camel_dict_to_snake_dict(e.response))
+                                     exception=e)
 
     return find_connection_response(connections=existing_conn)
 
@@ -328,10 +326,9 @@ def add_routes(connection, vpn_connection_id, routes_to_add):
         try:
             connection.create_vpn_connection_route(VpnConnectionId=vpn_connection_id,
                                                    DestinationCidrBlock=route)
-        except botocore.exceptions.ClientError as e:
+        except (BotoCoreError, ClientError) as e:
             raise VPNConnectionException(msg="Failed while adding route {0} to the VPN connection {1}.".format(route, vpn_connection_id),
-                                         exception=traceback.format_exc(),
-                                         response=e.response)
+                                         exception=e)
 
 
 def remove_routes(connection, vpn_connection_id, routes_to_remove):
@@ -339,10 +336,9 @@ def remove_routes(connection, vpn_connection_id, routes_to_remove):
         try:
             connection.delete_vpn_connection_route(VpnConnectionId=vpn_connection_id,
                                                    DestinationCidrBlock=route)
-        except botocore.exceptions.ClientError as e:
+        except (BotoCoreError, ClientError) as e:
             raise VPNConnectionException(msg="Failed to remove route {0} from the VPN connection {1}.".format(route, vpn_connection_id),
-                                         exception=traceback.format_exc(),
-                                         response=e.response)
+                                         exception=e)
 
 
 def create_filter(module_params, provided_filters):
@@ -456,15 +452,17 @@ def create_connection(connection, customer_gateway_id, static_only, vpn_gateway_
         raise VPNConnectionException(msg="No matching connection was found. To create a new connection you must provide "
                                      "both vpn_gateway_id and customer_gateway_id.")
     try:
-        vpn = connection.create_vpn_connection(DryRun=False,
-                                               Type=connection_type,
+        vpn = connection.create_vpn_connection(Type=connection_type,
                                                CustomerGatewayId=customer_gateway_id,
                                                VpnGatewayId=vpn_gateway_id,
                                                Options=options)
-    except botocore.exceptions.ClientError as e:
-        raise VPNConnectionException(msg="Failed to create VPN connection: {0}".format(e.message),
-                                     exception=traceback.format_exc(),
-                                     response=e.response)
+        connection.get_waiter('vpn_connection_available').wait(VpnConnectionIds=[vpn['VpnConnection']['VpnConnectionId']])
+    except WaiterError as e:
+        raise VPNConnectionException(msg="Failed to wait for VPN connection {0} to be available".format(vpn['VpnConnection']['VpnConnectionId']),
+                                     exception=e)
+    except (BotoCoreError, ClientError) as e:
+        raise VPNConnectionException(msg="Failed to create VPN connection",
+                                     exception=e)
 
     return vpn['VpnConnection']
 
@@ -472,36 +470,34 @@ def create_connection(connection, customer_gateway_id, static_only, vpn_gateway_
 def delete_connection(connection, vpn_connection_id):
     """ Deletes a VPN connection """
     try:
-        connection.delete_vpn_connection(DryRun=False,
-                                         VpnConnectionId=vpn_connection_id)
-    except botocore.exceptions.ClientError as e:
-        raise VPNConnectionException(msg="Failed to delete the VPN connection: {0}".format(e.message),
-                                     exception=traceback.format_exc(),
-                                     response=e.response)
+        connection.delete_vpn_connection(VpnConnectionId=vpn_connection_id)
+        connection.get_waiter('vpn_connection_deleted').wait(VpnConnectionIds=[vpn_connection_id])
+    except WaiterError as e:
+        raise VPNConnectionException(msg="Failed to wait for VPN connection {0} to be removed".format(vpn_connection_id),
+                                     exception=e)
+    except (BotoCoreError, ClientError) as e:
+        raise VPNConnectionException(msg="Failed to delete the VPN connection: {0}".format(vpn_connection_id),
+                                     exception=e)
 
 
 def add_tags(connection, vpn_connection_id, add):
     try:
-        connection.create_tags(DryRun=False,
-                               Resources=[vpn_connection_id],
+        connection.create_tags(Resources=[vpn_connection_id],
                                Tags=add)
-    except botocore.exceptions.ClientError as e:
+    except (BotoCoreError, ClientError) as e:
         raise VPNConnectionException(msg="Failed to add the tags: {0}.".format(add),
-                                     exception=traceback.format_exc(),
-                                     response=e.response)
+                                     exception=e)
 
 
 def remove_tags(connection, vpn_connection_id, remove):
     # format tags since they are a list in the format ['tag1', 'tag2', 'tag3']
     key_dict_list = [{'Key': tag} for tag in remove]
     try:
-        connection.delete_tags(DryRun=False,
-                               Resources=[vpn_connection_id],
+        connection.delete_tags(Resources=[vpn_connection_id],
                                Tags=key_dict_list)
-    except botocore.exceptions.ClientError as e:
+    except (BotoCoreError, ClientError) as e:
         raise VPNConnectionException(msg="Failed to remove the tags: {0}.".format(remove),
-                                     exception=traceback.format_exc(),
-                                     response=e.response)
+                                     exception=e)
 
 
 def check_for_update(connection, module_params, vpn_connection_id):
@@ -512,7 +508,7 @@ def check_for_update(connection, module_params, vpn_connection_id):
     purge_routes = module_params.get('purge_routes')
 
     vpn_connection = find_connection(connection, module_params, vpn_connection_id=vpn_connection_id)
-    current_attrs = ec2_utils.camel_dict_to_snake_dict(vpn_connection)
+    current_attrs = camel_dict_to_snake_dict(vpn_connection)
 
     # Initialize changes dict
     changes = {'tags_to_add': [],
@@ -521,14 +517,9 @@ def check_for_update(connection, module_params, vpn_connection_id):
                'routes_to_remove': []}
 
     # Get changes to tags
-    if 'tags' in current_attrs:
-        current_tags = ec2_utils.boto3_tag_list_to_ansible_dict(current_attrs['tags'], u'key', u'value')
-        tags_to_add, changes['tags_to_remove'] = ec2_utils.compare_aws_tags(current_tags, tags, purge_tags)
-        changes['tags_to_add'] = ec2_utils.ansible_dict_to_boto3_tag_list(tags_to_add)
-    elif tags:
-        current_tags = {}
-        tags_to_add, changes['tags_to_remove'] = ec2_utils.compare_aws_tags(current_tags, tags, purge_tags)
-        changes['tags_to_add'] = ec2_utils.ansible_dict_to_boto3_tag_list(tags_to_add)
+    current_tags = boto3_tag_list_to_ansible_dict(current_attrs.get('tags', []), u'key', u'value')
+    tags_to_add, changes['tags_to_remove'] = compare_aws_tags(current_tags, tags, purge_tags)
+    changes['tags_to_add'] = ansible_dict_to_boto3_tag_list(tags_to_add)
     # Get changes to routes
     if 'Routes' in vpn_connection:
         current_routes = [route['DestinationCidrBlock'] for route in vpn_connection['Routes']]
@@ -603,7 +594,7 @@ def get_check_mode_results(connection, module_params, vpn_connection_id=None, cu
     # get combined current tags and tags to set
     present_tags = module_params.get('tags')
     if current_state and 'Tags' in current_state:
-        current_tags = ec2_utils.boto3_tag_list_to_ansible_dict(current_state['Tags'])
+        current_tags = boto3_tag_list_to_ansible_dict(current_state['Tags'])
         if module_params.get('purge_tags'):
             if current_tags != present_tags:
                 changed = True
@@ -680,7 +671,7 @@ def ensure_present(connection, module_params, check_mode=False):
     if vpn_connection:
         vpn_connection = find_connection(connection, module_params, vpn_connection['VpnConnectionId'])
         if 'Tags' in vpn_connection:
-            vpn_connection['Tags'] = ec2_utils.boto3_tag_list_to_ansible_dict(vpn_connection['Tags'])
+            vpn_connection['Tags'] = boto3_tag_list_to_ansible_dict(vpn_connection['Tags'])
 
     return changed, vpn_connection
 
@@ -702,38 +693,23 @@ def ensure_absent(connection, module_params, check_mode=False):
 
 
 def main():
-    argument_spec = ec2_utils.ec2_argument_spec()
-    argument_spec.update(
-        dict(
-            state=dict(type='str', default='present', choices=['present', 'absent']),
-            filters=dict(type='dict', default={}),
-            vpn_gateway_id=dict(type='str'),
-            tags=dict(default={}, type='dict'),
-            connection_type=dict(default='ipsec.1', type='str'),
-            tunnel_options=dict(type='list', default=[]),
-            static_only=dict(default=False, type='bool'),
-            customer_gateway_id=dict(type='str'),
-            vpn_connection_id=dict(type='str'),
-            purge_tags=dict(type='bool', default=False),
-            routes=dict(type='list', default=[]),
-            purge_routes=dict(type='bool', default=False),
-        )
+    argument_spec = dict(
+        state=dict(type='str', default='present', choices=['present', 'absent']),
+        filters=dict(type='dict', default={}),
+        vpn_gateway_id=dict(type='str'),
+        tags=dict(default={}, type='dict'),
+        connection_type=dict(default='ipsec.1', type='str'),
+        tunnel_options=dict(type='list', default=[]),
+        static_only=dict(default=False, type='bool'),
+        customer_gateway_id=dict(type='str'),
+        vpn_connection_id=dict(type='str'),
+        purge_tags=dict(type='bool', default=False),
+        routes=dict(type='list', default=[]),
+        purge_routes=dict(type='bool', default=False),
     )
-    module = AnsibleModule(argument_spec=argument_spec,
-                           supports_check_mode=True)
-
-    if not ec2_utils.HAS_BOTO3:
-        module.fail_json(msg='boto3 required for this module')
-
-    # Retrieve any AWS settings from the environment.
-    region, ec2_url, aws_connect_kwargs = ec2_utils.get_aws_connection_info(module, boto3=True)
-
-    if not region:
-        module.fail_json(msg="Either region or AWS_REGION or EC2_REGION environment variable or boto config aws_region or ec2_region must be set.")
-
-    connection = ec2_utils.boto3_conn(module, conn_type='client',
-                                      resource='ec2', region=region,
-                                      endpoint=ec2_url, **aws_connect_kwargs)
+    module = AnsibleAWSModule(argument_spec=argument_spec,
+                              supports_check_mode=True)
+    connection = module.client('ec2')
 
     state = module.params.get('state')
     parameters = dict(module.params)
@@ -744,16 +720,12 @@ def main():
         elif state == 'absent':
             changed, response = ensure_absent(connection, parameters, module.check_mode)
     except VPNConnectionException as e:
-        if e.response and e.error_traceback:
-            module.fail_json(msg=e.msg, exception=e.error_traceback, **ec2_utils.camel_dict_to_snake_dict(e.response))
-        elif e.error_traceback:
-            module.fail_json(msg=e.msg, exception=e.error_traceback)
+        if e.exception:
+            module.fail_json_aws(e.exception, msg=e.msg)
         else:
             module.fail_json(msg=e.msg)
 
-    facts_result = dict(changed=changed, **ec2_utils.camel_dict_to_snake_dict(response))
-
-    module.exit_json(**facts_result)
+    module.exit_json(changed=changed, **camel_dict_to_snake_dict(response))
 
 if __name__ == '__main__':
     main()