diff --git a/nova/compute/manager.py b/nova/compute/manager.py index d3504b8fccf3..8688fdccaa2d 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -5502,8 +5502,16 @@ class ComputeManager(manager.Manager): 'mountpoint': bdm['mount_device']}, instance=instance) if bdm['attachment_id']: - self.volume_api.attachment_delete(context, - bdm['attachment_id']) + # Try to delete the attachment to make the volume + # available again. Note that DriverVolumeBlockDevice + # may have already deleted the attachment so ignore + # VolumeAttachmentNotFound. + try: + self.volume_api.attachment_delete( + context, bdm['attachment_id']) + except exception.VolumeAttachmentNotFound as exc: + LOG.debug('Ignoring VolumeAttachmentNotFound: %s', + exc, instance=instance) else: self.volume_api.unreserve_volume(context, bdm.volume_id) tb = traceback.format_exc() diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 0d2df5a4143d..7620a7406bfe 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -487,6 +487,60 @@ class ComputeVolumeTestCase(BaseTestCase): tb=mock.ANY), ]) + @mock.patch.object(compute_manager.LOG, 'debug') + @mock.patch.object(compute_utils, 'EventReporter') + @mock.patch('nova.context.RequestContext.elevated') + @mock.patch('nova.compute.utils.notify_about_volume_attach_detach') + def test_attach_volume_ignore_VolumeAttachmentNotFound( + self, mock_notify, mock_elevate, mock_event, mock_debug_log): + """Tests the scenario that the DriverVolumeBlockDevice.attach flow + already deleted the volume attachment before the + ComputeManager.attach_volume flow tries to rollback the attachment + record and delete it, which raises VolumeAttachmentNotFound and is + ignored. + """ + mock_elevate.return_value = self.context + + attachment_id = uuids.attachment_id + fake_bdm = objects.BlockDeviceMapping(**self.fake_volume) + fake_bdm.attachment_id = attachment_id + instance = self._create_fake_instance_obj() + expected_exception = test.TestingException() + + def fake_attach(*args, **kwargs): + raise expected_exception + + with test.nested( + mock.patch.object(driver_block_device.DriverVolumeBlockDevice, + 'attach'), + mock.patch.object(cinder.API, 'attachment_delete'), + mock.patch.object(objects.BlockDeviceMapping, + 'destroy') + ) as (mock_attach, mock_attach_delete, mock_destroy): + mock_attach.side_effect = fake_attach + mock_attach_delete.side_effect = \ + exception.VolumeAttachmentNotFound( + attachment_id=attachment_id) + self.assertRaises( + test.TestingException, self.compute.attach_volume, + self.context, instance, fake_bdm) + mock_destroy.assert_called_once_with() + mock_notify.assert_has_calls([ + mock.call(self.context, instance, 'fake-mini', + action='volume_attach', phase='start', + volume_id=uuids.volume_id), + mock.call(self.context, instance, 'fake-mini', + action='volume_attach', phase='error', + volume_id=uuids.volume_id, + exception=expected_exception, + tb=mock.ANY), + ]) + mock_event.assert_called_once_with( + self.context, 'compute_attach_volume', CONF.host, + instance.uuid) + self.assertIsInstance(mock_debug_log.call_args[0][1], + exception.VolumeAttachmentNotFound) + @mock.patch.object(compute_utils, 'EventReporter') def test_detach_volume_api_raises(self, mock_event): fake_bdm = objects.BlockDeviceMapping(**self.fake_volume)