Add retries to most ec2_ami AWS calls (#195)

* ec2_ami - Flag device_name as required, it's needed the apis * Add aws_retry decorator to most ec2_ami calls * Add Retries to ec2_ami_info * ec2_ami - Use waiter with retry * changelog * ec2_ami - mark integration tests stable...
ansible-collections · Nov 16, 2020 · 69e25d3 · 69e25d3
1 parent 0c13bdb
commit 69e25d3
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 28 deletions.
diff --git a/changelogs/fragments/195-ec2_ami-retries.yml b/changelogs/fragments/195-ec2_ami-retries.yml
@@ -0,0 +1,3 @@
+minor_changes:
+- ec2_ami - Add retries for ratelimiting related errors (https://github.com/ansible-collections/amazon.aws/pull/195).
+- ec2_ami_info - Add retries for ratelimiting related errors (https://github.com/ansible-collections/amazon.aws/pull/195).
diff --git a/plugins/module_utils/waiters.py b/plugins/module_utils/waiters.py
@@ -17,6 +17,25 @@
 ec2_data = {
     "version": 2,
     "waiters": {
+        "ImageAvailable": {
+            "operation": "DescribeImages",
+            "maxAttempts": 80,
+            "delay": 15,
+            "acceptors": [
+                {
+                    "state": "success",
+                    "matcher": "pathAll",
+                    "argument": "Images[].State",
+                    "expected": "available"
+                },
+                {
+                    "state": "failure",
+                    "matcher": "pathAny",
+                    "argument": "Images[].State",
+                    "expected": "failed"
+                }
+            ]
+        },
         "InternetGatewayExists": {
             "delay": 5,
             "maxAttempts": 40,
@@ -339,6 +358,12 @@ def rds_model(name):
 
 
 waiters_by_name = {
+    ('EC2', 'image_available'): lambda ec2: core_waiter.Waiter(
+        'image_available',
+        ec2_model('ImageAvailable'),
+        core_waiter.NormalizedOperationMethod(
+            ec2.describe_images
+        )),
     ('EC2', 'internet_gateway_exists'): lambda ec2: core_waiter.Waiter(
         'internet_gateway_exists',
         ec2_model('InternetGatewayExists'),

diff --git a/plugins/modules/ec2_ami.py b/plugins/modules/ec2_ami.py
@@ -80,6 +80,8 @@
           type: str
           description:
           - The device name. For example C(/dev/sda).
+          required: yes
+          aliases: ['DeviceName']
         virtual_name:
           type: str
           description:
@@ -369,9 +371,11 @@
 
 from ..module_utils.core import AnsibleAWSModule
 from ..module_utils.core import is_boto3_error_code
+from ..module_utils.ec2 import AWSRetry
 from ..module_utils.ec2 import ansible_dict_to_boto3_tag_list
 from ..module_utils.ec2 import boto3_tag_list_to_ansible_dict
 from ..module_utils.ec2 import compare_aws_tags
+from ..module_utils.waiters import get_waiter
 
 
 def get_block_device_mapping(image):
@@ -476,7 +480,7 @@ def create_image(module, connection):
         if instance_id:
             params['InstanceId'] = instance_id
             params['NoReboot'] = no_reboot
-            image_id = connection.create_image(**params).get('ImageId')
+            image_id = connection.create_image(aws_retry=True, **params).get('ImageId')
         else:
             if architecture:
                 params['Architecture'] = architecture
@@ -496,19 +500,19 @@ def create_image(module, connection):
                 params['KernelId'] = kernel_id
             if root_device_name:
                 params['RootDeviceName'] = root_device_name
-            image_id = connection.register_image(**params).get('ImageId')
+            image_id = connection.register_image(aws_retry=True, **params).get('ImageId')
     except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
         module.fail_json_aws(e, msg="Error registering image")
 
     if wait:
-        waiter = connection.get_waiter('image_available')
+        waiter = get_waiter(connection, 'image_available')
         delay = wait_timeout // 30
         max_attempts = 30
         waiter.wait(ImageIds=[image_id], WaiterConfig=dict(Delay=delay, MaxAttempts=max_attempts))
 
     if tags:
         try:
-            connection.create_tags(Resources=[image_id], Tags=ansible_dict_to_boto3_tag_list(tags))
+            connection.create_tags(aws_retry=True, Resources=[image_id], Tags=ansible_dict_to_boto3_tag_list(tags))
         except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
             module.fail_json_aws(e, msg="Error tagging image")
 
@@ -520,7 +524,7 @@ def create_image(module, connection):
             for user_id in launch_permissions.get('user_ids', []):
                 params['LaunchPermission']['Add'].append(dict(UserId=str(user_id)))
             if params['LaunchPermission']['Add']:
-                connection.modify_image_attribute(**params)
+                connection.modify_image_attribute(aws_retry=True, **params)
         except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
             module.fail_json_aws(e, msg="Error setting launch permissions for image %s" % image_id)
 
@@ -549,7 +553,7 @@ def deregister_image(module, connection):
     # When trying to re-deregister an already deregistered image it doesn't raise an exception, it just returns an object without image attributes.
     if 'ImageId' in image:
         try:
-            connection.deregister_image(ImageId=image_id)
+            connection.deregister_image(aws_retry=True, ImageId=image_id)
         except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
             module.fail_json_aws(e, msg="Error deregistering image")
     else:
@@ -568,14 +572,14 @@ def deregister_image(module, connection):
     exit_params = {'msg': "AMI deregister operation complete.", 'changed': True}
 
     if delete_snapshot:
-        try:
-            for snapshot_id in snapshots:
-                connection.delete_snapshot(SnapshotId=snapshot_id)
-        # Don't error out if root volume snapshot was already deregistered as part of deregister_image
-        except is_boto3_error_code('InvalidSnapshot.NotFound'):
-            pass
-        except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-            module.fail_json_aws(e, msg='Failed to delete snapshot.')
+        for snapshot_id in snapshots:
+            try:
+                connection.delete_snapshot(aws_retry=True, SnapshotId=snapshot_id)
+            # Don't error out if root volume snapshot was already deregistered as part of deregister_image
+            except is_boto3_error_code('InvalidSnapshot.NotFound'):
+                pass
+            except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
+                module.fail_json_aws(e, msg='Failed to delete snapshot.')
         exit_params['snapshots_deleted'] = snapshots
 
     module.exit_json(**exit_params)
@@ -606,7 +610,8 @@ def update_image(module, connection, image_id):
 
         if to_add or to_remove:
             try:
-                connection.modify_image_attribute(ImageId=image_id, Attribute='launchPermission',
+                connection.modify_image_attribute(aws_retry=True,
+                                                  ImageId=image_id, Attribute='launchPermission',
                                                   LaunchPermission=dict(Add=to_add, Remove=to_remove))
                 changed = True
             except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
@@ -619,22 +624,22 @@ def update_image(module, connection, image_id):
 
         if tags_to_remove:
             try:
-                connection.delete_tags(Resources=[image_id], Tags=[dict(Key=tagkey) for tagkey in tags_to_remove])
+                connection.delete_tags(aws_retry=True, Resources=[image_id], Tags=[dict(Key=tagkey) for tagkey in tags_to_remove])
                 changed = True
             except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
                 module.fail_json_aws(e, msg="Error updating tags")
 
         if tags_to_add:
             try:
-                connection.create_tags(Resources=[image_id], Tags=ansible_dict_to_boto3_tag_list(tags_to_add))
+                connection.create_tags(aws_retry=True, Resources=[image_id], Tags=ansible_dict_to_boto3_tag_list(tags_to_add))
                 changed = True
             except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
                 module.fail_json_aws(e, msg="Error updating tags")
 
     description = module.params.get('description')
     if description and description != image['Description']:
         try:
-            connection.modify_image_attribute(Attribute='Description ', ImageId=image_id, Description=dict(Value=description))
+            connection.modify_image_attribute(aws_retry=True, Attribute='Description ', ImageId=image_id, Description=dict(Value=description))
             changed = True
         except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
             module.fail_json_aws(e, msg="Error setting description for image %s" % image_id)
@@ -650,7 +655,7 @@ def update_image(module, connection, image_id):
 def get_image_by_id(module, connection, image_id):
     try:
         try:
-            images_response = connection.describe_images(ImageIds=[image_id])
+            images_response = connection.describe_images(aws_retry=True, ImageIds=[image_id])
         except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
             module.fail_json_aws(e, msg="Error retrieving image %s" % image_id)
         images = images_response.get('Images')
@@ -660,8 +665,10 @@ def get_image_by_id(module, connection, image_id):
         if no_images == 1:
             result = images[0]
             try:
-                result['LaunchPermissions'] = connection.describe_image_attribute(Attribute='launchPermission', ImageId=image_id)['LaunchPermissions']
-                result['ProductCodes'] = connection.describe_image_attribute(Attribute='productCodes', ImageId=image_id)['ProductCodes']
+                result['LaunchPermissions'] = connection.describe_image_attribute(aws_retry=True, Attribute='launchPermission',
+                                                                                  ImageId=image_id)['LaunchPermissions']
+                result['ProductCodes'] = connection.describe_image_attribute(aws_retry=True, Attribute='productCodes',
+                                                                             ImageId=image_id)['ProductCodes']
             except is_boto3_error_code('InvalidAMIID.Unavailable'):
                 pass
             except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
@@ -687,7 +694,7 @@ def rename_item_if_exists(dict_object, attribute, new_attribute, child_node=None
 
 def main():
     mapping_options = dict(
-        device_name=dict(type='str'),
+        device_name=dict(type='str', aliases=['DeviceName'], required=True),
         virtual_name=dict(
             type='str', aliases=['VirtualName'],
             deprecated_aliases=[dict(name='VirtualName', date='2022-06-01', collection_name='amazon.aws')]),
@@ -738,7 +745,7 @@ def main():
     if not any([module.params['image_id'], module.params['name']]):
         module.fail_json(msg="one of the following is required: name, image_id")
 
-    connection = module.client('ec2')
+    connection = module.client('ec2', retry_decorator=AWSRetry.jittered_backoff())
 
     if module.params.get('state') == 'absent':
         deregister_image(module, connection)

diff --git a/plugins/modules/ec2_ami_info.py b/plugins/modules/ec2_ami_info.py
@@ -207,6 +207,8 @@
 from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict
 
 from ..module_utils.core import AnsibleAWSModule
+from ..module_utils.core import is_boto3_error_code
+from ..module_utils.ec2 import AWSRetry
 from ..module_utils.ec2 import ansible_dict_to_boto3_filter_list
 from ..module_utils.ec2 import boto3_tag_list_to_ansible_dict
 
@@ -240,19 +242,23 @@ def list_ec2_images(ec2_client, module):
     filters = ansible_dict_to_boto3_filter_list(filters)
 
     try:
-        images = ec2_client.describe_images(ImageIds=image_ids, Filters=filters, Owners=owner_param, ExecutableUsers=executable_users)
+        images = ec2_client.describe_images(aws_retry=True, ImageIds=image_ids, Filters=filters, Owners=owner_param,
+                                            ExecutableUsers=executable_users)
         images = [camel_dict_to_snake_dict(image) for image in images["Images"]]
     except (ClientError, BotoCoreError) as err:
         module.fail_json_aws(err, msg="error describing images")
     for image in images:
         try:
             image['tags'] = boto3_tag_list_to_ansible_dict(image.get('tags', []))
             if module.params.get("describe_image_attributes"):
-                launch_permissions = ec2_client.describe_image_attribute(Attribute='launchPermission', ImageId=image['image_id'])['LaunchPermissions']
+                launch_permissions = ec2_client.describe_image_attribute(aws_retry=True, Attribute='launchPermission',
+                                                                         ImageId=image['image_id'])['LaunchPermissions']
                 image['launch_permissions'] = [camel_dict_to_snake_dict(perm) for perm in launch_permissions]
-        except (ClientError, BotoCoreError) as err:
+        except is_boto3_error_code('AuthFailure'):
             # describing launch permissions of images owned by others is not permitted, but shouldn't cause failures
             pass
+        except (ClientError, BotoCoreError) as err:
+            module.fail_json_aws(err, 'Failed to describe AMI')
 
     images.sort(key=lambda e: e.get('creation_date', ''))  # it may be possible that creation_date does not always exist
     module.exit_json(images=images)
@@ -272,7 +278,7 @@ def main():
     if module._module._name == 'ec2_ami_facts':
         module._module.deprecate("The 'ec2_ami_facts' module has been renamed to 'ec2_ami_info'", date='2021-12-01', collection_name='amazon.aws')
 
-    ec2_client = module.client('ec2')
+    ec2_client = module.client('ec2', retry_decorator=AWSRetry.jittered_backoff())
 
     list_ec2_images(ec2_client, module)
 

diff --git a/tests/integration/targets/ec2_ami/aliases b/tests/integration/targets/ec2_ami/aliases
@@ -1,4 +1,3 @@
 cloud/aws
 shippable/aws/group2
-unstable
 ec2_ami_info