From 067cd93424ea1e62c77744986a5479d1b99b0ffe Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Fri, 1 Oct 2021 12:21:57 +0100 Subject: [PATCH] block_device: Ignore VolumeAttachmentNotFound during detach Bug #1937084 details a race condition within Cinder where requests to delete an attachment and later delete the underlying volume can race leading to the initial request returning a 404 if the volume delete completes first. This change attempts to handle this within Nova during a detach as we ultimately don't care that the volume and/or volume attachment are no longer available within Cinder. This allows Nova to complete its' own cleanup of the BlockDeviceMapping record resulting in the volume no longer appearing attached in Nova's APIs. Closes-Bug: #1937084 Change-Id: I191552652d8ff5206abad7558c99bce27979dc84 --- .../regressions/test_bug_1937084.py | 28 ++++--------------- nova/tests/unit/virt/test_block_device.py | 14 +++++++++- nova/virt/block_device.py | 13 ++++++++- 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/nova/tests/functional/regressions/test_bug_1937084.py b/nova/tests/functional/regressions/test_bug_1937084.py index e0543a5fd995..3ef432ae5eb0 100644 --- a/nova/tests/functional/regressions/test_bug_1937084.py +++ b/nova/tests/functional/regressions/test_bug_1937084.py @@ -17,7 +17,6 @@ import mock from nova import context from nova import exception from nova import objects -from nova.tests.functional.api import client from nova.tests.functional import integrated_helpers @@ -64,32 +63,15 @@ class TestDetachAttachmentNotFound(integrated_helpers._IntegratedTestBase): ): # DELETE /servers/{server_id}/os-volume_attachments/{volume_id} is # async but as we are using CastAsCall it's sync in our func tests - ex = self.assertRaises( - client.OpenStackApiException, - self.api.delete_server_volume, + self.api.delete_server_volume( server_id, self.cinder.IMAGE_BACKED_VOL) - self.assertEqual(500, ex.response.status_code) mock_attachment_delete.assert_called_once() - # FIXME(lyarwood): This is the Nova portion of bug #1937084 where - # the original caller hasn't polled os-volume_attachments and sent - # a seperate DELETE request to c-api for the volume as soon as it - # has become available but before n-cpu has finished the original - # call. This leads to the sync request to c-api to delete the - # attachment returning a 404 that Nova translates into - # VolumeAttachmentNotFound. - # - # Replace this with the following once the exception is ignored: - # - # self.assertRaises( - # exception.VolumeBDMNotFound, - # objects.BlockDeviceMapping.get_by_volume_and_instance, - # context.get_admin_context(), - # self.cinder.IMAGE_BACKED_VOL, - # server_id) - # - bdm = objects.BlockDeviceMapping.get_by_volume_and_instance( + # Assert that the volume attachment is still removed in Nova + self.assertRaises( + exception.VolumeBDMNotFound, + objects.BlockDeviceMapping.get_by_volume_and_instance, context.get_admin_context(), self.cinder.IMAGE_BACKED_VOL, server_id) diff --git a/nova/tests/unit/virt/test_block_device.py b/nova/tests/unit/virt/test_block_device.py index 9fc97c0ff851..aff6c5ef199e 100644 --- a/nova/tests/unit/virt/test_block_device.py +++ b/nova/tests/unit/virt/test_block_device.py @@ -461,7 +461,11 @@ class TestDriverBlockDevice(test.NoDBTestCase): def test_call_wait_no_delete_volume(self): self._test_call_wait_func(False) - def test_volume_delete_attachment(self, include_shared_targets=False): + def test_volume_delete_attachment( + self, + include_shared_targets=False, + delete_attachment_raises=None + ): attachment_id = uuids.attachment driver_bdm = self.driver_classes['volume'](self.volume_bdm) driver_bdm['attachment_id'] = attachment_id @@ -488,6 +492,9 @@ class TestDriverBlockDevice(test.NoDBTestCase): ) as (mock_get_volume, mock_get_connector, mock_guard, vapi_attach_del): + if delete_attachment_raises: + vapi_attach_del.side_effect = delete_attachment_raises + driver_bdm.detach(elevated_context, instance, self.volume_api, self.virt_driver, attachment_id=attachment_id) @@ -499,6 +506,11 @@ class TestDriverBlockDevice(test.NoDBTestCase): def test_volume_delete_attachment_with_shared_targets(self): self.test_volume_delete_attachment(include_shared_targets=True) + def test_volume_delete_attachment_raises_attachment_not_found(self): + self.test_volume_delete_attachment( + delete_attachment_raises=exception.VolumeAttachmentNotFound( + attachment_id=uuids.attachment_id)) + @mock.patch.object(encryptors, 'get_encryption_metadata') @mock.patch.object(objects.BlockDeviceMapping, 'save') def _test_volume_attach(self, driver_bdm, bdm_dict, diff --git a/nova/virt/block_device.py b/nova/virt/block_device.py index 8adffdaf9a8a..4a4170317462 100644 --- a/nova/virt/block_device.py +++ b/nova/virt/block_device.py @@ -450,7 +450,18 @@ class DriverVolumeBlockDevice(DriverBlockDevice): volume_api.detach(context.elevated(), volume_id, instance.uuid, attachment_id) else: - volume_api.attachment_delete(context, self['attachment_id']) + try: + volume_api.attachment_delete(context, self['attachment_id']) + except exception.VolumeAttachmentNotFound: + LOG.info( + "Ignoring a volume attachment deletion failure as the " + "volume %(volume_id)s or the volume attachment " + "%(attachment_id)s disappeared during the request.", + { + 'volume_id': volume_id, + 'attachment_id': self['attachment_id'] + } + ) def detach(self, context, instance, volume_api, virt_driver, attachment_id=None, destroy_bdm=False):