Ensure block device instance creation outputs instances

Not waiting outputs results in a format that will never
be matched by the tests

Ensure instances get tidied up

Allow ec2:ReportInstanceStatus

ec2_instance: Improve test cleanup on failing tests

Improve describe/modify attribute error handling

Address feedback on PR

hacking/aws_config/testing_policies/compute-policy.json

@@ -86,7 +86,8 @@
                 "ec2:ModifyVpcAttribute",
                 "ec2:RegisterImage",
                 "ec2:ReleaseAddress",
-                "ec2:ReplaceRouteTableAssociation"
+                "ec2:ReplaceRouteTableAssociation",
+                "ec2:ReportInstanceStatus"
             ],
             "Resource": "*"
         },

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

@@ -1243,7 +1243,7 @@ def diff_instance_and_params(instance, params, ec2=None, skip=None):
 
     for mapping in param_mappings:
         if params.get(mapping.param_key) is not None and mapping.instance_key not in skip:
-            value = ec2.describe_instance_attribute(Attribute=mapping.attribute_name, InstanceId=id_)
+            value = AWSRetry.jittered_backoff()(ec2.describe_instance_attribute)(Attribute=mapping.attribute_name, InstanceId=id_)
             if params.get(mapping.param_key) is not None and value[mapping.instance_key]['Value'] != params.get(mapping.param_key):
                 arguments = dict(
                     InstanceId=instance['InstanceId'],
@@ -1496,7 +1496,7 @@ def handle_existing(existing_matches, changed, ec2, state):
         )
     changes = diff_instance_and_params(existing_matches[0], module.params)
     for c in changes:
-        ec2.modify_instance_attribute(**c)
+        AWSRetry.jittered_backoff()(ec2.modify_instance_attribute)(**c)
     changed |= bool(changes)
     changed |= add_or_update_instance_profile(existing_matches[0], module.params.get('instance_role'))
     changed |= change_network_attachments(existing_matches[0], module.params, ec2)

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

@@ -195,8 +195,8 @@ def get_or_create_policy_version(module, iam, policy, policy_document):
             try:
                 version = iam.create_policy_version(PolicyArn=policy['Arn'], PolicyDocument=policy_document)['PolicyVersion']
                 return version, True
