openstack/nova
Code Issues Proposed changes

Merge "Fix the wrong exception used to retry detach API calls"

Browse Source
This commit is contained in:
Zuul 2021-10-18 17:16:44 +00:00 committed by Gerrit Code Review
commit e14eef0719
2 changed files with 30 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
nova/tests/unit/volume/test_cinder.py
View File

@ -14,7 +14,6 @@
# under the License. # under the License.
from cinderclient import api_versions as cinder_api_versions from cinderclient import api_versions as cinder_api_versions
from cinderclient import apiclient as cinder_apiclient
from cinderclient import exceptions as cinder_exception from cinderclient import exceptions as cinder_exception
from cinderclient.v3 import limits as cinder_limits from cinderclient.v3 import limits as cinder_limits
from keystoneauth1 import loading as ks_loading from keystoneauth1 import loading as ks_loading
@ -546,11 +545,12 @@ 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', @mock.patch('nova.volume.cinder.cinderclient')
side_effect=cinder_apiclient.exceptions.InternalServerError)
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 = (
cinder_exception.ClientException(500))
self.assertRaises(cinder_apiclient.exceptions.InternalServerError, self.assertRaises(cinder_exception.ClientException,
self.api.attachment_delete, self.api.attachment_delete,
self.ctx, uuids.attachment_id) self.ctx, uuids.attachment_id)
@ -561,16 +561,17 @@ class CinderApiTestCase(test.NoDBTestCase):
self, mock_cinderclient): self, mock_cinderclient):
# generate exception, and then have a normal return on the next retry # generate exception, and then have a normal return on the next retry
mock_cinderclient.return_value.attachments.delete.side_effect = [ mock_cinderclient.return_value.attachments.delete.side_effect = [
cinder_apiclient.exceptions.InternalServerError, None] cinder_exception.ClientException(500), None]
attachment_id = uuids.attachment attachment_id = uuids.attachment
self.api.attachment_delete(self.ctx, attachment_id) self.api.attachment_delete(self.ctx, attachment_id)
self.assertEqual(2, mock_cinderclient.call_count) self.assertEqual(2, mock_cinderclient.call_count)
@mock.patch('nova.volume.cinder.cinderclient', @mock.patch('nova.volume.cinder.cinderclient')
side_effect=cinder_exception.BadRequest(code=400))
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 = (
cinder_exception.BadRequest(400))
self.assertRaises(exception.InvalidInput, self.assertRaises(exception.InvalidInput,
self.api.attachment_delete, self.api.attachment_delete,
@ -594,7 +595,7 @@ class CinderApiTestCase(test.NoDBTestCase):
@mock.patch('nova.volume.cinder.cinderclient') @mock.patch('nova.volume.cinder.cinderclient')
def test_attachment_complete_failed(self, mock_cinderclient): def test_attachment_complete_failed(self, mock_cinderclient):
mock_cinderclient.return_value.attachments.complete.side_effect = ( mock_cinderclient.return_value.attachments.complete.side_effect = (
cinder_exception.NotFound(404, '404')) cinder_exception.NotFound(404))
attachment_id = uuids.attachment attachment_id = uuids.attachment
ex = self.assertRaises(exception.VolumeAttachmentNotFound, ex = self.assertRaises(exception.VolumeAttachmentNotFound,
@ -667,27 +668,30 @@ class CinderApiTestCase(test.NoDBTestCase):
mock_cinderclient.assert_called_with(self.ctx, microversion=None) mock_cinderclient.assert_called_with(self.ctx, microversion=None)
mock_volumes.detach.assert_called_once_with('id1', 'fakeid') mock_volumes.detach.assert_called_once_with('id1', 'fakeid')
@mock.patch('nova.volume.cinder.cinderclient', @mock.patch('nova.volume.cinder.cinderclient')
side_effect=cinder_apiclient.exceptions.InternalServerError)
def test_detach_internal_server_error(self, mock_cinderclient): def test_detach_internal_server_error(self, mock_cinderclient):
mock_cinderclient.return_value.volumes.detach.side_effect = (
cinder_exception.ClientException(500))
self.assertRaises(cinder_apiclient.exceptions.InternalServerError, self.assertRaises(cinder_exception.ClientException,
self.api.detach, self.api.detach,
self.ctx, 'id1', instance_uuid='fake_uuid') self.ctx, 'id1', instance_uuid='fake_uuid')
self.assertEqual(5, mock_cinderclient.call_count) self.assertEqual(
5, mock_cinderclient.return_value.volumes.detach.call_count)
@mock.patch('nova.volume.cinder.cinderclient') @mock.patch('nova.volume.cinder.cinderclient')
def test_detach_internal_server_error_do_not_raise( def test_detach_internal_server_error_do_not_raise(
self, mock_cinderclient): self, mock_cinderclient):
# generate exception, and then have a normal return on the next retry # generate exception, and then have a normal return on the next retry
mock_cinderclient.return_value.volumes.detach.side_effect = [ mock_cinderclient.return_value.volumes.detach.side_effect = [
cinder_apiclient.exceptions.InternalServerError, None] cinder_exception.ClientException(500), None]
self.api.detach(self.ctx, 'id1', instance_uuid='fake_uuid', self.api.detach(self.ctx, 'id1', instance_uuid='fake_uuid',
attachment_id='fakeid') attachment_id='fakeid')
self.assertEqual(2, mock_cinderclient.call_count) self.assertEqual(
2, mock_cinderclient.return_value.volumes.detach.call_count)
@mock.patch('nova.volume.cinder.cinderclient', @mock.patch('nova.volume.cinder.cinderclient',
side_effect=cinder_exception.BadRequest(code=400)) side_effect=cinder_exception.BadRequest(code=400))
@ -818,11 +822,13 @@ class CinderApiTestCase(test.NoDBTestCase):
mock_volumes.terminate_connection.assert_called_once_with('id1', mock_volumes.terminate_connection.assert_called_once_with('id1',
'connector') 'connector')
@mock.patch('nova.volume.cinder.cinderclient', @mock.patch('nova.volume.cinder.cinderclient')
side_effect=cinder_apiclient.exceptions.InternalServerError)
def test_terminate_connection_internal_server_error( def test_terminate_connection_internal_server_error(
self, mock_cinderclient): self, mock_cinderclient):
self.assertRaises(cinder_apiclient.exceptions.InternalServerError, mock_cinderclient.return_value.volumes.terminate_connection.\
side_effect = cinder_exception.ClientException(500)
self.assertRaises(cinder_exception.ClientException,
self.api.terminate_connection, self.api.terminate_connection,
self.ctx, 'id1', 'connector') self.ctx, 'id1', 'connector')
@ -833,7 +839,7 @@ class CinderApiTestCase(test.NoDBTestCase):
self, mock_cinderclient): self, mock_cinderclient):
# generate exception, and then have a normal return on the next retry # generate exception, and then have a normal return on the next retry
mock_cinderclient.return_value.volumes.terminate_connection.\ mock_cinderclient.return_value.volumes.terminate_connection.\
side_effect = [cinder_apiclient.exceptions.InternalServerError, side_effect = [cinder_exception.ClientException(500),
None] None]
self.api.terminate_connection(self.ctx, 'id1', 'connector') self.api.terminate_connection(self.ctx, 'id1', 'connector')

10
nova/volume/cinder.py
View File

@ -25,7 +25,6 @@ import sys
import urllib import urllib
from cinderclient import api_versions as cinder_api_versions from cinderclient import api_versions as cinder_api_versions
from cinderclient import apiclient as cinder_apiclient
from cinderclient import client as cinder_client from cinderclient import client as cinder_client
from cinderclient import exceptions as cinder_exception from cinderclient import exceptions as cinder_exception
from keystoneauth1 import exceptions as keystone_exception from keystoneauth1 import exceptions as keystone_exception
@ -567,7 +566,8 @@ class API(object):
@translate_volume_exception @translate_volume_exception
@retrying.retry(stop_max_attempt_number=5, @retrying.retry(stop_max_attempt_number=5,
retry_on_exception=lambda e: retry_on_exception=lambda e:
type(e) == cinder_apiclient.exceptions.InternalServerError) (isinstance(e, cinder_exception.ClientException) and
e.code == 500))
def detach(self, context, volume_id, instance_uuid=None, def detach(self, context, volume_id, instance_uuid=None,
attachment_id=None): attachment_id=None):
client = cinderclient(context) client = cinderclient(context)
@ -632,7 +632,8 @@ class API(object):
@translate_volume_exception @translate_volume_exception
@retrying.retry(stop_max_attempt_number=5, @retrying.retry(stop_max_attempt_number=5,
retry_on_exception=lambda e: retry_on_exception=lambda e:
type(e) == cinder_apiclient.exceptions.InternalServerError) (isinstance(e, cinder_exception.ClientException) and
e.code == 500))
def terminate_connection(self, context, volume_id, connector): def terminate_connection(self, context, volume_id, connector):
return cinderclient(context).volumes.terminate_connection(volume_id, return cinderclient(context).volumes.terminate_connection(volume_id,
connector) connector)
@ -881,7 +882,8 @@ class API(object):
@translate_attachment_exception @translate_attachment_exception
@retrying.retry(stop_max_attempt_number=5, @retrying.retry(stop_max_attempt_number=5,
retry_on_exception=lambda e: retry_on_exception=lambda e:
type(e) == cinder_apiclient.exceptions.InternalServerError) (isinstance(e, cinder_exception.ClientException) and
e.code == 500))
def attachment_delete(self, context, attachment_id): def attachment_delete(self, context, attachment_id):
try: try:
cinderclient( cinderclient(