V3 Attach patches

cinder.diff

diff --git a/cinder/api/v3/attachments.py b/cinder/api/v3/attachments.py
index 120f6f9..2e31dbd 100644
--- a/cinder/api/v3/attachments.py
+++ b/cinder/api/v3/attachments.py
@@ -18,6 +18,7 @@ import webob
 from cinder.api import common
 from cinder.api.openstack import wsgi
 from cinder.api.v3.views import attachments as attachment_views
+from cinder import db
 from cinder import exception
 from cinder.i18n import _
 from cinder import objects
@@ -251,6 +252,17 @@ class AttachmentsController(wsgi.Controller):
         # or a dict?
         return attachment_views.ViewBuilder.detail(attachment_ref)
 
+    @wsgi.response(202)
+    @wsgi.Controller.api_version(API_VERSION)
+    @wsgi.action('os-complete')
+    def complete(self, req, id, body):
+        """Mark a volume attachment process as completed (in-use)."""
+        context = req.environ['cinder.context']
+        attachment_ref = (
+            objects.VolumeAttachment.get_by_id(context, id))
+        db.volume_attached(context.elevated(),
+                           attachment_ref.id)
+
     @wsgi.Controller.api_version(API_VERSION)
     def delete(self, req, id):
         """Delete an attachment.
diff --git a/cinder/db/api.py b/cinder/db/api.py
index 25c4065..f0dd23a 100644
--- a/cinder/db/api.py
+++ b/cinder/db/api.py
@@ -222,13 +222,18 @@ def volume_attach(context, values):
     return IMPL.volume_attach(context, values)
 
 
-def volume_attached(context, volume_id, instance_id, host_name, mountpoint,
+def volume_attaching(context, attachment_id, instance_id, host_name, mountpoint,
                     attach_mode='rw'):
-    """Ensure that a volume is set as attached."""
-    return IMPL.volume_attached(context, volume_id, instance_id, host_name,
+    """Ensure that a volume is set as attaching."""
+    return IMPL.volume_attaching(context, attachment_id, instance_id, host_name,
                                 mountpoint, attach_mode)
 
 
+def volume_attached(context, attachment_id):
+    """Ensure that a volume is set as attached."""
+    return IMPL.volume_attached(context, attachment_id)
+
+
 def volume_create(context, values):
     """Create a volume from the values dictionary."""
     return IMPL.volume_create(context, values)
diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index f7fcfa6..4e27318 100644
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -1450,13 +1450,13 @@ def volume_attach(context, values):
 
 
 @require_admin_context
-def volume_attached(context, attachment_id, instance_uuid, host_name,
+def volume_attaching(context, attachment_id, instance_uuid, host_name,
                     mountpoint, attach_mode='rw'):
     """This method updates a volume attachment entry.
 
     This function saves the information related to a particular
     attachment for a volume.  It also updates the volume record
