From 0b86ffa3a926df20c0db6c114410c64e3478ff5d Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Wed, 29 Sep 2021 14:53:12 -0700 Subject: [PATCH 1/4] Handle create_image in check_mode --- plugins/modules/ec2_ami.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/plugins/modules/ec2_ami.py b/plugins/modules/ec2_ami.py index 577f9230db5..71bcde64120 100644 --- a/plugins/modules/ec2_ami.py +++ b/plugins/modules/ec2_ami.py @@ -430,7 +430,7 @@ def get_ami_info(camel_image): ) -def create_image(module, connection): +def create_image(module, connection): # needs check mode --------------------------------------------------------- instance_id = module.params.get('instance_id') name = module.params.get('name') wait = module.params.get('wait') @@ -450,6 +450,13 @@ def create_image(module, connection): ramdisk_id = module.params.get('ramdisk_id') sriov_net_support = module.params.get('sriov_net_support') + if module.check_mode: + image = connection.describe_images(Filters=[{'Name':'name', 'Values':[str(name)]}]) + if not image['Images']: + module.exit_json(changed=True, msg='Would have created a AMI if not in check mode') + else: + module.exit_json(changed=False, msg='Error registering image: AMI name is already in use by another AMI') + try: params = { 'Name': name, @@ -544,7 +551,7 @@ def create_image(module, connection): **get_ami_info(get_image_by_id(module, connection, image_id))) -def deregister_image(module, connection): +def deregister_image(module, connection): # needs check mode ---------------------------------------------------- image_id = module.params.get('image_id') delete_snapshot = module.params.get('delete_snapshot') wait = module.params.get('wait') @@ -597,7 +604,7 @@ def deregister_image(module, connection): module.exit_json(**exit_params) -def update_image(module, connection, image_id): +def update_image(module, connection, image_id): # needs check mode ----------------------------------------------- launch_permissions = module.params.get('launch_permissions') image = get_image_by_id(module, connection, image_id) if image is None: @@ -749,7 +756,8 @@ def main(): argument_spec=argument_spec, required_if=[ ['state', 'absent', ['image_id']], - ] + ], + supports_check_mode=True, ) # Using a required_one_of=[['name', 'image_id']] overrides the message that should be provided by From cd4fb8fe2372abfc98666648555105a405709fec Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Wed, 29 Sep 2021 22:22:29 -0700 Subject: [PATCH 2/4] Handle deregister, update image in check_mode --- plugins/modules/ec2_ami.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/plugins/modules/ec2_ami.py b/plugins/modules/ec2_ami.py index 71bcde64120..465c5b7861a 100644 --- a/plugins/modules/ec2_ami.py +++ b/plugins/modules/ec2_ami.py @@ -430,7 +430,7 @@ def get_ami_info(camel_image): ) -def create_image(module, connection): # needs check mode --------------------------------------------------------- +def create_image(module, connection): instance_id = module.params.get('instance_id') name = module.params.get('name') wait = module.params.get('wait') @@ -453,7 +453,7 @@ def create_image(module, connection): # needs check mode ----------------------- if module.check_mode: image = connection.describe_images(Filters=[{'Name':'name', 'Values':[str(name)]}]) if not image['Images']: - module.exit_json(changed=True, msg='Would have created a AMI if not in check mode') + module.exit_json(changed=True, msg='Would have created a AMI if not in check mode.') else: module.exit_json(changed=False, msg='Error registering image: AMI name is already in use by another AMI') @@ -551,7 +551,7 @@ def create_image(module, connection): # needs check mode ----------------------- **get_ami_info(get_image_by_id(module, connection, image_id))) -def deregister_image(module, connection): # needs check mode ---------------------------------------------------- +def deregister_image(module, connection): image_id = module.params.get('image_id') delete_snapshot = module.params.get('delete_snapshot') wait = module.params.get('wait') @@ -571,6 +571,8 @@ def deregister_image(module, connection): # needs check mode ------------------- # 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: + if module.check_mode: + module.exit_json(changed=True, msg='Would have deregistered AMI if not in check mode.') try: connection.deregister_image(aws_retry=True, ImageId=image_id) except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: @@ -604,7 +606,7 @@ def deregister_image(module, connection): # needs check mode ------------------- module.exit_json(**exit_params) -def update_image(module, connection, image_id): # needs check mode ----------------------------------------------- +def update_image(module, connection, image_id): launch_permissions = module.params.get('launch_permissions') image = get_image_by_id(module, connection, image_id) if image is None: @@ -629,7 +631,8 @@ def update_image(module, connection, image_id): # needs check mode ------------- if to_add or to_remove: try: - connection.modify_image_attribute(aws_retry=True, + if not module.check_mode: + connection.modify_image_attribute(aws_retry=True, ImageId=image_id, Attribute='launchPermission', LaunchPermission=dict(Add=to_add, Remove=to_remove)) changed = True @@ -643,14 +646,16 @@ def update_image(module, connection, image_id): # needs check mode ------------- if tags_to_remove: try: - connection.delete_tags(aws_retry=True, Resources=[image_id], Tags=[dict(Key=tagkey) for tagkey in tags_to_remove]) + 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: - connection.create_tags(aws_retry=True, Resources=[image_id], Tags=ansible_dict_to_boto3_tag_list(tags_to_add)) + 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") @@ -658,12 +663,15 @@ def update_image(module, connection, image_id): # needs check mode ------------- description = module.params.get('description') if description and description != image['Description']: try: - connection.modify_image_attribute(aws_retry=True, Attribute='Description ', ImageId=image_id, Description=dict(Value=description)) + if not module.check_mode: + 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) if changed: + if module.check_mode: + module.exit_json(changed=True, msg='Would have updated AMI if not in check mode.') module.exit_json(msg="AMI updated.", changed=True, **get_ami_info(get_image_by_id(module, connection, image_id))) else: From 2bf267aec51003abceac60fda6b4c51f9440d5d4 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Thu, 30 Sep 2021 13:04:51 -0700 Subject: [PATCH 3/4] Integration tests for check mode --- plugins/modules/ec2_ami.py | 6 +- .../targets/ec2_ami/tasks/main.yml | 137 ++++++++++++++++++ 2 files changed, 140 insertions(+), 3 deletions(-) diff --git a/plugins/modules/ec2_ami.py b/plugins/modules/ec2_ami.py index 465c5b7861a..4086ea22851 100644 --- a/plugins/modules/ec2_ami.py +++ b/plugins/modules/ec2_ami.py @@ -451,7 +451,7 @@ def create_image(module, connection): sriov_net_support = module.params.get('sriov_net_support') if module.check_mode: - image = connection.describe_images(Filters=[{'Name':'name', 'Values':[str(name)]}]) + image = connection.describe_images(Filters=[{'Name': 'name', 'Values': [str(name)]}]) if not image['Images']: module.exit_json(changed=True, msg='Would have created a AMI if not in check mode.') else: @@ -633,8 +633,8 @@ def update_image(module, connection, image_id): try: if not module.check_mode: connection.modify_image_attribute(aws_retry=True, - ImageId=image_id, Attribute='launchPermission', - LaunchPermission=dict(Add=to_add, Remove=to_remove)) + ImageId=image_id, Attribute='launchPermission', + 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 of image %s" % image_id) diff --git a/tests/integration/targets/ec2_ami/tasks/main.yml b/tests/integration/targets/ec2_ami/tasks/main.yml index 313595d0dbf..1627eb772d4 100644 --- a/tests/integration/targets/ec2_ami/tasks/main.yml +++ b/tests/integration/targets/ec2_ami/tasks/main.yml @@ -87,6 +87,24 @@ # ============================================================ + - name: create an image from the instance (check mode) + ec2_ami: + instance_id: '{{ setup_instance.instance_ids[0] }}' + state: present + name: '{{ ec2_ami_name }}_ami' + description: '{{ ec2_ami_description }}' + tags: + Name: '{{ ec2_ami_name }}_ami' + wait: yes + root_device_name: '{{ ec2_ami_root_disk }}' + check_mode: true + register: check_mode_result + + - name: assert that check_mode result is changed + assert: + that: + - check_mode_result is changed + - name: create an image from the instance ec2_ami: instance_id: '{{ setup_instance.instance_ids[0] }}' @@ -124,6 +142,28 @@ # ============================================================ + - 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] }}' + device_mapping: + - device_name: /dev/sda1 + volume_size: 10 + delete_on_termination: true + volume_type: gp2 + - device_name: /dev/sdf + no_device: yes + state: present + wait: yes + root_device_name: '{{ ec2_ami_root_disk }}' + check_mode: true + register: check_mode_result + + - name: assert that check_mode result is changed + assert: + that: + - check_mode_result is changed + - name: create an image from the instance with attached devices with no_device true ec2_ami: name: '{{ ec2_ami_name }}_no_device_true_ami' @@ -250,6 +290,26 @@ # e2_ami_info filtering tests ends # ============================================================ + - name: delete the image (check mode) + ec2_ami: + instance_id: '{{ setup_instance.instance_ids[0] }}' + state: absent + delete_snapshot: yes + name: '{{ ec2_ami_name }}_ami' + description: '{{ ec2_ami_description }}' + image_id: '{{ result.image_id }}' + tags: + Name: '{{ ec2_ami_name }}_ami' + wait: yes + ignore_errors: true + check_mode: true + register: check_mode_result + + - name: assert that check_mode result is changed + assert: + that: + - check_mode_result is changed + - name: delete the image ec2_ami: instance_id: '{{ setup_instance.instance_ids[0] }}' @@ -319,6 +379,31 @@ # ============================================================ + - name: test default launch permissions idempotence (check mode) + ec2_ami: + description: '{{ ec2_ami_description }}' + state: present + name: '{{ ec2_ami_name }}_ami' + tags: + Name: '{{ ec2_ami_name }}_ami' + root_device_name: '{{ ec2_ami_root_disk }}' + image_id: '{{ result.image_id }}' + launch_permissions: + user_ids: [] + device_mapping: + - device_name: '{{ ec2_ami_root_disk }}' + volume_type: gp2 + size: 8 + delete_on_termination: true + snapshot_id: '{{ setup_snapshot.snapshot_id }}' + check_mode: true + register: check_mode_result + + - name: assert that check_mode result is not changed + assert: + that: + - check_mode_result is not changed + - name: test default launch permissions idempotence ec2_ami: description: '{{ ec2_ami_description }}' @@ -381,6 +466,23 @@ # ============================================================ + - name: update AMI launch permissions (check mode) + ec2_ami: + state: present + image_id: '{{ result.image_id }}' + description: '{{ ec2_ami_description }}' + tags: + Name: '{{ ec2_ami_name }}_ami' + launch_permissions: + group_names: ['all'] + check_mode: true + register: check_mode_result + + - name: assert that check_mode result is changed + assert: + that: + - check_mode_result is changed + - name: update AMI launch permissions ec2_ami: state: present @@ -399,6 +501,24 @@ # ============================================================ + - name: modify the AMI description (check mode) + ec2_ami: + state: present + image_id: '{{ result.image_id }}' + name: '{{ ec2_ami_name }}_ami' + description: '{{ ec2_ami_description }}CHANGED' + tags: + Name: '{{ ec2_ami_name }}_ami' + launch_permissions: + group_names: ['all'] + check_mode: true + register: check_mode_result + + - name: assert that check_mode result is changed + assert: + that: + - check_mode_result is changed + - name: modify the AMI description ec2_ami: state: present @@ -465,6 +585,23 @@ that: - "snapshot_result.snapshots[0].snapshot_id == ec2_ami_snapshot" + - name: delete ami for a second time (check mode) + ec2_ami: + instance_id: '{{ setup_instance.instance_ids[0] }}' + state: absent + name: '{{ ec2_ami_name }}_ami' + image_id: '{{ ec2_ami_image_id }}' + tags: + Name: '{{ ec2_ami_name }}_ami' + wait: yes + check_mode: true + register: check_mode_result + + - name: assert that check_mode result is not changed + assert: + that: + - check_mode_result is not changed + - name: delete ami for a second time ec2_ami: instance_id: '{{ setup_instance.instance_ids[0] }}' From 814ffc1d11446f6cf95719483857c7df330f420c Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Thu, 30 Sep 2021 22:47:10 -0700 Subject: [PATCH 4/4] Added changelogs fragment --- changelogs/fragments/516-ec2_ami_add_check_mode_support.yml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/fragments/516-ec2_ami_add_check_mode_support.yml diff --git a/changelogs/fragments/516-ec2_ami_add_check_mode_support.yml b/changelogs/fragments/516-ec2_ami_add_check_mode_support.yml new file mode 100644 index 00000000000..7511f2fac3c --- /dev/null +++ b/changelogs/fragments/516-ec2_ami_add_check_mode_support.yml @@ -0,0 +1,3 @@ +minor_changes: + - ec2_ami - add check_mode support + (https://github.com/ansible-collections/amazon.aws/pull/516).