Improve ec2_ami tests (#38987)

* Improve ec2_ami tests

Ensure that ec2_ami_image_id fact gets set immediately after AMI
creation so that they get torn down even if tests fail

Use YAML anchor to simplify AWS credential passing

Use aws_connection_info to reduce AWS credential boilerplate

Improve exception handling when updating image attributes

Error messages weren't correctly formatted to show image ids.

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

@@ -406,15 +406,6 @@ def create_image(module, connection):
             'Description': description
         }
 
-        images = connection.describe_images(
-            Filters=[
-                {
-                    'Name': 'name',
-                    'Values': [name]
-                }
-            ]
-        ).get('Images')
-
         block_device_mapping = None
 
         if device_mapping:
@@ -572,7 +563,7 @@ def update_image(module, connection, image_id):
                                                   LaunchPermission=dict(Add=to_add, Remove=to_remove))
                 changed = True
             except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
-                module.fail_json_aws(e, msg="Error updating launch permissions")
+                module.fail_json_aws(e, msg="Error updating launch permissions of image %s" % image_id)
 
     desired_tags = module.params.get('tags')
     if desired_tags is not None:
@@ -626,9 +617,9 @@ def get_image_by_id(module, connection, image_id):
                 result['ProductCodes'] = connection.describe_image_attribute(Attribute='productCodes', ImageId=image_id)['ProductCodes']
             except botocore.exceptions.ClientError as e:
                 if e.response['Error']['Code'] != 'InvalidAMIID.Unavailable':
-                    module.fail_json_aws(e, msg="Error retrieving image attributes %s" % image_id)
+                    module.fail_json_aws(e, msg="Error retrieving image attributes for image %s" % image_id)
             except botocore.exceptions.BotoCoreError as e:
-                module.fail_json_aws(e, msg="Error retrieving image attributes %s" % image_id)
+                module.fail_json_aws(e, msg="Error retrieving image attributes for image %s" % image_id)
             return result
         module.fail_json(msg="Invalid number of instances (%s) found for image_id: %s." % (str(len(images)), image_id))
     except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:

test/integration/targets/ec2_ami/tasks/main.yml

@@ -6,36 +6,34 @@
     # ============================================================
 
     # SETUP: vpc, ec2 key pair, subnet, security group, ec2 instance, snapshot
+    - name: set aws_connection_info fact
+      set_fact:
+        aws_connection_info: &aws_connection_info
+          aws_region: '{{aws_region}}'
+          aws_access_key: '{{aws_access_key}}'
+          aws_secret_key: '{{aws_secret_key}}'
+          security_token: '{{security_token}}'
+      no_log: yes
 
     - name: create a VPC to work in
       ec2_vpc_net:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         cidr_block: 10.0.0.0/24
         state: present
         name: '{{ ec2_ami_name }}_setup'
         resource_tags:
           Name: '{{ ec2_ami_name }}_setup'
+        <<: *aws_connection_info
       register: setup_vpc
 
     - name: create a key pair to use for creating an ec2 instance
       ec2_key:
         name: '{{ ec2_ami_name }}_setup'
         state: present
-        ec2_region: '{{ ec2_region }}'
+        <<: *aws_connection_info
-        ec2_access_key: '{{ ec2_access_key }}'
-        ec2_secret_key: '{{ ec2_secret_key }}'
-        security_token: '{{ security_token }}'
       register: setup_key
 
     - name: create a subnet to use for creating an ec2 instance
       ec2_vpc_subnet:
-        ec2_region: '{{ ec2_region }}'
-        ec2_access_key: '{{ ec2_access_key }}'
-        ec2_secret_key: '{{ ec2_secret_key }}'
-        security_token: '{{ security_token }}'
         az: '{{ ec2_region }}a'
         tags: '{{ ec2_ami_name }}_setup'
         vpc_id: '{{ setup_vpc.vpc.id }}'
@@ -43,26 +41,20 @@
         state: present
         resource_tags:
           Name: '{{ ec2_ami_name }}_setup'
+        <<: *aws_connection_info
       register: setup_subnet
 
     - name: create a security group to use for creating an ec2 instance
       ec2_group:
         name: '{{ ec2_ami_name }}_setup'
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         description: 'created by Ansible integration tests'
         state: present
         vpc_id: '{{ setup_vpc.vpc.id }}'
+        <<: *aws_connection_info
       register: setup_sg
 
     - name: provision ec2 instance to create an image
       ec2:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         key_name: '{{ setup_key.key.name }}'
         instance_type: t2.micro
         state: present
@@ -72,27 +64,21 @@
           '{{ec2_ami_name}}_instance_setup': 'integration_tests'
         group_id: '{{ setup_sg.group_id }}'
         vpc_subnet_id: '{{ setup_subnet.subnet.id }}'
+        <<: *aws_connection_info
       register: setup_instance
 
     - name: take a snapshot of the instance to create an image
       ec2_snapshot:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         instance_id: '{{ setup_instance.instance_ids[0] }}'
         device_name: /dev/xvda
         state: present
