Merge "prevent common kwargs from glance client failure"

This commit is contained in:
Zuul 2018-11-06 21:23:24 +00:00 committed by Gerrit Code Review
commit c295e395dc
2 changed files with 116 additions and 70 deletions

View File

@ -161,21 +161,35 @@ class GlanceClientWrapper(object):
self.api_server = next(self.api_servers)
return _glanceclient_from_endpoint(context, self.api_server, version)
def call(self, context, version, method, *args, **kwargs):
def call(self, context, version, method, controller=None, args=None,
kwargs=None):
"""Call a glance client method. If we get a connection error,
retry the request according to CONF.glance.num_retries.
:param context: RequestContext to use
:param version: Numeric version of the *Glance API* to use
:param method: string method name to execute on the glanceclient
:param controller: optional string name of the client controller to
use. Default (None) is to use the 'images'
controller
:param args: optional iterable of arguments to pass to the
glanceclient method, splatted as positional args
:param kwargs: optional dict of arguments to pass to the glanceclient,
splatted into named arguments
"""
args = args or []
kwargs = kwargs or {}
retry_excs = (glanceclient.exc.ServiceUnavailable,
glanceclient.exc.InvalidEndpoint,
glanceclient.exc.CommunicationError)
num_attempts = 1 + CONF.glance.num_retries
controller_name = controller or 'images'
for attempt in range(1, num_attempts + 1):
client = self.client or self._create_onetime_client(context,
version)
try:
controller = getattr(client,
kwargs.pop('controller', 'images'))
controller = getattr(client, controller_name)
result = getattr(controller, method)(*args, **kwargs)
if inspect.isgenerator(result):
# Convert generator results to a list, so that we can
@ -239,7 +253,7 @@ class GlanceImageServiceV2(object):
image is deleted.
"""
try:
image = self._client.call(context, 2, 'get', image_id)
image = self._client.call(context, 2, 'get', args=(image_id,))
except Exception:
_reraise_translated_image_exception(image_id)
@ -274,7 +288,7 @@ class GlanceImageServiceV2(object):
"""Calls out to Glance for a list of detailed image information."""
params = _extract_query_params_v2(kwargs)
try:
images = self._client.call(context, 2, 'list', **params)
images = self._client.call(context, 2, 'list', kwargs=params)
except Exception:
_reraise_translated_exception()
@ -320,7 +334,8 @@ class GlanceImageServiceV2(object):
LOG.exception("Download image error")
try:
image_chunks = self._client.call(context, 2, 'data', image_id)
image_chunks = self._client.call(
context, 2, 'data', args=(image_id,))
except Exception:
_reraise_translated_image_exception(image_id)
@ -466,14 +481,14 @@ class GlanceImageServiceV2(object):
def _add_location(self, context, image_id, location):
# 'show_multiple_locations' must be enabled in glance api conf file.
try:
return self._client.call(context, 2, 'add_location', image_id,
location, {})
return self._client.call(
context, 2, 'add_location', args=(image_id, location))
except glanceclient.exc.HTTPBadRequest:
_reraise_translated_exception()
def _upload_data(self, context, image_id, data):
self._client.call(context, 2, 'upload', image_id, data)
return self._client.call(context, 2, 'get', image_id)
self._client.call(context, 2, 'upload', args=(image_id, data))
return self._client.call(context, 2, 'get', args=(image_id,))
def _get_image_create_disk_format_default(self, context):
"""Gets an acceptable default image disk_format based on the schema.
@ -493,8 +508,8 @@ class GlanceImageServiceV2(object):
# Get the image schema - note we don't cache this value since it could
# change under us. This looks a bit funky, but what it's basically
# doing is calling glanceclient.v2.Client.schemas.get('image').
image_schema = self._client.call(context, 2, 'get', 'image',
controller='schemas')
image_schema = self._client.call(
context, 2, 'get', args=('image',), controller='schemas')
# get the disk_format schema property from the raw schema
disk_format_schema = (
image_schema.raw()['properties'].get('disk_format') if image_schema
@ -534,7 +549,7 @@ class GlanceImageServiceV2(object):
location = sent_service_image_meta.pop('location', None)
image = self._client.call(
context, 2, 'create', **sent_service_image_meta)
context, 2, 'create', kwargs=sent_service_image_meta)
image_id = image['id']
# Sending image location in a separate request.
@ -578,7 +593,7 @@ class GlanceImageServiceV2(object):
location = sent_service_image_meta.pop('location', None)
image_id = sent_service_image_meta['image_id']
image = self._client.call(
context, 2, 'update', **sent_service_image_meta)
context, 2, 'update', kwargs=sent_service_image_meta)
# Sending image location in a separate request.
if location:
@ -601,7 +616,7 @@ class GlanceImageServiceV2(object):
"""
try:
self._client.call(context, 2, 'delete', image_id)
self._client.call(context, 2, 'delete', args=(image_id,))
except glanceclient.exc.NotFound:
raise exception.ImageNotFound(image_id=image_id)
except glanceclient.exc.HTTPForbidden:

View File

@ -410,13 +410,13 @@ class TestGlanceClientWrapperRetries(test.NoDBTestCase):
self.flags(api_servers=api_servers, group='glance')
def assert_retry_attempted(self, sleep_mock, client, expected_url):
client.call(self.ctx, 1, 'get', 'meow')
client.call(self.ctx, 1, 'get', args=('meow',))
sleep_mock.assert_called_once_with(1)
self.assertEqual(str(client.api_server), expected_url)
def assert_retry_not_attempted(self, sleep_mock, client):
self.assertRaises(exception.GlanceConnectionFailed,
client.call, self.ctx, 1, 'get', 'meow')
client.call, self.ctx, 1, 'get', args=('meow',))
self.assertFalse(sleep_mock.called)
@mock.patch('time.sleep')
@ -490,12 +490,12 @@ class TestGlanceClientWrapperRetries(test.NoDBTestCase):
# sleep (which would be an indication of a retry)
self.assertRaises(exception.GlanceConnectionFailed,
client.call, self.ctx, 1, 'get', 'meow')
client.call, self.ctx, 1, 'get', args=('meow',))
self.assertEqual(str(client.api_server), 'http://host1:9292')
self.assertFalse(sleep_mock.called)
self.assertRaises(exception.GlanceConnectionFailed,
client.call, self.ctx, 1, 'get', 'meow')
client.call, self.ctx, 1, 'get', args=('meow',))
self.assertEqual(str(client.api_server), 'https://host2:9293')
self.assertFalse(sleep_mock.called)
@ -514,6 +514,33 @@ class TestGlanceClientWrapperRetries(test.NoDBTestCase):
create_client_mock.return_value = client_mock
class TestCommonPropertyNameConflicts(test.NoDBTestCase):
"""Tests that images that have common property names like "version" don't
cause an exception to be raised from the wacky GlanceClientWrapper magic
call() method.
:see https://bugs.launchpad.net/nova/+bug/1717547
"""
@mock.patch('nova.image.glance.GlanceClientWrapper._create_onetime_client')
def test_version_property_conflicts(self, mock_glance_client):
client = mock.MagicMock()
mock_glance_client.return_value = client
ctx = mock.sentinel.ctx
service = glance.GlanceImageServiceV2()
# Simulate the process of snapshotting a server that was launched with
# an image with the properties collection containing a (very
# commonly-named) "version" property.
image_meta = {
'id': 1,
'version': 'blows up',
}
# This call would blow up before the fix for 1717547
service.create(ctx, image_meta)
class TestDownloadNoDirectUri(test.NoDBTestCase):
"""Tests the download method of the GlanceImageServiceV2 when the
@ -532,8 +559,8 @@ class TestDownloadNoDirectUri(test.NoDBTestCase):
self.assertFalse(show_mock.called)
self.assertFalse(open_mock.called)
client.call.assert_called_once_with(ctx, 2, 'data',
mock.sentinel.image_id)
client.call.assert_called_once_with(
ctx, 2, 'data', args=(mock.sentinel.image_id,))
self.assertEqual(mock.sentinel.image_chunks, res)
@mock.patch.object(six.moves.builtins, 'open')
@ -548,8 +575,8 @@ class TestDownloadNoDirectUri(test.NoDBTestCase):
self.assertFalse(show_mock.called)
self.assertFalse(open_mock.called)
client.call.assert_called_once_with(ctx, 2, 'data',
mock.sentinel.image_id)
client.call.assert_called_once_with(
ctx, 2, 'data', args=(mock.sentinel.image_id,))
self.assertIsNone(res)
data.write.assert_has_calls(
[
@ -575,8 +602,8 @@ class TestDownloadNoDirectUri(test.NoDBTestCase):
dst_path=mock.sentinel.dst_path)
self.assertFalse(show_mock.called)
client.call.assert_called_once_with(ctx, 2, 'data',
mock.sentinel.image_id)
client.call.assert_called_once_with(
ctx, 2, 'data', args=(mock.sentinel.image_id,))
open_mock.assert_called_once_with(mock.sentinel.dst_path, 'wb')
fsync_mock.assert_called_once_with(writer)
self.assertIsNone(res)
@ -606,8 +633,8 @@ class TestDownloadNoDirectUri(test.NoDBTestCase):
self.assertFalse(show_mock.called)
self.assertFalse(open_mock.called)
client.call.assert_called_once_with(ctx, 2, 'data',
mock.sentinel.image_id)
client.call.assert_called_once_with(
ctx, 2, 'data', args=(mock.sentinel.image_id,))
self.assertIsNone(res)
data.write.assert_has_calls(
[
@ -721,8 +748,8 @@ class TestDownloadNoDirectUri(test.NoDBTestCase):
tran_mod.download.assert_called_once_with(ctx, mock.ANY,
mock.sentinel.dst_path,
mock.sentinel.loc_meta)
client.call.assert_called_once_with(ctx, 2, 'data',
mock.sentinel.image_id)
client.call.assert_called_once_with(
ctx, 2, 'data', args=(mock.sentinel.image_id,))
fsync_mock.assert_called_once_with(writer)
# NOTE(jaypipes): log messages call open() in part of the
# download path, so here, we just check that the last open()
@ -771,8 +798,8 @@ class TestDownloadNoDirectUri(test.NoDBTestCase):
mock.sentinel.image_id,
include_locations=True)
get_tran_mock.assert_called_once_with('file')
client.call.assert_called_once_with(ctx, 2, 'data',
mock.sentinel.image_id)
client.call.assert_called_once_with(
ctx, 2, 'data', args=(mock.sentinel.image_id,))
fsync_mock.assert_called_once_with(writer)
# NOTE(jaypipes): log messages call open() in part of the
# download path, so here, we just check that the last open()
@ -1229,8 +1256,8 @@ class TestShow(test.NoDBTestCase):
service = glance.GlanceImageServiceV2(client)
info = service.show(ctx, mock.sentinel.image_id)
client.call.assert_called_once_with(ctx, 2, 'get',
mock.sentinel.image_id)
client.call.assert_called_once_with(
ctx, 2, 'get', args=(mock.sentinel.image_id,))
is_avail_mock.assert_called_once_with(ctx, {})
trans_from_mock.assert_called_once_with({}, include_locations=False)
self.assertIn('mock', info)
@ -1248,8 +1275,8 @@ class TestShow(test.NoDBTestCase):
with testtools.ExpectedException(exception.ImageNotFound):
service.show(ctx, mock.sentinel.image_id)
client.call.assert_called_once_with(ctx, 2, 'get',
mock.sentinel.image_id)
client.call.assert_called_once_with(
ctx, 2, 'get', args=(mock.sentinel.image_id,))
is_avail_mock.assert_called_once_with(ctx, mock.sentinel.images_0)
self.assertFalse(trans_from_mock.called)
@ -1267,8 +1294,8 @@ class TestShow(test.NoDBTestCase):
with testtools.ExpectedException(exception.ImageNotAuthorized):
service.show(ctx, mock.sentinel.image_id)
client.call.assert_called_once_with(ctx, 2, 'get',
mock.sentinel.image_id)
client.call.assert_called_once_with(
ctx, 2, 'get', args=(mock.sentinel.image_id,))
self.assertFalse(is_avail_mock.called)
self.assertFalse(trans_from_mock.called)
reraise_mock.assert_called_once_with(mock.sentinel.image_id)
@ -1303,8 +1330,8 @@ class TestShow(test.NoDBTestCase):
ctx = mock.sentinel.ctx
service = glance.GlanceImageServiceV2(client)
image_info = service.show(ctx, glance_image.id)
client.call.assert_called_once_with(ctx, 2, 'get',
glance_image.id)
client.call.assert_called_once_with(
ctx, 2, 'get', args=(glance_image.id,))
NOVA_IMAGE_ATTRIBUTES = set(['size', 'disk_format', 'owner',
'container_format', 'status', 'id',
'name', 'created_at', 'updated_at',
@ -1328,7 +1355,8 @@ class TestShow(test.NoDBTestCase):
image_id = mock.sentinel.image_id
info = service.show(ctx, image_id, include_locations=True)
client.call.assert_called_once_with(ctx, 2, 'get', image_id)
client.call.assert_called_once_with(
ctx, 2, 'get', args=(image_id,))
avail_mock.assert_called_once_with(ctx, mock.sentinel.image)
trans_from_mock.assert_called_once_with(mock.sentinel.image,
include_locations=True)
@ -1350,7 +1378,8 @@ class TestShow(test.NoDBTestCase):
image_id = mock.sentinel.image_id
info = service.show(ctx, image_id, include_locations=True)
client.call.assert_called_once_with(ctx, 2, 'get', image_id)
client.call.assert_called_once_with(
ctx, 2, 'get', args=(image_id,))
expected = locations
expected.append({'url': mock.sentinel.duri, 'metadata': {}})
self.assertIn('locations', info)
@ -1374,8 +1403,8 @@ class TestShow(test.NoDBTestCase):
with testtools.ExpectedException(exception.ImageNotFound):
service.show(ctx, glance_image.id, show_deleted=False)
client.call.assert_called_once_with(ctx, 2, 'get',
glance_image.id)
client.call.assert_called_once_with(
ctx, 2, 'get', args=(glance_image.id,))
self.assertFalse(is_avail_mock.called)
self.assertFalse(trans_from_mock.called)
@ -1399,7 +1428,7 @@ class TestDetail(test.NoDBTestCase):
service = glance.GlanceImageServiceV2(client)
images = service.detail(ctx, **params)
client.call.assert_called_once_with(ctx, 2, 'list')
client.call.assert_called_once_with(ctx, 2, 'list', kwargs={})
is_avail_mock.assert_called_once_with(ctx, mock.sentinel.images_0)
trans_from_mock.assert_called_once_with(mock.sentinel.images_0)
self.assertEqual([mock.sentinel.trans_from], images)
@ -1419,7 +1448,7 @@ class TestDetail(test.NoDBTestCase):
service = glance.GlanceImageServiceV2(client)
images = service.detail(ctx, **params)
client.call.assert_called_once_with(ctx, 2, 'list')
client.call.assert_called_once_with(ctx, 2, 'list', kwargs={})
is_avail_mock.assert_called_once_with(ctx, mock.sentinel.images_0)
self.assertFalse(trans_from_mock.called)
self.assertEqual([], images)
@ -1433,10 +1462,8 @@ class TestDetail(test.NoDBTestCase):
service = glance.GlanceImageServiceV2(client)
service.detail(ctx, page_size=5, limit=10)
client.call.assert_called_once_with(ctx, 2, 'list',
filters={},
page_size=5,
limit=10)
client.call.assert_called_once_with(
ctx, 2, 'list', kwargs=dict(filters={}, page_size=5, limit=10))
@mock.patch('nova.image.glance._reraise_translated_exception')
@mock.patch('nova.image.glance._extract_query_params_v2')
@ -1456,7 +1483,7 @@ class TestDetail(test.NoDBTestCase):
with testtools.ExpectedException(exception.Forbidden):
service.detail(ctx, **params)
client.call.assert_called_once_with(ctx, 2, 'list')
client.call.assert_called_once_with(ctx, 2, 'list', kwargs={})
self.assertFalse(is_avail_mock.called)
self.assertFalse(trans_from_mock.called)
reraise_mock.assert_called_once_with()
@ -1486,8 +1513,8 @@ class TestCreate(test.NoDBTestCase):
# the call to glanceclient's update (since the image ID is
# supplied as a positional arg), and that the
# purge_props default is True.
client.call.assert_called_once_with(ctx, 2, 'create',
name=mock.sentinel.name)
client.call.assert_called_once_with(
ctx, 2, 'create', kwargs=dict(name=mock.sentinel.name))
trans_from_mock.assert_called_once_with({'id': '123'})
self.assertEqual(mock.sentinel.trans_from, image_meta)
@ -1523,7 +1550,7 @@ class TestCreate(test.NoDBTestCase):
image_meta = service.create(ctx, image_mock)
trans_to_mock.assert_called_once_with(image_mock)
# Verify that the disk_format and container_format kwargs are passed.
create_call_kwargs = client.call.call_args_list[0][1]
create_call_kwargs = client.call.call_args_list[0][1]['kwargs']
self.assertEqual('vdi', create_call_kwargs['disk_format'])
self.assertEqual('bare', create_call_kwargs['container_format'])
trans_from_mock.assert_called_once_with({'id': '123'})
@ -1580,7 +1607,7 @@ class TestCreate(test.NoDBTestCase):
mock.sentinel.ctx)
self.assertEqual(expected_disk_format, disk_format)
mock_client.call.assert_called_once_with(
mock.sentinel.ctx, 2, 'get', 'image', controller='schemas')
mock.sentinel.ctx, 2, 'get', args=('image',), controller='schemas')
def test_get_image_create_disk_format_default_no_schema(self):
"""Tests that if there is no disk_format schema we default to qcow2.
@ -1671,11 +1698,11 @@ class TestUpdate(test.NoDBTestCase):
# the call to glanceclient's update (since the image ID is
# supplied as a positional arg), and that the
# purge_props default is True.
client.call.assert_called_once_with(ctx, 2, 'update',
image_id=mock.sentinel.image_id,
name=mock.sentinel.name,
prop_to_keep='4',
remove_props=['prop_to_remove'])
client.call.assert_called_once_with(
ctx, 2, 'update', kwargs=dict(
image_id=mock.sentinel.image_id, name=mock.sentinel.name,
prop_to_keep='4', remove_props=['prop_to_remove'],
))
trans_from_mock.assert_called_once_with(mock.sentinel.image_meta)
self.assertEqual(mock.sentinel.trans_from, image_meta)
@ -1747,11 +1774,13 @@ class TestUpdate(test.NoDBTestCase):
self.assertRaises(exception.ImageNotAuthorized,
service.update, ctx, mock.sentinel.image_id,
image)
client.call.assert_called_once_with(ctx, 2, 'update',
image_id=mock.sentinel.image_id,
name=mock.sentinel.name,
prop_to_keep='4',
remove_props=['prop_to_remove'])
client.call.assert_called_once_with(
ctx, 2, 'update', kwargs=dict(
image_id=mock.sentinel.image_id,
name=mock.sentinel.name,
prop_to_keep='4',
remove_props=['prop_to_remove'],
))
reraise_mock.assert_called_once_with(mock.sentinel.image_id)
@ -1765,8 +1794,8 @@ class TestDelete(test.NoDBTestCase):
ctx = mock.sentinel.ctx
service = glance.GlanceImageServiceV2(client)
service.delete(ctx, mock.sentinel.image_id)
client.call.assert_called_once_with(ctx, 2, 'delete',
mock.sentinel.image_id)
client.call.assert_called_once_with(
ctx, 2, 'delete', args=(mock.sentinel.image_id,))
def test_delete_client_failure_v2(self):
client = mock.MagicMock()
@ -1895,10 +1924,12 @@ class TestExtractQueryParams(test.NoDBTestCase):
'kernel-id': 'some-id',
'updated_at': 'gte:some-date'}
client.call.assert_called_once_with(ctx, 2, 'list',
filters=expected_filters_v1,
page_size=5,
limit=10)
client.call.assert_called_once_with(
ctx, 2, 'list', kwargs=dict(
filters=expected_filters_v1,
page_size=5,
limit=10,
))
class TestTranslateToGlance(test.NoDBTestCase):