Add new attachment APIS

Replaces the original patch:
  https://review.openstack.org/#/c/387712/

Use the python-cinderclient WIP:
  https://review.openstack.org/#/c/387716/

Here's what you can do currently:
  `cinder attachment-create <volume-uuid>`
      Currently only tested/implemented the reserve piece
      Will create an attachment object, set volume to a
      status of 'reserved'
  `cinder attachment-list`
      Simple list output of attachments
  `cinder attachment-show <attachment-uuid>`
      Detailed list of specified attachment
  `cinder attachment-delete <attachment-uuid>`
      Removes an attachment

Change-Id: Ie15233c99d91de67279b56d27a5508c5ea98d769

cinder/api/openstack/api_version_request.py

@@ -77,6 +77,7 @@ REST_API_VERSION_HISTORY = """
     * 3.25 - Add ``volumes`` field to group list/detail and group show.
     * 3.26 - Add failover action and cluster listings accept new filters and
              return new data.
+    * 3.27 - Add attachment API
 """
 
 # The minimum and maximum versions of the API supported
@@ -84,7 +85,7 @@ REST_API_VERSION_HISTORY = """
 # minimum version of the API supported.
 # Explicitly using /v1 or /v2 enpoints will still work
 _MIN_API_VERSION = "3.0"
-_MAX_API_VERSION = "3.26"
+_MAX_API_VERSION = "3.27"
 _LEGACY_API_VERSION1 = "1.0"
 _LEGACY_API_VERSION2 = "2.0"
 

cinder/api/openstack/rest_api_version_history.rst

@@ -280,3 +280,7 @@ user documentation.
   - Cluster listing accepts ``replication_status``, ``frozen`` and
     ``active_backend_id`` as filters, and returns additional fields for each
     cluster: ``replication_status``, ``frozen``, ``active_backend_id``.
+
+3.27
+----
+  Added new attachment API's

cinder/api/v3/attachments.py

@@ -0,0 +1,253 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""The volumes attachments api."""
+
+from oslo_log import log as logging
+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 exception
+from cinder.i18n import _
+from cinder import objects
+from cinder import utils
+from cinder.volume import api as volume_api
+
+
+LOG = logging.getLogger(__name__)
+API_VERSION = '3.27'
+
+
+class AttachmentsController(wsgi.Controller):
+    """The Attachments API controller for the OpenStack API."""
+
+    _view_builder_class = attachment_views.ViewBuilder
+
+    allowed_filters = {'volume_id', 'status', 'instance_id', 'attach_status'}
+
+    def __init__(self, ext_mgr=None):
+        """Initialize controller class."""
+        self.volume_api = volume_api.API()
+        self.ext_mgr = ext_mgr
+        super(AttachmentsController, self).__init__()
+
+    @wsgi.Controller.api_version(API_VERSION)
+    def show(self, req, id):
+        """Return data about the given attachment."""
+        context = req.environ['cinder.context']
+        attachment = objects.VolumeAttachment.get_by_id(context, id)
+        return attachment_views.ViewBuilder.detail(attachment)
+
+    @wsgi.Controller.api_version(API_VERSION)
+    def index(self, req):
+        """Return a summary list of attachments."""
+        attachments = self._items(req, detailed=False)
+        return attachment_views.ViewBuilder.list(attachments)
+
+    @wsgi.Controller.api_version(API_VERSION)
+    def detail(self, req):
+        """Return a detailed list of attachments."""
+        attachments = self._items(req)
+        return attachment_views.ViewBuilder.list(req, attachments)
+
+    def _items(self, req, detailed=True):
+        """Return a list of attachments, transformed through view builder."""
+        context = req.environ['cinder.context']
+
+        # Pop out non search_opts and create local variables
+        search_opts = req.GET.copy()
+        sort_keys, sort_dirs = common.get_sort_params(search_opts)
+        marker, limit, offset = common.get_pagination_params(search_opts)
+        filters = dict(req.GET)
+        allowed = self.allowed_filters
+        if not allowed.issuperset(filters):
+            invalid_keys = set(filters).difference(allowed)
+            msg = _('Invalid filter keys: %s') % ', '.join(invalid_keys)
+            raise exception.InvalidInput(reason=msg)
+
+        # Filter out invalid options
+        allowed_search_options = ('status', 'volume_id',
+                                  'instance_id')
+        if search_opts.get('instance_id', None):
+            search_opts['instance_uuid'] = search_opts.get('instance_id')
+        utils.remove_invalid_filter_options(context, search_opts,
+                                            allowed_search_options)
+        return objects.VolumeAttachmentList.get_all(context)
+
+    @wsgi.Controller.api_version(API_VERSION)
+    @wsgi.response(202)
+    def create(self, req, body):
+        """Create an attachment.
+
+        This method can be used to create an empty attachment (reserve) or to
+        create and initialize a volume attachment based on the provided input
+        parameters.
+
+        If the caller does not yet have the connector information but needs to
+        reserve an attachment for the volume (ie Nova BootFromVolume) the
+        create can be called with just the volume-uuid and the server
+        identifier. This will reserve an attachment, mark the volume as
+        reserved and prevent any new attachment_create calls from being made
+        until the attachment is updated (completed).
+
+        The alternative is that the connection can be reserved and initialized
+        all at once with a single call if the caller has all of the required
+        information (connector data) at the time of the call.
+
+        NOTE: In Nova terms server == instance, the server_id parameter
+        referenced below is the uuid of the Instance, for non-nova consumers
+        this can be a server uuid or some other arbitrary unique identifier.
+
+        Expected format of the input parameter 'body':
+
+        .. code-block:: json
+
+            {
+                "attachment":
+                {
+                    "volume_uuid": "volume-uuid",
+                    "instance_uuid": "nova-server-uuid",
+                    "connector": None|<connector-object>,
+                }
+            }
+
+        Example connector:
+
+        .. code-block:: json
+
+            {
+                "connector":
+                {
+                    "initiator": "iqn.1993-08.org.debian:01:cad181614cec",
+                    "ip":"192.168.1.20",
+                    "platform": "x86_64",
+                    "host": "tempest-1",
+                    "os_type": "linux2",
+                    "multipath": False,
+                    "mountpoint": "/dev/vdb",
+                    "mode": None|"rw"|"ro",
+                }
+            }
+
+        NOTE all that's required for a reserve is volume_uuid
+        and a instance_uuid.
+
+        returns: A summary view of the attachment object
+        """
+        context = req.environ['cinder.context']
+        instance_uuid = body['attachment'].get('instance_uuid', None)
+        if not instance_uuid:
+            raise webob.exc.HTTPBadRequest(
+                explanation=_("Must specify 'instance_uuid' "
+                              "to create attachment."))
+
+        volume_ref = objects.Volume.get_by_id(
+            context,
+            body['attachment']['volume_uuid'])
+        connector = body['attachment'].get('connector', None)
+        err_msg = None
+        try:
+            attachment_ref = (
+                self.volume_api.attachment_create(context,
+                                                  volume_ref,
+                                                  instance_uuid,
+                                                  connector=connector))
+        except exception.CinderException as ex:
+            err_msg = _(
+                "Unable to create attachment for volume (%s).") % ex.msg
+            LOG.exception(err_msg)
+        except Exception as ex:
+            err_msg = _("Unable to create attachment for volume.")
+            LOG.exception(err_msg)
+        finally:
+            if err_msg:
+                raise webob.exc.HTTPInternalServerError(explanation=err_msg)
+        return attachment_views.ViewBuilder.detail(attachment_ref)
+
+    @wsgi.Controller.api_version(API_VERSION)
+    def update(self, req, id, body):
+        """Update an attachment record.
+
+        Update a reserved attachment record with connector information and set
+        up the appropriate connection_info from the driver.
+
+        Expected format of the input parameter 'body':
+
+        .. code-block:: json
+        {
+            "attachment":
+            {
+                "connector":
+                {
+                    "initiator": "iqn.1993-08.org.debian:01:cad181614cec",
+                    "ip":"192.168.1.20",
+                    "platform": "x86_64",
+                    "host": "tempest-1",
+                    "os_type": "linux2",
+                    "multipath": False,
+                    "mountpoint": "/dev/vdb",
+                    "mode": None|"rw"|"ro",
+                }
+            }
+
+        """
+        context = req.environ['cinder.context']
+        attachment_ref = (
+            objects.VolumeAttachment.get_by_id(context, id))
+        connector = body['attachment'].get('connector', None)
+        if not connector:
+            raise webob.exc.HTTPBadRequest(
+                explanation=_("Must specify 'connector' "
+                              "to update attachment."))
+        err_msg = None
+        try:
+            attachment_ref = (
+                self.volume_api.attachment_update(context,
+                                                  attachment_ref,
+                                                  connector))
+
+        except exception.CinderException as ex:
+            err_msg = (
+                _("Unable to create attachment for volume (%s).") % ex.msg)
+            LOG.exception(err_msg)
+        except Exception as ex:
+            err_msg = _("Unable to create attachment for volume.")
+            LOG.exception(err_msg)
+        finally:
+            if err_msg:
+                raise webob.exc.HTTPInternalServerError(explanation=err_msg)
+
+        # TODO(jdg): Test this out some more, do we want to return and object
+        # or a dict?
+        return attachment_views.ViewBuilder.detail(attachment_ref)
+
+    @wsgi.Controller.api_version(API_VERSION)
+    def delete(self, req, id):
+        """Delete an attachment.
+
+        Disconnects/Deletes the specified attachment, returns a list of any
+        known shared attachment-id's for the effected backend device.
+
+        returns: A summary list of any attachments sharing this connection
+
+        """
+        context = req.environ['cinder.context']
+        attachment = objects.VolumeAttachment.get_by_id(context, id)
+        attachments = self.volume_api.attachment_delete(context, attachment)
+        return attachment_views.ViewBuilder.list(attachments)
+
+
+def create_resource(ext_mgr):
+    """Create the wsgi resource for this controller."""
+    return wsgi.Resource(AttachmentsController(ext_mgr))

