Check if v2 is available and fallback
We have a basic implementation for a fallback mechanism that will use v1 rather than v2 when downloading schema files from glance-api fails. However, this is not sound. If the schemas are cached already, we won't check if v2 is available and fail to fallback. This patch fixes the aforementioned issue by getting the list of available versions from the server only when the API versions was not explicitly specified through the CLI. That is, for all commands that don't pass `--os-image-api-version 2`, we'll check v2's availability and we'll fallback to v1 if it isn't available. This patch also changes how we handle `/versions` calls in the client. The server has been, incorrectly, replying to requests to `/version` with a 300 error, which ended up in the client re-raising such exception. While I think 300 shouldn't raise an exception, I think we should handle that in a spearate patch. Therefore, this patch just avoids raising such exception when `/version` is explicitly called. This fallback behaviour and the check on `/versions` will be removed in future versions of the client. The later depends on this bug[0] being fixed. [0] https://bugs.launchpad.net/glance/+bug/1491350 Closes-bug: #1489381 Change-Id: Ibeba6bc86db2a97b8a2b4bd042248464cd792e5e
This commit is contained in:
Flavio Percoco
parent
90b7dc4141
commit
47423ebbb2
glanceclient
@ -91,7 +91,10 @@ class _BaseHTTPClient(object):
|
||||
if not resp.ok:
|
||||
LOG.debug("Request returned failure status %s." % resp.status_code)
|
||||
raise exc.from_response(resp, resp.content)
|
||||
elif resp.status_code == requests.codes.MULTIPLE_CHOICES:
|
||||
elif (resp.status_code == requests.codes.MULTIPLE_CHOICES and
|
||||
resp.request.path_url != '/versions'):
|
||||
# NOTE(flaper87): Eventually, we'll remove the check on `versions`
|
||||
# which is a bug (1491350) on the server.
|
||||
raise exc.from_response(resp)
|
||||
|
||||
content_type = resp.headers.get('Content-Type')
|
||||
|
||||
@ -553,7 +553,7 @@ class OpenStackImagesShell(object):
|
||||
client = glanceclient.Client(api_version, endpoint, **kwargs)
|
||||
return client
|
||||
|
||||
def _cache_schemas(self, options, home_dir='~/.glanceclient'):
|
||||
def _cache_schemas(self, options, client, home_dir='~/.glanceclient'):
|
||||
homedir = os.path.expanduser(home_dir)
|
||||
path_prefix = homedir
|
||||
if options.os_auth_url:
|
||||
@ -573,16 +573,11 @@ class OpenStackImagesShell(object):
|
||||
schema_file_paths = [os.path.join(path_prefix, x + '_schema.json')
|
||||
for x in ['image', 'namespace', 'resource_type']]
|
||||
|
||||
client = None
|
||||
failed_download_schema = 0
|
||||
for resource, schema_file_path in zip(resources, schema_file_paths):
|
||||
if (not os.path.exists(schema_file_path)) or options.get_schema:
|
||||
try:
|
||||
if not client:
|
||||
client = self._get_versioned_client('2', options,
|
||||
force_auth=True)
|
||||
schema = client.schemas.get(resource)
|
||||
|
||||
with open(schema_file_path, 'w') as f:
|
||||
f.write(json.dumps(schema.raw()))
|
||||
except Exception:
|
||||
@ -624,8 +619,21 @@ class OpenStackImagesShell(object):
|
||||
"Supported values are %s" % SUPPORTED_VERSIONS)
|
||||
utils.exit(msg=msg)
|
||||
|
||||
if api_version == 2:
|
||||
switch_version = self._cache_schemas(options)
|
||||
if not options.os_image_api_version and api_version == 2:
|
||||
switch_version = True
|
||||
client = self._get_versioned_client('2', options,
|
||||
force_auth=True)
|
||||
|
||||
resp, body = client.http_client.get('/versions')
|
||||
|
||||
for version in body['versions']:
|
||||
if version['id'].startswith('v2'):
|
||||
# NOTE(flaper87): We know v2 is enabled in the server,
|
||||
# which means we should be able to get the schemas and
|
||||
# move on.
|
||||
switch_version = self._cache_schemas(options, client)
|
||||
break
|
||||
|
||||
if switch_version:
|
||||
print('WARNING: The client is falling back to v1 because'
|
||||
' the accessing to v2 failed. This behavior will'
|
||||
|
||||
@ -80,7 +80,7 @@ class SimpleReadOnlyGlanceClientTest(base.ClientTestBase):
|
||||
params=param_image_id)
|
||||
|
||||
def test_help(self):
|
||||
help_text = self.glance('help')
|
||||
help_text = self.glance('--os-image-api-version 2 help')
|
||||
lines = help_text.split('\n')
|
||||
self.assertFirstLineStartsWith(lines, 'usage: glance')
|
||||
|
||||
|
||||
@ -147,17 +147,18 @@ class ShellTest(testutils.TestCase):
|
||||
|
||||
def test_help_unknown_command(self):
|
||||
shell = openstack_shell.OpenStackImagesShell()
|
||||
argstr = 'help foofoo'
|
||||
argstr = '--os-image-api-version 2 help foofoo'
|
||||
self.assertRaises(exc.CommandError, shell.main, argstr.split())
|
||||
|
||||
def test_help(self):
|
||||
shell = openstack_shell.OpenStackImagesShell()
|
||||
argstr = 'help'
|
||||
argstr = '--os-image-api-version 2 help'
|
||||
actual = shell.main(argstr.split())
|
||||
self.assertEqual(0, actual)
|
||||
|
||||
def test_help_on_subcommand_error(self):
|
||||
self.assertRaises(exc.CommandError, shell, 'help bad')
|
||||
self.assertRaises(exc.CommandError, shell,
|
||||
'--os-image-api-version 2 help bad')
|
||||
|
||||
def test_help_v2_no_schema(self):
|
||||
shell = openstack_shell.OpenStackImagesShell()
|
||||
@ -185,7 +186,9 @@ class ShellTest(testutils.TestCase):
|
||||
def test_cert_and_key_args_interchangeable(self,
|
||||
mock_versioned_client):
|
||||
# make sure --os-cert and --os-key are passed correctly
|
||||
args = '--os-cert mycert --os-key mykey image-list'
|
||||
args = ('--os-image-api-version 2 '
|
||||
'--os-cert mycert '
|
||||
'--os-key mykey image-list')
|
||||
shell(args)
|
||||
assert mock_versioned_client.called
|
||||
((api_version, args), kwargs) = mock_versioned_client.call_args
|
||||
@ -193,7 +196,9 @@ class ShellTest(testutils.TestCase):
|
||||
self.assertEqual('mykey', args.os_key)
|
||||
|
||||
# make sure we get the same thing with --cert-file and --key-file
|
||||
args = '--cert-file mycertfile --key-file mykeyfile image-list'
|
||||
args = ('--os-image-api-version 2 '
|
||||
'--cert-file mycertfile '
|
||||
'--key-file mykeyfile image-list')
|
||||
glance_shell = openstack_shell.OpenStackImagesShell()
|
||||
glance_shell.main(args.split())
|
||||
assert mock_versioned_client.called
|
||||
@ -381,6 +386,34 @@ class ShellTest(testutils.TestCase):
|
||||
msg = 'Unable to import module. Re-run with --debug for more info.'
|
||||
self.assertEqual(msg, str(e))
|
||||
|
||||
@mock.patch('glanceclient.v2.client.Client')
|
||||
@mock.patch('glanceclient.v1.images.ImageManager.list')
|
||||
def test_shell_v1_fallback_from_v2(self, v1_imgs, v2_client):
|
||||
self.make_env()
|
||||
cli2 = mock.MagicMock()
|
||||
v2_client.return_value = cli2
|
||||
cli2.http_client.get.return_value = (None, {'versions': []})
|
||||
args = 'image-list'
|
||||
glance_shell = openstack_shell.OpenStackImagesShell()
|
||||
glance_shell.main(args.split())
|
||||
self.assertFalse(cli2.schemas.get.called)
|
||||
self.assertTrue(v1_imgs.called)
|
||||
|
||||
@mock.patch.object(openstack_shell.OpenStackImagesShell,
|
||||
'_cache_schemas')
|
||||
@mock.patch('glanceclient.v2.client.Client')
|
||||
def test_shell_no_fallback_from_v2(self, v2_client, cache_schemas):
|
||||
self.make_env()
|
||||
cli2 = mock.MagicMock()
|
||||
v2_client.return_value = cli2
|
||||
cli2.http_client.get.return_value = (None,
|
||||
{'versions': [{'id': 'v2'}]})
|
||||
cache_schemas.return_value = False
|
||||
args = 'image-list'
|
||||
glance_shell = openstack_shell.OpenStackImagesShell()
|
||||
glance_shell.main(args.split())
|
||||
self.assertTrue(cli2.images.list.called)
|
||||
|
||||
@mock.patch('glanceclient.v1.client.Client')
|
||||
def test_auth_plugin_invocation_without_username_with_v1(self, v1_client):
|
||||
self.make_env(exclude='OS_USERNAME')
|
||||
@ -477,7 +510,7 @@ class ShellTestWithKeystoneV3Auth(ShellTest):
|
||||
self.assertRaises(exc.CommandError, glance_shell.main, args.split())
|
||||
|
||||
def test_bash_completion(self):
|
||||
stdout, stderr = self.shell('bash_completion')
|
||||
stdout, stderr = self.shell('--os-image-api-version 2 bash_completion')
|
||||
# just check we have some output
|
||||
required = [
|
||||
'--status',
|
||||
@ -542,8 +575,9 @@ class ShellCacheSchemaTest(testutils.TestCase):
|
||||
}
|
||||
schema_odict = OrderedDict(self.schema_dict)
|
||||
|
||||
self.shell._cache_schemas(self._make_args(options),
|
||||
home_dir=self.cache_dir)
|
||||
args = self._make_args(options)
|
||||
client = self.shell._get_versioned_client('2', args, force_auth=True)
|
||||
self.shell._cache_schemas(args, client, home_dir=self.cache_dir)
|
||||
|
||||
self.assertEqual(12, open.mock_calls.__len__())
|
||||
self.assertEqual(mock.call(self.cache_files[0], 'w'),
|
||||
@ -564,8 +598,9 @@ class ShellCacheSchemaTest(testutils.TestCase):
|
||||
}
|
||||
schema_odict = OrderedDict(self.schema_dict)
|
||||
|
||||
self.shell._cache_schemas(self._make_args(options),
|
||||
home_dir=self.cache_dir)
|
||||
args = self._make_args(options)
|
||||
client = self.shell._get_versioned_client('2', args, force_auth=True)
|
||||
self.shell._cache_schemas(args, client, home_dir=self.cache_dir)
|
||||
|
||||
self.assertEqual(12, open.mock_calls.__len__())
|
||||
self.assertEqual(mock.call(self.cache_files[0], 'w'),
|
||||
@ -585,8 +620,9 @@ class ShellCacheSchemaTest(testutils.TestCase):
|
||||
'os_auth_url': self.os_auth_url
|
||||
}
|
||||
|
||||
client = mock.MagicMock()
|
||||
self.shell._cache_schemas(self._make_args(options),
|
||||
home_dir=self.cache_dir)
|
||||
client, home_dir=self.cache_dir)
|
||||
|
||||
os.path.exists.assert_any_call(self.prefix_path)
|
||||
os.path.exists.assert_any_call(self.cache_files[0])
|
||||
@ -604,6 +640,8 @@ class ShellCacheSchemaTest(testutils.TestCase):
|
||||
|
||||
self.client.schemas.get.return_value = Exception()
|
||||
|
||||
client = mock.MagicMock()
|
||||
switch_version = self.shell._cache_schemas(self._make_args(options),
|
||||
client,
|
||||
home_dir=self.cache_dir)
|
||||
self.assertEqual(switch_version, True)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user