-    to mark the volume as attached.
+    to mark the volume as attaching.
 
     """
     if instance_uuid and not uuidutils.is_uuid_like(instance_uuid):
@@ -1480,6 +1480,35 @@ def volume_attached(context, attachment_id, instance_uuid, host_name,
 
         volume_ref = _volume_get(context, volume_attachment_ref['volume_id'],
                                  session=session)
+        volume_ref['status'] = 'attaching'
+        volume_ref['attach_status'] = fields.VolumeAttachStatus.ATTACHING
+        volume_ref.save(session=session)
+        return (volume_ref, updated_values)
+
+
+@require_admin_context
+def volume_attached(context, attachment_id):
+    """Update a volume attachment entry, marking it as complete.
+
+    This function saves the information related to a particular
+    attachment for a volume.  It also updates the volume record
+    to mark the volume as attached.
+
+    """
+    session = get_session()
+    with session.begin():
+        volume_attachment_ref = _attachment_get(context, attachment_id,
+                                                session=session)
+
+        updated_values = {'attach_status': fields.VolumeAttachStatus.ATTACHED,
+                          'attach_time': timeutils.utcnow(),
+                          'updated_at': literal_column('updated_at')}
+        volume_attachment_ref.update(updated_values)
+        volume_attachment_ref.save(session=session)
+        del updated_values['updated_at']
+
+        volume_ref = _volume_get(context, volume_attachment_ref['volume_id'],
+                                 session=session)
         volume_ref['status'] = 'in-use'
         volume_ref['attach_status'] = fields.VolumeAttachStatus.ATTACHED
         volume_ref.save(session=session)
diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index 20c7115..1a66dfd 100644
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -4368,15 +4368,15 @@ class VolumeManager(manager.CleanableManager,
                     {'attach_status':
                      fields.VolumeAttachStatus.ERROR_ATTACHING})
 
-        self.db.volume_attached(context.elevated(),
-                                attachment_ref.id,
-                                attachment_ref.instance_uuid,
-                                connector.get('host', ''),
-                                connector.get('mountpoint', 'na'),
-                                mode)
+        self.db.volume_attaching(context.elevated(),
+                                 attachment_ref.id,
+                                 attachment_ref.instance_uuid,
+                                 connector.get('host', ''),
+                                 connector.get('mountpoint', 'na'),
+                                 mode)
         vref.refresh()
         self._notify_about_volume_usage(context, vref, "attach.end")
-        LOG.info("Attach volume completed successfully.",
+        LOG.info("Update attachment completed successfully.",
                  resource=vref)
         attachment_ref = objects.VolumeAttachment.get_by_id(context,
                                                             attachment_id)

cinderclient.diff

diff --git a/cinderclient/v3/attachments.py b/cinderclient/v3/attachments.py
index 1eab8b1..6146626 100644
--- a/cinderclient/v3/attachments.py
+++ b/cinderclient/v3/attachments.py
@@ -66,3 +66,20 @@ class VolumeAttachmentManager(base.ManagerWithFind):
         resp = self._update('/attachments/%s' % id, body)
         return self.resource_class(self, resp['attachment'], loaded=True,
                                    resp=resp)
+
+    def complete(self, attachment):
+        """Mark the attachment as completed."""
+        resp, body = self._action_return_resp_and_body('os-complete',
+                                                       attachment,
+                                                       None)
+        return resp
+
+    def _action_return_resp_and_body(self, action, attachment, info=None,
+                                     **kwargs):
+        """Perform a attachments "action" and return response headers and body.
+
+        """
+        body = {action: info}
+        self.run_hooks('modify_body_for_action', body, **kwargs)
+        url = '/attachments/%s/action' % base.getid(attachment)
+        return self.api.client.post(url, body=body)
diff --git a/cinderclient/v3/shell.py b/cinderclient/v3/shell.py
index 9b6f115..c58df56 100644
--- a/cinderclient/v3/shell.py
+++ b/cinderclient/v3/shell.py
@@ -1937,6 +1937,16 @@ def do_attachment_delete(cs, args):
         cs.attachments.delete(attachment)
 
 
+@api_versions.wraps('3.27')
[email protected]('attachment',
+           metavar='<attachment>', nargs='+',
+           help='ID of attachment or attachments to delete.')
+def do_attachment_complete(cs, args):
+    """Complete an attachment for a cinder volume."""
+    for attachment in args.attachment:
+        cs.attachments.complete(attachment)
+
+
 @api_versions.wraps('3.0')
 def do_version_list(cs, args):
     """List all API versions."""

demo.bash

jgriffith@attach-3:~$ nova list; cinder list
+--------------------------------------+------+--------+------------+-------------+-------------------------------------------------------+
| ID                                   | Name | Status | Task State | Power State | Networks                                              |
+--------------------------------------+------+--------+------------+-------------+-------------------------------------------------------+
| 203f620b-3fad-433f-a121-96986a366047 | test | ACTIVE | -          | Running     | private=fd73:3030:915:0:f816:3eff:fe6a:2c01, 10.0.0.6 |
+--------------------------------------+------+--------+------------+-------------+-------------------------------------------------------+
+--------------------------------------+-----------+------+------+-------------+----------+-------------+
| ID                                   | Status    | Name | Size | Volume Type | Bootable | Attached to |
+--------------------------------------+-----------+------+------+-------------+----------+-------------+
| a80dd778-b263-42a8-9dae-4eec351d3cf3 | available | -    | 1    | lvmdriver-1 | false    |             |
+--------------------------------------+-----------+------+------+-------------+----------+-------------+
jgriffith@attach-3:~$ nova volume-attach 203f620b-3fad-433f-a121-96986a366047 a80dd778-b263-42a8-9dae-4eec351d3cf3
+----------+--------------------------------------+
| Property | Value                                |
+----------+--------------------------------------+
| device   | /dev/vdb                             |
| id       | a80dd778-b263-42a8-9dae-4eec351d3cf3 |
| serverId | 203f620b-3fad-433f-a121-96986a366047 |
| volumeId | a80dd778-b263-42a8-9dae-4eec351d3cf3 |
+----------+--------------------------------------+
jgriffith@attach-3:~$

# 1 second loop of cinder list cmd
+--------------------------------------+-----------+------+------+-------------+----------+-------------+
| ID                                   | Status    | Name | Size | Volume Type | Bootable | Attached to |
+--------------------------------------+-----------+------+------+-------------+----------+-------------+
| a80dd778-b263-42a8-9dae-4eec351d3cf3 | available | -    | 1    | lvmdriver-1 | false    |             |
+--------------------------------------+-----------+------+------+-------------+----------+-------------+
+--------------------------------------+-----------+------+------+-------------+----------+-------------+
| ID                                   | Status    | Name | Size | Volume Type | Bootable | Attached to |
+--------------------------------------+-----------+------+------+-------------+----------+-------------+
| a80dd778-b263-42a8-9dae-4eec351d3cf3 | available | -    | 1    | lvmdriver-1 | false    |             |
+--------------------------------------+-----------+------+------+-------------+----------+-------------+
+--------------------------------------+----------+------+------+-------------+----------+-------------+
| ID                                   | Status   | Name | Size | Volume Type | Bootable | Attached to |
+--------------------------------------+----------+------+------+-------------+----------+-------------+
| a80dd778-b263-42a8-9dae-4eec351d3cf3 | reserved | -    | 1    | lvmdriver-1 | false    |             |
+--------------------------------------+----------+------+------+-------------+----------+-------------+
+--------------------------------------+----------+------+------+-------------+----------+-------------+
| ID                                   | Status   | Name | Size | Volume Type | Bootable | Attached to |
+--------------------------------------+----------+------+------+-------------+----------+-------------+
| a80dd778-b263-42a8-9dae-4eec351d3cf3 | reserved | -    | 1    | lvmdriver-1 | false    |             |
+--------------------------------------+----------+------+------+-------------+----------+-------------+
+--------------------------------------+-----------+------+------+-------------+----------+-------------+
| ID                                   | Status    | Name | Size | Volume Type | Bootable | Attached to |
+--------------------------------------+-----------+------+------+-------------+----------+-------------+
| a80dd778-b263-42a8-9dae-4eec351d3cf3 | attaching | -    | 1    | lvmdriver-1 | false    |             |
+--------------------------------------+-----------+------+------+-------------+----------+-------------+
+--------------------------------------+--------+------+------+-------------+----------+--------------------------------------+
| ID                                   | Status | Name | Size | Volume Type | Bootable | Attached to                          |
+--------------------------------------+--------+------+------+-------------+----------+--------------------------------------+
| a80dd778-b263-42a8-9dae-4eec351d3cf3 | in-use | -    | 1    | lvmdriver-1 | false    | 203f620b-3fad-433f-a121-96986a366047 |
+--------------------------------------+--------+------+------+-------------+----------+--------------------------------------+
+--------------------------------------+--------+------+------+-------------+----------+--------------------------------------+
| ID                                   | Status | Name | Size | Volume Type | Bootable | Attached to                          |
+--------------------------------------+--------+------+------+-------------+----------+--------------------------------------+
| a80dd778-b263-42a8-9dae-4eec351d3cf3 | in-use | -    | 1    | lvmdriver-1 | false    | 203f620b-3fad-433f-a121-96986a366047 |
+--------------------------------------+--------+------+------+-------------+----------+--------------------------------------+

nova.diff

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 07d6024..b4f4ba0 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -4941,7 +4941,7 @@ class ComputeManager(manager.Manager):
             # the host connector, which will give us back the new attachment
             # connection_info.
             new_cinfo = self.volume_api.attachment_update(
-                context, new_attachment_id, connector)['connection_info']
+                context, new_attachment_id, connector)
 
         old_cinfo = jsonutils.loads(bdm['connection_info'])
         if old_cinfo and 'serial' not in old_cinfo:
diff --git a/nova/virt/block_device.py b/nova/virt/block_device.py
index ccdd0c1..600f9a3 100644
--- a/nova/virt/block_device.py
+++ b/nova/virt/block_device.py
@@ -447,7 +447,7 @@ class DriverVolumeBlockDevice(DriverBlockDevice):
         LOG.debug("Updating existing attachment record: %s",
                   attachment_id)
         self['connection_info'] = volume_api.attachment_update(
-            context, attachment_id, connector)['connection_info']
+            context, attachment_id, connector)
 
         connection_info_string = jsonutils.dumps(
             self['connection_info'])
@@ -471,6 +471,7 @@ class DriverVolumeBlockDevice(DriverBlockDevice):
                         context, self['connection_info'], instance,
                         self['mount_device'], disk_bus=self['disk_bus'],
                         device_type=self['device_type'], encryption=encryption)
+                volume_api.attachment_complete(context, self['attachment_id'])
             except Exception:
                 with excutils.save_and_reraise_exception():
                     LOG.exception("Driver failed to attach volume "
@@ -511,7 +512,7 @@ class DriverVolumeBlockDevice(DriverBlockDevice):
                                 virt_driver):
         connector = virt_driver.get_volume_connector(instance)
         return volume_api.attachment_update(context, self['attachment_id'],
-                                            connector)['connection_info']
+                                            connector)
 
     @update_db
     def refresh_connection_info(self, context, instance,
diff --git a/nova/volume/cinder.py b/nova/volume/cinder.py
index e7a8ae4..eae1f9d 100644
--- a/nova/volume/cinder.py
+++ b/nova/volume/cinder.py
@@ -21,6 +21,7 @@ Handles all requests relating to volumes + cinder.
 import collections
 import copy
 import functools
+import pprint
 import sys
 
 from cinderclient import api_versions as cinder_api_versions
@@ -215,16 +216,22 @@ def _untranslate_snapshot_summary_view(context, snapshot):
 
 def _translate_attachment_ref(attachment_ref):
     """Building old style connection_info by adding the 'data' key back."""
-    connection_info = attachment_ref.pop('connection_info', {})
-    attachment_ref['connection_info'] = {}
-    if connection_info.get('driver_volume_type'):
-        attachment_ref['connection_info']['driver_volume_type'] = \
-            connection_info['driver_volume_type']
-    attachment_ref['connection_info']['data'] = {}
-    for k, v in connection_info.items():
-        if k != "driver_volume_type":
-            attachment_ref['connection_info']['data'][k] = v
-    return attachment_ref
+    translated_con_info = {}
+    translated_con_info['attachment_id'] = \
+        attachment_ref['connection_info'].pop('attachment_id', None)
+    translated_con_info['driver_volume_type'] = \
+        attachment_ref['connection_info'].pop('driver_volume_type', None)
+    translated_con_info['data'] = attachment_ref.pop('connection_info', None)
+    translated_con_info['status'] = attachment_ref.pop('status', None)
+    translated_con_info['instance'] = attachment_ref.pop('instance', None)
+    translated_con_info['attach_mode'] = attachment_ref.pop('attach_mode', None)
+    translated_con_info['attached_at'] = attachment_ref.pop('attached_at', None)
+    translated_con_info['detached_at'] = attachment_ref.pop('detached_at', None)
+
+    # Now the catch all...
+    for k,v in attachment_ref.items():
+        translated_con_info[k] = v
+    return translated_con_info
 
 
 def translate_cinder_exception(method):
@@ -580,7 +587,16 @@ class API(object):
             attachment_ref = cinderclient(
                 context, '3.27', skip_version_check=True).attachments.update(
                     attachment_id, connector)
-            return _translate_attachment_ref(attachment_ref.to_dict())
+
+            LOG.info("attachment_ref:")
+            LOG.info("%s", pprint.pformat(attachment_ref.to_dict(), indent=2))
+
+            translated = _translate_attachment_ref(attachment_ref.to_dict())
+            translated['connector'] = connector
+
+            LOG.info("translated attachment_ref:")
+            LOG.info("%s", pprint.pformat(translated, indent=2))
+            return translated
         except cinder_exception.ClientException as ex:
             with excutils.save_and_reraise_exception():
                 LOG.error(('Update attachment failed for attachment '
@@ -602,3 +618,17 @@ class API(object):
                           {'id': attachment_id,
                            'msg': six.text_type(ex),
                            'code': getattr(ex, 'code', None)})
+
+    @translate_attachment_exception
+    def attachment_complete(self, context, attachment_id):
+        try:
+            cinderclient(
+                context, '3.27', skip_version_check=True).attachments.complete(
+                    attachment_id)
+        except cinder_exception.ClientException as ex:
+            with excutils.save_and_reraise_exception():
+                LOG.error(('Complete attachment failed for attachment '
+                           '%(id)s. Error: %(msg)s Code: %(code)s'),
+                          {'id': attachment_id,
+                           'msg': six.text_type(ex),
+                           'code': getattr(ex, 'code', None)})