diff --git a/tempest/api/compute/base.py b/tempest/api/compute/base.py index bee4716c88..8d249ff428 100644 --- a/tempest/api/compute/base.py +++ b/tempest/api/compute/base.py @@ -570,24 +570,33 @@ class BaseV2ComputeTest(api_version_utils.BaseMicroversionTest, attachment = self.servers_client.attach_volume( server['id'], **attach_kwargs)['volumeAttachment'] - # On teardown detach the volume and for multiattach volumes wait for - # the attachment to be removed. For non-multiattach volumes wait for - # the state of the volume to change to available. This is so we don't - # error out when trying to delete the volume during teardown. - if volume['multiattach']: - att = waiters.wait_for_volume_attachment_create( - self.volumes_client, volume['id'], server['id']) - self.addCleanup(waiters.wait_for_volume_attachment_remove, - self.volumes_client, volume['id'], - att['attachment_id']) - else: - self.addCleanup(waiters.wait_for_volume_resource_status, - self.volumes_client, volume['id'], 'available') - waiters.wait_for_volume_resource_status(self.volumes_client, - volume['id'], 'in-use') - # Ignore 404s on detach in case the server is deleted or the volume - # is already detached. + + # NOTE(lyarwood): During attach we initially wait for the volume + # attachment and then check the volume state. + waiters.wait_for_volume_attachment_create( + self.volumes_client, volume['id'], server['id']) + # TODO(lyarwood): Remove the following volume status checks and move to + # attachment status checks across all volumes now with the 3.27 + # microversion somehow. + if not volume['multiattach']: + waiters.wait_for_volume_resource_status( + self.volumes_client, volume['id'], 'in-use') + + # NOTE(lyarwood): On teardown (LIFO) initially wait for the volume + # attachment in Nova to be removed. While this technically happens last + # we want this to be the first waiter as if it fails we can then dump + # the contents of the console log. The final check of the volume state + # should be a no-op by this point and is just added for completeness + # when detaching non-multiattach volumes. + if not volume['multiattach']: + self.addCleanup( + waiters.wait_for_volume_resource_status, self.volumes_client, + volume['id'], 'available') + self.addCleanup( + waiters.wait_for_volume_attachment_remove_from_server, + self.servers_client, server['id'], volume['id']) self.addCleanup(self._detach_volume, server, volume) + return attachment def create_volume_snapshot(self, volume_id, name=None, description=None, diff --git a/tempest/common/waiters.py b/tempest/common/waiters.py index 3750b113f8..f6a4555fe8 100644 --- a/tempest/common/waiters.py +++ b/tempest/common/waiters.py @@ -356,23 +356,36 @@ def wait_for_volume_attachment_remove_from_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'] + + try: + volumes = client.list_volume_attachments( + server_id)['volumeAttachments'] + except lib_exc.NotFound: + # Ignore 404s on detach in case the server is deleted or the volume + # is already detached. + return 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: + console_output = client.get_console_output(server_id)['output'] + LOG.debug('Console output for %s\nbody=\n%s', + server_id, console_output) 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 + try: + volumes = client.list_volume_attachments( + server_id)['volumeAttachments'] + except lib_exc.NotFound: + # Ignore 404s on detach in case the server is deleted or the volume + # is already detached. + return + return def wait_for_volume_migration(client, volume_id, new_host): diff --git a/tempest/tests/common/test_waiters.py b/tempest/tests/common/test_waiters.py index f801243011..5cdbfbf7c2 100755 --- a/tempest/tests/common/test_waiters.py +++ b/tempest/tests/common/test_waiters.py @@ -453,11 +453,14 @@ class TestVolumeWaiters(base.TestCase): "volumeAttachments": [{"volumeId": uuids.volume_id}]} mock_list_volume_attachments = mock.Mock( side_effect=[volume_attached, volume_attached]) + mock_get_console_output = mock.Mock( + return_value={'output': 'output'}) mock_client = mock.Mock( spec=servers_client.ServersClient, build_interval=1, build_timeout=1, - list_volume_attachments=mock_list_volume_attachments) + list_volume_attachments=mock_list_volume_attachments, + get_console_output=mock_get_console_output) self.patch( 'time.time', side_effect=[0., 0.5, mock_client.build_timeout + 1.]) @@ -473,3 +476,22 @@ class TestVolumeWaiters(base.TestCase): mock_list_volume_attachments.assert_has_calls([ mock.call(uuids.server_id), mock.call(uuids.server_id)]) + + # Assert that we fetch console output + mock_get_console_output.assert_called_once_with(uuids.server_id) + + def test_wait_for_volume_attachment_remove_from_server_not_found(self): + mock_list_volume_attachments = mock.Mock( + side_effect=lib_exc.NotFound) + mock_client = mock.Mock( + spec=servers_client.ServersClient, + list_volume_attachments=mock_list_volume_attachments) + + # Assert that nothing is raised when lib_exc_NotFound is raised + # by the client call to list_volume_attachments + waiters.wait_for_volume_attachment_remove_from_server( + mock_client, mock.sentinel.server_id, mock.sentinel.volume_id) + + # Assert that list_volume_attachments was actually called + mock_list_volume_attachments.assert_called_once_with( + mock.sentinel.server_id)