From 10c7e718488a6daad5bcea97e00aece24179168e Mon Sep 17 00:00:00 2001
From: Lee Yarwood <lyarwood@redhat.com>
Date: Fri, 1 Oct 2021 12:05:15 +0100
Subject: [PATCH] Add regression test for bug #1937084

This regression test asserts the behaviour of Nova when Cinder raises a
404 during a DELETE request against an attachment.

In the context of bug 1937084 this could happen if a caller attempted to
DELETE a volume attachment through Nova's os-volume_attachments API and
then made a separate DELETE request against the underlying volume in
Cinder when it was marked as available.

Related-Bug: #1937084
Change-Id: I56106d16ed1d24793c4cddad0caa365a641ea4fd
---
 .../regressions/test_bug_1937084.py           | 95 +++++++++++++++++++
 1 file changed, 95 insertions(+)
 create mode 100644 nova/tests/functional/regressions/test_bug_1937084.py

diff --git a/nova/tests/functional/regressions/test_bug_1937084.py b/nova/tests/functional/regressions/test_bug_1937084.py
new file mode 100644
index 000000000000..e0543a5fd995
--- /dev/null
+++ b/nova/tests/functional/regressions/test_bug_1937084.py
@@ -0,0 +1,95 @@
+# Copyright 2021, Red Hat, Inc. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+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
+
+
+class TestDetachAttachmentNotFound(integrated_helpers._IntegratedTestBase):
+    """Regression test for the Nova portion of bug 1937084
+
+    This regression test asserts the behaviour of Nova when Cinder raises a 404
+    during a DELETE request against an attachment.
+
+    In the context of bug 1937084 this could happen if a caller attempted to
+    DELETE a volume attachment through Nova's os-volume_attachments API and
+    then made a separate DELETE request against the underlying volume in Cinder
+    when it was marked as available.
+    """
+
+    microversion = 'latest'
+
+    def test_delete_attachment_volume_not_found(self):
+        # Create a server and attach a single volume
+        server = self._create_server(networks='none')
+        server_id = server['id']
+        self.api.post_server_volume(
+            server_id,
+            {
+                'volumeAttachment': {
+                    'volumeId': self.cinder.IMAGE_BACKED_VOL
+                }
+            }
+        )
+        self._wait_for_volume_attach(server_id, self.cinder.IMAGE_BACKED_VOL)
+
+        # Assert that we have an active bdm for the attachment before we detach
+        bdm = objects.BlockDeviceMapping.get_by_volume_and_instance(
+            context.get_admin_context(),
+            self.cinder.IMAGE_BACKED_VOL,
+            server_id)
+
+        with mock.patch(
+            'nova.volume.cinder.API.attachment_delete',
+            side_effect=exception.VolumeAttachmentNotFound(
+                attachment_id=bdm.attachment_id)
+        ) as (
+            mock_attachment_delete
+        ):
+            # 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,
+                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(
+            context.get_admin_context(),
+            self.cinder.IMAGE_BACKED_VOL,
+            server_id)