Merge "Add check on get_endpoint returning None"

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

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,
+        self.endpoint = keystone.get_endpoint('glance',
+                                              session=_GLANCE_SESSION,
                                               auth=service_auth)
-        self.endpoint = adapter.get_endpoint()
 
         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:
+            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.'))
+                    '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)

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.
 

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,
+    endpoint = keystone.get_endpoint('neutron', session=session,
                                      auth=service_auth, **adapter_params)
-    endpoint = adapter.get_endpoint()
 
     user_auth = None
     if CONF.neutron.auth_type != 'none' and context.auth_token:

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:

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:

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,
+    inspector_url = keystone.get_endpoint('inspector', session=session,
                                           **adapter_params)
-    inspector_url = adapter.get_endpoint()
     # 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

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):
 

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.