Update detach to use V3 Cinder API
Use the new Cinder V3 attachment delete method during detach if the BDM has an attachment_id in it. This will only be present in the BDM if/when the new attachment_create API is called. Otherwise, we revert to the old calls. Edge cases are handled in separate patches. Partially Implements: blueprint cinder-new-attach-apis Co-Authored-By: Steve Noyes <steve.noyes@oracle.com> Change-Id: I91b9a60268354ffbed86b1e7d173906cfd7b97bd
This commit is contained in:
parent
1106477b78
commit
39e2760f35
|
@ -571,6 +571,10 @@ class AgentBuildExists(NovaException):
|
||||||
"architecture %(architecture)s exists.")
|
"architecture %(architecture)s exists.")
|
||||||
|
|
||||||
|
|
||||||
|
class VolumeAttachmentNotFound(NotFound):
|
||||||
|
msg_fmt = _("Volume attachment %(attachment_id)s could not be found.")
|
||||||
|
|
||||||
|
|
||||||
class VolumeNotFound(NotFound):
|
class VolumeNotFound(NotFound):
|
||||||
msg_fmt = _("Volume %(volume_id)s could not be found.")
|
msg_fmt = _("Volume %(volume_id)s could not be found.")
|
||||||
|
|
||||||
|
|
|
@ -101,6 +101,7 @@ class TestDriverBlockDevice(test.NoDBTestCase):
|
||||||
'boot_index': 0})
|
'boot_index': 0})
|
||||||
|
|
||||||
volume_driver_bdm = {
|
volume_driver_bdm = {
|
||||||
|
'attachment_id': None,
|
||||||
'mount_device': '/dev/sda1',
|
'mount_device': '/dev/sda1',
|
||||||
'connection_info': {"fake": "connection_info"},
|
'connection_info': {"fake": "connection_info"},
|
||||||
'delete_on_termination': False,
|
'delete_on_termination': False,
|
||||||
|
@ -129,6 +130,7 @@ class TestDriverBlockDevice(test.NoDBTestCase):
|
||||||
'boot_index': -1})
|
'boot_index': -1})
|
||||||
|
|
||||||
snapshot_driver_bdm = {
|
snapshot_driver_bdm = {
|
||||||
|
'attachment_id': None,
|
||||||
'mount_device': '/dev/sda2',
|
'mount_device': '/dev/sda2',
|
||||||
'connection_info': {"fake": "connection_info"},
|
'connection_info': {"fake": "connection_info"},
|
||||||
'delete_on_termination': True,
|
'delete_on_termination': True,
|
||||||
|
@ -157,6 +159,7 @@ class TestDriverBlockDevice(test.NoDBTestCase):
|
||||||
'boot_index': -1})
|
'boot_index': -1})
|
||||||
|
|
||||||
image_driver_bdm = {
|
image_driver_bdm = {
|
||||||
|
'attachment_id': None,
|
||||||
'mount_device': '/dev/sda2',
|
'mount_device': '/dev/sda2',
|
||||||
'connection_info': {"fake": "connection_info"},
|
'connection_info': {"fake": "connection_info"},
|
||||||
'delete_on_termination': True,
|
'delete_on_termination': True,
|
||||||
|
@ -185,6 +188,7 @@ class TestDriverBlockDevice(test.NoDBTestCase):
|
||||||
'boot_index': -1})
|
'boot_index': -1})
|
||||||
|
|
||||||
blank_driver_bdm = {
|
blank_driver_bdm = {
|
||||||
|
'attachment_id': None,
|
||||||
'mount_device': '/dev/sda2',
|
'mount_device': '/dev/sda2',
|
||||||
'connection_info': {"fake": "connection_info"},
|
'connection_info': {"fake": "connection_info"},
|
||||||
'delete_on_termination': True,
|
'delete_on_termination': True,
|
||||||
|
@ -383,6 +387,31 @@ class TestDriverBlockDevice(test.NoDBTestCase):
|
||||||
def test_call_wait_no_delete_volume(self):
|
def test_call_wait_no_delete_volume(self):
|
||||||
self._test_call_wait_func(False)
|
self._test_call_wait_func(False)
|
||||||
|
|
||||||
|
def test_volume_delete_attachment(self):
|
||||||
|
attachment_id = uuids.attachment
|
||||||
|
driver_bdm = self.driver_classes['volume'](self.volume_bdm)
|
||||||
|
driver_bdm['attachment_id'] = attachment_id
|
||||||
|
|
||||||
|
elevated_context = self.context.elevated()
|
||||||
|
instance_detail = {'id': '123', 'uuid': uuids.uuid,
|
||||||
|
'availability_zone': None}
|
||||||
|
instance = fake_instance.fake_instance_obj(self.context,
|
||||||
|
**instance_detail)
|
||||||
|
connector = {'ip': 'fake_ip', 'host': 'fake_host'}
|
||||||
|
|
||||||
|
with test.nested(
|
||||||
|
mock.patch.object(self.virt_driver, 'get_volume_connector',
|
||||||
|
return_value=connector),
|
||||||
|
mock.patch.object(self.volume_api, 'attachment_delete'),
|
||||||
|
) as (_, vapi_attach_del):
|
||||||
|
|
||||||
|
driver_bdm.detach(elevated_context, instance,
|
||||||
|
self.volume_api, self.virt_driver,
|
||||||
|
attachment_id=attachment_id)
|
||||||
|
|
||||||
|
vapi_attach_del.assert_called_once_with(elevated_context,
|
||||||
|
attachment_id)
|
||||||
|
|
||||||
def _test_volume_attach(self, driver_bdm, bdm_dict,
|
def _test_volume_attach(self, driver_bdm, bdm_dict,
|
||||||
fake_volume, fail_check_av_zone=False,
|
fake_volume, fail_check_av_zone=False,
|
||||||
driver_attach=False, fail_driver_attach=False,
|
driver_attach=False, fail_driver_attach=False,
|
||||||
|
|
|
@ -280,6 +280,33 @@ class CinderApiTestCase(test.NoDBTestCase):
|
||||||
mock_volumes.attach.assert_called_once_with('id1', 'uuid', 'point',
|
mock_volumes.attach.assert_called_once_with('id1', 'uuid', 'point',
|
||||||
mode='ro')
|
mode='ro')
|
||||||
|
|
||||||
|
@mock.patch('nova.volume.cinder.cinderclient')
|
||||||
|
def test_attachment_delete(self, mock_cinderclient):
|
||||||
|
mock_attachments = mock.MagicMock()
|
||||||
|
mock_cinderclient.return_value = \
|
||||||
|
mock.MagicMock(attachments=mock_attachments)
|
||||||
|
|
||||||
|
attachment_id = uuids.attachment
|
||||||
|
self.api.attachment_delete(self.ctx, attachment_id)
|
||||||
|
|
||||||
|
mock_cinderclient.assert_called_once_with(self.ctx)
|
||||||
|
mock_attachments.delete.assert_called_once_with(attachment_id)
|
||||||
|
|
||||||
|
@mock.patch('nova.volume.cinder.LOG')
|
||||||
|
@mock.patch('nova.volume.cinder.cinderclient')
|
||||||
|
def test_attachment_delete_failed(self, mock_cinderclient, mock_log):
|
||||||
|
mock_cinderclient.return_value.attachments.delete.side_effect = (
|
||||||
|
cinder_exception.NotFound(404, '404'))
|
||||||
|
|
||||||
|
attachment_id = uuids.attachment
|
||||||
|
ex = self.assertRaises(exception.VolumeAttachmentNotFound,
|
||||||
|
self.api.attachment_delete,
|
||||||
|
self.ctx,
|
||||||
|
attachment_id)
|
||||||
|
|
||||||
|
self.assertEqual(404, ex.code)
|
||||||
|
self.assertIn(attachment_id, ex.message)
|
||||||
|
|
||||||
@mock.patch('nova.volume.cinder.cinderclient')
|
@mock.patch('nova.volume.cinder.cinderclient')
|
||||||
def test_detach(self, mock_cinderclient):
|
def test_detach(self, mock_cinderclient):
|
||||||
mock_volumes = mock.MagicMock()
|
mock_volumes = mock.MagicMock()
|
||||||
|
|
|
@ -224,7 +224,8 @@ class DriverVolumeBlockDevice(DriverBlockDevice):
|
||||||
_legacy_fields = set(['connection_info', 'mount_device',
|
_legacy_fields = set(['connection_info', 'mount_device',
|
||||||
'delete_on_termination'])
|
'delete_on_termination'])
|
||||||
_new_fields = set(['guest_format', 'device_type',
|
_new_fields = set(['guest_format', 'device_type',
|
||||||
'disk_bus', 'boot_index'])
|
'disk_bus', 'boot_index',
|
||||||
|
'attachment_id'])
|
||||||
_fields = _legacy_fields | _new_fields
|
_fields = _legacy_fields | _new_fields
|
||||||
|
|
||||||
_valid_source = 'volume'
|
_valid_source = 'volume'
|
||||||
|
@ -349,9 +350,16 @@ class DriverVolumeBlockDevice(DriverBlockDevice):
|
||||||
'schost': stashed_connector.get('host')})
|
'schost': stashed_connector.get('host')})
|
||||||
connector = stashed_connector
|
connector = stashed_connector
|
||||||
|
|
||||||
volume_api.terminate_connection(context, volume_id, connector)
|
# NOTE(jdg): For now we need to actually inspect the bdm for an
|
||||||
volume_api.detach(context.elevated(), volume_id, instance.uuid,
|
# attachment_id as opposed to relying on what may have been passed
|
||||||
attachment_id)
|
# in, we want to force usage of the old detach flow for now and only
|
||||||
|
# use the new flow when we explicitly used it for the attach.
|
||||||
|
if not self['attachment_id']:
|
||||||
|
volume_api.terminate_connection(context, volume_id, connector)
|
||||||
|
volume_api.detach(context.elevated(), volume_id, instance.uuid,
|
||||||
|
attachment_id)
|
||||||
|
else:
|
||||||
|
volume_api.attachment_delete(context, self['attachment_id'])
|
||||||
|
|
||||||
@update_db
|
@update_db
|
||||||
def attach(self, context, instance, volume_api, virt_driver,
|
def attach(self, context, instance, volume_api, virt_driver,
|
||||||
|
|
|
@ -205,6 +205,19 @@ def translate_volume_exception(method):
|
||||||
return translate_cinder_exception(wrapper)
|
return translate_cinder_exception(wrapper)
|
||||||
|
|
||||||
|
|
||||||
|
def translate_attachment_exception(method):
|
||||||
|
"""Transforms the exception for the attachment but keeps its traceback intact.
|
||||||
|
"""
|
||||||
|
def wrapper(self, ctx, attachment_id, *args, **kwargs):
|
||||||
|
try:
|
||||||
|
res = method(self, ctx, attachment_id, *args, **kwargs)
|
||||||
|
except (keystone_exception.NotFound, cinder_exception.NotFound):
|
||||||
|
_reraise(exception.VolumeAttachmentNotFound(
|
||||||
|
attachment_id=attachment_id))
|
||||||
|
return res
|
||||||
|
return translate_cinder_exception(wrapper)
|
||||||
|
|
||||||
|
|
||||||
def translate_snapshot_exception(method):
|
def translate_snapshot_exception(method):
|
||||||
"""Transforms the exception for the snapshot but keeps its traceback
|
"""Transforms the exception for the snapshot but keeps its traceback
|
||||||
intact.
|
intact.
|
||||||
|
@ -480,3 +493,16 @@ class API(object):
|
||||||
{'status': status,
|
{'status': status,
|
||||||
'progress': '90%'}
|
'progress': '90%'}
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@translate_attachment_exception
|
||||||
|
def attachment_delete(self, context, attachment_id):
|
||||||
|
try:
|
||||||
|
cinderclient(
|
||||||
|
context).attachments.delete(attachment_id)
|
||||||
|
except cinder_exception.ClientException as ex:
|
||||||
|
with excutils.save_and_reraise_exception():
|
||||||
|
LOG.error(('Delete 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)})
|
||||||
|
|
Loading…
Reference in New Issue