-            except botocore.exceptions.ClientError as e:
-                pass
+            except botocore.exceptions.ClientError as second_e:
+                e = second_e
         # Handle both when the exception isn't LimitExceeded or
         # the second attempt still failed
         module.fail_json(msg="Couldn't create policy version: %s" % str(e),

test/integration/targets/ec2_instance/tasks/block_devices.yml

@@ -6,6 +6,7 @@
       security_token: "{{ security_token }}"
       region: "{{ aws_region }}"
   no_log: true
+
 - name: New instance with an extra block device
   ec2_instance:
     name: "{{ resource_prefix }}-test-ebs-vols"
@@ -20,12 +21,12 @@
     tags:
       TestId: "{{ resource_prefix }}"
     instance_type: t2.micro
-    wait: false
+    wait: true
     <<: *aws_connection_info
   register: in_test_vpc
 
-- name: Gather instance facts
-  ec2_instance_facts:
+- name: Gather instance info
+  ec2_instance_info:
     filters:
       "tag:Name": "{{ resource_prefix }}-test-ebs-vols"
     <<: *aws_connection_info
@@ -60,6 +61,7 @@
   ec2_instance_info:
     filters:
       "tag:Name": "{{ resource_prefix }}-test-ebs-vols"
+      "instance-state-name": "running"
     <<: *aws_connection_info
   register: presented_instance_fact
 

test/integration/targets/ec2_instance/tasks/cpu_options.yml

@@ -1,4 +1,5 @@
-- name: set connection information for all tasks
+- block:
+    - name: set connection information for all tasks
       set_fact:
         aws_connection_info: &aws_connection_info
           aws_access_key: "{{ aws_access_key }}"
@@ -7,7 +8,7 @@
           region: "{{ aws_region }}"
       no_log: true
 
-- name: create t3.nano instance with cpu_options
+    - name: create t3.nano instance with cpu_options
       ec2_instance:
         name: "{{ resource_prefix }}-test-t3nano-1-threads-per-core"
         image_id: "{{ ec2_ami_image[aws_region] }}"
@@ -22,13 +23,13 @@
         <<: *aws_connection_info
       register: instance_creation
 
-- name: instance with cpu_options created with the right options
+    - name: instance with cpu_options created with the right options
       assert:
         that:
           - instance_creation is success
           - instance_creation is changed
 
-- name: modify cpu_options on existing instance (warning displayed)
+    - name: modify cpu_options on existing instance (warning displayed)
       ec2_instance:
         state: present
         name: "{{ resource_prefix }}-test-t3nano-1-threads-per-core"
@@ -45,13 +46,13 @@
       register: cpu_options_update
       ignore_errors: yes
 
-- name: modify cpu_options has no effect on existing instance
+    - name: modify cpu_options has no effect on existing instance
       assert:
         that:
           - cpu_options_update is success
           - cpu_options_update is not changed
 
-- name: create t3.nano instance with cpu_options(check mode)
+    - name: create t3.nano instance with cpu_options(check mode)
       ec2_instance:
         name: "{{ resource_prefix }}-test-t3nano-1-threads-per-core-checkmode"
         image_id: "{{ ec2_ami_image[aws_region] }}"
@@ -65,21 +66,21 @@
         <<: *aws_connection_info
       check_mode: yes
 
-- name: "fact presented ec2 instance"
+    - name: "fact presented ec2 instance"
       ec2_instance_info:
         filters:
           "tag:Name": "{{ resource_prefix }}-test-t3nano-1-threads-per-core"
         <<: *aws_connection_info
       register: presented_instance_fact
 
-- name: "fact checkmode ec2 instance"
+    - name: "fact checkmode ec2 instance"
       ec2_instance_info:
         filters:
           "tag:Name": "{{ resource_prefix }}-test-t3nano-1-threads-per-core-checkmode"
         <<: *aws_connection_info
       register: checkmode_instance_fact
 
-- name: "Confirm existence of instance id."
+    - name: "Confirm existence of instance id."
       assert:
         that:
           - "{{ presented_instance_fact.instances | length }} > 0"
@@ -87,3 +88,14 @@
           - "{{ checkmode_instance_fact.instances | length }} == 0"
           - "{{ presented_instance_fact.instances.0.cpu_options.core_count }} == 1"
           - "{{ presented_instance_fact.instances.0.cpu_options.threads_per_core }} == 1"
+
+  always:
+    - name: Terminate instances
+      ec2_instance:
+        filters:
+          tag:TestId: "{{ resource_prefix }}"
+        state: absent
+        wait: false
+        <<: *aws_connection_info
+      register: result
+      ignore_errors: yes

test/integration/targets/ec2_instance/tasks/default_vpc_tests.yml

@@ -1,4 +1,5 @@
-- name: set connection information for all tasks
+- block:
+    - name: set connection information for all tasks
       set_fact:
         aws_connection_info: &aws_connection_info
           aws_access_key: "{{ aws_access_key }}"
@@ -6,7 +7,7 @@
           security_token: "{{ security_token }}"
           region: "{{ aws_region }}"
       no_log: true
-- name: Make instance in a default subnet of the VPC
+    - name: Make instance in a default subnet of the VPC
       ec2_instance:
         name: "{{ resource_prefix }}-test-default-vpc"
         image_id: "{{ ec2_ami_image[aws_region] }}"
@@ -18,7 +19,7 @@
         <<: *aws_connection_info
       register: in_default_vpc
 
-- name: Make instance in a default subnet of the VPC(check mode)
+    - name: Make instance in a default subnet of the VPC(check mode)
       ec2_instance:
         name: "{{ resource_prefix }}-test-default-vpc-checkmode"
         image_id: "{{ ec2_ami_image[aws_region] }}"
@@ -29,28 +30,37 @@
         <<: *aws_connection_info
       check_mode: yes
 
-- name: "fact presented ec2 instance"
+    - name: "fact presented ec2 instance"
       ec2_instance_info:
         filters:
           "tag:Name": "{{ resource_prefix }}-test-default-vpc"
         <<: *aws_connection_info
       register: presented_instance_fact
 
-- name: "fact checkmode ec2 instance"
+    - name: "fact checkmode ec2 instance"
       ec2_instance_info:
         filters:
           "tag:Name": "{{ resource_prefix }}-test-default-vpc-checkmode"
         <<: *aws_connection_info
       register: checkmode_instance_fact
 
-- name: "Confirm whether the check mode is working normally."
+    - name: "Confirm whether the check mode is working normally."
       assert:
         that:
           - "{{ presented_instance_fact.instances | length }} > 0"
           - "{{ checkmode_instance_fact.instances | length }} == 0"
 
-- name: Terminate instance
+    - name: Terminate instance
       ec2:
         instance_ids: "{{ in_default_vpc.instance_ids }}"
         state: absent
         <<: *aws_connection_info
+
+  always:
+    - name: Terminate test-default-vpc instance
+      ec2:
+        filters:
+          "tag:Name": "{{ resource_prefix }}-test-default-vpc"
+        state: absent
+        <<: *aws_connection_info
+      ignore_errors: yes

test/integration/targets/ec2_instance/tasks/external_resource_attach.yml

@@ -1,4 +1,5 @@
-- name: set connection information for all tasks
+- block:
+    - name: set connection information for all tasks
       set_fact:
         aws_connection_info: &aws_connection_info
           aws_access_key: "{{ aws_access_key }}"
@@ -6,22 +7,22 @@
           security_token: "{{ security_token }}"
           region: "{{ aws_region }}"
       no_log: true
-# Make custom ENIs and attach via the `network` parameter
-- ec2_eni:
+    # Make custom ENIs and attach via the `network` parameter
+    - ec2_eni:
         delete_on_termination: true
         subnet_id: "{{ testing_subnet_b.subnet.id }}"
         security_groups:
           - "{{ sg.group_id }}"
         <<: *aws_connection_info
       register: eni_a
-- ec2_eni:
+    - ec2_eni:
         delete_on_termination: true
         subnet_id: "{{ testing_subnet_b.subnet.id }}"
         security_groups:
           - "{{ sg.group_id }}"
         <<: *aws_connection_info
       register: eni_b
-- ec2_eni:
+    - ec2_eni:
         delete_on_termination: true
         subnet_id: "{{ testing_subnet_b.subnet.id }}"
         security_groups:
@@ -29,11 +30,11 @@
         <<: *aws_connection_info
       register: eni_c
 
-- ec2_key:
+    - ec2_key:
         name: "{{ resource_prefix }}_test_key"
         <<: *aws_connection_info
 
-- name: Make instance in the testing subnet created in the test VPC
+    - name: Make instance in the testing subnet created in the test VPC
       ec2_instance:
         name: "{{ resource_prefix }}-test-eni-vpc"
         key_name: "{{ resource_prefix }}_test_key"
@@ -49,19 +50,19 @@
         <<: *aws_connection_info
       register: in_test_vpc
 
-- name: "Gather {{ resource_prefix }}-test-eni-vpc facts"
-  ec2_instance_facts:
+    - name: "Gather {{ resource_prefix }}-test-eni-vpc info"
+      ec2_instance_info:
         filters:
           "tag:Name": '{{ resource_prefix }}-test-eni-vpc'
         <<: *aws_connection_info
       register: in_test_vpc_instance
 
-- assert:
+    - assert:
         that:
           - 'in_test_vpc_instance.instances.0.key_name == "{{ resource_prefix }}_test_key"'
           - '(in_test_vpc_instance.instances.0.network_interfaces | length) == 1'
 
-- name: Add a second interface
+    - name: Add a second interface
       ec2_instance:
         name: "{{ resource_prefix }}-test-eni-vpc"
         network:
@@ -79,7 +80,7 @@
       ignore_errors: yes
       retries: 10
 
-- name: Make instance in the testing subnet created in the test VPC(check mode)
+    - name: Make instance in the testing subnet created in the test VPC(check mode)
       ec2_instance:
         name: "{{ resource_prefix }}-test-eni-vpc-checkmode"
         key_name: "{{ resource_prefix }}_test_key"
@@ -94,27 +95,27 @@
         <<: *aws_connection_info
       check_mode: yes
 
-- name: "fact presented ec2 instance"
+    - name: "fact presented ec2 instance"
       ec2_instance_info:
         filters:
           "tag:Name": "{{ resource_prefix }}-test-eni-vpc"
         <<: *aws_connection_info
       register: presented_instance_fact
 
-- name: "fact checkmode ec2 instance"
+    - name: "fact checkmode ec2 instance"
       ec2_instance_info:
         filters:
           "tag:Name": "{{ resource_prefix }}-test-eni-vpc-checkmode"
         <<: *aws_connection_info
       register: checkmode_instance_fact
 
-- name: "Confirm existence of instance id."
+    - name: "Confirm existence of instance id."
       assert:
         that:
           - "{{ presented_instance_fact.instances | length }} > 0"
           - "{{ checkmode_instance_fact.instances | length }} == 0"
 
-- name: Terminate instance
+    - name: Terminate instance
       ec2_instance:
         filters:
           tag:TestId: "{{ resource_prefix }}"
@@ -122,20 +123,19 @@
         wait: false
         <<: *aws_connection_info
       register: result
-- assert:
-    that: result.changed
 
-- name: Terminate instance
+  always:
+    - name: Terminate instance
       ec2_instance:
         instance_ids: "{{ in_test_vpc.instance_ids }}"
         state: absent
         wait: false
         <<: *aws_connection_info
       register: result
-- assert:
-    that: not result.changed
+      ignore_errors: yes
 
-- ec2_key:
+    - ec2_key:
         name: "{{ resource_prefix }}_test_key"
         state: absent
         <<: *aws_connection_info
+      ignore_errors: yes

test/integration/targets/ec2_instance/tasks/iam_instance_role.yml

@@ -50,7 +50,7 @@
         image_id: "{{ ec2_ami_image[aws_region] }}"
         security_groups: "{{ sg.group_id }}"
         instance_type: t2.micro
-        instance_role: "ansible-test-sts-{{ resource_prefix }}-test-policy"
+        instance_role: "{{ iam_role.arn }}"
         <<: *aws_connection_info
       check_mode: yes
 
@@ -80,7 +80,7 @@
         image_id: "{{ ec2_ami_image[aws_region] }}"
         security_groups: "{{ sg.group_id }}"
         instance_type: t2.micro
-        instance_role: "ansible-test-sts-{{ resource_prefix }}-test-policy-2"
+        instance_role: "{{ iam_role_2.arn }}"
         <<: *aws_connection_info
       register: instance_with_updated_role
       until: instance_with_updated_role is not failed

test/integration/targets/ec2_instance/tasks/tags_and_vpc_settings.yml

@@ -1,4 +1,5 @@
-- name: set connection information for all tasks
+- block:
+    - name: set connection information for all tasks
       set_fact:
         aws_connection_info: &aws_connection_info
           aws_access_key: "{{ aws_access_key }}"
@@ -6,7 +7,7 @@
           security_token: "{{ security_token }}"
           region: "{{ aws_region }}"
       no_log: true
-- name: Make instance in the testing subnet created in the test VPC
+    - name: Make instance in the testing subnet created in the test VPC
       ec2_instance:
         name: "{{ resource_prefix }}-test-basic-vpc-create"
         image_id: "{{ ec2_ami_image[aws_region] }}"
@@ -26,7 +27,7 @@
         <<: *aws_connection_info
       register: in_test_vpc
 
-- name: Make instance in the testing subnet created in the test VPC(check mode)
+    - name: Make instance in the testing subnet created in the test VPC(check mode)
       ec2_instance:
         name: "{{ resource_prefix }}-test-basic-vpc-create-checkmode"
         image_id: "{{ ec2_ami_image[aws_region] }}"
@@ -45,7 +46,7 @@
         <<: *aws_connection_info
       check_mode: yes
 
-- name: Try to re-make the instance, hopefully this shows changed=False
+    - name: Try to re-make the instance, hopefully this shows changed=False
       ec2_instance:
         name: "{{ resource_prefix }}-test-basic-vpc-create"
         image_id: "{{ ec2_ami_image[aws_region] }}"
@@ -61,37 +62,37 @@
         instance_type: t2.micro
         <<: *aws_connection_info
       register: remake_in_test_vpc
-- name: "Remaking the same instance resulted in no changes"
+    - name: "Remaking the same instance resulted in no changes"
       assert:
         that: not remake_in_test_vpc.changed
-- name: check that instance IDs match anyway
+    - name: check that instance IDs match anyway
       assert:
         that: 'remake_in_test_vpc.instance_ids[0] == in_test_vpc.instance_ids[0]'
-- name: check that source_dest_check was set to false
+    - name: check that source_dest_check was set to false
       assert:
         that: 'not remake_in_test_vpc.instances[0].source_dest_check'
 
-- name: "fact presented ec2 instance"
+    - name: "fact presented ec2 instance"
       ec2_instance_info:
         filters:
           "tag:Name": "{{ resource_prefix }}-test-basic-vpc-create"
         <<: *aws_connection_info
       register: presented_instance_fact
 
-- name: "fact checkmode ec2 instance"
+    - name: "fact checkmode ec2 instance"
       ec2_instance_info:
         filters:
           "tag:Name": "{{ resource_prefix }}-test-basic-vpc-create-checkmode"
         <<: *aws_connection_info
       register: checkmode_instance_fact
 
-- name: "Confirm whether the check mode is working normally."
+    - name: "Confirm whether the check mode is working normally."
       assert:
         that:
           - "{{ presented_instance_fact.instances | length }} > 0"
           - "{{ checkmode_instance_fact.instances | length }} == 0"
 
-- name: Alter it by adding tags
+    - name: Alter it by adding tags
       ec2_instance:
         name: "{{ resource_prefix }}-test-basic-vpc-create"
         image_id: "{{ ec2_ami_image[aws_region] }}"
@@ -104,17 +105,17 @@
         <<: *aws_connection_info
       register: add_another_tag
 
-- ec2_instance_info:
+    - ec2_instance_info:
         instance_ids: "{{ add_another_tag.instance_ids }}"
         <<: *aws_connection_info
       register: check_tags
-- name: "Remaking the same instance resulted in no changes"
+    - name: "Remaking the same instance resulted in no changes"
       assert:
         that:
           - check_tags.instances[0].tags.Another == 'thing'
           - check_tags.instances[0].tags.Something == 'else'
 
-- name: Purge a tag
+    - name: Purge a tag
       ec2_instance:
         name: "{{ resource_prefix }}-test-basic-vpc-create"
         image_id: "{{ ec2_ami_image[aws_region] }}"
@@ -127,17 +128,29 @@
         instance_type: t2.micro
         <<: *aws_connection_info
 
-- ec2_instance_info:
+    - ec2_instance_info:
         instance_ids: "{{ add_another_tag.instance_ids }}"
         <<: *aws_connection_info
       register: check_tags
 
-- name: "Remaking the same instance resulted in no changes"
+    - name: "Remaking the same instance resulted in no changes"
       assert:
         that:
           - "'Something' not in check_tags.instances[0].tags"
 
-- name: Terminate instance
+    - name: check that subnet-default public IP rule was followed
+      assert:
+        that:
+          - check_tags.instances[0].public_dns_name == ""
+          - check_tags.instances[0].private_ip_address.startswith("10.22.33")
+          - check_tags.instances[0].subnet_id == testing_subnet_b.subnet.id
+    - name: check that tags were applied
+      assert:
+        that:
+          - check_tags.instances[0].tags.Name.startswith(resource_prefix)
+          - "'{{ check_tags.instances[0].state.name }}' in  ['pending', 'running']"
+
+    - name: Terminate instance
       ec2_instance:
         filters:
           tag:TestId: "{{ resource_prefix }}"
@@ -145,27 +158,16 @@
         wait: false
         <<: *aws_connection_info
       register: result
-- assert:
+    - assert:
         that: result.changed
 
-- name: Terminate instance
+  always:
+    - name: Terminate instance
       ec2_instance:
-    instance_ids: "{{ add_another_tag.instance_ids }}"
+        filters:
+          tag:TestId: "{{ resource_prefix }}"
         state: absent
         wait: false
         <<: *aws_connection_info
       register: result
-- assert:
-    that: not result.changed
-
-- name: check that subnet-default public IP rule was followed
-  assert:
-    that:
-      - check_tags.instances[0].public_dns_name == ""
-      - check_tags.instances[0].private_ip_address.startswith("10.22.33")
-      - check_tags.instances[0].subnet_id == testing_subnet_b.subnet.id
-- name: check that tags were applied
-  assert:
-    that:
-      - check_tags.instances[0].tags.Name.startswith(resource_prefix)
-      - "'{{ check_tags.instances[0].state.name }}' in  ['pending', 'running']"
+      ignore_errors: yes

test/integration/targets/ec2_instance/tasks/termination_protection.yml

@@ -1,4 +1,5 @@
-- name: set connection information for all tasks
+- block:
+    - name: set connection information for all tasks
       set_fact:
         aws_connection_info: &aws_connection_info
           aws_access_key: "{{ aws_access_key }}"
@@ -6,7 +7,8 @@
           security_token: "{{ security_token }}"
           region: "{{ aws_region }}"
       no_log: true
-- name: Make termination-protected instance in the testing subnet created in the test VPC
+
+    - name: Make termination-protected instance in the testing subnet created in the test VPC
       ec2_instance:
         name: "{{ resource_prefix }}-test-protected-instance-in-vpc"
         image_id: "{{ ec2_ami_image[aws_region] }}"
@@ -20,7 +22,7 @@
         <<: *aws_connection_info
       register: in_test_vpc
 
-- name: Make termination-protected instance in the testing subnet created in the test VPC(check mode)
+    - name: Make termination-protected instance in the testing subnet created in the test VPC(check mode)
       ec2_instance:
         name: "{{ resource_prefix }}-test-protected-instance-in-vpc-checkmode"
         image_id: "{{ ec2_ami_image[aws_region] }}"
@@ -33,28 +35,28 @@
         <<: *aws_connection_info
       check_mode: yes
 
-- name: "fact presented ec2 instance"
+    - name: "fact presented ec2 instance"
       ec2_instance_info:
         filters:
           "tag:Name": "{{ resource_prefix }}-test-protected-instance-in-vpc"
         <<: *aws_connection_info
       register: presented_instance_fact
 
-- name: "fact checkmode ec2 instance"
+    - name: "fact checkmode ec2 instance"
       ec2_instance_info:
         filters:
           "tag:Name": "{{ resource_prefix }}-test-protected-instance-in-vpc-checkmode"
         <<: *aws_connection_info
       register: checkmode_instance_fact
 
-- name: "Confirm whether the check mode is working normally."
+    - name: "Confirm whether the check mode is working normally."
       assert:
         that:
           - "{{ presented_instance_fact.instances | length }} > 0"
           - "'{{ presented_instance_fact.instances.0.state.name }}' in ['running', 'pending']"
           - "{{ checkmode_instance_fact.instances | length }} == 0"
 
-- name: Try to terminate the instance
+    - name: Try to terminate the instance
       ec2_instance:
         state: absent
         name: "{{ resource_prefix }}-test-protected-instance-in-vpc"
@@ -68,11 +70,11 @@
         <<: *aws_connection_info
       register: bad_terminate
       ignore_errors: yes
-- name: Cannot terminate protected instance
+    - name: Cannot terminate protected instance
       assert:
         that:
           - bad_terminate is failed
-- name: Alter termination protection setting
+    - name: Alter termination protection setting
       ec2_instance:
         name: "{{ resource_prefix }}-test-protected-instance-in-vpc"
         image_id: "{{ ec2_ami_image[aws_region] }}"
@@ -80,7 +82,7 @@
         termination_protection: false
         instance_type: t2.micro
         <<: *aws_connection_info
-- name: Try to terminate the instance again (should work)
+    - name: Try to terminate the instance again (should work)
       ec2_instance:
         name: "{{ resource_prefix }}-test-protected-instance-in-vpc"
         image_id: "{{ ec2_ami_image[aws_region] }}"
@@ -90,5 +92,16 @@
         wait: false
         <<: *aws_connection_info
       register: terminate_results
-- assert:
+    - assert:
         that: terminate_results is not failed
+
+  always:
+    - name: Terminate instance
+      ec2_instance:
+        filters:
+          tag:TestId: "{{ resource_prefix }}"
+        state: absent
+        wait: false
+        <<: *aws_connection_info
+      register: result
+      ignore_errors: yes