From 8428d1940da0ef73bc5a7e014c39eb0f8637504e Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Tue, 21 Sep 2021 19:13:34 -0700 Subject: [PATCH 01/13] Added check_mode support - create, remove snapshot --- plugins/modules/ec2_snapshot.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/plugins/modules/ec2_snapshot.py b/plugins/modules/ec2_snapshot.py index 317b1c7c507..df64143c035 100644 --- a/plugins/modules/ec2_snapshot.py +++ b/plugins/modules/ec2_snapshot.py @@ -91,7 +91,7 @@ device_name: /dev/sdb1 description: snapshot of /data from DB123 taken 2013/11/28 12:18:32 -# Snapshot of volume with tagging +# Snapshot of volume with tagging #not done yet // - amazon.aws.ec2_snapshot: instance_id: i-12345678 device_name: /dev/sdb1 @@ -288,6 +288,9 @@ def create_snapshot(module, ec2, description=None, wait=None, 'Tags': ansible_dict_to_boto3_tag_list(_tags), }] try: + if module.check_mode: + import q; q(volume) + module.exit_json(changed=True, volume_id=volume['VolumeId'], volume_size=volume['Size']) snapshot = _create_snapshot(ec2, **params) except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: module.fail_json_aws(e, msg="Failed to create snapshot") @@ -321,6 +324,8 @@ def create_snapshot(module, ec2, description=None, wait=None, def delete_snapshot(module, ec2, snapshot_id): + if module.check_mode: + module.exit_json(changed=True) try: ec2.delete_snapshot(aws_retry=True, SnapshotId=snapshot_id) except is_boto3_error_code('InvalidSnapshot.NotFound'): @@ -364,6 +369,7 @@ def create_snapshot_ansible_module(): required_if=required_if, required_one_of=required_one_of, required_together=required_together, + supports_check_mode=True ) return module From 7a2e5ef2d8e6f4b0eba90e48ff0b252760b44bac Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Wed, 22 Sep 2021 13:22:35 -0700 Subject: [PATCH 02/13] Added check_mode support to ec2_snapshot create, remove --- plugins/modules/ec2_snapshot.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/plugins/modules/ec2_snapshot.py b/plugins/modules/ec2_snapshot.py index df64143c035..778f17b5478 100644 --- a/plugins/modules/ec2_snapshot.py +++ b/plugins/modules/ec2_snapshot.py @@ -91,7 +91,7 @@ device_name: /dev/sdb1 description: snapshot of /data from DB123 taken 2013/11/28 12:18:32 -# Snapshot of volume with tagging #not done yet // +# Snapshot of volume with tagging - amazon.aws.ec2_snapshot: instance_id: i-12345678 device_name: /dev/sdb1 @@ -289,8 +289,7 @@ def create_snapshot(module, ec2, description=None, wait=None, }] try: if module.check_mode: - import q; q(volume) - module.exit_json(changed=True, volume_id=volume['VolumeId'], volume_size=volume['Size']) + module.exit_json(changed=True, msg='Would have created a snapshot if not in check mode', volume_id=volume['VolumeId'], volume_size=volume['Size']) snapshot = _create_snapshot(ec2, **params) except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: module.fail_json_aws(e, msg="Failed to create snapshot") @@ -325,7 +324,7 @@ def create_snapshot(module, ec2, description=None, wait=None, def delete_snapshot(module, ec2, snapshot_id): if module.check_mode: - module.exit_json(changed=True) + module.exit_json(changed=True, msg='Would have deleted snapshot if not in check mode') try: ec2.delete_snapshot(aws_retry=True, SnapshotId=snapshot_id) except is_boto3_error_code('InvalidSnapshot.NotFound'): From 4cb19c31881117e8e5f52d4e304ebe383d77158f Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Fri, 24 Sep 2021 13:59:10 -0700 Subject: [PATCH 03/13] Integration tests --- plugins/modules/ec2_snapshot.py | 3 +- .../targets/ec2_snapshot/tasks/main.yml | 125 +++++++++--------- 2 files changed, 65 insertions(+), 63 deletions(-) diff --git a/plugins/modules/ec2_snapshot.py b/plugins/modules/ec2_snapshot.py index 778f17b5478..4e26fea3bbd 100644 --- a/plugins/modules/ec2_snapshot.py +++ b/plugins/modules/ec2_snapshot.py @@ -289,7 +289,8 @@ def create_snapshot(module, ec2, description=None, wait=None, }] try: if module.check_mode: - module.exit_json(changed=True, msg='Would have created a snapshot if not in check mode', volume_id=volume['VolumeId'], volume_size=volume['Size']) + module.exit_json(changed=True, msg='Would have created a snapshot if not in check mode', + volume_id=volume['VolumeId'], volume_size=volume['Size']) snapshot = _create_snapshot(ec2, **params) except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: module.fail_json_aws(e, msg="Failed to create snapshot") diff --git a/tests/integration/targets/ec2_snapshot/tasks/main.yml b/tests/integration/targets/ec2_snapshot/tasks/main.yml index 6866789df78..037ece3146c 100644 --- a/tests/integration/targets/ec2_snapshot/tasks/main.yml +++ b/tests/integration/targets/ec2_snapshot/tasks/main.yml @@ -9,9 +9,6 @@ # Tests ec2_snapshot_info: # - Listing snapshots for filter: tag # -# Possible Bugs: -# - check_mode not supported -# - name: Integration testing for ec2_snapshot module_defaults: group/aws: @@ -23,7 +20,6 @@ collections: - community.aws - block: - name: Gather availability zones aws_az_facts: @@ -48,7 +44,7 @@ - untagged_snapshot.snapshots| length == 1 - untagged_snapshot.snapshots[0].volume_id == volume_detached.volume_id - - name: Setup an instance for testing + - name: Setup an instance for testing, make sure volumes are attached before next task ec2_instance: name: '{{ resource_prefix }}' instance_type: t2.nano @@ -60,25 +56,29 @@ volume_size: 8 delete_on_termination: true register: instance + retries: 5 + delay: 60 + until: "instance.instances[0].block_device_mappings | length > 0 and instance.instances[0].block_device_mappings[-1].ebs.status =='attached'" - set_fact: volume_id: '{{ instance.instances[0].block_device_mappings[0].ebs.volume_id }}' instance_id: '{{ instance.instances[0].instance_id }}' device_name: '{{ instance.instances[0].block_device_mappings[0].device_name }}' -# JR: Check mode not supported -# - name: Take snapshot (check mode) -# ec2_snapshot: -# instance_id: '{{ instance_id }}' -# check_mode: true -# snapshot_tags: -# Test: '{{ resource_prefix }}' -# register: result -# - assert: -# that: -# - result is changed - - - name: Take snapshot of volume + - name: Take snapshot (check mode) + ec2_snapshot: + instance_id: '{{ instance_id }}' + device_name: '{{ device_name }}' + snapshot_tags: + Test: '{{ resource_prefix }}' + check_mode: true + register: result + + - assert: + that: + - result is changed + + - name: Take snapshot of volume - first ec2_snapshot: volume_id: '{{ volume_id }}' register: result @@ -105,7 +105,7 @@ filters: "tag:Name": '{{ resource_prefix }}' register: info_check - check_mode: yes + check_mode: true - assert: that: @@ -116,18 +116,17 @@ - info_check.snapshots[0].volume_size == result.volume_size - info_check.snapshots[0].tags == result.tags -# JR: Check mode not supported -# - name: Take snapshot if most recent >1hr (False) (check mode) -# ec2_snapshot: -# volume_id: '{{ volume_id }}' -# snapshot_tags: -# Name: '{{ resource_prefix }}' -# last_snapshot_min_age: 60 -# check_mode: true -# register: result -# - assert: -# that: -# - result is not changed + - name: Take snapshot if most recent >1hr (False) (check mode) + ec2_snapshot: + volume_id: '{{ volume_id }}' + snapshot_tags: + Name: '{{ resource_prefix }}' + last_snapshot_min_age: 60 + check_mode: true + register: result + - assert: + that: + - result is not changed - name: Take snapshot if most recent >1hr (False) ec2_snapshot: @@ -150,20 +149,19 @@ pause: minutes: 1 -# JR: Check mode not supported -# - name: Take snapshot if most recent >1min (True) (check mode) -# ec2_snapshot: -# volume_id: '{{ volume_id }}' -# snapshot_tags: -# Name: '{{ resource_prefix }}' -# last_snapshot_min_age: 1 -# check_mode: true -# register: result -# - assert: -# that: -# - result is changed - - - name: Take snapshot if most recent >1min (True) + - name: Take snapshot if most recent >1min (True) (check mode) + ec2_snapshot: + volume_id: '{{ volume_id }}' + snapshot_tags: + Name: '{{ resource_prefix }}' + last_snapshot_min_age: 1 + check_mode: true + register: result + - assert: + that: + - result is changed + + - name: Take snapshot if most recent >1min (True) - second ec2_snapshot: volume_id: '{{ volume_id }}' last_snapshot_min_age: 1 @@ -181,18 +179,18 @@ - info_result.snapshots| length == 2 - result.snapshot_id in ( info_result.snapshots | map(attribute='snapshot_id') | list ) -# JR: Check mode not supported -# - name: Take snapshot with a tag (check mode) -# ec2_snapshot: -# volume_id: '{{ volume_id }}' -# snapshot_tags: -# MyTag: '{{ resource_prefix }}' -# register: result -# - assert: -# that: -# - result is changed - - - name: Take snapshot and tag it + - name: Take snapshot with a tag (check mode) + ec2_snapshot: + volume_id: '{{ volume_id }}' + snapshot_tags: + MyTag: '{{ resource_prefix }}' + check_mode: true + register: result + - assert: + that: + - result is changed + + - name: Take snapshot and tag it - third ec2_snapshot: volume_id: '{{ volume_id }}' snapshot_tags: @@ -222,11 +220,8 @@ "tag:Name": '{{ resource_prefix }}' register: info_result - - assert: - that: - - info_result.snapshots| length == 3 - - - ec2_snapshot: + - name: Generate extra snapshots - five more + ec2_snapshot: volume_id: '{{ volume_id }}' snapshot_tags: ResourcePrefix: '{{ resource_prefix }}' @@ -242,6 +237,12 @@ pause: minutes: 2 + - name: Get info about all snapshots for this test + ec2_snapshot_info: + filters: + "tag:Name": '{{ resource_prefix }}' + register: info_result + # check that snapshot_ids and max_results are mutually exclusive - name: Check that max_results and snapshot_ids are mutually exclusive ec2_snapshot_info: From 494bd17ffe8501a7a998d0fc78d584bc9bace920 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Fri, 24 Sep 2021 14:03:59 -0700 Subject: [PATCH 04/13] Revert "Integration tests" This reverts commit 4cb19c31881117e8e5f52d4e304ebe383d77158f. Revert push to wrong branch --- plugins/modules/ec2_snapshot.py | 3 +- .../targets/ec2_snapshot/tasks/main.yml | 125 +++++++++--------- 2 files changed, 63 insertions(+), 65 deletions(-) diff --git a/plugins/modules/ec2_snapshot.py b/plugins/modules/ec2_snapshot.py index 4e26fea3bbd..778f17b5478 100644 --- a/plugins/modules/ec2_snapshot.py +++ b/plugins/modules/ec2_snapshot.py @@ -289,8 +289,7 @@ def create_snapshot(module, ec2, description=None, wait=None, }] try: if module.check_mode: - module.exit_json(changed=True, msg='Would have created a snapshot if not in check mode', - volume_id=volume['VolumeId'], volume_size=volume['Size']) + module.exit_json(changed=True, msg='Would have created a snapshot if not in check mode', volume_id=volume['VolumeId'], volume_size=volume['Size']) snapshot = _create_snapshot(ec2, **params) except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: module.fail_json_aws(e, msg="Failed to create snapshot") diff --git a/tests/integration/targets/ec2_snapshot/tasks/main.yml b/tests/integration/targets/ec2_snapshot/tasks/main.yml index 037ece3146c..6866789df78 100644 --- a/tests/integration/targets/ec2_snapshot/tasks/main.yml +++ b/tests/integration/targets/ec2_snapshot/tasks/main.yml @@ -9,6 +9,9 @@ # Tests ec2_snapshot_info: # - Listing snapshots for filter: tag # +# Possible Bugs: +# - check_mode not supported +# - name: Integration testing for ec2_snapshot module_defaults: group/aws: @@ -20,6 +23,7 @@ collections: - community.aws + block: - name: Gather availability zones aws_az_facts: @@ -44,7 +48,7 @@ - untagged_snapshot.snapshots| length == 1 - untagged_snapshot.snapshots[0].volume_id == volume_detached.volume_id - - name: Setup an instance for testing, make sure volumes are attached before next task + - name: Setup an instance for testing ec2_instance: name: '{{ resource_prefix }}' instance_type: t2.nano @@ -56,29 +60,25 @@ volume_size: 8 delete_on_termination: true register: instance - retries: 5 - delay: 60 - until: "instance.instances[0].block_device_mappings | length > 0 and instance.instances[0].block_device_mappings[-1].ebs.status =='attached'" - set_fact: volume_id: '{{ instance.instances[0].block_device_mappings[0].ebs.volume_id }}' instance_id: '{{ instance.instances[0].instance_id }}' device_name: '{{ instance.instances[0].block_device_mappings[0].device_name }}' - - name: Take snapshot (check mode) - ec2_snapshot: - instance_id: '{{ instance_id }}' - device_name: '{{ device_name }}' - snapshot_tags: - Test: '{{ resource_prefix }}' - check_mode: true - register: result - - - assert: - that: - - result is changed - - - name: Take snapshot of volume - first +# JR: Check mode not supported +# - name: Take snapshot (check mode) +# ec2_snapshot: +# instance_id: '{{ instance_id }}' +# check_mode: true +# snapshot_tags: +# Test: '{{ resource_prefix }}' +# register: result +# - assert: +# that: +# - result is changed + + - name: Take snapshot of volume ec2_snapshot: volume_id: '{{ volume_id }}' register: result @@ -105,7 +105,7 @@ filters: "tag:Name": '{{ resource_prefix }}' register: info_check - check_mode: true + check_mode: yes - assert: that: @@ -116,17 +116,18 @@ - info_check.snapshots[0].volume_size == result.volume_size - info_check.snapshots[0].tags == result.tags - - name: Take snapshot if most recent >1hr (False) (check mode) - ec2_snapshot: - volume_id: '{{ volume_id }}' - snapshot_tags: - Name: '{{ resource_prefix }}' - last_snapshot_min_age: 60 - check_mode: true - register: result - - assert: - that: - - result is not changed +# JR: Check mode not supported +# - name: Take snapshot if most recent >1hr (False) (check mode) +# ec2_snapshot: +# volume_id: '{{ volume_id }}' +# snapshot_tags: +# Name: '{{ resource_prefix }}' +# last_snapshot_min_age: 60 +# check_mode: true +# register: result +# - assert: +# that: +# - result is not changed - name: Take snapshot if most recent >1hr (False) ec2_snapshot: @@ -149,19 +150,20 @@ pause: minutes: 1 - - name: Take snapshot if most recent >1min (True) (check mode) - ec2_snapshot: - volume_id: '{{ volume_id }}' - snapshot_tags: - Name: '{{ resource_prefix }}' - last_snapshot_min_age: 1 - check_mode: true - register: result - - assert: - that: - - result is changed - - - name: Take snapshot if most recent >1min (True) - second +# JR: Check mode not supported +# - name: Take snapshot if most recent >1min (True) (check mode) +# ec2_snapshot: +# volume_id: '{{ volume_id }}' +# snapshot_tags: +# Name: '{{ resource_prefix }}' +# last_snapshot_min_age: 1 +# check_mode: true +# register: result +# - assert: +# that: +# - result is changed + + - name: Take snapshot if most recent >1min (True) ec2_snapshot: volume_id: '{{ volume_id }}' last_snapshot_min_age: 1 @@ -179,18 +181,18 @@ - info_result.snapshots| length == 2 - result.snapshot_id in ( info_result.snapshots | map(attribute='snapshot_id') | list ) - - name: Take snapshot with a tag (check mode) - ec2_snapshot: - volume_id: '{{ volume_id }}' - snapshot_tags: - MyTag: '{{ resource_prefix }}' - check_mode: true - register: result - - assert: - that: - - result is changed - - - name: Take snapshot and tag it - third +# JR: Check mode not supported +# - name: Take snapshot with a tag (check mode) +# ec2_snapshot: +# volume_id: '{{ volume_id }}' +# snapshot_tags: +# MyTag: '{{ resource_prefix }}' +# register: result +# - assert: +# that: +# - result is changed + + - name: Take snapshot and tag it ec2_snapshot: volume_id: '{{ volume_id }}' snapshot_tags: @@ -220,8 +222,11 @@ "tag:Name": '{{ resource_prefix }}' register: info_result - - name: Generate extra snapshots - five more - ec2_snapshot: + - assert: + that: + - info_result.snapshots| length == 3 + + - ec2_snapshot: volume_id: '{{ volume_id }}' snapshot_tags: ResourcePrefix: '{{ resource_prefix }}' @@ -237,12 +242,6 @@ pause: minutes: 2 - - name: Get info about all snapshots for this test - ec2_snapshot_info: - filters: - "tag:Name": '{{ resource_prefix }}' - register: info_result - # check that snapshot_ids and max_results are mutually exclusive - name: Check that max_results and snapshot_ids are mutually exclusive ec2_snapshot_info: From 0c3da0558130b1225d49c029f8119a40a124ba43 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Fri, 24 Sep 2021 14:09:49 -0700 Subject: [PATCH 05/13] Integration tests --- .../targets/ec2_snapshot/tasks/main.yml | 125 +++++++++--------- 1 file changed, 63 insertions(+), 62 deletions(-) diff --git a/tests/integration/targets/ec2_snapshot/tasks/main.yml b/tests/integration/targets/ec2_snapshot/tasks/main.yml index 6866789df78..037ece3146c 100644 --- a/tests/integration/targets/ec2_snapshot/tasks/main.yml +++ b/tests/integration/targets/ec2_snapshot/tasks/main.yml @@ -9,9 +9,6 @@ # Tests ec2_snapshot_info: # - Listing snapshots for filter: tag # -# Possible Bugs: -# - check_mode not supported -# - name: Integration testing for ec2_snapshot module_defaults: group/aws: @@ -23,7 +20,6 @@ collections: - community.aws - block: - name: Gather availability zones aws_az_facts: @@ -48,7 +44,7 @@ - untagged_snapshot.snapshots| length == 1 - untagged_snapshot.snapshots[0].volume_id == volume_detached.volume_id - - name: Setup an instance for testing + - name: Setup an instance for testing, make sure volumes are attached before next task ec2_instance: name: '{{ resource_prefix }}' instance_type: t2.nano @@ -60,25 +56,29 @@ volume_size: 8 delete_on_termination: true register: instance + retries: 5 + delay: 60 + until: "instance.instances[0].block_device_mappings | length > 0 and instance.instances[0].block_device_mappings[-1].ebs.status =='attached'" - set_fact: volume_id: '{{ instance.instances[0].block_device_mappings[0].ebs.volume_id }}' instance_id: '{{ instance.instances[0].instance_id }}' device_name: '{{ instance.instances[0].block_device_mappings[0].device_name }}' -# JR: Check mode not supported -# - name: Take snapshot (check mode) -# ec2_snapshot: -# instance_id: '{{ instance_id }}' -# check_mode: true -# snapshot_tags: -# Test: '{{ resource_prefix }}' -# register: result -# - assert: -# that: -# - result is changed - - - name: Take snapshot of volume + - name: Take snapshot (check mode) + ec2_snapshot: + instance_id: '{{ instance_id }}' + device_name: '{{ device_name }}' + snapshot_tags: + Test: '{{ resource_prefix }}' + check_mode: true + register: result + + - assert: + that: + - result is changed + + - name: Take snapshot of volume - first ec2_snapshot: volume_id: '{{ volume_id }}' register: result @@ -105,7 +105,7 @@ filters: "tag:Name": '{{ resource_prefix }}' register: info_check - check_mode: yes + check_mode: true - assert: that: @@ -116,18 +116,17 @@ - info_check.snapshots[0].volume_size == result.volume_size - info_check.snapshots[0].tags == result.tags -# JR: Check mode not supported -# - name: Take snapshot if most recent >1hr (False) (check mode) -# ec2_snapshot: -# volume_id: '{{ volume_id }}' -# snapshot_tags: -# Name: '{{ resource_prefix }}' -# last_snapshot_min_age: 60 -# check_mode: true -# register: result -# - assert: -# that: -# - result is not changed + - name: Take snapshot if most recent >1hr (False) (check mode) + ec2_snapshot: + volume_id: '{{ volume_id }}' + snapshot_tags: + Name: '{{ resource_prefix }}' + last_snapshot_min_age: 60 + check_mode: true + register: result + - assert: + that: + - result is not changed - name: Take snapshot if most recent >1hr (False) ec2_snapshot: @@ -150,20 +149,19 @@ pause: minutes: 1 -# JR: Check mode not supported -# - name: Take snapshot if most recent >1min (True) (check mode) -# ec2_snapshot: -# volume_id: '{{ volume_id }}' -# snapshot_tags: -# Name: '{{ resource_prefix }}' -# last_snapshot_min_age: 1 -# check_mode: true -# register: result -# - assert: -# that: -# - result is changed - - - name: Take snapshot if most recent >1min (True) + - name: Take snapshot if most recent >1min (True) (check mode) + ec2_snapshot: + volume_id: '{{ volume_id }}' + snapshot_tags: + Name: '{{ resource_prefix }}' + last_snapshot_min_age: 1 + check_mode: true + register: result + - assert: + that: + - result is changed + + - name: Take snapshot if most recent >1min (True) - second ec2_snapshot: volume_id: '{{ volume_id }}' last_snapshot_min_age: 1 @@ -181,18 +179,18 @@ - info_result.snapshots| length == 2 - result.snapshot_id in ( info_result.snapshots | map(attribute='snapshot_id') | list ) -# JR: Check mode not supported -# - name: Take snapshot with a tag (check mode) -# ec2_snapshot: -# volume_id: '{{ volume_id }}' -# snapshot_tags: -# MyTag: '{{ resource_prefix }}' -# register: result -# - assert: -# that: -# - result is changed - - - name: Take snapshot and tag it + - name: Take snapshot with a tag (check mode) + ec2_snapshot: + volume_id: '{{ volume_id }}' + snapshot_tags: + MyTag: '{{ resource_prefix }}' + check_mode: true + register: result + - assert: + that: + - result is changed + + - name: Take snapshot and tag it - third ec2_snapshot: volume_id: '{{ volume_id }}' snapshot_tags: @@ -222,11 +220,8 @@ "tag:Name": '{{ resource_prefix }}' register: info_result - - assert: - that: - - info_result.snapshots| length == 3 - - - ec2_snapshot: + - name: Generate extra snapshots - five more + ec2_snapshot: volume_id: '{{ volume_id }}' snapshot_tags: ResourcePrefix: '{{ resource_prefix }}' @@ -242,6 +237,12 @@ pause: minutes: 2 + - name: Get info about all snapshots for this test + ec2_snapshot_info: + filters: + "tag:Name": '{{ resource_prefix }}' + register: info_result + # check that snapshot_ids and max_results are mutually exclusive - name: Check that max_results and snapshot_ids are mutually exclusive ec2_snapshot_info: From c1e98843872c8e3724939cd845ab6eee0d50ff76 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Fri, 24 Sep 2021 14:16:25 -0700 Subject: [PATCH 06/13] Sanity fix --- plugins/modules/ec2_snapshot.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/plugins/modules/ec2_snapshot.py b/plugins/modules/ec2_snapshot.py index 778f17b5478..49e7da0ccd7 100644 --- a/plugins/modules/ec2_snapshot.py +++ b/plugins/modules/ec2_snapshot.py @@ -71,12 +71,10 @@ required: false default: 0 type: int - author: "Will Thames (@willthames)" extends_documentation_fragment: - amazon.aws.aws - amazon.aws.ec2 - ''' EXAMPLES = ''' @@ -84,13 +82,11 @@ - amazon.aws.ec2_snapshot: volume_id: vol-abcdef12 description: snapshot of /data from DB123 taken 2013/11/28 12:18:32 - # Snapshot of volume mounted on device_name attached to instance_id - amazon.aws.ec2_snapshot: instance_id: i-12345678 device_name: /dev/sdb1 description: snapshot of /data from DB123 taken 2013/11/28 12:18:32 - # Snapshot of volume with tagging - amazon.aws.ec2_snapshot: instance_id: i-12345678 @@ -98,12 +94,10 @@ snapshot_tags: frequency: hourly source: /data - # Remove a snapshot - amazon.aws.ec2_snapshot: snapshot_id: snap-abcd1234 state: absent - # Create a snapshot only if the most recent one is older than 1 hour - amazon.aws.ec2_snapshot: volume_id: vol-abcdef12 @@ -289,7 +283,8 @@ def create_snapshot(module, ec2, description=None, wait=None, }] try: if module.check_mode: - module.exit_json(changed=True, msg='Would have created a snapshot if not in check mode', volume_id=volume['VolumeId'], volume_size=volume['Size']) + module.exit_json(changed=True, msg='Would have created a snapshot if not in check mode', + volume_id=volume['VolumeId'], volume_size=volume['Size']) snapshot = _create_snapshot(ec2, **params) except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: module.fail_json_aws(e, msg="Failed to create snapshot") From 45141ca681ac03c4cdc80ba3ee4e225132be396f Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Fri, 24 Sep 2021 14:39:42 -0700 Subject: [PATCH 07/13] Added changelogs fragment --- .../fragments/512-ec2_snapshot_add_check_mode_support.yml.yml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/fragments/512-ec2_snapshot_add_check_mode_support.yml.yml diff --git a/changelogs/fragments/512-ec2_snapshot_add_check_mode_support.yml.yml b/changelogs/fragments/512-ec2_snapshot_add_check_mode_support.yml.yml new file mode 100644 index 00000000000..bebb68ada24 --- /dev/null +++ b/changelogs/fragments/512-ec2_snapshot_add_check_mode_support.yml.yml @@ -0,0 +1,3 @@ +minor_changes: + - ec2_snapshot - add check_mode support + (https://github.com/ansible-collections/amazon.aws/pull/512) \ No newline at end of file From 0f6fe15e492a54c35b86948b417e0948d78301f9 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Fri, 24 Sep 2021 14:41:06 -0700 Subject: [PATCH 08/13] Changelogs fragment - line at EOF --- .../fragments/512-ec2_snapshot_add_check_mode_support.yml.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/fragments/512-ec2_snapshot_add_check_mode_support.yml.yml b/changelogs/fragments/512-ec2_snapshot_add_check_mode_support.yml.yml index bebb68ada24..6bf8a066fbd 100644 --- a/changelogs/fragments/512-ec2_snapshot_add_check_mode_support.yml.yml +++ b/changelogs/fragments/512-ec2_snapshot_add_check_mode_support.yml.yml @@ -1,3 +1,3 @@ minor_changes: - ec2_snapshot - add check_mode support - (https://github.com/ansible-collections/amazon.aws/pull/512) \ No newline at end of file + (https://github.com/ansible-collections/amazon.aws/pull/512) From 06187fce31f40f6d22b941cca6b05aaa4690f55a Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Fri, 24 Sep 2021 14:48:19 -0700 Subject: [PATCH 09/13] Remove debugging text --- tests/integration/targets/ec2_snapshot/tasks/main.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/targets/ec2_snapshot/tasks/main.yml b/tests/integration/targets/ec2_snapshot/tasks/main.yml index 037ece3146c..179c07face3 100644 --- a/tests/integration/targets/ec2_snapshot/tasks/main.yml +++ b/tests/integration/targets/ec2_snapshot/tasks/main.yml @@ -78,7 +78,7 @@ that: - result is changed - - name: Take snapshot of volume - first + - name: Take snapshot of volume ec2_snapshot: volume_id: '{{ volume_id }}' register: result @@ -161,7 +161,7 @@ that: - result is changed - - name: Take snapshot if most recent >1min (True) - second + - name: Take snapshot if most recent >1min (True) ec2_snapshot: volume_id: '{{ volume_id }}' last_snapshot_min_age: 1 @@ -190,7 +190,7 @@ that: - result is changed - - name: Take snapshot and tag it - third + - name: Take snapshot and tag it ec2_snapshot: volume_id: '{{ volume_id }}' snapshot_tags: @@ -220,7 +220,7 @@ "tag:Name": '{{ resource_prefix }}' register: info_result - - name: Generate extra snapshots - five more + - name: Generate extra snapshots ec2_snapshot: volume_id: '{{ volume_id }}' snapshot_tags: From 8c37000ab1dc3042a177c6024e52e9996ca53676 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Mon, 27 Sep 2021 16:35:54 -0700 Subject: [PATCH 10/13] Modified based on feedback --- .../512-ec2_snapshot_add_check_mode_support.yml.yml | 2 +- plugins/modules/ec2_snapshot.py | 6 +++++- tests/integration/targets/ec2_snapshot/tasks/main.yml | 4 ++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/changelogs/fragments/512-ec2_snapshot_add_check_mode_support.yml.yml b/changelogs/fragments/512-ec2_snapshot_add_check_mode_support.yml.yml index 6bf8a066fbd..acc069ad913 100644 --- a/changelogs/fragments/512-ec2_snapshot_add_check_mode_support.yml.yml +++ b/changelogs/fragments/512-ec2_snapshot_add_check_mode_support.yml.yml @@ -1,3 +1,3 @@ minor_changes: - ec2_snapshot - add check_mode support - (https://github.com/ansible-collections/amazon.aws/pull/512) + (https://github.com/ansible-collections/amazon.aws/pull/512). diff --git a/plugins/modules/ec2_snapshot.py b/plugins/modules/ec2_snapshot.py index 49e7da0ccd7..d14ecb57b0b 100644 --- a/plugins/modules/ec2_snapshot.py +++ b/plugins/modules/ec2_snapshot.py @@ -82,11 +82,13 @@ - amazon.aws.ec2_snapshot: volume_id: vol-abcdef12 description: snapshot of /data from DB123 taken 2013/11/28 12:18:32 + # Snapshot of volume mounted on device_name attached to instance_id - amazon.aws.ec2_snapshot: instance_id: i-12345678 device_name: /dev/sdb1 description: snapshot of /data from DB123 taken 2013/11/28 12:18:32 + # Snapshot of volume with tagging - amazon.aws.ec2_snapshot: instance_id: i-12345678 @@ -94,10 +96,12 @@ snapshot_tags: frequency: hourly source: /data + # Remove a snapshot - amazon.aws.ec2_snapshot: snapshot_id: snap-abcd1234 state: absent + # Create a snapshot only if the most recent one is older than 1 hour - amazon.aws.ec2_snapshot: volume_id: vol-abcdef12 @@ -363,7 +367,7 @@ def create_snapshot_ansible_module(): required_if=required_if, required_one_of=required_one_of, required_together=required_together, - supports_check_mode=True + supports_check_mode=True, ) return module diff --git a/tests/integration/targets/ec2_snapshot/tasks/main.yml b/tests/integration/targets/ec2_snapshot/tasks/main.yml index 179c07face3..27c394822d3 100644 --- a/tests/integration/targets/ec2_snapshot/tasks/main.yml +++ b/tests/integration/targets/ec2_snapshot/tasks/main.yml @@ -220,6 +220,10 @@ "tag:Name": '{{ resource_prefix }}' register: info_result + - assert: + that: + - info_result.snapshots | length == 3 + - name: Generate extra snapshots ec2_snapshot: volume_id: '{{ volume_id }}' From 48e54df5fcecacee3bb8ad5047260a38d0e7eb96 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Mon, 27 Sep 2021 19:38:28 -0700 Subject: [PATCH 11/13] Fix: deletion idempotentcy in check mode --- plugins/modules/ec2_snapshot.py | 6 ++++- .../targets/ec2_snapshot/tasks/main.yml | 27 +++++++++++++++++-- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/plugins/modules/ec2_snapshot.py b/plugins/modules/ec2_snapshot.py index d14ecb57b0b..852102457de 100644 --- a/plugins/modules/ec2_snapshot.py +++ b/plugins/modules/ec2_snapshot.py @@ -323,7 +323,11 @@ def create_snapshot(module, ec2, description=None, wait=None, def delete_snapshot(module, ec2, snapshot_id): if module.check_mode: - module.exit_json(changed=True, msg='Would have deleted snapshot if not in check mode') + try: + _describe_snapshots(ec2, SnapshotIds=[(snapshot_id)]) + module.exit_json(changed=True, msg='Would have deleted snapshot if not in check mode') + except is_boto3_error_code('InvalidSnapshot.NotFound'): + module.exit_json(changed=False, msg='Invalid snapshot ID - snapshot not found') try: ec2.delete_snapshot(aws_retry=True, SnapshotId=snapshot_id) except is_boto3_error_code('InvalidSnapshot.NotFound'): diff --git a/tests/integration/targets/ec2_snapshot/tasks/main.yml b/tests/integration/targets/ec2_snapshot/tasks/main.yml index 27c394822d3..79b1255f1fc 100644 --- a/tests/integration/targets/ec2_snapshot/tasks/main.yml +++ b/tests/integration/targets/ec2_snapshot/tasks/main.yml @@ -55,9 +55,8 @@ ebs: volume_size: 8 delete_on_termination: true + state: running register: instance - retries: 5 - delay: 60 until: "instance.instances[0].block_device_mappings | length > 0 and instance.instances[0].block_device_mappings[-1].ebs.status =='attached'" - set_fact: @@ -300,12 +299,36 @@ that: - info_result.snapshots | length == 3 + # delete the tagged snapshot - check mode + - name: Delete the tagged snapshot (check mode) + ec2_snapshot: + state: absent + snapshot_id: '{{ tagged_snapshot_id }}' + register: delete_result_check_mode + check_mode: true + + - assert: + that: + - delete_result_check_mode is changed + # delete the tagged snapshot - name: Delete the tagged snapshot ec2_snapshot: state: absent snapshot_id: '{{ tagged_snapshot_id }}' + # delete the tagged snapshot again (results in InvalidSnapshot.NotFound) + - name: Delete already removed snapshot (check mode) + ec2_snapshot: + state: absent + snapshot_id: '{{ tagged_snapshot_id }}' + register: delete_result_second_check_mode + check_mode: true + + - assert: + that: + - delete_result_second_check_mode is not changed + - name: Get info about all snapshots for this test ec2_snapshot_info: filters: From 9e4848a1996640929fcc26e56a71522d2d2bddd1 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Tue, 28 Sep 2021 13:58:24 -0700 Subject: [PATCH 12/13] Modified based on feedback --- tests/integration/targets/ec2_snapshot/tasks/main.yml | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/integration/targets/ec2_snapshot/tasks/main.yml b/tests/integration/targets/ec2_snapshot/tasks/main.yml index 79b1255f1fc..00f874f51dc 100644 --- a/tests/integration/targets/ec2_snapshot/tasks/main.yml +++ b/tests/integration/targets/ec2_snapshot/tasks/main.yml @@ -49,15 +49,14 @@ name: '{{ resource_prefix }}' instance_type: t2.nano image_id: '{{ ec2_ami_id }}' - wait: yes volumes: - device_name: /dev/xvda ebs: volume_size: 8 delete_on_termination: true state: running + wait: true register: instance - until: "instance.instances[0].block_device_mappings | length > 0 and instance.instances[0].block_device_mappings[-1].ebs.status =='attached'" - set_fact: volume_id: '{{ instance.instances[0].block_device_mappings[0].ebs.volume_id }}' @@ -240,12 +239,6 @@ pause: minutes: 2 - - name: Get info about all snapshots for this test - ec2_snapshot_info: - filters: - "tag:Name": '{{ resource_prefix }}' - register: info_result - # check that snapshot_ids and max_results are mutually exclusive - name: Check that max_results and snapshot_ids are mutually exclusive ec2_snapshot_info: From 561821d023f8a609a89cce8f2ca676ef616112cc Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Fri, 1 Oct 2021 00:07:46 -0700 Subject: [PATCH 13/13] Added delete integration test (idempotent) --- tests/integration/targets/ec2_snapshot/tasks/main.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/integration/targets/ec2_snapshot/tasks/main.yml b/tests/integration/targets/ec2_snapshot/tasks/main.yml index 00f874f51dc..63fadd60d38 100644 --- a/tests/integration/targets/ec2_snapshot/tasks/main.yml +++ b/tests/integration/targets/ec2_snapshot/tasks/main.yml @@ -322,6 +322,16 @@ that: - delete_result_second_check_mode is not changed + - name: Delete already removed snapshot (idempotent) + ec2_snapshot: + state: absent + snapshot_id: '{{ tagged_snapshot_id }}' + register: delete_result_second_idempotent + + - assert: + that: + - delete_result_second_idempotent is not changed + - name: Get info about all snapshots for this test ec2_snapshot_info: filters: