openstack/cinder
Code Issues Proposed changes

Make attachment_update set status to attaching

Browse Source 
The new attachment_update method in Cinder's API creates an attachment
object and populates it with the provided connector info. In addition,
we set the volumes status to in-use and update the attachment object
status to "attached".

This isn't really accurate though, because we don't know if the volume
is actually attached (connected) by the consumer or not.  Also a big
side effect here is that currently all of our tests and automation
use volume-status to determine if a volume is fully connected/ready
for use and that everything went well.  It's used as an ack in most
cases.

This change goes back to using multiple states to signify where a
an attachment is in it's life-cycle:
1. reserved
     We've created an empty attachment record but haven't done anything
     with it yet.
2. attaching
     We provided a connector and set up the TGT so that everything is
     ready for a consumer to connect/use it.
3. in-use
     An ACK back from the consumer letting us know that they connected
     it successfully and are doing their thing.

Some consumers don't need or care about this last step, and we're going
to provide a means to set it straight to attached/in-use, but for
this bug we don't need to introduce that particular *feature*.

Sadly, this requires a micro-version bump and a new API call to
toggle the state from the API, pushing us to 3.44.

closes-bug #1710295

Change-Id: I57631d3deddb2d7cd244584e82206ee17fe2dd78
This commit is contained in:
John Griffith 2017-08-11 17:19:06 -06:00
parent 9f189858d7
commit 7f69969345
9 changed files with 62 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
cinder/api/openstack/api_version_request.py
View File

@ -106,6 +106,7 @@ REST_API_VERSION_HISTORY = """
'volume:extend_attached_volume' policy rule. Extend in reserved
state is intentionally NOT allowed.
* 3.43 - Support backup CRUD with metadata.
* 3.44 - Add attachment-complete.
"""
# The minimum and maximum versions of the API supported
@ -113,7 +114,7 @@ REST_API_VERSION_HISTORY = """
# minimum version of the API supported.
# Explicitly using /v1 or /v2 endpoints will still work
_MIN_API_VERSION = "3.0"
_MAX_API_VERSION = "3.43"
_MAX_API_VERSION = "3.44"
_LEGACY_API_VERSION1 = "1.0"
_LEGACY_API_VERSION2 = "2.0"

4
cinder/api/openstack/rest_api_version_history.rst
View File

@ -365,3 +365,7 @@ user documentation.
3.43
----
Support backup CRUD with metadata.
3.44
----
Support attachment completion.

15
cinder/api/v3/attachments.py
View File

@ -27,6 +27,7 @@ from cinder.volume import api as volume_api
LOG = logging.getLogger(__name__)
API_VERSION = '3.27'
ATTACHMENT_COMPLETION_VERSION = '3.44'
class AttachmentsController(wsgi.Controller):
@ -266,6 +267,20 @@ class AttachmentsController(wsgi.Controller):
attachments = self.volume_api.attachment_delete(context, attachment)
return attachment_views.ViewBuilder.list(attachments)
@wsgi.response(202)
@wsgi.Controller.api_version(ATTACHMENT_COMPLETION_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))
volume_ref = objects.Volume.get_by_id(
context,
attachment_ref.volume_id)
attachment_ref.update({'attach_status': 'attached'})
volume_ref.update({'status': 'in-use', 'attach_status': 'attached'})
def create_resource(ext_mgr):
"""Create the wsgi resource for this controller."""

4
cinder/db/api.py
View File

@ -223,10 +223,10 @@ def volume_attach(context, values):
def volume_attached(context, volume_id, instance_id, host_name, mountpoint,
attach_mode='rw'):
attach_mode='rw', mark_attached=True):
"""Ensure that a volume is set as attached."""
return IMPL.volume_attached(context, volume_id, instance_id, host_name,
mountpoint, attach_mode)
mountpoint, attach_mode, mark_attached)
def volume_create(context, values):

21
cinder/db/sqlalchemy/api.py
View File

@ -1451,14 +1451,25 @@ def volume_attach(context, values):
@require_admin_context
def volume_attached(context, attachment_id, instance_uuid, host_name,
mountpoint, attach_mode='rw'):
mountpoint, attach_mode, mark_attached):
"""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 attached or attaching.
The mark_attached argument is a boolean, when set to True,
we mark the volume as 'in-use' and the 'attachment' as
'attached', if False, we use 'attaching' for both of these
status settings.
"""
attach_status = fields.VolumeAttachStatus.ATTACHED
volume_status = 'in-use'
if not mark_attached:
attach_status = fields.VolumeAttachStatus.ATTACHING
volume_status = 'attaching'
if instance_uuid and not uuidutils.is_uuid_like(instance_uuid):
raise exception.InvalidUUID(uuid=instance_uuid)
@ -1468,7 +1479,7 @@ def volume_attached(context, attachment_id, instance_uuid, host_name,
session=session)
updated_values = {'mountpoint': mountpoint,
'attach_status': fields.VolumeAttachStatus.ATTACHED,
'attach_status': attach_status,
'instance_uuid': instance_uuid,
'attached_host': host_name,
'attach_time': timeutils.utcnow(),
@ -1480,8 +1491,8 @@ 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'] = 'in-use'
volume_ref['attach_status'] = fields.VolumeAttachStatus.ATTACHED
volume_ref['status'] = volume_status
volume_ref['attach_status'] = attach_status
volume_ref.save(session=session)
return (volume_ref, updated_values)

4
cinder/tests/unit/attachments/test_attachments_manager.py
View File

@ -119,8 +119,8 @@ class AttachmentManagerTestCase(test.TestCase):
self.assertEqual('rw', new_attachment_ref['attach_mode'])
new_volume_ref = db.volume_get(self.context, vref.id)
self.assertEqual('in-use', new_volume_ref.status)
self.assertEqual(fields.VolumeAttachStatus.ATTACHED,
self.assertEqual('attaching', new_volume_ref.status)
self.assertEqual(fields.VolumeAttachStatus.ATTACHING,
new_volume_ref.attach_status)
def test_attachment_delete(self):

3
cinder/tests/unit/objects/test_volume_attachment.py
View File

@ -92,7 +92,8 @@ class TestVolumeAttachment(test_objects.BaseObjectsTestCase):
fake.INSTANCE_ID,
'fake_host',
'/dev/sda',
'rw')
'rw',
True)
self.assertEqual('/dev/sda', attachment.mountpoint)
self.assertEqual(fake.INSTANCE_ID, attachment.instance_uuid)
self.assertEqual(fields.VolumeAttachStatus.ATTACHED,

15
cinder/tests/unit/volume/test_volume.py
View File

@ -2793,6 +2793,21 @@ class VolumeTestCase(base.BaseVolumeTestCase):
manager._set_resource_host(volume)
save_mock.assert_not_called()
def test_volume_attach_attaching(self):
"""Test volume_attach ."""
instance_uuid = '12345678-1234-5678-1234-567812345678'
volume = tests_utils.create_volume(self.context, **self.volume_params)
attachment = db.volume_attach(self.context,
{'volume_id': volume['id'],
'attached_host': 'fake-host'})
db.volume_attached(self.context, attachment['id'], instance_uuid,
'fake-host', 'vdb', mark_attached=False)
volume_api = cinder.volume.api.API()
volume = volume_api.get(self.context, volume['id'])
self.assertEqual("attaching", volume['status'])
self.assertEqual("attaching", volume['attach_status'])
class VolumeTestCaseLocks(base.BaseVolumeTestCase):
MOCK_TOOZ = False

8
cinder/volume/manager.py
View File

@ -4373,13 +4373,13 @@ class VolumeManager(manager.CleanableManager,
attachment_ref.instance_uuid,
connector.get('host', ''),
connector.get('mountpoint', 'na'),
mode)
mode,
False)
vref.refresh()
attachment_ref.refresh()
self._notify_about_volume_usage(context, vref, "attach.end")
LOG.info("Attach volume completed successfully.",
LOG.info("attachment_update completed successfully.",
resource=vref)
attachment_ref = objects.VolumeAttachment.get_by_id(context,
attachment_id)
return connection_info
def _connection_terminate(self, context, volume,