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

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)

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,

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):