Add retry to cinder API calls related to volume detach
When shutting down an instance for which volume needs to be deleted, if cinder RPC timeout expires before cinder volume driver terminates connection, then an unknown cinder exception is received and the volume is not removed. This fix adds a retry mechanism directly in cinder API calls attachment_delete, terminate_connection, and detach. Change-Id: I3c9ae47d0ceb64fa3082a01cb7df27faa4f5a00d Closes-Bug: #1834659 (cherry picked from commit01c334cbdd
) (cherry picked from commit118ee68257
)
This commit is contained in:
parent
ad282233be
commit
1d66884af7
|
@ -14,6 +14,7 @@
|
|||
# under the License.
|
||||
|
||||
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.v2 import limits as cinder_limits
|
||||
from keystoneauth1 import loading as ks_loading
|
||||
|
@ -546,6 +547,38 @@ class CinderApiTestCase(test.NoDBTestCase):
|
|||
mock_cinderclient.assert_called_once_with(self.ctx, '3.44',
|
||||
skip_version_check=True)
|
||||
|
||||
@mock.patch('nova.volume.cinder.cinderclient',
|
||||
side_effect=cinder_apiclient.exceptions.InternalServerError)
|
||||
def test_attachment_delete_internal_server_error(self, mock_cinderclient):
|
||||
|
||||
self.assertRaises(cinder_apiclient.exceptions.InternalServerError,
|
||||
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_internal_server_error_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_apiclient.exceptions.InternalServerError, 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',
|
||||
side_effect=cinder_exception.BadRequest(code=400))
|
||||
def test_attachment_delete_bad_request_exception(self, mock_cinderclient):
|
||||
|
||||
self.assertRaises(exception.InvalidInput,
|
||||
self.api.attachment_delete,
|
||||
self.ctx, uuids.attachment_id)
|
||||
|
||||
self.assertEqual(1, mock_cinderclient.call_count)
|
||||
|
||||
@mock.patch('nova.volume.cinder.cinderclient')
|
||||
def test_attachment_complete(self, mock_cinderclient):
|
||||
mock_attachments = mock.MagicMock()
|
||||
|
@ -635,6 +668,38 @@ class CinderApiTestCase(test.NoDBTestCase):
|
|||
mock_cinderclient.assert_called_with(self.ctx, microversion=None)
|
||||
mock_volumes.detach.assert_called_once_with('id1', 'fakeid')
|
||||
|
||||
@mock.patch('nova.volume.cinder.cinderclient',
|
||||
side_effect=cinder_apiclient.exceptions.InternalServerError)
|
||||
def test_detach_internal_server_error(self, mock_cinderclient):
|
||||
|
||||
self.assertRaises(cinder_apiclient.exceptions.InternalServerError,
|
||||
self.api.detach,
|
||||
self.ctx, 'id1', instance_uuid='fake_uuid')
|
||||
|
||||
self.assertEqual(5, mock_cinderclient.call_count)
|
||||
|
||||
@mock.patch('nova.volume.cinder.cinderclient')
|
||||
def test_detach_internal_server_error_do_not_raise(
|
||||
self, mock_cinderclient):
|
||||
# generate exception, and then have a normal return on the next retry
|
||||
mock_cinderclient.return_value.volumes.detach.side_effect = [
|
||||
cinder_apiclient.exceptions.InternalServerError, None]
|
||||
|
||||
self.api.detach(self.ctx, 'id1', instance_uuid='fake_uuid',
|
||||
attachment_id='fakeid')
|
||||
|
||||
self.assertEqual(2, mock_cinderclient.call_count)
|
||||
|
||||
@mock.patch('nova.volume.cinder.cinderclient',
|
||||
side_effect=cinder_exception.BadRequest(code=400))
|
||||
def test_detach_bad_request_exception(self, mock_cinderclient):
|
||||
|
||||
self.assertRaises(exception.InvalidInput,
|
||||
self.api.detach,
|
||||
self.ctx, 'id1', instance_uuid='fake_uuid')
|
||||
|
||||
self.assertEqual(1, mock_cinderclient.call_count)
|
||||
|
||||
@mock.patch('nova.volume.cinder.cinderclient')
|
||||
def test_attachment_get(self, mock_cinderclient):
|
||||
mock_attachment = mock.MagicMock()
|
||||
|
@ -754,6 +819,38 @@ class CinderApiTestCase(test.NoDBTestCase):
|
|||
mock_volumes.terminate_connection.assert_called_once_with('id1',
|
||||
'connector')
|
||||
|
||||
@mock.patch('nova.volume.cinder.cinderclient',
|
||||
side_effect=cinder_apiclient.exceptions.InternalServerError)
|
||||
def test_terminate_connection_internal_server_error(
|
||||
self, mock_cinderclient):
|
||||
self.assertRaises(cinder_apiclient.exceptions.InternalServerError,
|
||||
self.api.terminate_connection,
|
||||
self.ctx, 'id1', 'connector')
|
||||
|
||||
self.assertEqual(5, mock_cinderclient.call_count)
|
||||
|
||||
@mock.patch('nova.volume.cinder.cinderclient')
|
||||
def test_terminate_connection_internal_server_error_do_not_raise(
|
||||
self, mock_cinderclient):
|
||||
# generate exception, and then have a normal return on the next retry
|
||||
mock_cinderclient.return_value.volumes.terminate_connection.\
|
||||
side_effect = [cinder_apiclient.exceptions.InternalServerError,
|
||||
None]
|
||||
|
||||
self.api.terminate_connection(self.ctx, 'id1', 'connector')
|
||||
|
||||
self.assertEqual(2, mock_cinderclient.call_count)
|
||||
|
||||
@mock.patch('nova.volume.cinder.cinderclient',
|
||||
side_effect=cinder_exception.BadRequest(code=400))
|
||||
def test_terminate_connection_bad_request_exception(
|
||||
self, mock_cinderclient):
|
||||
self.assertRaises(exception.InvalidInput,
|
||||
self.api.terminate_connection,
|
||||
self.ctx, 'id1', 'connector')
|
||||
|
||||
self.assertEqual(1, mock_cinderclient.call_count)
|
||||
|
||||
@mock.patch('nova.volume.cinder.cinderclient')
|
||||
def test_delete(self, mock_cinderclient):
|
||||
mock_volumes = mock.MagicMock()
|
||||
|
|
|
@ -24,6 +24,7 @@ import functools
|
|||
import sys
|
||||
|
||||
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 exceptions as cinder_exception
|
||||
from keystoneauth1 import exceptions as keystone_exception
|
||||
|
@ -33,6 +34,7 @@ from oslo_serialization import jsonutils
|
|||
from oslo_utils import encodeutils
|
||||
from oslo_utils import excutils
|
||||
from oslo_utils import strutils
|
||||
import retrying
|
||||
import six
|
||||
from six.moves import urllib
|
||||
|
||||
|
@ -559,6 +561,9 @@ class API(object):
|
|||
mountpoint, mode=mode)
|
||||
|
||||
@translate_volume_exception
|
||||
@retrying.retry(stop_max_attempt_number=5,
|
||||
retry_on_exception=lambda e:
|
||||
type(e) == cinder_apiclient.exceptions.InternalServerError)
|
||||
def detach(self, context, volume_id, instance_uuid=None,
|
||||
attachment_id=None):
|
||||
client = cinderclient(context)
|
||||
|
@ -622,6 +627,9 @@ class API(object):
|
|||
exc.code if hasattr(exc, 'code') else None)})
|
||||
|
||||
@translate_volume_exception
|
||||
@retrying.retry(stop_max_attempt_number=5,
|
||||
retry_on_exception=lambda e:
|
||||
type(e) == cinder_apiclient.exceptions.InternalServerError)
|
||||
def terminate_connection(self, context, volume_id, connector):
|
||||
return cinderclient(context).volumes.terminate_connection(volume_id,
|
||||
connector)
|
||||
|
@ -868,6 +876,9 @@ class API(object):
|
|||
'code': getattr(ex, 'code', None)})
|
||||
|
||||
@translate_attachment_exception
|
||||
@retrying.retry(stop_max_attempt_number=5,
|
||||
retry_on_exception=lambda e:
|
||||
type(e) == cinder_apiclient.exceptions.InternalServerError)
|
||||
def attachment_delete(self, context, attachment_id):
|
||||
try:
|
||||
cinderclient(
|
||||
|
|
Loading…
Reference in New Issue