Remove race due to 1907084

The test_attach_scsi_disk_with_config_drive test cases is racy at volume
detach. Nova first detach the volume in cinder then later deletes the
BlockDeviceMapping from the nova DB. Test waits for the volume to become
available and then checks the list of volume attachments in the nova API
then it can see that the attachment is still there.

Closes-Bug: #1907084
Change-Id: I814ae3215f39d1e8407c4ca1c7117a314941c80b

tempest/api/compute/admin/test_volume.py

@@ -112,7 +112,5 @@ class AttachSCSIVolumeTestJSON(BaseAttachSCSIVolumeTest):
             server['id'], attachment['volumeId'])
         waiters.wait_for_volume_resource_status(
             self.volumes_client, attachment['volumeId'], 'available')
-        volume_after_detach = self.servers_client.list_volume_attachments(
-            server['id'])['volumeAttachments']
-        self.assertEqual(0, len(volume_after_detach),
-                         "Failed to detach volume")
+        waiters.wait_for_volume_attachment_remove_from_server(
+            self.servers_client, server['id'], attachment['volumeId'])

tempest/common/waiters.py

@@ -317,6 +317,32 @@ def wait_for_volume_attachment_remove(client, volume_id, attachment_id):
              'seconds', attachment_id, volume_id, time.time() - start)
 
 
+def wait_for_volume_attachment_remove_from_server(
+        client, server_id, volume_id):
+    """Waits for a volume to be removed from a given server.
+
+    This waiter checks the compute API if the volume attachment is removed.
+    """
+    start = int(time.time())
+    volumes = client.list_volume_attachments(server_id)['volumeAttachments']
+
+    while any(volume for volume in volumes if volume['volumeId'] == volume_id):
+        time.sleep(client.build_interval)
+
+        timed_out = int(time.time()) - start >= client.build_timeout
+        if timed_out:
+            message = ('Volume %s failed to detach from server %s within '
+                       'the required time (%s s) from the compute API '
+                       'perspective' %
+                       (volume_id, server_id, client.build_timeout))
+            raise lib_exc.TimeoutException(message)
+
+        volumes = client.list_volume_attachments(server_id)[
+            'volumeAttachments']
+
+    return volumes
+
+
 def wait_for_volume_migration(client, volume_id, new_host):
     """Waits for a Volume to move to a new host."""
     body = client.show_volume(volume_id)['volume']

tempest/tests/common/test_waiters.py

@@ -20,6 +20,7 @@ from oslo_utils.fixture import uuidsentinel as uuids
 from tempest.common import waiters
 from tempest import exceptions
 from tempest.lib import exceptions as lib_exc
+from tempest.lib.services.compute import servers_client
 from tempest.lib.services.volume.v2 import volumes_client
 from tempest.tests import base
 import tempest.tests.utils as utils
@@ -384,3 +385,54 @@ class TestVolumeWaiters(base.TestCase):
                                                   uuids.attachment_id)
         # Assert that show volume is only called once before we return
         show_volume.assert_called_once_with(uuids.volume_id)
+
+    def test_wait_for_volume_attachment_remove_from_server(self):
+        volume_attached = {
+            "volumeAttachments": [{"volumeId": uuids.volume_id}]}
+        volume_not_attached = {"volumeAttachments": []}
+        mock_list_volume_attachments = mock.Mock(
+            side_effect=[volume_attached, volume_not_attached])
+        mock_client = mock.Mock(
+            spec=servers_client.ServersClient,
+            build_interval=1,
+            build_timeout=1,
+            list_volume_attachments=mock_list_volume_attachments)
+        self.patch(
+            'time.time',
+            side_effect=[0., 0.5, mock_client.build_timeout + 1.])
+        self.patch('time.sleep')
+
+        waiters.wait_for_volume_attachment_remove_from_server(
+            mock_client, uuids.server_id, uuids.volume_id)
+
+        # Assert that list_volume_attachments is called until the attachment is
+        # removed.
+        mock_list_volume_attachments.assert_has_calls([
+            mock.call(uuids.server_id),
+            mock.call(uuids.server_id)])
+
+    def test_wait_for_volume_attachment_remove_from_server_timeout(self):
+        volume_attached = {
+            "volumeAttachments": [{"volumeId": uuids.volume_id}]}
+        mock_list_volume_attachments = mock.Mock(
+            side_effect=[volume_attached, volume_attached])
+        mock_client = mock.Mock(
+            spec=servers_client.ServersClient,
+            build_interval=1,
+            build_timeout=1,
+            list_volume_attachments=mock_list_volume_attachments)
+        self.patch(
+            'time.time',
+            side_effect=[0., 0.5, mock_client.build_timeout + 1.])
+        self.patch('time.sleep')
+
+        self.assertRaises(
+            lib_exc.TimeoutException,
+            waiters.wait_for_volume_attachment_remove_from_server,
+            mock_client, uuids.server_id, uuids.volume_id)
+
+        # Assert that list_volume_attachments is called until the attachment is
+        # removed.
+        mock_list_volume_attachments.assert_has_calls([
+            mock.call(uuids.server_id),
+            mock.call(uuids.server_id)])