Merge "Introduce keystoneauth adapters for clients"

devstack/lib/ironic

@@ -1059,9 +1059,13 @@ function configure_ironic_api {
     cp -p $IRONIC_DIR/etc/ironic/policy.json $IRONIC_POLICY_JSON
 }
 
-function configure_auth_for {
+# configure_client_for() - is used by configure_ironic_conductor.
+# Sets options to instantiate clients for other services
+# single argument - config section to fill
+function configure_client_for {
     local service_config_section
     service_config_section=$1
+    # keystoneauth auth plugin options
     iniset $IRONIC_CONF_FILE $service_config_section auth_type password
     iniset $IRONIC_CONF_FILE $service_config_section auth_url $KEYSTONE_SERVICE_URI
     iniset $IRONIC_CONF_FILE $service_config_section username ironic
@@ -1069,24 +1073,39 @@ function configure_auth_for {
     iniset $IRONIC_CONF_FILE $service_config_section project_name $SERVICE_PROJECT_NAME
     iniset $IRONIC_CONF_FILE $service_config_section user_domain_id default
     iniset $IRONIC_CONF_FILE $service_config_section project_domain_id default
+    # keystoneauth session options
     iniset $IRONIC_CONF_FILE $service_config_section cafile $SSL_BUNDLE_FILE
 }
 
+# TODO(pas-ha) this function is for transition period only,
+# after all clients are moved to use keystoneauth adapters, it will be merged
+# into configure_client_for function
+function configure_adapter_for {
+    local service_config_section
+    service_config_section=$1
+    # keystoneauth adapter options
+    # NOTE(pas-ha) relying on defaults for valid_interfaces being "internal,public" in ironic
+    iniset $IRONIC_CONF_FILE $service_config_section region_name $REGION_NAME
+}
+
 # configure_ironic_conductor() - Is used by configure_ironic().
 # Sets conductor specific settings.
 function configure_ironic_conductor {
 
-    # set keystone region for all services
+    # NOTE(pas-ha) service_catalog section is used to discover
-    iniset $IRONIC_CONF_FILE keystone region_name $REGION_NAME
+    # ironic API endpoint from keystone catalog
+    local client_sections="neutron swift glance inspector cinder service_catalog"
+    for conf_section in $client_sections; do
+        configure_client_for $conf_section
+    done
 
-    # set keystone auth plugin options for services
+    # TODO(pas-ha) this block is for transition period only,
-    configure_auth_for neutron
+    # after all clients are moved to use keystoneauth adapters,
-    configure_auth_for swift
+    # it will be deleted
-    configure_auth_for glance
+    local sections_with_adapter="service_catalog"
-    configure_auth_for inspector
+    for conf_section in $sections_with_adapter; do
-    configure_auth_for cinder
+        configure_adapter_for $conf_section
-    # this one is needed for lookup of Ironic API endpoint via Keystone
+    done
-    configure_auth_for service_catalog
 
     cp $IRONIC_DIR/etc/ironic/rootwrap.conf $IRONIC_ROOTWRAP_CONF
     cp -r $IRONIC_DIR/etc/ironic/rootwrap.d $IRONIC_CONF_DIR
@@ -1237,8 +1256,6 @@ function create_ironic_accounts {
         get_or_create_service "ironic" "baremetal" "Ironic baremetal provisioning service"
         get_or_create_endpoint "baremetal" \
             "$REGION_NAME" \
-            "$IRONIC_SERVICE_PROTOCOL://$IRONIC_HOSTPORT" \
-            "$IRONIC_SERVICE_PROTOCOL://$IRONIC_HOSTPORT" \
             "$IRONIC_SERVICE_PROTOCOL://$IRONIC_HOSTPORT"
 
         # Create ironic service user

etc/ironic/ironic.conf.sample

@@ -1026,10 +1026,15 @@
 # Seconds between conductor heart beats. (integer value)
 #heartbeat_interval = 10
 
-# URL of Ironic API service. If not set ironic can get the
+# DEPRECATED: URL of Ironic API service. If not set ironic can
-# current value from the keystone service catalog. If set, the
+# get the current value from the keystone service catalog. If
-# value must start with either http:// or https://. (uri
+# set, the value must start with either http:// or https://.
-# value)
+# (uri value)
+# This option is deprecated for removal.
+# Its value may be silently ignored in the future.
+# Reason: Use [service_catalog]endpoint_override option
+# instead if required to use a specific ironic api address,
+# for example in noauth mode.
 #api_url = <None>
 
 # Maximum time (in seconds) since the last check-in of a
@@ -3545,12 +3550,28 @@
 # Domain name to scope to (string value)
 #domain_name = <None>
 
+# Always use this endpoint URL for requests for this client.
+# (string value)
+#endpoint_override = <None>
+
 # Verify HTTPS connections. (boolean value)
 #insecure = false
 
 # PEM encoded client certificate key file (string value)
 #keyfile = <None>
 
+# The maximum major version of a given API, intended to be
+# used as the upper bound of a range with min_version.
+# Mutually exclusive with version. (string value)
+#max_version = <None>
+
+# The minimum major version of a given API, intended to be
+# used as the lower bound of a range with max_version.
+# Mutually exclusive with version. If min_version is given
+# with no max_version it is as if max version is "latest".
+# (string value)
+#min_version = <None>
+
 # User's password (string value)
 #password = <None>
 
@@ -3568,6 +3589,18 @@
 # Deprecated group/name - [service_catalog]/tenant_name
 #project_name = <None>
 
+# The default region_name for endpoint URL discovery. (string
+# value)
+#region_name = <None>
+
+# The default service_name for endpoint URL discovery. (string
+# value)
+#service_name = <None>
+
+# The default service_type for endpoint URL discovery. (string
+# value)
+#service_type = baremetal
+
 # Tenant ID (string value)
 #tenant_id = <None>
 
@@ -3593,6 +3626,15 @@
 # Deprecated group/name - [service_catalog]/user_name
 #username = <None>
 
+# List of interfaces, in order of preference, for endpoint
+# URL. (list value)
+#valid_interfaces = internal,public
+
+# Minimum Major API version within a given Major API version
+# for endpoint URL discovery. Mutually exclusive with
+# min_version and max_version (string value)
+#version = <None>
+
 
 [snmp]
 

ironic/common/keystone.py

@@ -20,6 +20,7 @@ from oslo_log import log as logging
 import six
 
 from ironic.common import exception
+from ironic.conf import auth as auth_conf
 from ironic.conf import CONF
 
 
@@ -86,7 +87,22 @@ def get_auth(group, **auth_kwargs):
     return auth
 
 
-# NOTE(pas-ha) Used by neutronclient and resolving ironic API only
+@ks_exceptions
+def get_adapter(group, **adapter_kwargs):
+    """Loads adapter from options in a configuration file section.
+
+    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
+
+    """
+    return kaloading.load_adapter_from_conf_options(CONF, group,
+                                                    **adapter_kwargs)
+
+
+# NOTE(pas-ha) Used by neutronclient and glanceclient only
 # FIXME(pas-ha) remove this while moving to kesytoneauth adapters
 @ks_exceptions
 def get_service_url(session, **kwargs):
@@ -103,7 +119,5 @@ def get_service_url(session, **kwargs):
 
     if 'interface' in kwargs:
         return session.get_endpoint(**kwargs)
-    try:
+    return session.get_endpoint(interface=auth_conf.DEFAULT_VALID_INTERFACES,
-        return session.get_endpoint(interface='internal', **kwargs)
+                                **kwargs)
-    except kaexception.EndpointNotFound:
-        return session.get_endpoint(interface='public', **kwargs)

ironic/conf/auth.py

@@ -15,13 +15,16 @@
 import copy
 
 from keystoneauth1 import loading as kaloading
+from oslo_config import cfg
 from oslo_log import log
 
 
 LOG = log.getLogger(__name__)
 
+DEFAULT_VALID_INTERFACES = ['internal', 'public']
 
-def register_auth_opts(conf, group):
+
+def register_auth_opts(conf, group, service_type=None):
     """Register session- and auth-related options
 
     Registers only basic auth options shared by all auth plugins.
@@ -29,9 +32,14 @@ def register_auth_opts(conf, group):
     """
     kaloading.register_session_conf_options(conf, group)
     kaloading.register_auth_conf_options(conf, group)
+    if service_type:
+        kaloading.register_adapter_conf_options(conf, group)
+        conf.set_default('valid_interfaces', DEFAULT_VALID_INTERFACES,
+                         group=group)
+        conf.set_default('service_type', service_type, group=group)
 
 
-def add_auth_opts(options):
+def add_auth_opts(options, service_type=None):
     """Add auth options to sample config
 
     As these are dynamically registered at runtime,
@@ -55,5 +63,12 @@ def add_auth_opts(options):
         plugin = kaloading.get_plugin_loader(name)
         add_options(opts, kaloading.get_auth_plugin_conf_options(plugin))
     add_options(opts, kaloading.get_session_conf_options())
+    if service_type:
+        adapter_opts = kaloading.get_adapter_conf_options(
+            include_deprecated=False)
+        # adding defaults for valid interfaces
+        cfg.set_defaults(adapter_opts, service_type=service_type,
+                         valid_interfaces=DEFAULT_VALID_INTERFACES)
+        add_options(opts, adapter_opts)
     opts.sort(key=lambda x: x.name)
     return opts

ironic/conf/conductor.py

@@ -30,6 +30,11 @@ opts = [
                help=_('Seconds between conductor heart beats.')),
     cfg.URIOpt('api_url',
                schemes=('http', 'https'),
+               deprecated_for_removal=True,
+               deprecated_reason=_("Use [service_catalog]endpoint_override "
+                                   "option instead if required to use "
+                                   "a specific ironic api address, "
+                                   "for example in noauth mode."),
                help=_('URL of Ironic API service. If not set ironic can '
                       'get the current value from the keystone service '
                       'catalog. If set, the value must start with either '

ironic/conf/service_catalog.py

@@ -26,8 +26,9 @@ SERVICE_CATALOG_GROUP = cfg.OptGroup(
 
 
 def register_opts(conf):
-    auth.register_auth_opts(conf, SERVICE_CATALOG_GROUP.name)
+    auth.register_auth_opts(conf, SERVICE_CATALOG_GROUP.name,
+                            service_type='baremetal')
 
 
 def list_opts():
-    return auth.add_auth_opts([])
+    return auth.add_auth_opts([], service_type='baremetal')

ironic/drivers/modules/deploy_utils.py

@@ -77,9 +77,7 @@ _IRONIC_SESSION = None
 def _get_ironic_session():
     global _IRONIC_SESSION
     if not _IRONIC_SESSION:
-        auth = keystone.get_auth('service_catalog')
+        _IRONIC_SESSION = keystone.get_session('service_catalog')
-        _IRONIC_SESSION = keystone.get_session('service_catalog',
-                                               auth=auth)
     return _IRONIC_SESSION
 
 
@@ -94,11 +92,21 @@ def get_ironic_api_url():
 
     either from config of from Keystone catalog.
     """
-    ironic_api = CONF.conductor.api_url
+    adapter_opts = {'session': _get_ironic_session()}
-    if not ironic_api:
+    # NOTE(pas-ha) force 'none' auth plugin for noauth mode
+    if CONF.auth_strategy != 'keystone':
+        CONF.set_override('auth_type', 'none', group='service_catalog')
+    adapter_opts['auth'] = keystone.get_auth('service_catalog')
+
+    # TODO(pas-ha) remove in Rocky
+    # 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
+    if CONF.keystone.region_name and not CONF.service_catalog.region_name:
+        adapter_opts['region_name'] = CONF.keystone.region_name
+    adapter = keystone.get_adapter('service_catalog', **adapter_opts)
     try:
-            ironic_session = _get_ironic_session()
+        ironic_api = adapter.get_endpoint()
-            ironic_api = keystone.get_service_url(ironic_session)
     except (exception.KeystoneFailure,
             exception.CatalogNotFound,
             exception.KeystoneUnauthorized) as e:

ironic/tests/unit/common/test_keystone.py

@@ -12,7 +12,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-from keystoneauth1 import exceptions as ksexception
 from keystoneauth1 import loading as kaloading
 import mock
 from oslo_config import cfg
@@ -32,7 +31,8 @@ class KeystoneTestCase(base.TestCase):
                     group='keystone')
         self.test_group = 'test_group'
         self.cfg_fixture.conf.register_group(cfg.OptGroup(self.test_group))
-        ironic_auth.register_auth_opts(self.cfg_fixture.conf, self.test_group)
+        ironic_auth.register_auth_opts(self.cfg_fixture.conf, self.test_group,
+                                       service_type='vikings')
         self.config(auth_type='password',
                     group=self.test_group)
         # NOTE(pas-ha) this is due to auth_plugin options
@@ -76,20 +76,19 @@ class KeystoneTestCase(base.TestCase):
         self.assertEqual('spam', keystone.get_service_url(session, **params))
         session.get_endpoint.assert_called_once_with(**params)
 
-    def test_get_service_url_internal(self):
+    def test_get_service_url(self):
         session = mock.Mock()
         session.get_endpoint.return_value = 'spam'
         params = {'ham': 'eggs'}
         self.assertEqual('spam', keystone.get_service_url(session, **params))
-        session.get_endpoint.assert_called_once_with(interface='internal',
+        session.get_endpoint.assert_called_once_with(
-                                                     **params)
+            interface=['internal', 'public'], **params)
 
-    def test_get_service_url_internal_fail(self):
+    def test_get_adapter_from_config(self):
-        session = mock.Mock()
+        self.config(valid_interfaces=['internal', 'public'],
-        session.get_endpoint.side_effect = [ksexception.EndpointNotFound(),
+                    group=self.test_group)
-                                            'spam']
+        session = keystone.get_session(self.test_group)
-        params = {'ham': 'eggs'}
+        adapter = keystone.get_adapter(self.test_group, session=session,
-        self.assertEqual('spam', keystone.get_service_url(session, **params))
+                                       interface='admin')
-        session.get_endpoint.assert_has_calls([
+        self.assertEqual('admin', adapter.interface)
-            mock.call(interface='internal', **params),
+        self.assertEqual(session, adapter.session)
-            mock.call(interface='public', **params)])

ironic/tests/unit/drivers/modules/test_deploy_utils.py

@@ -1214,38 +1214,42 @@ class OtherFunctionTestCase(db_base.DbTestCase):
         mock_clean_up_caches.assert_called_once_with(None, 'master_dir',
                                                      [('uuid', 'path')])
 
+    @mock.patch('ironic.common.keystone.get_auth')
     @mock.patch.object(utils, '_get_ironic_session')
-    @mock.patch('ironic.common.keystone.get_service_url')
+    def test_get_ironic_api_url_from_config(self, mock_ks, mock_auth):
-    def test_get_ironic_api_url_from_config(self, mock_get_url, mock_ks):
         mock_sess = mock.Mock()
         mock_ks.return_value = mock_sess
         fake_api_url = 'http://foo/'
-        mock_get_url.side_effect = exception.KeystoneFailure
         self.config(api_url=fake_api_url, group='conductor')
-        url = utils.get_ironic_api_url()
         # also checking for stripped trailing slash
-        self.assertEqual(fake_api_url[:-1], url)
+        self.assertEqual(fake_api_url[:-1], utils.get_ironic_api_url())
-        self.assertFalse(mock_get_url.called)
 
+    @mock.patch('ironic.common.keystone.get_auth')
     @mock.patch.object(utils, '_get_ironic_session')
-    @mock.patch('ironic.common.keystone.get_service_url')
+    @mock.patch('ironic.common.keystone.get_adapter')
-    def test_get_ironic_api_url_from_keystone(self, mock_get_url, mock_ks):
+    def test_get_ironic_api_url_from_keystone(self, mock_ka, mock_ks,
+                                              mock_auth):
         mock_sess = mock.Mock()
         mock_ks.return_value = mock_sess
         fake_api_url = 'http://foo/'
-        mock_get_url.return_value = fake_api_url
+        mock_ka.return_value.get_endpoint.return_value = fake_api_url
+        # NOTE(pas-ha) endpoint_override is None by default
         self.config(api_url=None, group='conductor')
         url = utils.get_ironic_api_url()
         # also checking for stripped trailing slash
         self.assertEqual(fake_api_url[:-1], url)
-        mock_get_url.assert_called_with(mock_sess)
+        mock_ka.assert_called_with('service_catalog', session=mock_sess,
+                                   auth=mock_auth.return_value)
+        mock_ka.return_value.get_endpoint.assert_called_once_with()
 
+    @mock.patch('ironic.common.keystone.get_auth')
     @mock.patch.object(utils, '_get_ironic_session')
-    @mock.patch('ironic.common.keystone.get_service_url')
+    @mock.patch('ironic.common.keystone.get_adapter')
-    def test_get_ironic_api_url_fail(self, mock_get_url, mock_ks):
+    def test_get_ironic_api_url_fail(self, mock_ka, mock_ks, mock_auth):
         mock_sess = mock.Mock()
         mock_ks.return_value = mock_sess
-        mock_get_url.side_effect = exception.KeystoneFailure()
+        mock_ka.return_value.get_endpoint.side_effect = (
+            exception.KeystoneFailure())
         self.config(api_url=None, group='conductor')
         self.assertRaises(exception.InvalidParameterValue,
                           utils.get_ironic_api_url)

ironic/tests/unit/drivers/modules/test_iscsi_deploy.py

@@ -521,7 +521,9 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase):
                               iscsi_deploy.validate, task)
         mock_get_url.assert_called_once_with()
 
-    def test_validate_invalid_root_device_hints(self):
+    @mock.patch('ironic.drivers.modules.deploy_utils.get_ironic_api_url')
+    def test_validate_invalid_root_device_hints(self, mock_get_url):
+        mock_get_url.return_value = 'http://spam.ham/baremetal'
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
             task.node.properties['root_device'] = {'size': 'not-int'}

releasenotes/notes/keystoneauth-adapter-opts-ca4f68f568e6cf6f.yaml

@@ -0,0 +1,41 @@
+---
+upgrade:
+  - |
+    To facilitate automatic discovery of services from the keystone catalog,
+    the configuration file sections for service clients may include these
+    configuration options: ``service_type``, ``service_name``,
+    ``valid_interfaces``, ``region_name`` and other keystoneauth Adaper-related
+    options.
+
+    These options together must uniquely specify an endpoint for a service
+    registered in the keystone catalog.
+    Consult the ``keystoneauth`` library documentation for full list of
+    available options, their meaning and possible values.
+
+    Default values for ``service_type`` are set by ironic to sane defaults
+    based on required services and their entries in ``service-types-authority``.
+
+    The ``valid_interfaces`` option defaults to ``['internal', 'public']``.
+
+    The ``region_name`` option defaults to ``None`` and must be explicitly set
+    for multi-regional setup for endpoint discovery to succeed.
+
+    The configuration file sections where these new options are available are:
+
+    - service_catalog
+
+features:
+  - |
+    Options for service endpoint discovery can now be set per-service in
+    appropriate service configuration file sections:
+
+    - ``[service_catalog]`` for baremetal API discovery
+
+deprecations:
+  - |
+    Configuration option ``[conductor]api_url`` is deprecated
+    and will be ignored in the Rocky release.
+    Instead, use ``[service_catalog]endpoint_override`` configuration option
+    to set specific ironic API address if automatic discovery of ironic API
+    endpoint from keystone catalog is not desired.
+    This new option defaults to ``None`` and must be set explicitly if needed.