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

@@ -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 is None:
-        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:
         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')
-
+    changed = False
-    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))
-
     # 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):
+    if volume.attachment_state() is not None:
-    vol = get_volume(module, ec2)
+        adata = volume.attach_data
-    if not vol or vol.attachment_state() is None:
+        if adata.instance_id != instance.id:
-        module.exit_json(changed=False)
+            module.fail_json(msg = "Volume %s is already attached to another instance: %s"
+                             % (volume.id, adata.instance_id))
     else:
-        vol.detach()
+        try:
-        module.exit_json(changed=True)
+            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
-
+        detach_vol_flag = True
-    ec2 = ec2_connect(module)
+    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({
+            returned_volumes.append(get_volume_info(v, state))
-                '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
-                }
-            })
 
         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)
+        volume, changed = create_volume(module, ec2, zone)
-        if instance:
+        if detach_vol_flag:
-            attach_volume(module, ec2, volume, inst)
+            volume, changed = detach_volume(module, ec2, volume)    
-        else:
+        elif inst is not None:
-            detach_volume(module, ec2)    
+            volume, changed = attach_volume(module, ec2, volume, inst)
-        module.exit_json(volume_id=volume.id, device=device_name, volume_type=volume.type)
+
+        # 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 *