From 9519601401ee116a9197fe3b5d571495a96912e9 Mon Sep 17 00:00:00 2001 From: Eric Fried Date: Fri, 4 Aug 2017 16:54:08 -0500 Subject: [PATCH] Get auth from context for glance endpoint Change the Adapter loading for glance to use the auth from the user context instead of exposing and requiring it in the conf. With this change, it is possible to leave the [glance] conf section empty and still be able to discover the image API endpoint from the service catalog. Note that, when we do this, we often end up with the user auth being a _ContextAuthPlugin, which doesn't conform to the characteristics of keystoneauth1.identity.base.BaseIdentityPlugin as augmented in keystoneauth1 3.1.0. This requires a series of workarounds until bug 1709118 is fixed. These, along with workarounds for bugs 1707993 and 1707995, are subsumed with this change set in a (hopefully temporary) helper method nova.utils.get_endpoint. This lays the foundation for other services that should use user context for authentication - those via which Nova is acting on behalf of the user, i.e. cinder, keystone, and (sometimes) neutron[1]. (Services such as placement and ironic (and sometimes neutron) should continue to use admin auth context loaded from the conf.) [1] https://github.com/openstack/nova/blob/bb4faf40dfb02237af119646a5ebd960b072b31e/nova/network/neutronv2/api.py#L149-L160 Co-Authored-By: Eric Fried Partial-Implements: bp use-ksa-adapter-for-endpoints Change-Id: I4e755b9c66ec8bc3af0393e81cffd91c56064717 --- nova/api/openstack/compute/servers.py | 2 +- nova/api/openstack/compute/views/images.py | 3 +- nova/compute/manager.py | 2 +- nova/conf/glance.py | 8 ++- nova/conf/utils.py | 10 +++- nova/context.py | 2 +- nova/image/glance.py | 32 ++++++----- nova/notifications/base.py | 2 +- .../unit/api/openstack/compute/test_images.py | 20 +++---- .../openstack/compute/test_server_actions.py | 10 ++-- nova/tests/unit/compute/test_compute.py | 20 ++++--- nova/tests/unit/compute/test_compute_utils.py | 16 +++--- nova/tests/unit/image/test_glance.py | 6 +-- nova/tests/unit/test_context.py | 5 +- nova/tests/unit/test_utils.py | 53 ++++++++++++++++++ nova/utils.py | 54 +++++++++++++++++++ nova/virt/xenapi/image/glance.py | 2 +- .../glance-via-ksa-5646eb3d5db51c54.yaml | 5 +- 18 files changed, 188 insertions(+), 64 deletions(-) diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 9c9f87c6a9..93c160a0ea 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -1023,7 +1023,7 @@ class ServersController(wsgi.Controller): # build location of newly-created image entity image_id = str(image['id']) - image_ref = glance.generate_image_url(image_id) + image_ref = glance.generate_image_url(image_id, context) resp = webob.Response(status_int=202) resp.headers['Location'] = image_ref diff --git a/nova/api/openstack/compute/views/images.py b/nova/api/openstack/compute/views/images.py index 0e73f8d4a8..5cf7cab95b 100644 --- a/nova/api/openstack/compute/views/images.py +++ b/nova/api/openstack/compute/views/images.py @@ -132,7 +132,8 @@ class ViewBuilder(common.ViewBuilder): def _get_alternate_link(self, request, identifier): """Create an alternate link for a specific image id.""" - glance_url = glance.generate_glance_url() + glance_url = glance.generate_glance_url( + request.environ['nova.context']) glance_url = self._update_glance_link_prefix(glance_url) return '/'.join([glance_url, self._collection_name, diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 3ea26bf970..d3994f0fbc 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -2946,7 +2946,7 @@ class ComputeManager(manager.Manager): # image_ref, not the new one. Since the DB has been updated # to point to the new one... we have to override it. # TODO(jaypipes): Move generate_image_url() into the nova.image.api - orig_image_ref_url = glance.generate_image_url(orig_image_ref) + orig_image_ref_url = glance.generate_image_url(orig_image_ref, context) extra_usage_info = {'image_ref_url': orig_image_ref_url} compute_utils.notify_usage_exists( self.notifier, context, instance, diff --git a/nova/conf/glance.py b/nova/conf/glance.py index 43a31d6081..af61955e2c 100644 --- a/nova/conf/glance.py +++ b/nova/conf/glance.py @@ -149,17 +149,15 @@ def register_opts(conf): conf.register_group(glance_group) conf.register_opts(glance_opts, group=glance_group) - confutils.register_ksa_opts(conf, glance_group, DEFAULT_SERVICE_TYPE, - deprecated_opts=deprecated_ksa_opts) + confutils.register_ksa_opts( + conf, glance_group, DEFAULT_SERVICE_TYPE, include_auth=False, + deprecated_opts=deprecated_ksa_opts) def list_opts(): return {glance_group: ( glance_opts + ks_loading.get_session_conf_options() + - ks_loading.get_auth_plugin_conf_options('password') + - ks_loading.get_auth_plugin_conf_options('v2password') + - ks_loading.get_auth_plugin_conf_options('v3password') + confutils.get_ksa_adapter_opts(DEFAULT_SERVICE_TYPE, deprecated_opts=deprecated_ksa_opts)) } diff --git a/nova/conf/utils.py b/nova/conf/utils.py index f24ef44baa..da9644408f 100644 --- a/nova/conf/utils.py +++ b/nova/conf/utils.py @@ -55,7 +55,8 @@ def _dummy_opt(name): return cfg.Opt(name, type=lambda x: None) -def register_ksa_opts(conf, group, default_service_type, deprecated_opts=None): +def register_ksa_opts(conf, group, default_service_type, include_auth=True, + deprecated_opts=None): """Register keystoneauth auth, Session, and Adapter opts. :param conf: oslo_config.cfg.CONF in which to register the options @@ -63,6 +64,10 @@ def register_ksa_opts(conf, group, default_service_type, deprecated_opts=None): options. :param default_service_type: Default for the service_type conf option on the Adapter. + :param include_auth: For service types where Nova is acting on behalf of + the user, auth should come from the user context. + In those cases, set this arg to False to avoid + registering ksa auth options. :param deprecated_opts: dict of deprecated opts to register with the ksa Session or Adapter opts. See docstring for the deprecated_opts param of: @@ -72,7 +77,8 @@ def register_ksa_opts(conf, group, default_service_type, deprecated_opts=None): group = getattr(group, 'name', group) ks_loading.register_session_conf_options( conf, group, deprecated_opts=deprecated_opts) - ks_loading.register_auth_conf_options(conf, group) + if include_auth: + ks_loading.register_auth_conf_options(conf, group) conf.register_opts(get_ksa_adapter_opts( default_service_type, deprecated_opts=deprecated_opts), group=group) # Have to register dummies for the version-related opts we removed diff --git a/nova/context.py b/nova/context.py index c02d35719b..b17c156ab7 100644 --- a/nova/context.py +++ b/nova/context.py @@ -118,7 +118,7 @@ class RequestContext(context.RequestContext): if service_catalog: # Only include required parts of service_catalog self.service_catalog = [s for s in service_catalog - if s.get('type') in ('block-storage', 'volumev3', + if s.get('type') in ('image', 'block-storage', 'volumev3', 'key-manager', 'placement')] else: # if list is empty or none diff --git a/nova/image/glance.py b/nova/image/glance.py index 388083b002..47d7c4e030 100644 --- a/nova/image/glance.py +++ b/nova/image/glance.py @@ -56,7 +56,8 @@ CONF = nova.conf.CONF _SESSION = None -def _glanceclient_from_endpoint(context, endpoint, version): +def _session_and_auth(context): + # Session is cached, but auth needs to be pulled from context each time. global _SESSION if not _SESSION: @@ -65,21 +66,25 @@ def _glanceclient_from_endpoint(context, endpoint, version): auth = service_auth.get_auth_plugin(context) - # TODO(johngarbutt) eventually we should default to getting the - # endpoint URL from the service catalog. - return glanceclient.Client(version, session=_SESSION, auth=auth, + return _SESSION, auth + + +def _glanceclient_from_endpoint(context, endpoint, version): + sess, auth = _session_and_auth(context) + + return glanceclient.Client(version, session=sess, auth=auth, endpoint_override=endpoint, global_request_id=context.global_id) -def generate_glance_url(): +def generate_glance_url(context): """Return a random glance url from the api servers we know about.""" - return next(get_api_servers()) + return next(get_api_servers(context)) -def generate_image_url(image_ref): +def generate_image_url(image_ref, context): """Generate an image URL from an image_ref.""" - return "%s/images/%s" % (generate_glance_url(), image_ref) + return "%s/images/%s" % (generate_glance_url(context), image_ref) def _endpoint_from_image_ref(image_href): @@ -106,7 +111,7 @@ def generate_identity_headers(context, status='Confirmed'): } -def get_api_servers(): +def get_api_servers(context): """Shuffle a list of service endpoints and return an iterator that will cycle through the list, looping around to the beginning if necessary. """ @@ -117,13 +122,12 @@ def get_api_servers(): api_servers = CONF.glance.api_servers random.shuffle(api_servers) else: - # TODO(efried): Plumb in a reasonable auth from callers' contexts + sess, auth = _session_and_auth(context) ksa_adap = utils.get_ksa_adapter( nova.conf.glance.DEFAULT_SERVICE_TYPE, + ksa_auth=auth, ksa_session=sess, min_version='2.0', max_version='2.latest') - # TODO(efried): Use ksa_adap.get_endpoint() when bug #1707995 is fixed. - api_servers = [ksa_adap.endpoint_override or - ksa_adap.get_endpoint_data().catalog_url] + api_servers = [utils.get_endpoint(ksa_adap)] return itertools.cycle(api_servers) @@ -149,7 +153,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.api_server = next(self.api_servers) return _glanceclient_from_endpoint(context, self.api_server, version) diff --git a/nova/notifications/base.py b/nova/notifications/base.py index eb6c919f7d..3b7d07092b 100644 --- a/nova/notifications/base.py +++ b/nova/notifications/base.py @@ -394,7 +394,7 @@ def info_from_instance(context, instance, network_info, modifications. """ - image_ref_url = glance.generate_image_url(instance.image_ref) + image_ref_url = glance.generate_image_url(instance.image_ref, context) instance_type = instance.get_flavor() instance_type_name = instance_type.get('name', '') diff --git a/nova/tests/unit/api/openstack/compute/test_images.py b/nova/tests/unit/api/openstack/compute/test_images.py index de77ab0f9f..6fb31bcce7 100644 --- a/nova/tests/unit/api/openstack/compute/test_images.py +++ b/nova/tests/unit/api/openstack/compute/test_images.py @@ -92,7 +92,7 @@ class ImagesControllerTestV21(test.NoDBTestCase): "rel": "alternate", "type": "application/vnd.openstack.image", "href": self.alternate % - (glance.generate_glance_url(), + (glance.generate_glance_url('ctx'), 123), }], }, @@ -136,7 +136,7 @@ class ImagesControllerTestV21(test.NoDBTestCase): "type": "application/vnd.openstack.image", "href": self.alternate % - (glance.generate_glance_url(), + (glance.generate_glance_url('ctx'), 124), }], }, @@ -196,7 +196,7 @@ class ImagesControllerTestV21(test.NoDBTestCase): image_125["links"][0]["href"] = "%s/125" % self.url_prefix image_125["links"][1]["href"] = "%s/125" % self.bookmark_prefix image_125["links"][2]["href"] = ( - "%s/images/125" % glance.generate_glance_url()) + "%s/images/125" % glance.generate_glance_url('ctx')) image_126 = copy.deepcopy(self.expected_image_124["image"]) image_126['id'] = '126' @@ -206,7 +206,7 @@ class ImagesControllerTestV21(test.NoDBTestCase): image_126["links"][0]["href"] = "%s/126" % self.url_prefix image_126["links"][1]["href"] = "%s/126" % self.bookmark_prefix image_126["links"][2]["href"] = ( - "%s/images/126" % glance.generate_glance_url()) + "%s/images/126" % glance.generate_glance_url('ctx')) image_127 = copy.deepcopy(self.expected_image_124["image"]) image_127['id'] = '127' @@ -216,7 +216,7 @@ class ImagesControllerTestV21(test.NoDBTestCase): image_127["links"][0]["href"] = "%s/127" % self.url_prefix image_127["links"][1]["href"] = "%s/127" % self.bookmark_prefix image_127["links"][2]["href"] = ( - "%s/images/127" % glance.generate_glance_url()) + "%s/images/127" % glance.generate_glance_url('ctx')) image_128 = copy.deepcopy(self.expected_image_124["image"]) image_128['id'] = '128' @@ -226,7 +226,7 @@ class ImagesControllerTestV21(test.NoDBTestCase): image_128["links"][0]["href"] = "%s/128" % self.url_prefix image_128["links"][1]["href"] = "%s/128" % self.bookmark_prefix image_128["links"][2]["href"] = ( - "%s/images/128" % glance.generate_glance_url()) + "%s/images/128" % glance.generate_glance_url('ctx')) image_129 = copy.deepcopy(self.expected_image_124["image"]) image_129['id'] = '129' @@ -236,7 +236,7 @@ class ImagesControllerTestV21(test.NoDBTestCase): image_129["links"][0]["href"] = "%s/129" % self.url_prefix image_129["links"][1]["href"] = "%s/129" % self.bookmark_prefix image_129["links"][2]["href"] = ( - "%s/images/129" % glance.generate_glance_url()) + "%s/images/129" % glance.generate_glance_url('ctx')) image_130 = copy.deepcopy(self.expected_image_123["image"]) image_130['id'] = '130' @@ -247,7 +247,7 @@ class ImagesControllerTestV21(test.NoDBTestCase): image_130["links"][0]["href"] = "%s/130" % self.url_prefix image_130["links"][1]["href"] = "%s/130" % self.bookmark_prefix image_130["links"][2]["href"] = ( - "%s/images/130" % glance.generate_glance_url()) + "%s/images/130" % glance.generate_glance_url('ctx')) image_131 = copy.deepcopy(self.expected_image_123["image"]) image_131['id'] = '131' @@ -258,7 +258,7 @@ class ImagesControllerTestV21(test.NoDBTestCase): image_131["links"][0]["href"] = "%s/131" % self.url_prefix image_131["links"][1]["href"] = "%s/131" % self.bookmark_prefix image_131["links"][2]["href"] = ( - "%s/images/131" % glance.generate_glance_url()) + "%s/images/131" % glance.generate_glance_url('ctx')) expected = [self.expected_image_123["image"], self.expected_image_124["image"], @@ -352,7 +352,7 @@ class ImagesControllerTestV21(test.NoDBTestCase): view = images_view.ViewBuilder() request = self.http_request.blank(self.url_base + 'images/1') generated_url = view._get_alternate_link(request, 1) - actual_url = "%s/images/1" % glance.generate_glance_url() + actual_url = "%s/images/1" % glance.generate_glance_url('ctx') self.assertEqual(generated_url, actual_url) def _check_response(self, controller_method, response, expected_code): diff --git a/nova/tests/unit/api/openstack/compute/test_server_actions.py b/nova/tests/unit/api/openstack/compute/test_server_actions.py index 359c79020b..206d58029d 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_server_actions.py @@ -896,7 +896,7 @@ class ServerActionsControllerTestV21(test.TestCase): location = response.headers['Location'] self.assertEqual(self.image_url + '123' if self.image_url else - glance.generate_image_url('123'), + glance.generate_image_url('123', self.context), location) def test_create_image_v2_45(self): @@ -997,8 +997,9 @@ class ServerActionsControllerTestV21(test.TestCase): FAKE_UUID, body=body) location = response.headers['Location'] - image_id = location.replace(self.image_url or - glance.generate_image_url(''), '') + image_id = location.replace( + self.image_url or glance.generate_image_url('', self.context), + '') image = image_service.show(None, image_id) self.assertEqual(image['name'], 'snapshot_of_volume_backed') @@ -1145,7 +1146,8 @@ class ServerActionsControllerTestV21(test.TestCase): location = response.headers['Location'] self.assertEqual(self.image_url + '123' if self.image_url else - glance.generate_image_url('123'), location) + glance.generate_image_url('123', self.context), + location) def test_create_image_with_too_much_metadata(self): body = { diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 1cfa02ecc0..c0b120931f 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -2362,7 +2362,8 @@ class ComputeTestCase(BaseTestCase, self.assertIn('display_name', payload) self.assertIn('created_at', payload) self.assertIn('launched_at', payload) - image_ref_url = glance.generate_image_url(FAKE_IMAGE_REF) + image_ref_url = glance.generate_image_url(FAKE_IMAGE_REF, + self.context) self.assertEqual(payload['image_ref_url'], image_ref_url) msg = fake_notifier.NOTIFICATIONS[0] self.assertIn('rescue_image_name', msg.payload) @@ -2402,7 +2403,8 @@ class ComputeTestCase(BaseTestCase, self.assertIn('display_name', payload) self.assertIn('created_at', payload) self.assertIn('launched_at', payload) - image_ref_url = glance.generate_image_url(FAKE_IMAGE_REF) + image_ref_url = glance.generate_image_url(FAKE_IMAGE_REF, + self.context) self.assertEqual(payload['image_ref_url'], image_ref_url) self.compute.terminate_instance(self.context, instance, [], []) @@ -4149,7 +4151,7 @@ class ComputeTestCase(BaseTestCase, self.assertIn('launched_at', payload) self.assertIn('fixed_ips', payload) self.assertTrue(payload['launched_at']) - image_ref_url = glance.generate_image_url(FAKE_IMAGE_REF) + image_ref_url = glance.generate_image_url(FAKE_IMAGE_REF, self.context) self.assertEqual(payload['image_ref_url'], image_ref_url) self.assertEqual('Success', payload['message']) self.compute.terminate_instance(self.context, instance, [], []) @@ -4279,7 +4281,7 @@ class ComputeTestCase(BaseTestCase, self.assertIn('deleted_at', payload) self.assertEqual(payload['terminated_at'], utils.strtime(cur_time)) self.assertEqual(payload['deleted_at'], utils.strtime(cur_time)) - image_ref_url = glance.generate_image_url(FAKE_IMAGE_REF) + image_ref_url = glance.generate_image_url(FAKE_IMAGE_REF, self.context) self.assertEqual(payload['image_ref_url'], image_ref_url) @mock.patch.object(fake.FakeDriver, "macs_for_instance") @@ -4968,8 +4970,9 @@ class ComputeTestCase(BaseTestCase, inst_ref.refresh() - image_ref_url = glance.generate_image_url(image_ref) - new_image_ref_url = glance.generate_image_url(new_image_ref) + image_ref_url = glance.generate_image_url(image_ref, self.context) + new_image_ref_url = glance.generate_image_url(new_image_ref, + self.context) self.assertEqual(len(fake_notifier.NOTIFICATIONS), 3) msg = fake_notifier.NOTIFICATIONS[0] @@ -5057,7 +5060,8 @@ class ComputeTestCase(BaseTestCase, self.assertIn('created_at', payload) self.assertIn('launched_at', payload) self.assertEqual(payload['launched_at'], utils.strtime(cur_time)) - image_ref_url = glance.generate_image_url(FAKE_IMAGE_REF) + image_ref_url = glance.generate_image_url(FAKE_IMAGE_REF, + self.context) self.assertEqual(payload['image_ref_url'], image_ref_url) self.compute.terminate_instance(self.context, instance, [], []) @@ -5109,7 +5113,7 @@ class ComputeTestCase(BaseTestCase, self.assertIn('display_name', payload) self.assertIn('created_at', payload) self.assertIn('launched_at', payload) - image_ref_url = glance.generate_image_url(FAKE_IMAGE_REF) + image_ref_url = glance.generate_image_url(FAKE_IMAGE_REF, self.context) self.assertEqual(payload['image_ref_url'], image_ref_url) self.compute.terminate_instance(self.context, instance, [], []) diff --git a/nova/tests/unit/compute/test_compute_utils.py b/nova/tests/unit/compute/test_compute_utils.py index 2453d49cad..3f7c773903 100644 --- a/nova/tests/unit/compute/test_compute_utils.py +++ b/nova/tests/unit/compute/test_compute_utils.py @@ -449,8 +449,8 @@ class UsageInfoTestCase(test.TestCase): "Key %s not in payload" % attr) self.assertEqual(payload['image_meta'], {'md_key1': 'val1', 'md_key2': 'val2'}) - image_ref_url = "%s/images/%s" % (glance.generate_glance_url(), - uuids.fake_image_ref) + image_ref_url = "%s/images/%s" % ( + glance.generate_glance_url(self.context), uuids.fake_image_ref) self.assertEqual(payload['image_ref_url'], image_ref_url) self.compute.terminate_instance(self.context, instance, [], []) @@ -485,8 +485,8 @@ class UsageInfoTestCase(test.TestCase): self.assertIn(attr, payload, "Key %s not in payload" % attr) self.assertEqual(payload['image_meta'], {'md_key1': 'val1', 'md_key2': 'val2'}) - image_ref_url = "%s/images/%s" % (glance.generate_glance_url(), - uuids.fake_image_ref) + image_ref_url = "%s/images/%s" % ( + glance.generate_glance_url(self.context), uuids.fake_image_ref) self.assertEqual(payload['image_ref_url'], image_ref_url) def test_notify_about_instance_action(self): @@ -778,8 +778,8 @@ class UsageInfoTestCase(test.TestCase): 'audit_period_ending', 'image_meta'): self.assertIn(attr, payload, "Key %s not in payload" % attr) self.assertEqual(payload['image_meta'], {}) - image_ref_url = "%s/images/%s" % (glance.generate_glance_url(), - uuids.fake_image_ref) + image_ref_url = "%s/images/%s" % ( + glance.generate_glance_url(self.context), uuids.fake_image_ref) self.assertEqual(payload['image_ref_url'], image_ref_url) def test_notify_about_instance_usage(self): @@ -814,8 +814,8 @@ class UsageInfoTestCase(test.TestCase): self.assertEqual(payload['image_meta'], {'md_key1': 'val1', 'md_key2': 'val2'}) self.assertEqual(payload['image_name'], 'fake_name') - image_ref_url = "%s/images/%s" % (glance.generate_glance_url(), - uuids.fake_image_ref) + image_ref_url = "%s/images/%s" % ( + glance.generate_glance_url(self.context), uuids.fake_image_ref) self.assertEqual(payload['image_ref_url'], image_ref_url) self.compute.terminate_instance(self.context, instance, [], []) diff --git a/nova/tests/unit/image/test_glance.py b/nova/tests/unit/image/test_glance.py index cf86935835..32a431a2cb 100644 --- a/nova/tests/unit/image/test_glance.py +++ b/nova/tests/unit/image/test_glance.py @@ -1580,7 +1580,7 @@ class TestGlanceApiServers(test.NoDBTestCase): 'http://10.0.2.2:9294'] expected_servers = set(glance_servers) self.flags(api_servers=glance_servers, group='glance') - api_servers = glance.get_api_servers() + api_servers = glance.get_api_servers('context') # In len(expected_servers) cycles, we should get all the endpoints self.assertEqual(expected_servers, {next(api_servers) for _ in expected_servers}) @@ -1589,7 +1589,7 @@ class TestGlanceApiServers(test.NoDBTestCase): def test_get_api_servers_get_ksa_adapter(self, mock_epd): """Test get_api_servers via nova.utils.get_ksa_adapter().""" self.flags(api_servers=None, group='glance') - api_servers = glance.get_api_servers() + api_servers = glance.get_api_servers(mock.Mock()) self.assertEqual(mock_epd.return_value.catalog_url, next(api_servers)) # Still get itertools.cycle behavior self.assertEqual(mock_epd.return_value.catalog_url, next(api_servers)) @@ -1598,7 +1598,7 @@ class TestGlanceApiServers(test.NoDBTestCase): # Now test with endpoint_override - get_endpoint_data is not called. mock_epd.reset_mock() self.flags(endpoint_override='foo', group='glance') - api_servers = glance.get_api_servers() + api_servers = glance.get_api_servers(mock.Mock()) self.assertEqual('foo', next(api_servers)) self.assertEqual('foo', next(api_servers)) mock_epd.assert_not_called() diff --git a/nova/tests/unit/test_context.py b/nova/tests/unit/test_context.py index c2152898ac..2603c67bbd 100644 --- a/nova/tests/unit/test_context.py +++ b/nova/tests/unit/test_context.py @@ -97,7 +97,7 @@ class ContextTestCase(test.NoDBTestCase): service_catalog=None) self.assertEqual([], ctxt.service_catalog) - def test_service_catalog_cinder_only(self): + def test_service_catalog_filter(self): service_catalog = [ {u'type': u'compute', u'name': u'nova'}, {u'type': u's3', u'name': u's3'}, @@ -110,7 +110,8 @@ class ContextTestCase(test.NoDBTestCase): {u'type': None, u'name': u'S_withouttype'}, {u'type': u'vo', u'name': u'S_partofvolume'}] - volume_catalog = [{u'type': u'volumev3', u'name': u'cinderv3'}, + volume_catalog = [{u'type': u'image', u'name': u'glance'}, + {u'type': u'volumev3', u'name': u'cinderv3'}, {u'type': u'block-storage', u'name': u'cinder'}] ctxt = context.RequestContext('111', '222', service_catalog=service_catalog) diff --git a/nova/tests/unit/test_utils.py b/nova/tests/unit/test_utils.py index 2bda15549f..629d7a942a 100644 --- a/nova/tests/unit/test_utils.py +++ b/nova/tests/unit/test_utils.py @@ -20,6 +20,8 @@ import os.path import tempfile import eventlet +from keystoneauth1 import adapter as ks_adapter +from keystoneauth1 import exceptions as ks_exc from keystoneauth1.identity import base as ks_identity from keystoneauth1 import session as ks_session import mock @@ -1385,3 +1387,54 @@ class GetKSAAdapterTestCase(test.NoDBTestCase): self.load_adap.assert_called_once_with( utils.CONF, 'cinder', session=self.sess, auth=self.auth, min_version=None, max_version=None) + + +class GetEndpointTestCase(test.NoDBTestCase): + def setUp(self): + super(GetEndpointTestCase, self).setUp() + self.adap = mock.create_autospec(ks_adapter.Adapter, instance=True) + self.adap.endpoint_override = None + self.adap.service_type = 'stype' + self.adap.interface = ['admin', 'public'] + + def test_endpoint_override(self): + self.adap.endpoint_override = 'foo' + self.assertEqual('foo', utils.get_endpoint(self.adap)) + self.adap.get_endpoint_data.assert_not_called() + self.adap.get_endpoint.assert_not_called() + + def test_image_good(self): + self.adap.service_type = 'image' + self.adap.get_endpoint_data.return_value.catalog_url = 'url' + self.assertEqual('url', utils.get_endpoint(self.adap)) + self.adap.get_endpoint_data.assert_called_once_with() + self.adap.get_endpoint.assert_not_called() + + def test_image_bad(self): + self.adap.service_type = 'image' + self.adap.get_endpoint_data.side_effect = AttributeError + self.adap.get_endpoint.return_value = 'url' + self.assertEqual('url', utils.get_endpoint(self.adap)) + self.adap.get_endpoint_data.assert_called_once_with() + self.adap.get_endpoint.assert_called_once_with() + + def test_nonimage_good(self): + self.adap.get_endpoint.return_value = 'url' + self.assertEqual('url', utils.get_endpoint(self.adap)) + self.adap.get_endpoint_data.assert_not_called() + self.adap.get_endpoint.assert_called_once_with() + + def test_nonimage_try_interfaces(self): + self.adap.get_endpoint.side_effect = (ks_exc.EndpointNotFound, 'url') + self.assertEqual('url', utils.get_endpoint(self.adap)) + self.adap.get_endpoint_data.assert_not_called() + self.assertEqual(2, self.adap.get_endpoint.call_count) + self.assertEqual('admin', self.adap.interface) + + def test_nonimage_try_interfaces_fail(self): + self.adap.get_endpoint.side_effect = ks_exc.EndpointNotFound + self.assertRaises(ks_exc.EndpointNotFound, + utils.get_endpoint, self.adap) + self.adap.get_endpoint_data.assert_not_called() + self.assertEqual(3, self.adap.get_endpoint.call_count) + self.assertEqual('public', self.adap.interface) diff --git a/nova/utils.py b/nova/utils.py index 09665b2810..f83f5e5c77 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -33,6 +33,7 @@ import tempfile import time import eventlet +from keystoneauth1 import exceptions as ks_exc from keystoneauth1 import loading as ks_loading import netaddr from os_service_types import service_types @@ -1325,3 +1326,56 @@ def get_ksa_adapter(service_type, ksa_auth=None, ksa_session=None, return ks_loading.load_adapter_from_conf_options( CONF, confgrp, session=ksa_session, auth=ksa_auth, min_version=min_version, max_version=max_version) + + +def get_endpoint(ksa_adapter): + """Get the endpoint URL represented by a keystoneauth1 Adapter. + + This method is equivalent to what + + ksa_adapter.get_endpoint() + + should do, if it weren't for a panoply of bugs. + + :param ksa_adapter: keystoneauth1.adapter.Adapter, appropriately set up + with an endpoint_override; or service_type, interface + (list) and auth/service_catalog. + :return: String endpoint URL. + :raise EndpointNotFound: If endpoint discovery fails. + """ + # TODO(efried): This will be unnecessary once bug #1707993 is fixed. + # (At least for the non-image case, until 1707995 is fixed.) + if ksa_adapter.endpoint_override: + return ksa_adapter.endpoint_override + # TODO(efried): Remove this once bug #1707995 is fixed. + if ksa_adapter.service_type == 'image': + try: + return ksa_adapter.get_endpoint_data().catalog_url + except AttributeError: + # ksa_adapter.auth is a _ContextAuthPlugin, which doesn't have + # get_endpoint_data. Fall through to using get_endpoint(). + pass + # TODO(efried): The remainder of this method reduces to + # TODO(efried): return ksa_adapter.get_endpoint() + # TODO(efried): once bug #1709118 is fixed. + # NOTE(efried): Id9bd19cca68206fc64d23b0eaa95aa3e5b01b676 may also do the + # trick, once it's in a ksa release. + # The EndpointNotFound exception happens when _ContextAuthPlugin is in play + # because its get_endpoint() method isn't yet set up to handle interface as + # a list. (It could also happen with a real auth if the endpoint isn't + # there; but that's covered below.) + try: + return ksa_adapter.get_endpoint() + except ks_exc.EndpointNotFound: + pass + + interfaces = list(ksa_adapter.interface) + for interface in interfaces: + ksa_adapter.interface = interface + try: + return ksa_adapter.get_endpoint() + except ks_exc.EndpointNotFound: + pass + raise ks_exc.EndpointNotFound( + "Could not find requested endpoint for any of the following " + "interfaces: %s" % interfaces) diff --git a/nova/virt/xenapi/image/glance.py b/nova/virt/xenapi/image/glance.py index e77640c11f..96550a4bd6 100644 --- a/nova/virt/xenapi/image/glance.py +++ b/nova/virt/xenapi/image/glance.py @@ -35,7 +35,7 @@ LOG = logging.getLogger(__name__) class GlanceStore(object): def _call_glance_plugin(self, context, instance, session, fn, image_id, params): - glance_api_servers = glance.get_api_servers() + glance_api_servers = glance.get_api_servers(context) sr_path = vm_utils.get_sr_path(session) extra_headers = glance.generate_identity_headers(context) diff --git a/releasenotes/notes/glance-via-ksa-5646eb3d5db51c54.yaml b/releasenotes/notes/glance-via-ksa-5646eb3d5db51c54.yaml index 1039b6d719..96539aa430 100644 --- a/releasenotes/notes/glance-via-ksa-5646eb3d5db51c54.yaml +++ b/releasenotes/notes/glance-via-ksa-5646eb3d5db51c54.yaml @@ -2,9 +2,10 @@ upgrade: - | Nova now uses keystoneauth1 configuration to set up communication with the - image service. Use keystoneauth1 loading parameters for auth, Session, and + image service. Use keystoneauth1 loading parameters for Session and Adapter setup in the ``[glance]`` conf section. This includes using ``endpoint_override`` in favor of ``api_servers``. The ``[glance]api_servers`` conf option is still supported, but should only be used if you need multiple endpoints and are unable to use a load balancer - for some reason. + for some reason. However, note that no configuration is necessary with an + appropriate service catalog entry for the image service.