From 3aa3238cd0323ea8fc4cd4a2e5c1f990d87bfc04 Mon Sep 17 00:00:00 2001 From: Nate Potter Date: Wed, 2 Dec 2015 15:54:08 +0000 Subject: [PATCH] Use the catalog to retrieve glance_api_servers Currently the config option glance_api_servers must be set to give the correct list of glance API URLs. This patch enables the use of the catalog to find the glance endpoint when this config option is set to None, and changes its default to None as well. Closes-bug: #1371644 DocImpact: The config options glance_host and glance_port have been removed, the default value for glance_api_servers has been changed to None, and a new config option glance_catalog_info has been added to glance_opts. Change-Id: I261da528eb008f6b2520aef043c06697bd5ce352 --- cinder/common/config.py | 9 +----- cinder/context.py | 3 +- cinder/image/glance.py | 45 ++++++++++++++++++++------ cinder/tests/unit/image/test_glance.py | 23 +++++++++++-- cinder/tests/unit/test_context.py | 2 +- 5 files changed, 60 insertions(+), 22 deletions(-) diff --git a/cinder/common/config.py b/cinder/common/config.py index 9a27bf639..93a1bf8ba 100644 --- a/cinder/common/config.py +++ b/cinder/common/config.py @@ -56,15 +56,8 @@ global_opts = [ cfg.StrOpt('my_ip', default=netutils.get_my_ipv4(), help='IP address of this host'), - cfg.StrOpt('glance_host', - default='$my_ip', - help='Default glance host name or IP'), - cfg.IntOpt('glance_port', - default=9292, - min=1, max=65535, - help='Default glance port'), cfg.ListOpt('glance_api_servers', - default=['$glance_host:$glance_port'], + default=None, help='A list of the URLs of glance API servers available to ' 'cinder ([http[s]://][hostname|ip]:port). If protocol ' 'is not specified it defaults to http.'), diff --git a/cinder/context.py b/cinder/context.py index 8c43d9884..7f9fcd5ad 100644 --- a/cinder/context.py +++ b/cinder/context.py @@ -91,7 +91,8 @@ class RequestContext(context.RequestContext): # Only include required parts of service_catalog self.service_catalog = [s for s in service_catalog if s.get('type') in - ('identity', 'compute', 'object-store')] + ('identity', 'compute', 'object-store', + 'image')] else: # if list is empty or none self.service_catalog = [] diff --git a/cinder/image/glance.py b/cinder/image/glance.py index d0785718f..ddc87113d 100644 --- a/cinder/image/glance.py +++ b/cinder/image/glance.py @@ -36,7 +36,7 @@ from six.moves import range from six.moves import urllib from cinder import exception -from cinder.i18n import _LE, _LW +from cinder.i18n import _, _LE, _LW glance_opts = [ @@ -45,6 +45,12 @@ glance_opts = [ help='A list of url schemes that can be downloaded directly ' 'via the direct_url. Currently supported schemes: ' '[file].'), + cfg.StrOpt('glance_catalog_info', + default='image:glance:publicURL', + help='Info to match when looking for glance in the service ' + 'catalog. Format is: separated values of the form: ' + ':: - ' + 'Only used if glance_api_servers are not provided.'), ] glance_core_properties_opts = [ cfg.ListOpt('glance_core_properties', @@ -97,23 +103,44 @@ def _create_glance_client(context, netloc, use_ssl, version=None): return glanceclient.Client(str(version), endpoint, **params) -def get_api_servers(): +def get_api_servers(context): """Return Iterable over shuffled api servers. - Shuffle a list of CONF.glance_api_servers and return an iterator + Shuffle a list of glance_api_servers and return an iterator that will cycle through the list, looping around to the beginning - if necessary. + if necessary. If CONF.glance_api_servers is None then they will + be retrieved from the catalog. """ api_servers = [] - for api_server in CONF.glance_api_servers: + api_servers_info = [] + + if CONF.glance_api_servers is None: + info = CONF.glance_catalog_info + try: + service_type, service_name, endpoint_type = info.split(':') + except ValueError: + raise exception.InvalidConfigurationValue(_( + "Failed to parse the configuration option " + "'glance_catalog_info', must be in the form " + "::")) + for entry in context.service_catalog: + if entry.get('type') == service_type: + api_servers.append( + entry.get('endpoints')[0].get(endpoint_type)) + else: + for api_server in CONF.glance_api_servers: + api_servers.append(api_server) + + for api_server in api_servers: if '//' not in api_server: api_server = 'http://' + api_server url = urllib.parse.urlparse(api_server) netloc = url.netloc use_ssl = (url.scheme == 'https') - api_servers.append((netloc, use_ssl)) - random.shuffle(api_servers) - return itertools.cycle(api_servers) + api_servers_info.append((netloc, use_ssl)) + + random.shuffle(api_servers_info) + return itertools.cycle(api_servers_info) class GlanceClientWrapper(object): @@ -149,7 +176,7 @@ class GlanceClientWrapper(object): def _create_onetime_client(self, context, version): """Create a client that will be used for one call.""" if self.api_servers is None: - self.api_servers = get_api_servers() + self.api_servers = get_api_servers(context) self.netloc, self.use_ssl = next(self.api_servers) return _create_glance_client(context, self.netloc, diff --git a/cinder/tests/unit/image/test_glance.py b/cinder/tests/unit/image/test_glance.py index c8fcbebbf..f8cd0653f 100644 --- a/cinder/tests/unit/image/test_glance.py +++ b/cinder/tests/unit/image/test_glance.py @@ -15,6 +15,7 @@ import datetime +import itertools import glanceclient.exc import mock @@ -94,8 +95,12 @@ class TestGlanceImageService(test.TestCase): super(TestGlanceImageService, self).setUp() client = glance_stubs.StubGlanceClient() + service_catalog = [{u'type': u'image', u'name': u'glance', + u'endpoints': [{ + u'publicURL': u'http://example.com:9292'}]}] self.service = self._create_image_service(client) self.context = context.RequestContext('fake', 'fake', auth_token=True) + self.context.service_catalog = service_catalog self.stubs.Set(glance.time, 'sleep', lambda s: None) def _create_image_service(self, client): @@ -123,6 +128,11 @@ class TestGlanceImageService(test.TestCase): updated_at=self.NOW_GLANCE_FORMAT, deleted_at=self.NOW_GLANCE_FORMAT) + def test_get_api_servers(self): + result = glance.get_api_servers(self.context) + expected = (u'example.com:9292', False) + self.assertEqual(expected, next(result)) + def test_create_with_instance_id(self): """Ensure instance_id is persisted as an image-property.""" fixture = {'name': 'test image', @@ -533,7 +543,10 @@ class TestGlanceImageService(test.TestCase): @mock.patch('six.moves.builtins.open') @mock.patch('shutil.copyfileobj') - def test_download_from_direct_file(self, mock_copyfileobj, mock_open): + @mock.patch('cinder.image.glance.get_api_servers', + return_value=itertools.cycle([(False, 'localhost:9292')])) + def test_download_from_direct_file(self, api_servers, + mock_copyfileobj, mock_open): fixture = self._make_fixture(name='test image', locations=[{'url': 'file:///tmp/test'}]) image_id = self.service.create(self.context, fixture)['id'] @@ -545,7 +558,9 @@ class TestGlanceImageService(test.TestCase): @mock.patch('six.moves.builtins.open') @mock.patch('shutil.copyfileobj') - def test_download_from_direct_file_non_file(self, + @mock.patch('cinder.image.glance.get_api_servers', + return_value=itertools.cycle([(False, 'localhost:9292')])) + def test_download_from_direct_file_non_file(self, api_servers, mock_copyfileobj, mock_open): fixture = self._make_fixture(name='test image', direct_url='swift+http://test/image') @@ -688,7 +703,9 @@ class TestGlanceClientVersion(test.TestCase): self.assertEqual('2', _mockglanceclient.call_args[0][0]) @mock.patch('cinder.image.glance.glanceclient.Client') - def test_call_glance_version_by_arg(self, _mockglanceclient): + @mock.patch('cinder.image.glance.get_api_servers', + return_value=itertools.cycle([(False, 'localhost:9292')])) + def test_call_glance_version_by_arg(self, api_servers, _mockglanceclient): """Test glance version set by arg to GlanceClientWrapper""" glance_wrapper = glance.GlanceClientWrapper() glance_wrapper.call('fake_context', 'method', version=2) diff --git a/cinder/tests/unit/test_context.py b/cinder/tests/unit/test_context.py index c39c03d83..66173b899 100644 --- a/cinder/tests/unit/test_context.py +++ b/cinder/tests/unit/test_context.py @@ -82,7 +82,7 @@ class ContextTestCase(test.TestCase): object_catalog = [{u'name': u'swift', u'type': u'object-store'}] ctxt = context.RequestContext('111', '222', service_catalog=service_catalog) - self.assertEqual(3, len(ctxt.service_catalog)) + self.assertEqual(4, len(ctxt.service_catalog)) return_compute = [v for v in ctxt.service_catalog if v['type'] == u'compute'] return_object = [v for v in ctxt.service_catalog if