Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ec2_vol: Add check_mode support to ec2_vol #509

Merged
3 changes: 3 additions & 0 deletions changelogs/fragments/509-ec2_vol_add_check_mode_support.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
minor_changes:
- ec2_vol - add check_mode support
(https://github.com/ansible-collections/amazon.aws/pull/509).
18 changes: 18 additions & 0 deletions plugins/modules/ec2_vol.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,8 @@ def update_volume(module, ec2_conn, volume):
changed = iops_changed or size_changed or type_changed or throughput_changed or multi_attach_changed

if changed:
if module.check_mode:
module.exit_json(changed=True, msg='Would have updated volume if not in check mode.')
response = ec2_conn.modify_volume(**req_obj)

volume['size'] = response.get('VolumeModification').get('TargetSize')
Expand All @@ -457,6 +459,9 @@ def create_volume(module, ec2_conn, zone):

volume = get_volume(module, ec2_conn)

if module.check_mode:
module.exit_json(changed=True, msg='Would have created a volume if not in check mode.')

if volume is None:
Comment on lines +462 to 465
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor niggle for the future (not worth blocking this over). I find it's less likely to lead to weird edge cases if you make your check_mode tests as late as possible. Here for example, if someone re-wrote the flow of main() and you'd sometimes be passed a volume, performing the check_mode test before you test volume would result in broken idempotency.


try:
Expand Down Expand Up @@ -514,6 +519,10 @@ def attach_volume(module, ec2_conn, volume_dict, instance_dict, device_name):

attachment_data = get_attachment_data(volume_dict, wanted_state='attached')
if attachment_data:
if module.check_mode:
if attachment_data[0].get('status') in ['attached', 'attaching']:
module.exit_json(changed=False, msg='IN CHECK MODE - volume already attached to instance: {1}.'.format(
attachment_data[0].get('instance_id', None)))
if not volume_dict['multi_attach_enabled']:
# volumes without MultiAttach Enabled can be attached to 1 instance only
if attachment_data[0].get('instance_id', None) != instance_dict['instance_id']:
Expand All @@ -523,6 +532,8 @@ def attach_volume(module, ec2_conn, volume_dict, instance_dict, device_name):
return volume_dict, changed

try:
if module.check_mode:
module.exit_json(changed=True, msg='Would have attached volume if not in check mode.')
attach_response = ec2_conn.attach_volume(aws_retry=True, Device=device_name,
InstanceId=instance_dict['instance_id'],
VolumeId=volume_dict['volume_id'])
Expand Down Expand Up @@ -610,6 +621,8 @@ def detach_volume(module, ec2_conn, volume_dict):
attachment_data = get_attachment_data(volume_dict, wanted_state='attached')
# The ID of the instance must be specified if you are detaching a Multi-Attach enabled volume.
for attachment in attachment_data:
if module.check_mode:
module.exit_json(changed=True, msg='Would have detached volume if not in check mode.')
ec2_conn.detach_volume(aws_retry=True, InstanceId=attachment['instance_id'], VolumeId=volume_dict['volume_id'])
waiter = ec2_conn.get_waiter('volume_available')
waiter.wait(
Expand Down Expand Up @@ -662,6 +675,8 @@ def get_mapped_block_device(instance_dict=None, device_name=None):


def ensure_tags(module, connection, res_id, res_type, tags, purge_tags):
if module.check_mode:
return {}, True
changed = ensure_ec2_tags(connection, module, res_id, res_type, tags, purge_tags, ['InvalidVolume.NotFound'])
final_tags = describe_ec2_tags(connection, module, res_id, res_type)

Expand Down Expand Up @@ -696,6 +711,7 @@ def main():
['volume_type', 'io1', ['iops']],
['volume_type', 'io2', ['iops']],
],
supports_check_mode=True,
)