+        <<: *aws_connection_info
       register: setup_snapshot
 
     # ============================================================
 
     - name: test clean failure if not providing image_id or name with state=present
       ec2_ami:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         instance_id: '{{ setup_instance.instance_ids[0] }}'
         state: present
         description: '{{ ec2_ami_description }}'
@@ -100,6 +86,7 @@
           Name: '{{ ec2_ami_name }}_ami'
         wait: yes
         root_device_name: /dev/xvda
+        <<: *aws_connection_info
       register: result
       ignore_errors: yes
 
@@ -113,10 +100,6 @@
 
     - name: create an image from the instance
       ec2_ami:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         instance_id: '{{ setup_instance.instance_ids[0] }}'
         state: present
         name: '{{ ec2_ami_name }}_ami'
@@ -125,8 +108,13 @@
           Name: '{{ ec2_ami_name }}_ami'
         wait: yes
         root_device_name: /dev/xvda
+        <<: *aws_connection_info
       register: result
 
+    - name: set image id fact for deletion later
+      set_fact:
+        ec2_ami_image_id: "{{ result.image_id }}"
+
     - name: assert that image has been created
       assert:
         that:
@@ -134,19 +122,12 @@
           - "result.image_id.startswith('ami-')"
           - "'Name' in result.tags and result.tags.Name == ec2_ami_name + '_ami'"
 
-    - name: set image id fact for deletion later
-      set_fact:
-        ec2_ami_image_id: "{{ result.image_id }}"
-
     # ============================================================
 
     - name: gather facts about the image created
       ec2_ami_facts:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         image_ids: '{{ ec2_ami_image_id }}'
+        <<: *aws_connection_info
       register: ami_facts_result
       ignore_errors: true
 
@@ -159,10 +140,6 @@
 
     - name: delete the image
       ec2_ami:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         instance_id: '{{ setup_instance.instance_ids[0] }}'
         state: absent
         delete_snapshot: yes
@@ -172,6 +149,7 @@
         tags:
           Name: '{{ ec2_ami_name }}_ami'
         wait: yes
+        <<: *aws_connection_info
       ignore_errors: true
       register: result
 
@@ -185,11 +163,8 @@
 
     - name: test removing an ami if no image ID is provided (expected failed=true)
       ec2_ami:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         state: absent
+        <<: *aws_connection_info
       register: result
       ignore_errors: yes
 
@@ -203,10 +178,6 @@
 
     - name: create an image from the snapshot
       ec2_ami:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         name: '{{ ec2_ami_name }}_ami'
         description: '{{ ec2_ami_description }}'
         state: present
@@ -221,28 +192,25 @@
             size: 8
             delete_on_termination: true
             snapshot_id: '{{ setup_snapshot.snapshot_id }}'
+        <<: *aws_connection_info
       register: result
       ignore_errors: true
 
+    - name: set image id fact for deletion later
+      set_fact:
+        ec2_ami_image_id: "{{ result.image_id }}"
+        ec2_ami_snapshot: "{{ result.block_device_mapping['/dev/xvda'].snapshot_id }}"
+
     - name: assert a new ami has been created
       assert:
         that:
           - "result.changed"
           - "result.image_id.startswith('ami-')"
 
-    - name: set image id fact for deletion later
-      set_fact:
-        ec2_ami_image_id: "{{ result.image_id }}"
-        ec2_ami_snapshot: "{{ result.block_device_mapping['/dev/xvda'].snapshot_id }}"
-
     # ============================================================
 
     - name: test default launch permissions idempotence
       ec2_ami:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         description: '{{ ec2_ami_description }}'
         state: present
         name: '{{ ec2_ami_name }}_ami'
@@ -258,6 +226,7 @@
             size: 8
             delete_on_termination: true
             snapshot_id: '{{ setup_snapshot.snapshot_id }}'
+        <<: *aws_connection_info
       register: result
 
     - name: assert a new ami has not been created
@@ -270,16 +239,13 @@
 
     - name: add a tag to the AMI
       ec2_ami:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         state: present
         description: '{{ ec2_ami_description }}'
         image_id: '{{ result.image_id }}'
         name: '{{ ec2_ami_name }}_ami'
         tags:
           New: Tag
+        <<: *aws_connection_info
       register: result
 
     - name: assert a tag was added
@@ -290,10 +256,6 @@
 
     - name: use purge_tags to remove a tag from the AMI
       ec2_ami:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         state: present
         description: '{{ ec2_ami_description }}'
         image_id: '{{ result.image_id }}'
@@ -301,6 +263,7 @@
         tags:
           New: Tag
         purge_tags: yes
+        <<: *aws_connection_info
       register: result
 
     - name: assert a tag was removed
@@ -313,10 +276,6 @@
 
     - name: update AMI launch permissions
       ec2_ami:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         state: present
         image_id: '{{ result.image_id }}'
         description: '{{ ec2_ami_description }}'
