diff --git a/nova/tests/unit/volume/test_cinder.py b/nova/tests/unit/volume/test_cinder.py index 780953398101..c6bb2c10cad5 100644 --- a/nova/tests/unit/volume/test_cinder.py +++ b/nova/tests/unit/volume/test_cinder.py @@ -521,16 +521,15 @@ class CinderApiTestCase(test.NoDBTestCase): @mock.patch('nova.volume.cinder.cinderclient') def test_attachment_delete_failed(self, mock_cinderclient, mock_log): mock_cinderclient.return_value.attachments.delete.side_effect = ( - cinder_exception.NotFound(404, '404')) + cinder_exception.BadRequest(400, '400')) attachment_id = uuids.attachment - ex = self.assertRaises(exception.VolumeAttachmentNotFound, + ex = self.assertRaises(exception.InvalidInput, self.api.attachment_delete, self.ctx, attachment_id) - self.assertEqual(404, ex.code) - self.assertIn(attachment_id, six.text_type(ex)) + self.assertEqual(400, ex.code) @mock.patch('nova.volume.cinder.cinderclient', side_effect=exception.CinderAPIVersionNotAvailable( @@ -546,6 +545,16 @@ class CinderApiTestCase(test.NoDBTestCase): mock_cinderclient.assert_called_once_with(self.ctx, '3.44', skip_version_check=True) + @mock.patch('nova.volume.cinder.cinderclient') + def test_attachment_delete_not_found(self, mock_cinderclient): + mock_cinderclient.return_value.attachments.delete.side_effect = ( + cinder_exception.ClientException(404)) + + attachment_id = uuids.attachment + self.api.attachment_delete(self.ctx, attachment_id) + + self.assertEqual(1, mock_cinderclient.call_count) + @mock.patch('nova.volume.cinder.cinderclient') def test_attachment_delete_internal_server_error(self, mock_cinderclient): mock_cinderclient.return_value.attachments.delete.side_effect = ( @@ -569,6 +578,29 @@ class CinderApiTestCase(test.NoDBTestCase): self.assertEqual(2, mock_cinderclient.call_count) + @mock.patch('nova.volume.cinder.cinderclient') + def test_attachment_delete_gateway_timeout(self, mock_cinderclient): + mock_cinderclient.return_value.attachments.delete.side_effect = ( + cinder_exception.ClientException(504)) + + self.assertRaises(cinder_exception.ClientException, + self.api.attachment_delete, + self.ctx, uuids.attachment_id) + + self.assertEqual(5, mock_cinderclient.call_count) + + @mock.patch('nova.volume.cinder.cinderclient') + def test_attachment_delete_gateway_timeout_do_not_raise( + self, mock_cinderclient): + # generate exception, and then have a normal return on the next retry + mock_cinderclient.return_value.attachments.delete.side_effect = [ + cinder_exception.ClientException(504), None] + + attachment_id = uuids.attachment + self.api.attachment_delete(self.ctx, attachment_id) + + self.assertEqual(2, mock_cinderclient.call_count) + @mock.patch('nova.volume.cinder.cinderclient') def test_attachment_delete_bad_request_exception(self, mock_cinderclient): mock_cinderclient.return_value.attachments.delete.side_effect = ( diff --git a/nova/volume/cinder.py b/nova/volume/cinder.py index 1eaa5bcbda94..08f8fa0b0186 100644 --- a/nova/volume/cinder.py +++ b/nova/volume/cinder.py @@ -884,19 +884,23 @@ class API(object): @retrying.retry(stop_max_attempt_number=5, retry_on_exception=lambda e: (isinstance(e, cinder_exception.ClientException) and - e.code == 500)) + e.code in (500, 504))) def attachment_delete(self, context, attachment_id): try: cinderclient( context, '3.44', skip_version_check=True).attachments.delete( attachment_id) except cinder_exception.ClientException as ex: - with excutils.save_and_reraise_exception(): - LOG.error('Delete attachment failed for attachment ' - '%(id)s. Error: %(msg)s Code: %(code)s', - {'id': attachment_id, - 'msg': six.text_type(ex), - 'code': getattr(ex, 'code', None)}) + if ex.code == 404: + LOG.warning('Attachment %(id)s does not exist. Ignoring.', + {'id': attachment_id}) + else: + with excutils.save_and_reraise_exception(): + LOG.error('Delete attachment failed for attachment ' + '%(id)s. Error: %(msg)s Code: %(code)s', + {'id': attachment_id, + 'msg': six.text_type(ex), + 'code': getattr(ex, 'code', None)}) @translate_attachment_exception def attachment_complete(self, context, attachment_id): diff --git a/releasenotes/notes/bug-1978444-db46df5f3d5ea19e.yaml b/releasenotes/notes/bug-1978444-db46df5f3d5ea19e.yaml new file mode 100644 index 000000000000..6c1980407452 --- /dev/null +++ b/releasenotes/notes/bug-1978444-db46df5f3d5ea19e.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + `Bug #1978444 `_: Now nova + retries deleting a volume attachment in case Cinder API returns + ``504 Gateway Timeout``. Also, ``404 Not Found`` is now ignored and + leaves only a warning message.