diff --git a/nova/compute/api.py b/nova/compute/api.py index 8cf7a52a0bcc..c3a552313d13 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -4282,12 +4282,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) diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index 7d8bf5e06aa0..4238a7f7426a 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -1220,19 +1220,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) diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 1d57f07df3e8..3246db54fa8f 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -10804,8 +10804,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',