param_id = module.params.get('id')
Expand Down Expand Up @@ -837,6 +853,8 @@ def main():
if not name and not param_id:
module.fail_json('A volume name or id is required for deletion')
if volume:
if module.check_mode:
module.exit_json(changed=True, msg='Would have deleted volume if not in check mode.')
detach_volume(module, ec2_conn, volume_dict=volume)
changed = delete_volume(module, ec2_conn, volume_id=volume['volume_id'])
module.exit_json(changed=changed)
Expand Down
163 changes: 163 additions & 0 deletions tests/integration/targets/ec2_vol/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,20 @@

# # ==== ec2_vol tests ===============================================

- name: create a volume (validate module defaults - check_mode)
ec2_vol:
volume_size: 1
zone: "{{ availability_zone }}"
tags:
ResourcePrefix: "{{ resource_prefix }}"
check_mode: true
register: volume1_check_mode

- assert:
that:
- volume1_check_mode is changed


- name: create a volume (validate module defaults)
ec2_vol:
volume_size: 1
Expand Down Expand Up @@ -147,6 +161,23 @@
that:
- vol1_snapshot.changed

- name: create a volume from a snapshot (check_mode)
ec2_vol:
snapshot: "{{ vol1_snapshot.snapshot_id }}"
encrypted: yes
volume_type: gp2
volume_size: 1
zone: "{{ availability_zone }}"
tags:
ResourcePrefix: "{{ resource_prefix }}"
check_mode: true
register: volume3_check_mode

- name: check task return attributes
assert:
that:
- volume3_check_mode.changed

- name: create a volume from a snapshot
ec2_vol:
snapshot: "{{ vol1_snapshot.snapshot_id }}"
Expand All @@ -171,6 +202,19 @@
image_id: "{{ ec2_ami_id }}"
wait: True

- name: attach existing volume to an instance (check_mode)
ec2_vol:
id: "{{ volume1.volume_id }}"
instance: "{{ test_instance.instance_ids[0] }}"
device_name: /dev/sdg
delete_on_termination: no
check_mode: true
register: vol_attach_result_check_mode

- assert:
that:
- vol_attach_result_check_mode is changed

- name: attach existing volume to an instance
ec2_vol:
id: "{{ volume1.volume_id }}"
Expand All @@ -190,6 +234,19 @@
- vol_attach_result.volume.attachment_set[0].device == '/dev/sdg'
- not vol_attach_result.volume.attachment_set[0].delete_on_termination

- name: attach existing volume to an instance (idempotent - check_mode)
ec2_vol:
id: "{{ volume1.volume_id }}"
instance: "{{ test_instance.instance_ids[0] }}"
device_name: /dev/sdg
delete_on_termination: no
check_mode: true
register: vol_attach_result_check_mode

- assert:
that:
- vol_attach_result_check_mode is not changed

- name: attach existing volume to an instance (idempotent)
ec2_vol:
id: "{{ volume1.volume_id }}"
Expand All @@ -204,6 +261,26 @@
- "not vol_attach_result.changed"
- vol_attach_result.volume.attachment_set[0].status in ['attached', 'attaching']

- name: attach a new volume to an instance (check_mode)
ec2_vol:
instance: "{{ test_instance.instance_ids[0] }}"
device_name: /dev/sdh
volume_size: 1
volume_type: gp2
name: '{{ resource_prefix }} - sdh'
tags:
"lowercase spaced": 'hello cruel world'
"Title Case": 'Hello Cruel World'
CamelCase: 'SimpleCamelCase'
snake_case: 'simple_snake_case'
ResourcePrefix: "{{ resource_prefix }}"
check_mode: true
register: new_vol_attach_result_check_mode

- assert:
that:
- new_vol_attach_result_check_mode is changed

- name: attach a new volume to an instance
ec2_vol:
instance: "{{ test_instance.instance_ids[0] }}"
Expand Down Expand Up @@ -234,6 +311,21 @@
- new_vol_attach_result.volume.tags["snake_case"] == 'simple_snake_case'
- new_vol_attach_result.volume.tags["Name"] == '{{ resource_prefix }} - sdh'