cinder/api/v3/router.py

@@ -24,6 +24,7 @@ import cinder.api.openstack
 from cinder.api.v2 import limits
 from cinder.api.v2 import snapshot_metadata
 from cinder.api.v2 import types
+from cinder.api.v3 import attachments
 from cinder.api.v3 import backups
 from cinder.api.v3 import clusters
 from cinder.api.v3 import consistencygroups
@@ -175,6 +176,12 @@ class APIRouter(cinder.api.openstack.APIRouter):
                         controller=self.resources['backups'],
                         collection={'detail': 'GET'})
 
+        self.resources['attachments'] = attachments.create_resource(ext_mgr)
+        mapper.resource("attachment", "attachments",
+                        controller=self.resources['attachments'],
+                        collection={'detail': 'GET', 'summary': 'GET'},
+                        member={'action': 'POST'})
+
         self.resources['workers'] = workers.create_resource()
         mapper.resource('worker', 'workers',
                         controller=self.resources['workers'],

cinder/api/v3/views/attachments.py

@@ -0,0 +1,57 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from oslo_utils import timeutils
+
+
+class ViewBuilder(object):
+    """Model an attachment API response as a python dictionary."""
+
+    _collection_name = "attachments"
+
+    @staticmethod
+    def _normalize(date):
+        if date:
+            return timeutils.normalize_time(date)
+        return ''
+
+    @classmethod
+    def detail(cls, attachment, flat=False):
+        """Detailed view of an attachment."""
+        result = cls.summary(attachment, flat=True)
+        result.update(
+            attached_at=cls._normalize(attachment.attach_time),
+            detached_at=cls._normalize(attachment.detach_time),
+            attach_mode=attachment.attach_mode,
+            connection_info=getattr(attachment, 'connection_info', None),)
+        if flat:
+            return result
+        return {'attachment': result}
+
+    @staticmethod
+    def summary(attachment, flat=False):
+        """Non detailed view of an attachment."""
+        result = {
+            'id': attachment.id,
+            'status': attachment.attach_status,
+            'instance': attachment.instance_uuid,
+            'volume_id': attachment.volume_id, }
+        if flat:
+            return result
+        return {'attachment': result}
+
+    @classmethod
+    def list(cls, attachments, detail=False):
+        """Build a view of a list of attachments."""
+        func = cls.detail if detail else cls.summary
+        return {'attachments': [func(attachment, flat=True) for attachment in
+                                attachments]}

cinder/db/api.py

@@ -382,6 +382,11 @@ def volume_attachment_get_all_by_project(context, project_id, filters=None,
                                                      sort_dirs)
 
 
+def attachment_destroy(context, attachment_id):
+    """Destroy the attachment or raise if it does not exist."""
+    return IMPL.attachment_destroy(context, attachment_id)
+
+
 def volume_update_status_based_on_attachment(context, volume_id):
     """Update volume status according to attached instance id"""
     return IMPL.volume_update_status_based_on_attachment(context, volume_id)
@@ -1734,6 +1739,33 @@ class Condition(object):
             raise ValueError(_('Condition has no field.'))
         return field
 
+###################
+
+
+def attachment_specs_get(context, attachment_id):
+    """Get all specs for an attachment."""
+    return IMPL.attachment_specs_get(context, attachment_id)
+
+
+def attachment_specs_delete(context, attachment_id, key):
+    """Delete the given attachment specs item."""
+    return IMPL.attachment_specs_delete(context, attachment_id, key)
+
+
+def attachment_specs_update_or_create(context,
+                                      attachment_id,
+                                      specs):
+    """Create or update attachment specs.
+
+    This adds or modifies the key/value pairs specified in the attachment
+    specs dict argument.
+    """
+    return IMPL.attachment_specs_update_or_create(context,
+                                                  attachment_id,
+                                                  specs)
+
+###################
+
 
 class Not(Condition):
     """Class for negated condition values for conditional_update.

cinder/db/sqlalchemy/api.py

@@ -1771,7 +1771,8 @@ def _volume_get(context, volume_id, session=None, joined_load=True):
 
 def _attachment_get_all(context, filters=None, marker=None, limit=None,
                         offset=None, sort_keys=None, sort_dirs=None):
-    project_id = filters.pop('project_id', None)
+
+    project_id = filters.pop('project_id', None) if filters else None
     if filters and not is_valid_model_filters(models.VolumeAttachment,
                                               filters):
         return []
@@ -1856,12 +1857,14 @@ def volume_attachment_get_all_by_host(context, host):
 
 @require_context
 def volume_attachment_get(context, attachment_id):
+    """Fetch the specified attachment record."""
     return _attachment_get(context, attachment_id)
 
 
 @require_context
 def volume_attachment_get_all_by_instance_uuid(context,
                                                instance_uuid):
+    """Fetch all attachment records associated with the specified instance."""
     session = get_session()
     with session.begin():
         result = model_query(context, models.VolumeAttachment,
@@ -1892,6 +1895,102 @@ def volume_attachment_get_all_by_project(context, project_id, filters=None,
                                sort_dirs)
 
 
+@require_admin_context
+@_retry_on_deadlock
+def attachment_destroy(context, attachment_id):
+    """Destroy the specified attachment record."""
+    utcnow = timeutils.utcnow()
+    session = get_session()
+    with session.begin():
+        updated_values = {'attach_status': 'deleted',
+                          'deleted': True,
+                          'deleted_at': utcnow,
+                          'updated_at': literal_column('updated_at')}
+        model_query(context, models.VolumeAttachment, session=session).\
+            filter_by(id=attachment_id).\
+            update(updated_values)
+        model_query(context, models.AttachmentSpecs, session=session).\
+            filter_by(attachment_id=attachment_id).\
+            update({'deleted': True,
+                    'deleted_at': utcnow,
+                    'updated_at': literal_column('updated_at')})
+    del updated_values['updated_at']
+    return updated_values
+
+
+def _attachment_specs_query(context, attachment_id, session=None):
+    return model_query(context, models.AttachmentSpecs, session=session,
+                       read_deleted="no").\
+        filter_by(attachment_id=attachment_id)
+
+
+@require_context
+def attachment_specs_get(context, attachment_id):
+    """Fetch the attachment_specs for the specified attachment record."""
+    rows = _attachment_specs_query(context, attachment_id).\
+        all()
+
+    result = {row['key']: row['value'] for row in rows}
+    return result
+
+
+@require_context
+def attachment_specs_delete(context, attachment_id, key):
+    """Delete attachment_specs for the specified attachment record."""
+    session = get_session()
+    with session.begin():
+        _attachment_specs_get_item(context,
+                                   attachment_id,
+                                   key,
+                                   session)
+        _attachment_specs_query(context, attachment_id, session).\
+            filter_by(key=key).\
+            update({'deleted': True,
+                    'deleted_at': timeutils.utcnow(),
+                    'updated_at': literal_column('updated_at')})
+
+
+@require_context
+def _attachment_specs_get_item(context,
+                               attachment_id,
+                               key,
+                               session=None):
+    result = _attachment_specs_query(
+        context, attachment_id, session=session).\
+        filter_by(key=key).\
+        first()
+
+    if not result:
+        raise exception.AttachmentSpecsNotFound(
+            specs_key=key,
+            attachment_id=attachment_id)
+
+    return result
+
+
+@handle_db_data_error
+@require_context
+def attachment_specs_update_or_create(context,
+                                      attachment_id,
+                                      specs):
+    """Update attachment_specs for the specified attachment record."""
+    session = get_session()
+    with session.begin():
+        spec_ref = None
+        for key, value in specs.items():
+            try:
+                spec_ref = _attachment_specs_get_item(
+                    context, attachment_id, key, session)
+            except exception.AttachmentSpecsNotFound:
+                spec_ref = models.AttachmentSpecs()
+            spec_ref.update({"key": key, "value": value,
+                             "attachment_id": attachment_id,
+                             "deleted": False})
+            spec_ref.save(session=session)
+
+        return specs
+
+
 @require_context
 def volume_get(context, volume_id):
     return _volume_get(context, volume_id)

cinder/db/sqlalchemy/migrate_repo/versions/091_add_attachment_specs.py

@@ -0,0 +1,40 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from sqlalchemy import Boolean, Column, DateTime, ForeignKey, Integer
+from sqlalchemy import MetaData, String, Table
+
+
+def upgrade(migrate_engine):
+    """Add attachment_specs table."""
+
+    meta = MetaData()
+    meta.bind = migrate_engine
+    Table('volume_attachment', meta, autoload=True)
+
+    attachment_specs = Table(
+        'attachment_specs', meta,
+        Column('created_at', DateTime(timezone=False)),
+        Column('updated_at', DateTime(timezone=False)),
+        Column('deleted_at', DateTime(timezone=False)),
+        Column('deleted', Boolean(), default=False),
+        Column('id', Integer, primary_key=True, nullable=False),
+        Column('attachment_id', String(36),
+               ForeignKey('volume_attachment.id'),
+               nullable=False),
+        Column('key', String(255)),
+        Column('value', String(255)),
+        mysql_engine='InnoDB',
+        mysql_charset='utf8'
+    )
+
+    attachment_specs.create()

cinder/db/sqlalchemy/models.py

@@ -874,3 +874,24 @@ class Worker(BASE, CinderBase):
         backref="workers",
         foreign_keys=service_id,
         primaryjoin='Worker.service_id == Service.id')
+
+
+class AttachmentSpecs(BASE, CinderBase):
+    """Represents attachment specs as k/v pairs for a volume_attachment."""
+
+    __tablename__ = 'attachment_specs'
+    id = Column(Integer, primary_key=True)
+    key = Column(String(255))
+    value = Column(String(255))
+    attachment_id = (
+        Column(String(36),
+               ForeignKey('volume_attachment.id'),
+               nullable=False))
+    volume_attachment = relationship(
+        VolumeAttachment,
+        backref="attachment_specs",
+        foreign_keys=attachment_id,
+        primaryjoin='and_('
+        'AttachmentSpecs.attachment_id == VolumeAttachment.id,'
+        'AttachmentSpecs.deleted == False)'
+    )

cinder/exception.py

@@ -1347,3 +1347,12 @@ class RdxAPICommandException(VolumeDriverException):
 
 class RdxAPIConnectionException(VolumeDriverException):
     message = _("Reduxio API Connection Exception")
+
+
+class AttachmentSpecsNotFound(NotFound):
+    message = _("Attachment %(attachment_id)s has no "
+                "key %(specs_key)s.")
+
+
+class InvalidAttachment(Invalid):
+    message = _("Invalid attachment: %(reason)s")

cinder/objects/fields.py

@@ -148,11 +148,13 @@ class VolumeAttachStatus(BaseCinderEnum):
     ATTACHED = 'attached'
     ATTACHING = 'attaching'
     DETACHED = 'detached'
+    RESERVED = 'reserved'
     ERROR_ATTACHING = 'error_attaching'
     ERROR_DETACHING = 'error_detaching'
+    DELETED = 'deleted'
 
     ALL = (ATTACHED, ATTACHING, DETACHED, ERROR_ATTACHING,
-           ERROR_DETACHING)
+           ERROR_DETACHING, RESERVED, DELETED)
 
 
 class VolumeAttachStatusField(BaseEnumField):

cinder/objects/volume_attachment.py

@@ -132,6 +132,11 @@ class VolumeAttachment(base.CinderPersistentObject, base.CinderObject,
             db_attachment = db.volume_attach(self._context, updates)
         self._from_db_object(self._context, self, db_attachment)
 
+    def destroy(self):
+        updated_values = db.attachment_destroy(self._context, self.id)
+        self.update(updated_values)
+        self.obj_reset_changes(updated_values.keys())
+
 
 @base.CinderObjectRegistry.register
 class VolumeAttachmentList(base.ObjectListBase, base.CinderObject):

cinder/tests/unit/attachments/test_attachments_api.py

@@ -0,0 +1,139 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import mock
+from oslo_config import cfg
+
+from cinder import context
+from cinder import db
+from cinder import objects
+from cinder import test
+from cinder.tests.unit import fake_constants as fake
+from cinder.tests.unit import utils as tests_utils
+from cinder.volume import api as volume_api
+from cinder.volume import configuration as conf
+
+CONF = cfg.CONF
+
+
+class AttachmentManagerTestCase(test.TestCase):
+    """Attachment related test for volume/api.py."""
+
+    def setUp(self):
+        """Setup test class."""
+        super(AttachmentManagerTestCase, self).setUp()
+        self.configuration = mock.Mock(conf.Configuration)
+        self.context = context.get_admin_context()
+        self.context.user_id = fake.USER_ID
+        self.project_id = fake.PROJECT3_ID
+        self.context.project_id = self.project_id
+        self.volume_api = volume_api.API()
+
+    @mock.patch('cinder.volume.api.check_policy')
+    def test_attachment_create_no_connector(self, mock_policy):
+        """Test attachment_create no connector."""
+        volume_params = {'status': 'available'}
+
+        vref = tests_utils.create_volume(self.context, **volume_params)
+        aref = self.volume_api.attachment_create(self.context,
+                                                 vref,
+                                                 fake.UUID2)
+        self.assertEqual(fake.UUID2, aref.instance_uuid)
+        self.assertIsNone(aref.attach_time)
+        self.assertEqual('reserved', aref.attach_status)
+        self.assertIsNone(aref.attach_mode)
+        self.assertEqual(vref.id, aref.volume_id)
+        self.assertEqual({}, aref.connection_info)
+
+    @mock.patch('cinder.volume.api.check_policy')
+    @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_update')
+    def test_attachment_create_with_connector(self,
+                                              mock_rpc_attachment_update,
+                                              mock_policy):
+        """Test attachment_create with connector."""
+        volume_params = {'status': 'available'}
+
+        vref = tests_utils.create_volume(self.context, **volume_params)
+        connector = {'fake': 'connector'}
+        self.volume_api.attachment_create(self.context,
+                                          vref,
+                                          fake.UUID2,
+                                          connector)
+        mock_rpc_attachment_update.assert_called_once_with(self.context,
+                                                           mock.ANY,
+                                                           connector,
+                                                           mock.ANY)
+
+    @mock.patch('cinder.volume.api.check_policy')
+    @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_delete')
+    def test_attachment_delete_reserved(self,
+                                        mock_rpc_attachment_delete,
+                                        mock_policy):
+        """Test attachment_delete with reserved."""
+        volume_params = {'status': 'available'}
+
+        vref = tests_utils.create_volume(self.context, **volume_params)
+        aref = self.volume_api.attachment_create(self.context,
+                                                 vref,
+                                                 fake.UUID2)
+        aobj = objects.VolumeAttachment.get_by_id(self.context,
+                                                  aref.id)
+        self.assertEqual('reserved', aref.attach_status)
+        self.assertEqual(vref.id, aref.volume_id)
+        self.volume_api.attachment_delete(self.context,
+                                          aobj)
+
+        # Since it's just reserved and never finalized, we should never make an
+        # rpc call
+        mock_rpc_attachment_delete.assert_not_called()
+
+    @mock.patch('cinder.volume.api.check_policy')
+    @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_delete')
+    @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_update')
+    def test_attachment_create_update_and_delete(
+            self,
+            mock_rpc_attachment_update,
+            mock_rpc_attachment_delete,
+            mock_policy):
+        """Test attachment_delete."""
+        volume_params = {'status': 'available'}
+
+        vref = tests_utils.create_volume(self.context, **volume_params)
+        aref = self.volume_api.attachment_create(self.context,
+                                                 vref,
+                                                 fake.UUID2)
+        aref = objects.VolumeAttachment.get_by_id(self.context,
+                                                  aref.id)
+        vref = objects.Volume.get_by_id(self.context,
+                                        vref.id)
+
+        connector = {'fake': 'connector'}
+        self.volume_api.attachment_update(self.context,
+                                          aref,
+                                          connector)
+        # We mock the actual call that updates the status
+        # so force it here
+        values = {'volume_id': vref.id,
+                  'volume_host': vref.host,
+                  'attach_status': 'attached',
+                  'instance_uuid': fake.UUID2}
+        aref = db.volume_attach(self.context, values)
+
+        aref = objects.VolumeAttachment.get_by_id(self.context,
+                                                  aref.id)
+        self.assertEqual(vref.id, aref.volume_id)
+        self.volume_api.attachment_delete(self.context,
+                                          aref)
+
+        mock_rpc_attachment_delete.assert_called_once_with(self.context,
+                                                           aref.id,
+                                                           mock.ANY)

cinder/tests/unit/attachments/test_attachments_manager.py

@@ -0,0 +1,99 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import mock
+from oslo_config import cfg
+from oslo_utils import importutils
+
+from cinder import context
+from cinder import db
+from cinder import exception
+from cinder import test
+from cinder.tests.unit import fake_constants as fake
+from cinder.tests.unit import utils as tests_utils
+from cinder.volume import configuration as conf
+
+CONF = cfg.CONF
+
+
+class AttachmentManagerTestCase(test.TestCase):
+    """Attachment related test for volume.manager.py."""
+
+    def setUp(self):
+        """Setup test class."""
+        super(AttachmentManagerTestCase, self).setUp()
+        self.manager = importutils.import_object(CONF.volume_manager)
+        self.configuration = mock.Mock(conf.Configuration)
+        self.context = context.get_admin_context()
+        self.context.user_id = fake.USER_ID
+        self.project_id = fake.PROJECT3_ID
+        self.context.project_id = self.project_id
+        self.manager.driver.set_initialized()
+        self.manager.stats = {'allocated_capacity_gb': 100,
+                              'pools': {}}
+
+    def test_attachment_update(self):
+        """Test attachment_update."""
+        volume_params = {'status': 'available'}
+        connector = {
+            "initiator": "iqn.1993-08.org.debian:01:cad181614cec",
+            "ip": "192.168.1.20",
+            "platform": "x86_64",
+            "host": "tempest-1",
+            "os_type": "linux2",
+            "multipath": False}
+
+        vref = tests_utils.create_volume(self.context, **volume_params)
+        self.manager.create_volume(self.context, vref)
+        values = {'volume_id': vref.id,
+                  'volume_host': vref.host,
+                  'attach_status': 'reserved',
+                  'instance_uuid': fake.UUID1}
+        attachment_ref = db.volume_attach(self.context, values)
+        with mock.patch.object(self.manager,
+                               '_notify_about_volume_usage',
+                               return_value=None):
+            expected = {
+                'encrypted': False,
+                'qos_specs': None,
+                'access_mode': 'rw',
+                'driver_volume_type': 'iscsi',
+                'attachment_id': attachment_ref.id}
+
+            self.assertEqual(expected,
+                             self.manager.attachment_update(
+                                 self.context,
+                                 vref,
+                                 connector,
+                                 attachment_ref.id))
+
+    def test_attachment_delete(self):
+        """Test attachment_delete."""
+        volume_params = {'status': 'available'}
+
+        vref = tests_utils.create_volume(self.context, **volume_params)
+        self.manager.create_volume(self.context, vref)
+        values = {'volume_id': vref.id,
+                  'volume_host': vref.host,
+                  'attach_status': 'reserved',
+                  'instance_uuid': fake.UUID1}
+        attachment_ref = db.volume_attach(self.context, values)
+        attachment_ref = db.volume_attachment_get(
+            self.context,
+            attachment_ref['id'])
+        self.manager.attachment_delete(self.context,
+                                       attachment_ref['id'],
+                                       vref)
+        self.assertRaises(exception.VolumeAttachmentNotFound,
+                          db.volume_attachment_get,
+                          self.context,
+                          attachment_ref.id)

cinder/tests/unit/fake_constants.py

@@ -80,3 +80,10 @@ GROUP_ID = '9a965cc6-ee3a-468d-a721-cebb193f696f'
 GROUP2_ID = '40a85639-abc3-4461-9230-b131abd8ee07'
 GROUP_SNAPSHOT_ID = '1e2ab152-44f0-11e6-819f-000c29d19d84'
 GROUP_SNAPSHOT2_ID = '33e2ff04-44f0-11e6-819f-000c29d19d84'
+
+# I don't care what it's used for, I just want a damn UUID
+UUID1 = '84d0c5f7-2349-401c-8672-f76214d13cab'
+UUID2 = '25406d50-e645-4e62-a9ef-1f53f9cba13f'
+UUID3 = '29c80662-3a9f-4844-a585-55cd3cd180b5'
+UUID4 = '4cd72b2b-5a4f-4f24-93dc-7c0212002916'
+UUID5 = '0a574d83-cacf-42b9-8f9f-8f4faa6d4746'

cinder/tests/unit/objects/test_objects.py

@@ -41,9 +41,9 @@ object_data = {
     'ServiceList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e',
     'Snapshot': '1.3-69dfbe3244992478a0174cb512cd7f27',
     'SnapshotList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e',
-    'Volume': '1.6-8a56256db74c0642dca1a30739d88074',
+    'Volume': '1.6-7d3bc8577839d5725670d55e480fe95f',
     'VolumeList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e',
-    'VolumeAttachment': '1.1-e98b04a372a303b01bedab1e47ee9f6d',
+    'VolumeAttachment': '1.1-ed82a5fdd56655e14d9f86396c130aea',
     'VolumeAttachmentList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e',
     'VolumeProperties': '1.1-cadac86b2bdc11eb79d1dcea988ff9e8',
     'VolumeType': '1.3-a5d8c3473db9bc3bbcdbab9313acf4d1',

cinder/volume/api.py

@@ -86,6 +86,7 @@ CONF.import_opt('glance_core_properties', 'cinder.image.glance')
 
 LOG = logging.getLogger(__name__)
 QUOTAS = quota.QUOTAS
+AO_LIST = objects.VolumeAttachmentList
 
 
 def wrap_check_policy(func):
@@ -98,7 +99,6 @@ def wrap_check_policy(func):
     def wrapped(self, context, target_obj, *args, **kwargs):
         check_policy(context, func.__name__, target_obj)
         return func(self, context, target_obj, *args, **kwargs)
-
     return wrapped
 
 
@@ -1906,6 +1906,98 @@ class API(base.Base):
             else:
                 return bool(val)
 
+    def _attachment_reserve(self, ctxt, vref, instance_uuid=None):
+        # NOTE(jdg): Reserved is a special case, we're avoiding allowing
+        # creation of other new reserves/attachments while in this state
+        # so we avoid contention issues with shared connections
+
+        # FIXME(JDG):  We want to be able to do things here like reserve a
+        # volume for Nova to do BFV WHILE the volume may be in the process of
+        # downloading image, we add downloading here; that's easy enough but
+        # we've got a race inbetween with the attaching/detaching that we do
+        # locally on the Cinder node.  Just come up with an easy way to
+        # determine if we're attaching to the Cinder host for some work or if
+        # we're being used by the outside world.
+        expected = {'multiattach': vref.multiattach,
+                    'status': (('available', 'in-use', 'downloading')
+                               if vref.multiattach
+                               else ('available', 'downloading'))}
+        result = vref.conditional_update({'status': 'reserved'}, expected)
+        if not result:
+            msg = (_('Volume %(vol_id)s status must be %(statuses)s') %
+                   {'vol_id': vref.id,
+                    'statuses': utils.build_or_str(expected['status'])})
+            raise exception.InvalidVolume(reason=msg)
+
+        values = {'volume_id': vref.id,
+                  'volume_host': vref.host,
+                  'attach_status': 'reserved',
+                  'instance_uuid': instance_uuid}
+        db_ref = self.db.volume_attach(ctxt.elevated(), values)
+        return objects.VolumeAttachment.get_by_id(ctxt, db_ref['id'])
+
+    @wrap_check_policy
+    def attachment_create(self,
+                          ctxt,
+                          volume_ref,
+                          instance_uuid,
+                          connector=None):
+        """Create an attachment record for the specified volume."""
+        connection_info = {}
+        attachment_ref = self._attachment_reserve(ctxt,
+                                                  volume_ref,
+                                                  instance_uuid)
+        if connector:
+            connection_info = (
+                self.volume_rpcapi.attachment_update(ctxt,
+                                                     volume_ref,
+                                                     connector,
+                                                     attachment_ref.id))
+        attachment_ref.connection_info = connection_info
+        attachment_ref.save()
+        return attachment_ref
+
+    @wrap_check_policy
+    def attachment_update(self, ctxt, attachment_ref, connector):
+        """Update an existing attachment record."""
+        # Valid items to update (connector includes mode and mountpoint):
+        #   1. connector (required)
+        #     a. mode (if None use value from attachment_ref)
+        #     b. mountpoint (if None use value from attachment_ref)
+        #     c. instance_uuid(if None use value from attachment_ref)
+
+        # We fetch the volume object and pass it to the rpc call because we
+        # need to direct this to the correct host/backend
+
+        volume_ref = objects.Volume.get_by_id(ctxt, attachment_ref.volume_id)
+        connection_info = (
+            self.volume_rpcapi.attachment_update(ctxt,
+                                                 volume_ref,
+                                                 connector,
+                                                 attachment_ref.id))
+        attachment_ref.connection_info = connection_info
+        attachment_ref.save()
+        return attachment_ref
+
+    @wrap_check_policy
+    def attachment_delete(self, ctxt, attachment):
+        volume = objects.Volume.get_by_id(ctxt, attachment.volume_id)
+        if attachment.attach_status == 'reserved':
+            attachment.destroy()
+        else:
+            self.volume_rpcapi.attachment_delete(ctxt,
+                                                 attachment.id,
+                                                 volume)
+        remaining_attachments = AO_LIST.get_all_by_volume_id(ctxt, volume.id)
+
+        # TODO(jdg): Make this check attachments_by_volume_id when we
+        # implement multi-attach for real
+        if len(remaining_attachments) < 1:
+            volume.status = 'available'
+            volume.attach_status = 'detached'
+            volume.save()
+        return remaining_attachments
+
 
 class HostAPI(base.Base):
     """Sub-set of the Volume Manager API for managing host operations."""

cinder/volume/manager.py

@@ -110,6 +110,7 @@ VALID_CREATE_CG_SRC_SNAP_STATUS = (fields.SnapshotStatus.AVAILABLE,)
 VALID_CREATE_GROUP_SRC_SNAP_STATUS = (fields.SnapshotStatus.AVAILABLE,)
 VALID_CREATE_CG_SRC_CG_STATUS = ('available',)
 VALID_CREATE_GROUP_SRC_GROUP_STATUS = ('available',)
+VA_LIST = objects.VolumeAttachmentList
 
 volume_manager_opts = [
     cfg.StrOpt('volume_driver',
@@ -1002,11 +1003,11 @@ class VolumeManager(manager.CleanableManager,
             host_name) if host_name else None
         if instance_uuid:
             attachments = (
-                objects.VolumeAttachmentList.get_all_by_instance_uuid(
+                VA_LIST.get_all_by_instance_uuid(
                     context, instance_uuid))
         else:
             attachments = (
-                objects.VolumeAttachmentList.get_all_by_host(
+                VA_LIST.get_all_by_host(
                     context, host_name_sanitized))
         if attachments:
             # check if volume<->instance mapping is already tracked in DB
@@ -1378,85 +1379,7 @@ class VolumeManager(manager.CleanableManager,
                         exc_info=True, resource={'type': 'image',
                                                  'id': image_id})
 
-    def initialize_connection(self, context, volume, connector):
+    def _parse_connection_options(self, context, volume, conn_info):
-        """Prepare volume for connection from host represented by connector.
-
-        This method calls the driver initialize_connection and returns
-        it to the caller.  The connector parameter is a dictionary with
-        information about the host that will connect to the volume in the
-        following format::
-
-            {
-                'ip': ip,
-                'initiator': initiator,
-            }
-
-        ip: the ip address of the connecting machine
-
-        initiator: the iscsi initiator name of the connecting machine.
-        This can be None if the connecting machine does not support iscsi
-        connections.
-
-        driver is responsible for doing any necessary security setup and
-        returning a connection_info dictionary in the following format::
-
-            {
-                'driver_volume_type': driver_volume_type,
-                'data': data,
-            }
-
-        driver_volume_type: a string to identify the type of volume.  This
-                           can be used by the calling code to determine the
-                           strategy for connecting to the volume. This could
-                           be 'iscsi', 'rbd', 'sheepdog', etc.
-
-        data: this is the data that the calling code will use to connect
-              to the volume. Keep in mind that this will be serialized to
-              json in various places, so it should not contain any non-json
-              data types.
-        """
-
-        # NOTE(flaper87): Verify the driver is enabled
-        # before going forward. The exception will be caught
-        # and the volume status updated.
-        utils.require_driver_initialized(self.driver)
-        try:
-            self.driver.validate_connector(connector)
-        except exception.InvalidConnectorException as err:
-            raise exception.InvalidInput(reason=six.text_type(err))
-        except Exception as err:
-            err_msg = (_("Validate volume connection failed "
-                         "(error: %(err)s).") % {'err': six.text_type(err)})
-            LOG.exception(err_msg, resource=volume)
-            raise exception.VolumeBackendAPIException(data=err_msg)
-
-        try:
-            model_update = self.driver.create_export(context.elevated(),
-                                                     volume, connector)
-        except exception.CinderException:
-            err_msg = (_("Create export for volume failed."))
-            LOG.exception(err_msg, resource=volume)
-            raise exception.VolumeBackendAPIException(data=err_msg)
-
-        try:
-            if model_update:
-                volume.update(model_update)
-                volume.save()
-        except exception.CinderException as ex:
-            LOG.exception(_LE("Model update failed."), resource=volume)
-            raise exception.ExportFailure(reason=six.text_type(ex))
-
-        try:
-            conn_info = self.driver.initialize_connection(volume, connector)
-        except Exception as err:
-            err_msg = (_("Driver initialize connection failed "
-                         "(error: %(err)s).") % {'err': six.text_type(err)})
-            LOG.exception(err_msg, resource=volume)
-
-            self.driver.remove_export(context.elevated(), volume)
-
-            raise exception.VolumeBackendAPIException(data=err_msg)
-
         # Add qos_specs to connection info
         typeid = volume.volume_type_id
         specs = None
@@ -1498,6 +1421,91 @@ class VolumeManager(manager.CleanableManager,
                  resource=volume)
         return conn_info
 
+    def initialize_connection(self, context, volume, connector):
+        """Prepare volume for connection from host represented by connector.
+
+        This method calls the driver initialize_connection and returns
+        it to the caller.  The connector parameter is a dictionary with
+        information about the host that will connect to the volume in the
+        following format::
+
+            {
+                'ip': ip,
+                'initiator': initiator,
+            }
+
+        ip: the ip address of the connecting machine
+
+        initiator: the iscsi initiator name of the connecting machine.
+        This can be None if the connecting machine does not support iscsi
+        connections.
+
+        driver is responsible for doing any necessary security setup and
+        returning a connection_info dictionary in the following format::
+
+            {
+                'driver_volume_type': driver_volume_type,
+                'data': data,
+            }
+
+        driver_volume_type: a string to identify the type of volume.  This
+                           can be used by the calling code to determine the
+                           strategy for connecting to the volume. This could
+                           be 'iscsi', 'rbd', 'sheepdog', etc.
+
+        data: this is the data that the calling code will use to connect
+              to the volume. Keep in mind that this will be serialized to
+              json in various places, so it should not contain any non-json
+              data types.
+        """
+        # NOTE(flaper87): Verify the driver is enabled
+        # before going forward. The exception will be caught
+        # and the volume status updated.
+
+        # TODO(jdg): Add deprecation warning
+        utils.require_driver_initialized(self.driver)
+        try:
+            self.driver.validate_connector(connector)
+        except exception.InvalidConnectorException as err:
+            raise exception.InvalidInput(reason=six.text_type(err))
+        except Exception as err:
+            err_msg = (_("Validate volume connection failed "
+                         "(error: %(err)s).") % {'err': six.text_type(err)})
+            LOG.exception(err_msg, resource=volume)
+            raise exception.VolumeBackendAPIException(data=err_msg)
+
+        try:
+            model_update = self.driver.create_export(context.elevated(),
+                                                     volume, connector)
+        except exception.CinderException as ex:
+            msg = _("Create export of volume failed (%s)") % ex.msg
+            LOG.exception(msg, resource=volume)
+            raise exception.VolumeBackendAPIException(data=msg)
+
+        try:
+            if model_update:
+                volume.update(model_update)
+                volume.save()
+        except exception.CinderException as ex:
+            LOG.exception(_LE("Model update failed."), resource=volume)
+            raise exception.ExportFailure(reason=six.text_type(ex))
+
+        try:
+            conn_info = self.driver.initialize_connection(volume, connector)
+        except Exception as err:
+            err_msg = (_("Driver initialize connection failed "
+                         "(error: %(err)s).") % {'err': six.text_type(err)})
+            LOG.exception(err_msg, resource=volume)
+
+            self.driver.remove_export(context.elevated(), volume)
+
+            raise exception.VolumeBackendAPIException(data=err_msg)
+
+        conn_info = self._parse_connection_options(context, volume, conn_info)
+        LOG.info(_LI("Initialize volume connection completed successfully."),
+                 resource=volume)
+        return conn_info
+
     def terminate_connection(self, context, volume_id, connector, force=False):
         """Cleanup connection from host represented by connector.
 
@@ -1522,7 +1530,6 @@ class VolumeManager(manager.CleanableManager,
 
     def remove_export(self, context, volume_id):
         """Removes an export for a volume."""
-
         utils.require_driver_initialized(self.driver)
         volume_ref = self.db.volume_get(context, volume_id)
         try:
@@ -4435,3 +4442,211 @@ class VolumeManager(manager.CleanableManager,
     def secure_file_operations_enabled(self, ctxt, volume):
         secure_enabled = self.driver.secure_file_operations_enabled()
         return secure_enabled
+
+    def _connection_create(self, ctxt, volume, attachment, connector):
+        try:
+            self.driver.validate_connector(connector)
+        except exception.InvalidConnectorException as err:
+            raise exception.InvalidInput(reason=six.text_type(err))
+        except Exception as err:
+            err_msg = (_("Validate volume connection failed "
+                         "(error: %(err)s).") % {'err': six.text_type(err)})
+            LOG.error(err_msg, resource=volume)
+            raise exception.VolumeBackendAPIException(data=err_msg)
+
+        try:
+            model_update = self.driver.create_export(ctxt.elevated(),
+                                                     volume, connector)
+        except exception.CinderException as ex:
+            err_msg = (_("Create export for volume failed (%s).") % ex.msg)
+            LOG.exception(err_msg, resource=volume)
+            raise exception.VolumeBackendAPIException(data=err_msg)
+
+        try:
+            if model_update:
+                volume.update(model_update)
+                volume.save()
+        except exception.CinderException as ex:
+            LOG.exception(_LE("Model update failed."), resource=volume)
+            raise exception.ExportFailure(reason=six.text_type(ex))
+
+        try:
+            conn_info = self.driver.initialize_connection(volume, connector)
+        except Exception as err:
+            err_msg = (_("Driver initialize connection failed "
+                         "(error: %(err)s).") % {'err': six.text_type(err)})
+            LOG.exception(err_msg, resource=volume)
+            self.driver.remove_export(ctxt.elevated(), volume)
+            raise exception.VolumeBackendAPIException(data=err_msg)
+        conn_info = self._parse_connection_options(ctxt, volume, conn_info)
+
+        # NOTE(jdg): Get rid of the nested dict (data key)
+        conn_data = conn_info.pop('data', {})
+        connection_info = conn_data.copy()
+        connection_info.update(conn_info)
+        values = {'volume_id': volume.id,
+                  'attach_status': 'attaching', }
+
+        self.db.volume_attachment_update(ctxt, attachment.id, values)
+        self.db.attachment_specs_update_or_create(
+            ctxt,
+            attachment.id,
+            connector)
+
+        connection_info['attachment_id'] = attachment.id
+        return connection_info
+
+    def attachment_update(self,
+                          context,
+                          vref,
+                          connector,
+                          attachment_id):
+        """Update/Finalize an attachment.
+
+        This call updates a valid attachment record to associate with a volume
+        and provide the caller with the proper connection info.  Note that
+        this call requires an `attachment_ref`.  It's expected that prior to
+        this call that the volume and an attachment UUID has been reserved.
+
+        param: vref: Volume object to create attachment for
+        param: connector: Connector object to use for attachment creation
+        param: attachment_ref: ID of the attachment record to update
+        """
+
+        mode = connector.get('mode', 'rw')
+        self._notify_about_volume_usage(context, vref, 'attach.start')
+        attachment_ref = objects.VolumeAttachment.get_by_id(context,
+                                                            attachment_id)
+        connection_info = self._connection_create(context,
+                                                  vref,
+                                                  attachment_ref,
+                                                  connector)
+        # FIXME(jdg): get rid of this admin_meta option here, the only thing
+        # it does is enforce that a volume is R/O, that should be done via a
+        # type and not *more* metadata
+        volume_metadata = self.db.volume_admin_metadata_update(
+            context.elevated(),
+            attachment_ref.volume_id,
+            {'attached_mode': mode}, False)
+
+        if volume_metadata.get('readonly') == 'True' and mode != 'ro':
+            self.db.volume_update(context, vref.id,
+                                  {'status': 'error_attaching'})
+            self.message_api.create(
+                context, defined_messages.ATTACH_READONLY_VOLUME,
+                context.project_id, resource_type=resource_types.VOLUME,
+                resource_uuid=vref.id)
+            raise exception.InvalidVolumeAttachMode(mode=mode,
+                                                    volume_id=vref.id)
+        try:
+            utils.require_driver_initialized(self.driver)
+            self.driver.attach_volume(context,
+                                      vref,
+                                      attachment_ref.instance_uuid,
+                                      connector.get('hostname', ''),
+                                      connector.get('mountpoint', 'na'))
+        except Exception:
+            with excutils.save_and_reraise_exception():
+                self.db.volume_attachment_update(
+                    context, attachment_ref.id,
+                    {'attach_status': 'error_attaching'})
+
+        self.db.volume_attached(context.elevated(),
+                                attachment_ref.id,
+                                attachment_ref.instance_uuid,
+                                connector.get('hostname', ''),
+                                connector.get('mountpoint', 'na'),
+                                mode)
+        vref.refresh()
+        self._notify_about_volume_usage(context, vref, "attach.end")
+        LOG.info(_LI("Attach volume completed successfully."),
+                 resource=vref)
+        attachment_ref = objects.VolumeAttachment.get_by_id(context,
+                                                            attachment_id)
+        return connection_info
+
+    def _connection_terminate(self, context, volume,
+                              attachment, force=False):
+        """Remove a volume connection, but leave attachment."""
+        utils.require_driver_initialized(self.driver)
+
+        # TODO(jdg): Add an object method to cover this
+        connector = self.db.attachment_specs_get(
+            context,
+            attachment.id)
+
+        try:
+            shared_connections = self.driver.terminate_connection(volume,
+                                                                  connector,
+                                                                  force=force)
+            if not isinstance(shared_connections, bool):
+                shared_connections = False
+
+        except Exception as err:
+            err_msg = (_('Terminate volume connection failed: %(err)s')
+                       % {'err': six.text_type(err)})
+            LOG.exception(err_msg, resource=volume)
+            raise exception.VolumeBackendAPIException(data=err_msg)
+        LOG.info(_LI("Terminate volume connection completed successfully."),
+                 resource=volume)
+        # NOTE(jdg): Return True/False if there are other outstanding
+        # attachments that share this connection.  If True should signify
+        # caller to preserve the actual host connection (work should be
+        # done in the brick connector as it has the knowledge of what's
+        # going on here.
+        return shared_connections
+
+    def attachment_delete(self, context, attachment_id, vref):
+        """Delete/Detach the specified attachment.
+
+        Notifies the backend device that we're detaching the specified
+        attachment instance.
+
+        param: vref: Volume object associated with the attachment
+        param: attachment: Attachment reference object to remove
+
+        NOTE if the attachment reference is None, we remove all existing
+        attachments for the specified volume object.
+        """
+        has_shared_connection = False
+        attachment_ref = objects.VolumeAttachment.get_by_id(context,
+                                                            attachment_id)
+        if not attachment_ref:
+            for attachment in VA_LIST.get_all_by_volume_id(context, vref.id):
+                if self._do_attachment_delete(context, vref, attachment):
+                    has_shared_connection = True
+        else:
+            has_shared_connection = (
+                self._do_attachment_delete(context, vref, attachment_ref))
+        return has_shared_connection
+
+    def _do_attachment_delete(self, context, vref, attachment):
+        utils.require_driver_initialized(self.driver)
+        self._notify_about_volume_usage(context, vref, "detach.start")
+        has_shared_connection = self._connection_terminate(context,
+                                                           vref,
+                                                           attachment)
+        self.driver.detach_volume(context, vref, attachment)
+        try:
+            LOG.debug('Deleting attachment %(attachment_id)s.',
+                      {'attachment_id': attachment.id},
+                      resource=vref)
+            self.driver.detach_volume(context, vref, attachment)
+            self.driver.remove_export(context.elevated(), vref)
+        except Exception:
+            # FIXME(jdg): Obviously our volume object is going to need some
+            # changes to deal with multi-attach and figuring out how to
+            # represent a single failed attach out of multiple attachments
+
+            # TODO(jdg): object method here
+            self.db.volume_attachment_update(
+                context, attachment.get('id'),
+                {'attach_status': 'error_detaching'})
+        else:
+            self.db.volume_detached(context.elevated(), vref.id,
+                                    attachment.get('id'))
+            self.db.volume_admin_metadata_delete(context.elevated(),
+                                                 vref.id,
+                                                 'attached_mode')
+        self._notify_about_volume_usage(context, vref, "detach.end")
+        return has_shared_connection

cinder/volume/rpcapi.py

@@ -123,9 +123,10 @@ class VolumeAPI(rpc.RPCAPI):
         3.7  - Adds do_cleanup method to do volume cleanups from other nodes
                that we were doing in init_host.
         3.8  - Make failover_host cluster aware and add failover_completed.
+        3.9  - Adds new attach/detach methods
     """
 
-    RPC_API_VERSION = '3.8'
+    RPC_API_VERSION = '3.9'
     RPC_DEFAULT_VERSION = '3.0'
     TOPIC = constants.VOLUME_TOPIC
     BINARY = 'cinder-volume'
@@ -406,6 +407,35 @@ class VolumeAPI(rpc.RPCAPI):
         cctxt.cast(ctxt, 'delete_group_snapshot',
                    group_snapshot=group_snapshot)
 
+    def attachment_update(self, ctxt, vref, connector, attachment_id):
+        if not self.client.can_send_version('3.9'):
+            msg = _('One of cinder-volume services is too old to accept '
+                    'such request. Are you running mixed Newton-Ocata'
+                    'cinder-schedulers?')
+            raise exception.ServiceTooOld(msg)
+
+        version = self._compat_ver('3.9')
+        cctxt = self._get_cctxt(vref.host, version=version)
+        return cctxt.call(ctxt,
+                          'attachment_update',
+                          vref=vref,
+                          connector=connector,
+                          attachment_id=attachment_id)
+
+    def attachment_delete(self, ctxt, attachment_id, vref):
+        if not self.client.can_send_version('3.9'):
+            msg = _('One of cinder-volume services is too old to accept '
+                    'such request. Are you running mixed Newton-Ocata'
+                    'cinder-schedulers?')
+            raise exception.ServiceTooOld(msg)
+
+        version = self._compat_ver('3.9')
+        cctxt = self._get_cctxt(vref.host, version=version)
+        return cctxt.call(ctxt,
+                          'attachment_delete',
+                          attachment_id=attachment_id,
+                          vref=vref)
+
     def do_cleanup(self, ctxt, cleanup_request):
         """Perform this service/cluster resource cleanup as requested."""
         if not self.client.can_send_version('3.7'):

doc/source/devref/attach_detach_conventions_v2.rst

@@ -0,0 +1,135 @@
+..
+      Licensed under the Apache License, Version 2.0 (the "License"); you may
+      not use this file except in compliance with the License. You may obtain
+      a copy of the License at
+
+          http://www.apache.org/licenses/LICENSE-2.0
+
+      Unless required by applicable law or agreed to in writing, software
+      distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+      WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+      License for the specific language governing permissions and limitations
+      under the License.
+
+=============================
+Volume Attach/Detach workflow
+=============================
+
+Previously there were six API calls associated with attach/detach of volumes in
+Cinder (3 calls for each operation).  As the projects grew and the
+functionality of *simple* things like attach/detach evolved things have become
+a bit vague and we have a number of race issues during the calls that
+continually cause us some problems.
+
+Additionally, the existing code path makes things like multi-attach extremely
+difficult to implement due to no real good tracking mechansim of attachment
+info.
+
+To try and improve this we've proposed a new Attachments Object and API.  Now
+we keep an Attachment record for each attachment that we want to perform as
+opposed to trying to infer the information from the Volume Object.
+
+Attachment Object
+=================
+
+We actually already had a VolumeAttachment Table in the db, however we
+weren't really using it, or at least using it efficiently. For V2 of attach
+implementation (V3 API) flow we'll use the Attachment Table (object) as
+the primary handle for managing attachment(s) for a volume.
+
+In addition, we also introduce the AttachmentSpecs Table which will store the
+connector information for an Attachment so we no longer have the problem of
+lost connector info, or trying to reassemble it.
+
+New API and Flow
+=================
+
+```
+attachment_create <instance_uuid> <volume_id>
+```
+
+The attachment_create call simply creates an empty Attachment record for the
+specified Volume with an optional Instance UUID field set.  This is
+particularly useful for cases like Nova Boot from Volume where Nova hasn't sent
+the job to the actual Compute host yet, but needs to make initial preparations
+to reserve the volume for use, so here we can reserve the volume and indicate
+that we will be attaching it to <Instance-UUID> in the future.
+
+Alternatively, the caller may provide a connector in which case the Cinder API
+will create the attachment and perform the update on the attachment to set the
+connector info and return the connection data needed to make a connection.
+
+The attachment_create call can be used in one of two ways:
+1. Create an empty Attachment object (reserve)
+   attachment_create call.  In this case the attachment_create call requires
+   an instance_uuid and a volume_uuid, and just creates an empty attachment
+   object and returns the UUID of said attachment to the caller.
+
+2. Create and Complete the Attachment process in one call.  The Reserve process
+   is only needed in certain cases, in many cases Nova actually has enough
+   information to do everything in a single call.  Also, non-nova consumers
+   typically don't require the granularity of a separate reserve at all.
+
+   To perform the complete operation, include the connector data in the
+   attachment_create call and the Cinder API will perform the reserve and
+   initialize the connection in the single request.
+
+
+
+param instance-uuid: The ID of the Instance we'll be attaching to
+param volume-id: The ID of the volume to reserve an Attachment record for
+rtyp: string:`VolumeAttachmentID`
+
+```
+cinder --os-volume-api-version 3.27 attachment-create --instance <instance-uuid>  <volume-id>
+```
+
+param volume_id: The ID of the volume to create attachment for.
+parm attachment_id: The ID of a previously reserved attachment.
+
+param connector: Dictionary of connection info
+param mode: `rw` or `ro` (defaults to `rw` if omitted).
+param mountpoint: Mountpoint of remote attachment.
+rtype: :class:`VolumeAttachment`
+
+Example connector:
+    {'initiator': 'iqn.1993-08.org.debian:01:cad181614cec',
+     'ip':'192.168.1.20',
+     'platform': 'x86_64',
+     'host': 'tempest-1',
+     'os_type': 'linux2',
+     'multipath': False}
+
+```
+cinder --os-volume-api-version 3.27 attachment-create --initiator iqn.1993-08.org.debian:01:29353d53fa41 --ip 1.1.1.1 --host blah --instance <instance-id> <volume-id>
+```
+
+Returns a dictionary including the connector and attachment_id:
+
+```
++-------------------+-----------------------------------------------------------------------+
+| Property          | Value                                                                 |
++-------------------+-----------------------------------------------------------------------+
+| access_mode       | rw                                                                    |
+| attachment_id     | 6ab061ad-5c45-48f3-ad9c-bbd3b6275bf2                                  |
+| auth_method       | CHAP                                                                  |
+| auth_password     | kystSioDKHSV2j9y                                                      |
+| auth_username     | hxGUgiWvsS4GqAQcfA78                                                  |
+| encrypted         | False                                                                 |
+| qos_specs         | None                                                                  |
+| target_discovered | False                                                                 |
+| target_iqn        | iqn.2010-10.org.openstack:volume-23212c97-5ed7-42d7-b433-dbf8fc38ec35 |
+| target_lun        | 0                                                                     |
+| target_portal     | 192.168.0.9:3260                                                      |
+| volume_id         | 23212c97-5ed7-42d7-b433-dbf8fc38ec35                                  |
++-------------------+-----------------------------------------------------------------------+
+```
+
+
+attachment-delete
+=================
+
+```
+cinder --os-volume-api-version 3.27 attachment-delete 6ab061ad-5c45-48f3-ad9c-bbd3b6275bf2
+```
+