Retry attachment delete API call for 504 Gateway Timeout
When cinder-api runs behind a load balancer(eg haproxy), the load balancer can return 504 Gateway Timeout when cinder-api does not respond within timeout. This change ensures nova retries deleting a volume attachment in that case. Also this change makes nova ignore 404 in the API call. This is required because cinder might continue deleting the attachment even if the load balancer returns 504. This also helps us in the situation where the volume attachment was accidentally removed by users. Conflicts: nova/tests/unit/volume/test_cinder.py nova/volume/cinder.py NOTE(melwitt): The conflicts are due to the following changes not in Victoria: * I23bb9e539d08f5c6202909054c2dd49b6c7a7a0e (Remove six.text_type (1/2)) * I779bd1446dc1f070fa5100ccccda7881fa508d79 (Remove six.text_type (2/2)) Closes-Bug: #1978444 Change-Id: I593011d9f4c43cdae7a3d53b556c6e2a2b939989 (cherry picked from commit8f4b740ca5
) (cherry picked from commitb94ffb1123
) (cherry picked from commit14f9b7627e
) (cherry picked from commit9b1c078112
)
This commit is contained in:
parent
ba3a6f3855
commit
3cb1e35b5e
|
@ -521,16 +521,15 @@ class CinderApiTestCase(test.NoDBTestCase):
|
||||||
@mock.patch('nova.volume.cinder.cinderclient')
|
@mock.patch('nova.volume.cinder.cinderclient')
|
||||||
def test_attachment_delete_failed(self, mock_cinderclient, mock_log):
|
def test_attachment_delete_failed(self, mock_cinderclient, mock_log):
|
||||||
mock_cinderclient.return_value.attachments.delete.side_effect = (
|
mock_cinderclient.return_value.attachments.delete.side_effect = (
|
||||||
cinder_exception.NotFound(404, '404'))
|
cinder_exception.BadRequest(400, '400'))
|
||||||
|
|
||||||
attachment_id = uuids.attachment
|
attachment_id = uuids.attachment
|
||||||
ex = self.assertRaises(exception.VolumeAttachmentNotFound,
|
ex = self.assertRaises(exception.InvalidInput,
|
||||||
self.api.attachment_delete,
|
self.api.attachment_delete,
|
||||||
self.ctx,
|
self.ctx,
|
||||||
attachment_id)
|
attachment_id)
|
||||||
|
|
||||||
self.assertEqual(404, ex.code)
|
self.assertEqual(400, ex.code)
|
||||||
self.assertIn(attachment_id, six.text_type(ex))
|
|
||||||
|
|
||||||
@mock.patch('nova.volume.cinder.cinderclient',
|
@mock.patch('nova.volume.cinder.cinderclient',
|
||||||
side_effect=exception.CinderAPIVersionNotAvailable(
|
side_effect=exception.CinderAPIVersionNotAvailable(
|
||||||
|
@ -546,6 +545,16 @@ class CinderApiTestCase(test.NoDBTestCase):
|
||||||
mock_cinderclient.assert_called_once_with(self.ctx, '3.44',
|
mock_cinderclient.assert_called_once_with(self.ctx, '3.44',
|
||||||
skip_version_check=True)
|
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')
|
@mock.patch('nova.volume.cinder.cinderclient')
|
||||||
def test_attachment_delete_internal_server_error(self, mock_cinderclient):
|
def test_attachment_delete_internal_server_error(self, mock_cinderclient):
|
||||||
mock_cinderclient.return_value.attachments.delete.side_effect = (
|
mock_cinderclient.return_value.attachments.delete.side_effect = (
|
||||||
|
@ -569,6 +578,29 @@ class CinderApiTestCase(test.NoDBTestCase):
|
||||||
|
|
||||||
self.assertEqual(2, mock_cinderclient.call_count)
|
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')
|
@mock.patch('nova.volume.cinder.cinderclient')
|
||||||
def test_attachment_delete_bad_request_exception(self, mock_cinderclient):
|
def test_attachment_delete_bad_request_exception(self, mock_cinderclient):
|
||||||
mock_cinderclient.return_value.attachments.delete.side_effect = (
|
mock_cinderclient.return_value.attachments.delete.side_effect = (
|
||||||
|
|
|
@ -884,13 +884,17 @@ class API(object):
|
||||||
@retrying.retry(stop_max_attempt_number=5,
|
@retrying.retry(stop_max_attempt_number=5,
|
||||||
retry_on_exception=lambda e:
|
retry_on_exception=lambda e:
|
||||||
(isinstance(e, cinder_exception.ClientException) and
|
(isinstance(e, cinder_exception.ClientException) and
|
||||||
e.code == 500))
|
e.code in (500, 504)))
|
||||||
def attachment_delete(self, context, attachment_id):
|
def attachment_delete(self, context, attachment_id):
|
||||||
try:
|
try:
|
||||||
cinderclient(
|
cinderclient(
|
||||||
context, '3.44', skip_version_check=True).attachments.delete(
|
context, '3.44', skip_version_check=True).attachments.delete(
|
||||||
attachment_id)
|
attachment_id)
|
||||||
except cinder_exception.ClientException as ex:
|
except cinder_exception.ClientException as ex:
|
||||||
|
if ex.code == 404:
|
||||||
|
LOG.warning('Attachment %(id)s does not exist. Ignoring.',
|
||||||
|
{'id': attachment_id})
|
||||||
|
else:
|
||||||
with excutils.save_and_reraise_exception():
|
with excutils.save_and_reraise_exception():
|
||||||
LOG.error('Delete attachment failed for attachment '
|
LOG.error('Delete attachment failed for attachment '
|
||||||
'%(id)s. Error: %(msg)s Code: %(code)s',
|
'%(id)s. Error: %(msg)s Code: %(code)s',
|
||||||
|
|
|
@ -0,0 +1,7 @@
|
||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
`Bug #1978444 <https://bugs.launchpad.net/nova/+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.
|
Loading…
Reference in New Issue