- name: attach a new volume to an instance (idempotent - check_mode)
ec2_vol:
instance: "{{ test_instance.instance_ids[0] }}"
device_name: /dev/sdh
volume_size: 1
volume_type: gp2
tags:
ResourcePrefix: "{{ resource_prefix }}"
check_mode: true
register: new_vol_attach_result_idem_check_mode
ignore_errors: true

- assert:
that:
- new_vol_attach_result_idem_check_mode is not changed

- name: attach a new volume to an instance (idempotent)
ec2_vol:
Expand Down Expand Up @@ -288,6 +380,20 @@
- new_vol_attach_result.volume.tags["ResourcePrefix"] == resource_prefix
- new_vol_attach_result.volume.tags["Name"] == '{{ resource_prefix }} - sdh'

- name: create a volume from a snapshot and attach to the instance (check_mode)
ec2_vol:
instance: "{{ test_instance.instance_ids[0] }}"
device_name: /dev/sdi
snapshot: "{{ vol1_snapshot.snapshot_id }}"
tags:
ResourcePrefix: "{{ resource_prefix }}"
check_mode: true
register: attach_new_vol_from_snapshot_result_check_mode

- assert:
that:
- attach_new_vol_from_snapshot_result_check_mode is changed


- name: create a volume from a snapshot and attach to the instance
ec2_vol:
Expand Down Expand Up @@ -352,6 +458,19 @@
that:
- not changed_gp3_volume.changed

- name: change existing volume to gp3 (check_mode)
ec2_vol:
id: "{{ new_vol_attach_result.volume_id }}"
zone: "{{ availability_zone }}"
volume_type: gp3
modify_volume: yes
check_mode: true
register: changed_gp3_volume_check_mode

- assert:
that:
- changed_gp3_volume_check_mode is changed

- name: change existing volume to gp3
ec2_vol:
id: "{{ new_vol_attach_result.volume_id }}"
Expand Down Expand Up @@ -435,6 +554,17 @@
vars:
v: "{{ verify_gp3_change.volumes[0] }}"

- name: detach volume from the instance (check_mode)
ec2_vol:
id: "{{ new_vol_attach_result.volume_id }}"
instance: ""
check_mode: true
register: new_vol_attach_result_check_mode

- assert:
that:
- new_vol_attach_result_check_mode is changed

- name: detach volume from the instance
ec2_vol:
id: "{{ new_vol_attach_result.volume_id }}"
Expand All @@ -447,6 +577,17 @@
- new_vol_attach_result.changed
- new_vol_attach_result.volume.status == 'available'

- name: detach volume from the instance (idempotent - check_mode)
ec2_vol:
id: "{{ new_vol_attach_result.volume_id }}"
instance: ""
register: new_vol_attach_result_idem_check_mode

- name: check task return attributes
assert:
that:
- not new_vol_attach_result_idem_check_mode.changed

- name: detach volume from the instance (idempotent)
ec2_vol:
id: "{{ new_vol_attach_result.volume_id }}"
Expand All @@ -458,6 +599,17 @@
that:
- not new_vol_attach_result_idem.changed

- name: delete volume (check_mode)
ec2_vol:
id: "{{ volume2.volume_id }}"
state: absent
check_mode: true
register: delete_volume_result_check_mode

- assert:
that:
- delete_volume_result_check_mode is changed

- name: delete volume
ec2_vol:
id: "{{ volume2.volume_id }}"
Expand All @@ -469,6 +621,17 @@
that:
- "delete_volume_result.changed"

- name: delete volume (idempotent - check_mode)
ec2_vol:
id: "{{ volume2.volume_id }}"
state: absent
check_mode: true
register: delete_volume_result_check_mode

- assert:
that:
- delete_volume_result_check_mode is not changed

- name: delete volume (idempotent)
ec2_vol:
id: "{{ volume2.volume_id }}"
Expand Down