Don't call begin_detaching when detaching volume from shelved vm

When shelve an instance, if the instance has volume attached,
with new attach/detach flow, we will delete the old attachment
and create a new attachment, the volume status will be ``reserved``.

If the user tries to detach these volumes, it fails due to that
Cinder does not allow a begin_detaching() call on a `reserved` volume.

Actually for shelved instances, we can just skip this step and
directly detach it.

Change-Id: Ib1799feebbd8f4b0f389168939df7e5e90c8add1
closes-bug: #1808089
(cherry picked from commit 41b982c9fe)
(cherry picked from commit e7dc43bf84)
This commit is contained in:
Kevin_Zheng 2018-12-13 19:31:13 +08:00 committed by Matt Riedemann
parent 9cc0a58cf1
commit b8704a1d1c
3 changed files with 12 additions and 11 deletions

View File

@ -4123,12 +4123,17 @@ class API(base.Base):
def detach_volume(self, context, instance, bdms):
self._local_cleanup_bdm_volumes(bdms, instance, context)
try:
self.volume_api.begin_detaching(context, volume['id'])
except exception.InvalidInput as exc:
raise exception.InvalidVolume(reason=exc.format_message())
bdms = [objects.BlockDeviceMapping.get_by_volume_id(
context, volume['id'], instance.uuid)]
# The begin_detaching() call only works with in-use volumes,
# which will not be the case for volumes attached to a shelved
# offloaded server via the attachments API since those volumes
# will have `reserved` status.
if not bdms[0].attachment_id:
try:
self.volume_api.begin_detaching(context, volume['id'])
except exception.InvalidInput as exc:
raise exception.InvalidVolume(reason=exc.format_message())
self._record_action_start(
context, instance,
instance_actions.DETACH_VOLUME)

View File

@ -1195,19 +1195,16 @@ class ServerTestV220(ServersTestBase):
self.assertIsNone(attach_response['device'])
# Test detach volume
with test.nested(mock.patch.object(volume.cinder.API,
'begin_detaching'),
mock.patch.object(objects.BlockDeviceMappingList,
with test.nested(mock.patch.object(objects.BlockDeviceMappingList,
'get_by_instance_uuid'),
mock.patch.object(compute_api.API,
'_local_cleanup_bdm_volumes')
) as (mock_check, mock_get_bdms, mock_clean_vols):
) as (mock_get_bdms, mock_clean_vols):
mock_get_bdms.return_value = fake_bdms
attachment_id = mock_get_bdms.return_value[0]['volume_id']
self.api.api_delete('/servers/%s/os-volume_attachments/%s' %
(server_id, attachment_id))
self.assertTrue(mock_check.called)
self.assertTrue(mock_clean_vols.called)
self._delete_server(server_id)

View File

@ -10706,8 +10706,7 @@ class ComputeAPITestCase(BaseTestCase):
self.compute_api._detach_volume_shelved_offloaded(self.context,
instance,
volume)
mock_begin_detaching.assert_called_once_with(self.context,
volume['id'])
mock_begin_detaching.assert_not_called()
self.assertTrue(mock_local_cleanup.called)
@mock.patch.object(nova.volume.cinder.API, 'begin_detaching',