Use module_util helper for tagging AMIs (ansible-collections#520)

Use module_util helper for tagging AMIs SUMMARY We have some helpers for tagging EC2 resources, use them for tagging AMIs. ISSUE TYPE Feature Pull Request COMPONENT NAME ec2_ami ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis <None> Reviewed-by: None <None>
alinabuzachis · Oct 4, 2021 · b9f0bda · b9f0bda
1 parent 0325024
commit b9f0bda
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 52 deletions.
diff --git a/changelogs/fragments/520-ec2_ami-tagging.yml b/changelogs/fragments/520-ec2_ami-tagging.yml
@@ -0,0 +1,4 @@
+minor_changes:
+- ec2_ami - use module_util helper for tagging AMIs (https://github.com/ansible-collections/amazon.aws/pull/520).
+bugfixes:
+- ec2_ami - fix problem when creating an AMI from an instance with ephemeral volumes (https://github.com/ansible-collections/amazon.aws/issues/511).
diff --git a/plugins/modules/ec2_ami.py b/plugins/modules/ec2_ami.py
@@ -372,9 +372,9 @@
 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.ec2 import ensure_ec2_tags
+from ..module_utils.ec2 import add_ec2_tags
 from ..module_utils.waiters import get_waiter
 
 
@@ -525,15 +525,14 @@ def create_image(module, connection):
         waiter.wait(ImageIds=[image_id], WaiterConfig=dict(Delay=delay, MaxAttempts=max_attempts))
 
     if tags:
-        resources_to_tag = [image_id]
         image_info = get_image_by_id(module, connection, image_id)
+        add_ec2_tags(connection, module, image_id, tags)
         if image_info and image_info.get('BlockDeviceMappings'):
             for mapping in image_info.get('BlockDeviceMappings'):
-                resources_to_tag.append(mapping.get('Ebs').get('SnapshotId'))
-        try:
-            connection.create_tags(aws_retry=True, Resources=resources_to_tag, 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")
+                # We can only tag Ebs volumes
+                if 'Ebs' not in mapping:
+                    continue
+                add_ec2_tags(connection, module, mapping.get('Ebs').get('SnapshotId'), tags)
 
     if launch_permissions:
         try:
@@ -641,24 +640,7 @@ def update_image(module, connection, image_id):
 
     desired_tags = module.params.get('tags')
     if desired_tags is not None:
-        current_tags = boto3_tag_list_to_ansible_dict(image.get('Tags'))
-        tags_to_add, tags_to_remove = compare_aws_tags(current_tags, desired_tags, purge_tags=module.params.get('purge_tags'))
-
-        if tags_to_remove:
-            try:
-                if not module.check_mode:
-                    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:
-                if not module.check_mode:
-                    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")
+        changed |= ensure_ec2_tags(connection, module, image_id, tags=desired_tags, purge_tags=module.params.get('purge_tags'))
 
     description = module.params.get('description')
     if description and description != image['Description']:

diff --git a/tests/integration/targets/ec2_ami/tasks/main.yml b/tests/integration/targets/ec2_ami/tasks/main.yml
@@ -46,21 +46,28 @@
       register: setup_sg
 
     - name: provision ec2 instance to create an image
-      ec2:
+      ec2_instance:
+        state: running
         key_name: '{{ setup_key.key.name }}'
         instance_type: t2.micro
-        state: present
-        image: '{{ ec2_ami_id }}'
-        wait: yes
-        instance_tags:
+        image_id: '{{ ec2_ami_id }}'
+        tags:
           '{{ec2_ami_name}}_instance_setup': 'integration_tests'
-        group_id: '{{ setup_sg.group_id }}'
+        security_group: '{{ setup_sg.group_id }}'
         vpc_subnet_id: '{{ setup_subnet.subnet.id }}'
+        volumes:
+          - device_name: /dev/sdc
+            virtual_name: ephemeral1
+        wait: yes
       register: setup_instance
 
+    - name: Store EC2 Instance ID
+      set_fact:
+        ec2_instance_id: '{{ setup_instance.instances[0].instance_id }}'
+
     - name: take a snapshot of the instance to create an image
       ec2_snapshot:
-        instance_id: '{{ setup_instance.instance_ids[0] }}'
+        instance_id: '{{ ec2_instance_id }}'
         device_name: '{{ ec2_ami_root_disk }}'
         state: present
       register: setup_snapshot
@@ -69,7 +76,7 @@
 
     - name: test clean failure if not providing image_id or name with state=present
       ec2_ami:
-        instance_id: '{{ setup_instance.instance_ids[0] }}'
+        instance_id: '{{ ec2_instance_id }}'
         state: present
         description: '{{ ec2_ami_description }}'
         tags:
@@ -89,7 +96,7 @@
 
     - name: create an image from the instance (check mode)
       ec2_ami:
-        instance_id: '{{ setup_instance.instance_ids[0] }}'
+        instance_id: '{{ ec2_instance_id }}'
         state: present
         name: '{{ ec2_ami_name }}_ami'
         description: '{{ ec2_ami_description }}'
@@ -107,7 +114,7 @@
 
     - name: create an image from the instance
       ec2_ami:
-        instance_id: '{{ setup_instance.instance_ids[0] }}'
+        instance_id: '{{ ec2_instance_id }}'
         state: present
         name: '{{ ec2_ami_name }}_ami'
         description: '{{ ec2_ami_description }}'
@@ -145,7 +152,7 @@
     - name: create an image from the instance with attached devices with no_device true (check mode)
       ec2_ami:
         name: '{{ ec2_ami_name }}_no_device_true_ami'
-        instance_id: '{{ setup_instance.instance_ids[0] }}'
+        instance_id: '{{ ec2_instance_id }}'
         device_mapping:
           - device_name: /dev/sda1
             volume_size: 10
@@ -167,7 +174,7 @@
     - name: create an image from the instance with attached devices with no_device true
       ec2_ami:
         name: '{{ ec2_ami_name }}_no_device_true_ami'
-        instance_id: '{{ setup_instance.instance_ids[0] }}'
+        instance_id: '{{ ec2_instance_id }}'
         device_mapping:
           - device_name: /dev/sda1
             volume_size: 10
@@ -193,7 +200,7 @@
     - name: create an image from the instance with attached devices with no_device false
       ec2_ami:
         name: '{{ ec2_ami_name }}_no_device_false_ami'
-        instance_id: '{{ setup_instance.instance_ids[0] }}'
+        instance_id: '{{ ec2_instance_id }}'
         device_mapping:
           - device_name: /dev/sda1
             volume_size: 10
@@ -292,7 +299,7 @@
 
     - name: delete the image (check mode)
       ec2_ami:
-        instance_id: '{{ setup_instance.instance_ids[0] }}'
+        instance_id: '{{ ec2_instance_id }}'
         state: absent
         delete_snapshot: yes
         name: '{{ ec2_ami_name }}_ami'
@@ -312,7 +319,7 @@
 
     - name: delete the image
       ec2_ami:
-        instance_id: '{{ setup_instance.instance_ids[0] }}'
+        instance_id: '{{ ec2_instance_id }}'
         state: absent
         delete_snapshot: yes
         name: '{{ ec2_ami_name }}_ami'
@@ -558,7 +565,7 @@
 
     - name: delete ami without deleting the snapshot (default is not to delete)
       ec2_ami:
-        instance_id: '{{ setup_instance.instance_ids[0] }}'
+        instance_id: '{{ ec2_instance_id }}'
         state: absent
         name: '{{ ec2_ami_name }}_ami'
         image_id: '{{ ec2_ami_image_id }}'
@@ -587,7 +594,7 @@
 
     - name: delete ami for a second time (check mode)
       ec2_ami:
-        instance_id: '{{ setup_instance.instance_ids[0] }}'
+        instance_id: '{{ ec2_instance_id }}'
         state: absent
         name: '{{ ec2_ami_name }}_ami'
         image_id: '{{ ec2_ami_image_id }}'
@@ -604,7 +611,7 @@
 
     - name: delete ami for a second time
       ec2_ami:
-        instance_id: '{{ setup_instance.instance_ids[0] }}'
+        instance_id: '{{ ec2_instance_id }}'
         state: absent
         name: '{{ ec2_ami_name }}_ami'
         image_id: '{{ ec2_ami_image_id }}'
@@ -660,15 +667,11 @@
       ignore_errors: yes
 
     - name: remove setup ec2 instance
-      ec2:
-        instance_type: t2.micro
-        instance_ids: '{{ setup_instance.instance_ids }}'
+      ec2_instance:
         state: absent
-        wait: yes
-        instance_tags:
-          '{{ec2_ami_name}}_instance_setup': 'integration_tests'
-        group_id: '{{ setup_sg.group_id }}'
-        vpc_subnet_id: '{{ setup_subnet.subnet.id }}'
+        instance_ids:
+        - '{{ ec2_instance_id }}'
+        wait: true
       ignore_errors: yes
 
     - name: remove setup keypair