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 commit41b982c9fe
) (cherry picked from commite7dc43bf84
)
This commit is contained in:
parent
9cc0a58cf1
commit
b8704a1d1c
|
@ -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)
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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',
|
||||
|
|
Loading…
Reference in New Issue