@@ -324,6 +283,7 @@
           Name: '{{ ec2_ami_name }}_ami'
         launch_permissions:
           group_names: ['all']
+        <<: *aws_connection_info
       register: result
 
     - name: assert launch permissions were updated
@@ -335,10 +295,6 @@
 
     - name: modify the AMI description
       ec2_ami:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         state: present
         image_id: '{{ result.image_id }}'
         name: '{{ ec2_ami_name }}_ami'
@@ -347,6 +303,7 @@
           Name: '{{ ec2_ami_name }}_ami'
         launch_permissions:
           group_names: ['all']
+        <<: *aws_connection_info
       register: result
 
     - name: assert the description changed
@@ -358,10 +315,6 @@
 
     - name: remove public launch permissions
       ec2_ami:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         state: present
         image_id: '{{ result.image_id }}'
         name: '{{ ec2_ami_name }}_ami'
@@ -369,6 +322,7 @@
           Name: '{{ ec2_ami_name }}_ami'
         launch_permissions:
           group_names: []
+        <<: *aws_connection_info
       register: result
 
     - name: assert launch permissions were updated
@@ -380,10 +334,6 @@
 
     - name: delete ami without deleting the snapshot (default is not to delete)
       ec2_ami:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         instance_id: '{{ setup_instance.instance_ids[0] }}'
         state: absent
         name: '{{ ec2_ami_name }}_ami'
@@ -391,6 +341,7 @@
         tags:
           Name: '{{ ec2_ami_name }}_ami'
         wait: yes
+        <<: *aws_connection_info
       ignore_errors: true
       register: result
 
@@ -404,10 +355,7 @@
       ec2_snapshot_facts:
         snapshot_ids:
           - '{{ ec2_ami_snapshot }}'
-        ec2_region: '{{ec2_region}}'
+        <<: *aws_connection_info
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
       register: snapshot_result
 
     - name: assert the snapshot wasn't deleted
@@ -417,10 +365,6 @@
 
     - name: delete ami for a second time
       ec2_ami:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         instance_id: '{{ setup_instance.instance_ids[0] }}'
         state: absent
         name: '{{ ec2_ami_name }}_ami'
@@ -428,6 +372,7 @@
         tags:
           Name: '{{ ec2_ami_name }}_ami'
         wait: yes
+        <<: *aws_connection_info
       register: result
 
     - name: assert that image does not exist
@@ -450,32 +395,22 @@
 
     - name: delete ami
       ec2_ami:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         state: absent
         image_id: "{{ ec2_ami_image_id }}"
         name: '{{ ec2_ami_name }}_ami'
         wait: yes
+        <<: *aws_connection_info
       ignore_errors: yes
 
     - name: remove setup snapshot of ec2 instance
       ec2_snapshot:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         state: absent
         snapshot_id: '{{ setup_snapshot.snapshot_id }}'
+        <<: *aws_connection_info
       ignore_errors: yes
 
     - name: remove setup ec2 instance
       ec2:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         instance_type: t2.micro
         instance_ids: '{{ setup_instance.instance_ids }}'
         state: absent
@@ -484,36 +419,27 @@
           '{{ec2_ami_name}}_instance_setup': 'integration_tests'
         group_id: '{{ setup_sg.group_id }}'
         vpc_subnet_id: '{{ setup_subnet.subnet.id }}'
+        <<: *aws_connection_info
       ignore_errors: yes
 
     - name: remove setup keypair
       ec2_key:
         name: '{{ec2_ami_name}}_setup'
         state: absent
-        ec2_region: '{{ec2_region}}'
+        <<: *aws_connection_info
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
       ignore_errors: yes
 
     - name: remove setup security group
       ec2_group:
         name: '{{ ec2_ami_name }}_setup'
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         description: 'created by Ansible integration tests'
         state: absent
         vpc_id: '{{ setup_vpc.vpc.id }}'
+        <<: *aws_connection_info
       ignore_errors: yes
 
     - name: remove setup subnet
       ec2_vpc_subnet:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         az: '{{ ec2_region }}a'
         tags: '{{ec2_ami_name}}_setup'
         vpc_id: '{{ setup_vpc.vpc.id }}'
@@ -521,17 +447,15 @@
         state: absent
         resource_tags:
           Name: '{{ ec2_ami_name }}_setup'
+        <<: *aws_connection_info
       ignore_errors: yes
 
     - name: remove setup VPC
       ec2_vpc_net:
-        ec2_region: '{{ec2_region}}'
-        ec2_access_key: '{{ec2_access_key}}'
-        ec2_secret_key: '{{ec2_secret_key}}'
-        security_token: '{{security_token}}'
         cidr_block: 10.0.0.0/24
         state: absent
         name: '{{ ec2_ami_name }}_setup'
         resource_tags:
           Name: '{{ ec2_ami_name }}_setup'
+        <<: *aws_connection_info
       ignore_errors: yes