diff --git a/ironic/common/cinder.py b/ironic/common/cinder.py index 5b229b05d9..c2afc021cc 100644 --- a/ironic/common/cinder.py +++ b/ironic/common/cinder.py @@ -55,11 +55,10 @@ def get_client(context): if CONF.cinder.url and not CONF.cinder.endpoint_override: adapter_opts['endpoint_override'] = CONF.cinder.url - adapter = keystone.get_adapter('cinder', session=session, - auth=service_auth, **adapter_opts) # TODO(pas-ha) use versioned endpoint data to select required # cinder api version - cinder_url = adapter.get_endpoint() + cinder_url = keystone.get_endpoint('cinder', session=session, + auth=service_auth, **adapter_opts) # TODO(pas-ha) investigate possibility of passing a user context here, # similar to what neutron/glance-related code does # NOTE(pas-ha) cinderclient has both 'connect_retries' (passed to diff --git a/ironic/common/glance_service/image_service.py b/ironic/common/glance_service/image_service.py index 6c926a6691..e3d74d38cf 100644 --- a/ironic/common/glance_service/image_service.py +++ b/ironic/common/glance_service/image_service.py @@ -74,9 +74,9 @@ def check_image_service(func): # so we can pass session and auth separately, makes things easier service_auth = keystone.get_auth('glance') - adapter = keystone.get_adapter('glance', session=_GLANCE_SESSION, - auth=service_auth) - self.endpoint = adapter.get_endpoint() + self.endpoint = keystone.get_endpoint('glance', + session=_GLANCE_SESSION, + auth=service_auth) user_auth = None # NOTE(pas-ha) our ContextHook removes context.auth_token in noauth @@ -296,14 +296,14 @@ class GlanceImageService(object): endpoint_url = CONF.glance.swift_endpoint_url if not endpoint_url: swift_session = swift.get_swift_session() - adapter = keystone.get_adapter('swift', session=swift_session) - endpoint_url = adapter.get_endpoint() - - if not endpoint_url: - raise exception.MissingParameterValue(_( - 'Swift temporary URLs require a Swift endpoint URL, but it ' - 'was not found in the service catalog. ' - 'You must provide "swift_endpoint_url" as a config option.')) + try: + endpoint_url = keystone.get_endpoint('swift', + session=swift_session) + except exception.CatalogNotFound: + raise exception.MissingParameterValue(_( + 'Swift temporary URLs require a Swift endpoint URL, ' + 'but it was not found in the service catalog. You must ' + 'provide "swift_endpoint_url" as a config option.')) # Strip /v1/AUTH_%(tenant_id)s, if present endpoint_url = re.sub('/v1/AUTH_[^/]+/?$', '', endpoint_url) diff --git a/ironic/common/keystone.py b/ironic/common/keystone.py index ba167a1b1c..c310369447 100644 --- a/ironic/common/keystone.py +++ b/ironic/common/keystone.py @@ -103,6 +103,25 @@ def get_adapter(group, **adapter_kwargs): **adapter_kwargs) +def get_endpoint(group, **adapter_kwargs): + """Get an endpoint from an adapter. + + The adapter_kwargs will be passed directly to keystoneauth1 Adapter + and will override the values loaded from config. + Consult keystoneauth1 docs for available adapter options. + + :param group: name of the config section to load adapter options from + :raises: CatalogNotFound if the endpoint is not found + """ + result = get_adapter(group, **adapter_kwargs).get_endpoint() + if not result: + service_type = adapter_kwargs.get('service_type', 'baremetal') + endpoint_type = adapter_kwargs.get('endpoint_type', 'internal') + raise exception.CatalogNotFound( + service_type=service_type, endpoint_type=endpoint_type) + return result + + def get_service_auth(context, endpoint, service_auth): """Create auth plugin wrapping both user and service auth. diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index b2834c90c4..f74e80440c 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -76,9 +76,8 @@ def get_client(token=None, context=None): else: if CONF.neutron.url and not CONF.neutron.endpoint_override: adapter_params['endpoint_override'] = CONF.neutron.url - adapter = keystone.get_adapter('neutron', session=session, - auth=service_auth, **adapter_params) - endpoint = adapter.get_endpoint() + endpoint = keystone.get_endpoint('neutron', session=session, + auth=service_auth, **adapter_params) user_auth = None if CONF.neutron.auth_type != 'none' and context.auth_token: diff --git a/ironic/common/swift.py b/ironic/common/swift.py index d485ff45cc..b1f5dec34b 100644 --- a/ironic/common/swift.py +++ b/ironic/common/swift.py @@ -63,8 +63,8 @@ class SwiftAPI(object): # TODO(pas-ha) pass the context here and use token from context # with service auth params['session'] = session = get_swift_session() - adapter = keystone.get_adapter('swift', session=session) - params['os_options'] = {'object_storage_url': adapter.get_endpoint()} + endpoint = keystone.get_endpoint('swift', session=session) + params['os_options'] = {'object_storage_url': endpoint} # deconstruct back session-related options params['timeout'] = session.timeout if session.verify is False: diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 64e21f5615..52e0c92794 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -108,9 +108,8 @@ def get_ironic_api_url(): # NOTE(pas-ha) if both set, the new options win if CONF.conductor.api_url and not CONF.service_catalog.endpoint_override: adapter_opts['endpoint_override'] = CONF.conductor.api_url - adapter = keystone.get_adapter('service_catalog', **adapter_opts) try: - ironic_api = adapter.get_endpoint() + ironic_api = keystone.get_endpoint('service_catalog', **adapter_opts) except (exception.KeystoneFailure, exception.CatalogNotFound, exception.KeystoneUnauthorized) as e: diff --git a/ironic/drivers/modules/inspector.py b/ironic/drivers/modules/inspector.py index 00749bd2ce..658d973ab8 100644 --- a/ironic/drivers/modules/inspector.py +++ b/ironic/drivers/modules/inspector.py @@ -56,9 +56,8 @@ def _get_client(context): adapter_params = {} if CONF.inspector.service_url and not CONF.inspector.endpoint_override: adapter_params['endpoint_override'] = CONF.inspector.service_url - adapter = keystone.get_adapter('inspector', session=session, - **adapter_params) - inspector_url = adapter.get_endpoint() + inspector_url = keystone.get_endpoint('inspector', session=session, + **adapter_params) # TODO(pas-ha) investigate possibility of passing user context here, # similar to what neutron/glance-related code does # NOTE(pas-ha) ironic-inspector-client has no Adaper-based diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 1c65844efa..e166e14015 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -1315,6 +1315,17 @@ class OtherFunctionTestCase(db_base.DbTestCase): self.assertRaises(exception.InvalidParameterValue, utils.get_ironic_api_url) + @mock.patch('ironic.common.keystone.get_auth') + @mock.patch.object(utils, '_get_ironic_session') + @mock.patch('ironic.common.keystone.get_adapter') + def test_get_ironic_api_url_none(self, mock_ka, mock_ks, mock_auth): + mock_sess = mock.Mock() + mock_ks.return_value = mock_sess + mock_ka.return_value.get_endpoint.return_value = None + self.config(api_url=None, group='conductor') + self.assertRaises(exception.InvalidParameterValue, + utils.get_ironic_api_url) + class GetSingleNicTestCase(db_base.DbTestCase): diff --git a/releasenotes/notes/api-none-cdb95e58b69a5c50.yaml b/releasenotes/notes/api-none-cdb95e58b69a5c50.yaml new file mode 100644 index 0000000000..4944293854 --- /dev/null +++ b/releasenotes/notes/api-none-cdb95e58b69a5c50.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixes a confusing ``AttributeError`` if an adapter returns ``None`` for + the bare metal API. + - | + Prevents the adapter configuration options from getting ignored if + a matching endpoint cannot be found. An error is now raised.