From 5207165789a9a33cd7b7d203c641d5e11f396981 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Fri, 1 Oct 2021 15:04:19 +0200 Subject: [PATCH 1/5] Use module_util helper for tagging AMIs --- plugins/modules/ec2_ami.py | 31 +++++-------------------------- 1 file changed, 5 insertions(+), 26 deletions(-) diff --git a/plugins/modules/ec2_ami.py b/plugins/modules/ec2_ami.py index 4086ea22851..5367d257078 100644 --- 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,11 @@ 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") + add_ec2_tags(connection, module, mapping.get('Ebs').get('SnapshotId'), tags) if launch_permissions: try: @@ -641,24 +637,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']: From 3f01b58133d8d19b2de5e5b4cbcdc2974a70cd41 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Fri, 1 Oct 2021 15:06:55 +0200 Subject: [PATCH 2/5] changelog --- changelogs/fragments/520-ec2_ami-tagging.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/520-ec2_ami-tagging.yml diff --git a/changelogs/fragments/520-ec2_ami-tagging.yml b/changelogs/fragments/520-ec2_ami-tagging.yml new file mode 100644 index 00000000000..c2f78dcf5db --- /dev/null +++ b/changelogs/fragments/520-ec2_ami-tagging.yml @@ -0,0 +1,2 @@ +minor_changes: +- ec2_ami - use module_util helper for tagging AMIs (https://github.com/ansible-collections/amazon.aws/pull/520). From a7e0f2026db5fbd5146b132adad7cc769d7d786e Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Fri, 1 Oct 2021 15:30:38 +0200 Subject: [PATCH 3/5] Skip trying to tag empemeral volumes --- changelogs/fragments/520-ec2_ami-tagging.yml | 2 ++ plugins/modules/ec2_ami.py | 3 +++ 2 files changed, 5 insertions(+) diff --git a/changelogs/fragments/520-ec2_ami-tagging.yml b/changelogs/fragments/520-ec2_ami-tagging.yml index c2f78dcf5db..275c0117213 100644 --- a/changelogs/fragments/520-ec2_ami-tagging.yml +++ b/changelogs/fragments/520-ec2_ami-tagging.yml @@ -1,2 +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 index 5367d257078..5b13cbbf8a8 100644 --- a/plugins/modules/ec2_ami.py +++ b/plugins/modules/ec2_ami.py @@ -529,6 +529,9 @@ def create_image(module, connection): add_ec2_tags(connection, module, image_id, tags) if image_info and image_info.get('BlockDeviceMappings'): for mapping in image_info.get('BlockDeviceMappings'): + # 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: From 9644f8acbf1d12347aa51b692851e96236434a62 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Fri, 1 Oct 2021 21:17:49 +0200 Subject: [PATCH 4/5] Switch ec2_ami integration tests over to ec2_instance --- .../targets/ec2_ami/tasks/main.yml | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/tests/integration/targets/ec2_ami/tasks/main.yml b/tests/integration/targets/ec2_ami/tasks/main.yml index 1627eb772d4..35af1d0e352 100644 --- a/tests/integration/targets/ec2_ami/tasks/main.yml +++ b/tests/integration/targets/ec2_ami/tasks/main.yml @@ -46,21 +46,25 @@ 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 }}' + 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 +73,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 +93,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 +111,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 +149,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 +171,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 +197,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 +296,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 +316,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 +562,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 +591,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 +608,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 +664,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 From 0d8942b8dc51bf335681354d5fb93d8b3a5e182d Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Fri, 1 Oct 2021 21:18:14 +0200 Subject: [PATCH 5/5] Add emphemeral volume to test instance - these should be ignored during tagging. https://github.com/ansible-collections/amazon.aws/issues/511 --- tests/integration/targets/ec2_ami/tasks/main.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/integration/targets/ec2_ami/tasks/main.yml b/tests/integration/targets/ec2_ami/tasks/main.yml index 35af1d0e352..a98bf390b11 100644 --- a/tests/integration/targets/ec2_ami/tasks/main.yml +++ b/tests/integration/targets/ec2_ami/tasks/main.yml @@ -55,6 +55,9 @@ '{{ec2_ami_name}}_instance_setup': 'integration_tests' 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