From 11c0be06fb2fc7e59abcdfe46c9fba9887c30dbf Mon Sep 17 00:00:00 2001 From: Rob Date: Thu, 1 Oct 2015 13:23:41 +1000 Subject: [PATCH] Update ec2_vol.py Changed=true now reported on new volume. Only detach volume when instance is specified as 'None' or '' rather than whenever instance is not specified at all Fix regression caused by 6b27cdc where by no volume is created if id or Name is not supplied Remove unnecessary empty aliases Corrected example to use acceptable parameter for ions Added exception handling to get_all_instances call Moved the attachment state validation code to attach_volume function rather than create_volume function Refactored attach_volume and detach_volume so that changed state can be passed back to call Created get_volume_info function so that state=present and state=list can return the same data. Also added instance_id as a returned value in attachment_set dict Updated aws connection method so that boto profile can be used --- lib/ansible/modules/cloud/amazon/ec2_vol.py | 220 +++++++++++--------- 1 file changed, 124 insertions(+), 96 deletions(-) diff --git a/lib/ansible/modules/cloud/amazon/ec2_vol.py b/lib/ansible/modules/cloud/amazon/ec2_vol.py index 6e5e20beeaa..e780bd133b0 100644 --- a/lib/ansible/modules/cloud/amazon/ec2_vol.py +++ b/lib/ansible/modules/cloud/amazon/ec2_vol.py @@ -27,41 +27,35 @@ options: - instance ID if you wish to attach the volume. Since 1.9 you can set to None to detach. required: false default: null - aliases: [] name: description: - volume Name tag if you wish to attach an existing volume (requires instance) required: false default: null - aliases: [] version_added: "1.6" id: description: - volume id if you wish to attach an existing volume (requires instance) or remove an existing volume required: false default: null - aliases: [] version_added: "1.6" volume_size: description: - size of volume (in GB) to create. required: false default: null - aliases: [] volume_type: description: - Type of EBS volume; standard (magnetic), gp2 (SSD), io1 (Provisioned IOPS). "Standard" is the old EBS default and continues to remain the Ansible default for backwards compatibility. required: false default: standard - aliases: [] version_added: "1.9" iops: description: - the provisioned IOPs you want to associate with this volume (integer). required: false default: 100 - aliases: [] version_added: "1.3" encrypted: description: @@ -73,13 +67,6 @@ options: - device id to override device mapping. Assumes /dev/sdf for Linux/UNIX and /dev/xvdf for Windows. required: false default: null - aliases: [] - region: - description: - - The AWS region to use. If not specified then the value of the EC2_REGION environment variable, if any, is used. - required: false - default: null - aliases: ['aws_region', 'ec2_region'] zone: description: - zone in which to create the volume, if unset uses the zone the instance is in (if set) @@ -98,7 +85,6 @@ options: required: false default: "yes" choices: ["yes", "no"] - aliases: [] version_added: "1.5" state: description: @@ -122,7 +108,7 @@ EXAMPLES = ''' - ec2_vol: instance: XXXXXX volume_size: 5 - iops: 200 + iops: 100 device_name: sdd # Example using snapshot id @@ -193,6 +179,7 @@ from distutils.version import LooseVersion try: import boto.ec2 + from boto.exception import BotoServerError HAS_BOTO = True except ImportError: HAS_BOTO = False @@ -204,6 +191,11 @@ def get_volume(module, ec2): zone = module.params.get('zone') filters = {} volume_ids = None + + # If no name or id supplied, just try volume creation based on module parameters + if id is None and name is None: + return None + if zone: filters['availability_zone'] = zone if name: @@ -223,18 +215,20 @@ def get_volume(module, ec2): module.fail_json(msg=msg) else: return None + if len(vols) > 1: module.fail_json(msg="Found more than one volume in zone (if specified) with name: %s" % name) return vols[0] def get_volumes(module, ec2): + instance = module.params.get('instance') - if not instance: - module.fail_json(msg = "Instance must be specified to get volumes") - try: - vols = ec2.get_all_volumes(filters={'attachment.instance-id': instance}) + if not instance: + vols = ec2.get_all_volumes() + else: + vols = ec2.get_all_volumes(filters={'attachment.instance-id': instance}) except boto.exception.BotoServerError, e: module.fail_json(msg = "%s: %s" % (e.error_code, e.error_message)) return vols @@ -258,7 +252,9 @@ def boto_supports_volume_encryption(): """ return hasattr(boto, 'Version') and LooseVersion(boto.Version) >= LooseVersion('2.29.0') + def create_volume(module, ec2, zone): + changed = False name = module.params.get('name') id = module.params.get('id') instance = module.params.get('instance') @@ -271,30 +267,15 @@ def create_volume(module, ec2, zone): if iops: volume_type = 'io1' - if instance == 'None' or instance == '': - instance = None - volume = get_volume(module, ec2) - if volume: - if volume.attachment_state() is not None: - if instance is None: - return volume - adata = volume.attach_data - if adata.instance_id != instance: - module.fail_json(msg = "Volume %s is already attached to another instance: %s" - % (name or id, adata.instance_id)) - else: - module.exit_json(msg="Volume %s is already mapped on instance %s: %s" % - (name or id, adata.instance_id, adata.device), - volume_id=id, - device=adata.device, - changed=False) - else: + if volume is None: try: if boto_supports_volume_encryption(): volume = ec2.create_volume(volume_size, zone, snapshot, volume_type, iops, encrypted) + changed = True else: volume = ec2.create_volume(volume_size, zone, snapshot, volume_type, iops) + changed = True while volume.status != 'available': time.sleep(3) @@ -305,52 +286,89 @@ def create_volume(module, ec2, zone): except boto.exception.BotoServerError, e: module.fail_json(msg = "%s: %s" % (e.error_code, e.error_message)) - return volume + return volume, changed def attach_volume(module, ec2, volume, instance): + device_name = module.params.get('device_name') - - if device_name and instance: - try: - attach = volume.attach(instance.id, device_name) - while volume.attachment_state() != 'attached': - time.sleep(3) - volume.update() - except boto.exception.BotoServerError, e: - module.fail_json(msg = "%s: %s" % (e.error_code, e.error_message)) - + changed = False + # If device_name isn't set, make a choice based on best practices here: # http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/block-device-mapping-concepts.html - + # In future this needs to be more dynamic but combining block device mapping best practices # (bounds for devices, as above) with instance.block_device_mapping data would be tricky. For me ;) - + # Use password data attribute to tell whether the instance is Windows or Linux - if device_name is None and instance: + if device_name is None: try: if not ec2.get_password_data(instance.id): device_name = '/dev/sdf' - attach = volume.attach(instance.id, device_name) - while volume.attachment_state() != 'attached': - time.sleep(3) - volume.update() else: device_name = '/dev/xvdf' - attach = volume.attach(instance.id, device_name) - while volume.attachment_state() != 'attached': - time.sleep(3) - volume.update() except boto.exception.BotoServerError, e: module.fail_json(msg = "%s: %s" % (e.error_code, e.error_message)) - -def detach_volume(module, ec2): - vol = get_volume(module, ec2) - if not vol or vol.attachment_state() is None: - module.exit_json(changed=False) + + if volume.attachment_state() is not None: + adata = volume.attach_data + if adata.instance_id != instance.id: + module.fail_json(msg = "Volume %s is already attached to another instance: %s" + % (volume.id, adata.instance_id)) else: - vol.detach() - module.exit_json(changed=True) + try: + volume.attach(instance.id, device_name) + while volume.attachment_state() != 'attached': + time.sleep(3) + volume.update() + changed = True + except boto.exception.BotoServerError, e: + module.fail_json(msg = "%s: %s" % (e.error_code, e.error_message)) + + return volume, changed + +def detach_volume(module, ec2, volume): + + changed = False + + if volume.attachment_state() is not None: + adata = volume.attach_data + volume.detach() + while volume.attachment_state() is not None: + time.sleep(3) + volume.update() + changed = True + + return volume, changed + +def get_volume_info(volume, state): + + # If we're just listing volumes then do nothing, else get the latest update for the volume + if state != 'list': + volume.update() + + volume_info = {} + attachment = volume.attach_data + + volume_info = { + 'create_time': volume.create_time, + 'id': volume.id, + 'iops': volume.iops, + 'size': volume.size, + 'snapshot_id': volume.snapshot_id, + 'status': volume.status, + 'type': volume.type, + 'zone': volume.zone, + 'attachment_set': { + 'attach_time': attachment.attach_time, + 'device': attachment.device, + 'instance_id': attachment.instance_id, + 'status': attachment.status + }, + 'tags': volume.tags + } + + return volume_info def main(): argument_spec = ec2_argument_spec() @@ -384,11 +402,30 @@ def main(): zone = module.params.get('zone') snapshot = module.params.get('snapshot') state = module.params.get('state') - + + # Ensure we have the zone or can get the zone + if instance is None and zone is None and state == 'present': + module.fail_json(msg="You must specify either instance or zone") + + # Set volume detach flag if instance == 'None' or instance == '': instance = None - - ec2 = ec2_connect(module) + detach_vol_flag = True + else: + detach_vol_flag = False + + # Set changed flag + changed = False + + region, ec2_url, aws_connect_params = get_aws_connection_info(module) + + if region: + try: + ec2 = connect_to_aws(boto.ec2, region, **aws_connect_params) + except (boto.exception.NoAuthHandlerFound, StandardError), e: + module.fail_json(msg=str(e)) + else: + module.fail_json(msg="region must be specified") if state == 'list': returned_volumes = [] @@ -397,22 +434,7 @@ def main(): for v in vols: attachment = v.attach_data - returned_volumes.append({ - 'create_time': v.create_time, - 'id': v.id, - 'iops': v.iops, - 'size': v.size, - 'snapshot_id': v.snapshot_id, - 'status': v.status, - 'type': v.type, - 'zone': v.zone, - 'attachment_set': { - 'attach_time': attachment.attach_time, - 'device': attachment.device, - 'status': attachment.status, - 'deleteOnTermination': attachment.deleteOnTermination - } - }) + returned_volumes.append(get_volume_info(v, state)) module.exit_json(changed=False, volumes=returned_volumes) @@ -423,8 +445,12 @@ def main(): # instance is specified but zone isn't. # Useful for playbooks chaining instance launch with volume create + attach and where the # zone doesn't matter to the user. + inst = None if instance: - reservation = ec2.get_all_instances(instance_ids=instance) + try: + reservation = ec2.get_all_instances(instance_ids=instance) + except BotoServerError as e: + module.fail_json(msg=e.message) inst = reservation[0].instances[0] zone = inst.placement @@ -443,17 +469,19 @@ def main(): if volume_size and (id or snapshot): module.fail_json(msg="Cannot specify volume_size together with id or snapshot") - - if state == 'absent': - delete_volume(module, ec2) - + if state == 'present': - volume = create_volume(module, ec2, zone) - if instance: - attach_volume(module, ec2, volume, inst) - else: - detach_volume(module, ec2) - module.exit_json(volume_id=volume.id, device=device_name, volume_type=volume.type) + volume, changed = create_volume(module, ec2, zone) + if detach_vol_flag: + volume, changed = detach_volume(module, ec2, volume) + elif inst is not None: + volume, changed = attach_volume(module, ec2, volume, inst) + + # Add device, volume_id and volume_type parameters separately to maintain backward compatability + volume_info = get_volume_info(volume, state) + module.exit_json(changed=changed, volume=volume_info, device=volume_info['attachment_set']['device'], volume_id=volume_info['id'], volume_type=volume_info['type']) + elif state == 'absent': + delete_volume(module, ec2) # import module snippets from ansible.module_utils.basic import *