diff --git a/devstack/lib/ironic b/devstack/lib/ironic index e044891304..ee8119624b 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -1113,7 +1113,7 @@ function configure_ironic_conductor { # TODO(pas-ha) this block is for transition period only, # after all clients are moved to use keystoneauth adapters, # it will be deleted - local sections_with_adapter="service_catalog glance cinder" + local sections_with_adapter="service_catalog glance cinder inspector" for conf_section in $sections_with_adapter; do configure_adapter_for $conf_section done diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 99f1fed87f..a384d7ec90 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -1895,12 +1895,28 @@ # fake_inspector driver. (boolean value) #enabled = false +# Always use this endpoint URL for requests for this client. +# (string value) +#endpoint_override = + # Verify HTTPS connections. (boolean value) #insecure = false # PEM encoded client certificate key file (string value) #keyfile = +# 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 = + +# 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 = + # User's password (string value) #password = @@ -1918,8 +1934,24 @@ # Deprecated group/name - [inspector]/tenant_name #project_name = -# ironic-inspector HTTP endpoint. If this is not set, the -# service catalog will be used. (string value) +# The default region_name for endpoint URL discovery. (string +# value) +#region_name = + +# The default service_name for endpoint URL discovery. (string +# value) +#service_name = + +# The default service_type for endpoint URL discovery. (string +# value) +#service_type = baremetal-introspection + +# DEPRECATED: ironic-inspector HTTP endpoint. If this is not +# set, the service catalog will be used. (string value) +# This option is deprecated for removal. +# Its value may be silently ignored in the future. +# Reason: Use [inspector]/endpoint_override option instead to +# set a specific ironic-inspector API URL to connect to. #service_url = # period (in seconds) to check status of nodes on inspection @@ -1951,6 +1983,15 @@ # Deprecated group/name - [inspector]/user_name #username = +# 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 = + [ipmi] diff --git a/ironic/conf/inspector.py b/ironic/conf/inspector.py index 336f53d743..3f56ac999c 100644 --- a/ironic/conf/inspector.py +++ b/ironic/conf/inspector.py @@ -23,6 +23,10 @@ opts = [ 'This option does not affect new-style dynamic drivers ' 'and the fake_inspector driver.')), cfg.StrOpt('service_url', + deprecated_for_removal=True, + deprecated_reason=_("Use [inspector]/endpoint_override option " + "instead to set a specific " + "ironic-inspector API URL to connect to."), help=_('ironic-inspector HTTP endpoint. If this is not set, ' 'the service catalog will be used.')), cfg.IntOpt('status_check_period', default=60, @@ -33,8 +37,9 @@ opts = [ def register_opts(conf): conf.register_opts(opts, group='inspector') - auth.register_auth_opts(conf, 'inspector') + auth.register_auth_opts(conf, 'inspector', + service_type='baremetal-introspection') def list_opts(): - return auth.add_auth_opts(opts) + return auth.add_auth_opts(opts, service_type='baremetal-introspection') diff --git a/ironic/drivers/modules/inspector.py b/ironic/drivers/modules/inspector.py index 4edfd956ba..2642a83a66 100644 --- a/ironic/drivers/modules/inspector.py +++ b/ironic/drivers/modules/inspector.py @@ -39,22 +39,36 @@ INSPECTOR_API_VERSION = (1, 0) _INSPECTOR_SESSION = None -def _get_inspector_session(): - if CONF.auth_strategy != 'keystone': - return - +def _get_inspector_session(**kwargs): global _INSPECTOR_SESSION if not _INSPECTOR_SESSION: - _INSPECTOR_SESSION = keystone.get_session( - 'inspector', auth=keystone.get_auth('inspector')) + _INSPECTOR_SESSION = keystone.get_session('inspector', **kwargs) return _INSPECTOR_SESSION -def _get_client(): +def _get_client(context): """Helper to get inspector client instance.""" + # NOTE(pas-ha) remove in Rocky + if CONF.auth_strategy != 'keystone': + CONF.set_override('auth_type', 'none', group='inspector') + service_auth = keystone.get_auth('inspector') + session = _get_inspector_session(auth=service_auth) + 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() + # 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 + # SessionClient, so we'll resolve inspector API form adapter loaded + # form config options + # TODO(pas-ha) rewrite when inspectorclient is based on ksa Adapter, + # also add global_request_id to the call return client.ClientV1(api_version=INSPECTOR_API_VERSION, - inspector_url=CONF.inspector.service_url, - session=_get_inspector_session()) + session=session, + inspector_url=inspector_url) class Inspector(base.InspectInterface): @@ -137,7 +151,7 @@ class Inspector(base.InspectInterface): def _start_inspection(node_uuid, context): """Call to inspector to start inspection.""" try: - _get_client().introspect(node_uuid) + _get_client(context).introspect(node_uuid) except Exception as exc: LOG.exception('Exception during contacting ironic-inspector ' 'for inspection of node %(node)s: %(err)s', @@ -166,7 +180,7 @@ def _check_status(task): task.node.uuid) try: - status = _get_client().get_status(node.uuid) + status = _get_client(task.context).get_status(node.uuid) except Exception: # NOTE(dtantsur): get_status should not normally raise # let's assume it's a transient failure and retry later diff --git a/ironic/tests/unit/drivers/modules/test_inspector.py b/ironic/tests/unit/drivers/modules/test_inspector.py index 73b1abcb07..010d04b95d 100644 --- a/ironic/tests/unit/drivers/modules/test_inspector.py +++ b/ironic/tests/unit/drivers/modules/test_inspector.py @@ -14,6 +14,7 @@ import eventlet import ironic_inspector_client as client import mock +from ironic.common import context from ironic.common import driver_factory from ironic.common import exception from ironic.common import states @@ -52,6 +53,63 @@ class DisabledTestCase(db_base.DbTestCase): inspector.Inspector() +@mock.patch('ironic.common.keystone.get_adapter', autospec=True) +@mock.patch('ironic.common.keystone.get_service_auth', autospec=True, + return_value=mock.sentinel.sauth) +@mock.patch('ironic.common.keystone.get_auth', autospec=True, + return_value=mock.sentinel.auth) +@mock.patch('ironic.common.keystone.get_session', autospec=True, + return_value=mock.sentinel.session) +@mock.patch.object(client.ClientV1, '__init__', return_value=None) +class GetClientTestCase(db_base.DbTestCase): + + def setUp(self): + super(GetClientTestCase, self).setUp() + # NOTE(pas-ha) force-reset global inspector session object + inspector._INSPECTOR_SESSION = None + self.api_version = (1, 0) + self.context = context.RequestContext(global_request_id='global') + + def test__get_client(self, mock_init, mock_session, mock_auth, + mock_sauth, mock_adapter): + mock_adapter.return_value.get_endpoint.return_value = 'inspector_url' + inspector._get_client(self.context) + mock_init.assert_called_once_with( + session=mock.sentinel.session, + api_version=self.api_version, + inspector_url='inspector_url') + self.assertEqual(0, mock_sauth.call_count) + self.assertEqual(1, mock_session.call_count) + + def test__get_client_standalone(self, mock_init, mock_session, mock_auth, + mock_sauth, mock_adapter): + self.config(auth_strategy='noauth') + mock_adapter.return_value.get_endpoint.return_value = 'inspector_url' + inspector._get_client(self.context) + self.assertEqual('none', inspector.CONF.inspector.auth_type) + mock_init.assert_called_once_with( + session=mock.sentinel.session, + api_version=self.api_version, + inspector_url='inspector_url') + self.assertEqual(0, mock_sauth.call_count) + self.assertEqual(1, mock_session.call_count) + + def test__get_client_url(self, mock_init, mock_session, mock_auth, + mock_sauth, mock_adapter): + self.config(service_url='meow', group='inspector') + mock_adapter.return_value.get_endpoint.return_value = 'meow' + inspector._get_client(self.context) + mock_init.assert_called_once_with( + session=mock.sentinel.session, + api_version=self.api_version, + inspector_url='meow') + mock_adapter.assert_called_once_with('inspector', + session=mock.sentinel.session, + endpoint_override='meow') + self.assertEqual(0, mock_sauth.call_count) + self.assertEqual(1, mock_session.call_count) + + class BaseTestCase(db_base.DbTestCase): def setUp(self): super(BaseTestCase, self).setUp() @@ -65,8 +123,6 @@ class BaseTestCase(db_base.DbTestCase): self.task.node = self.node self.task.driver = self.driver self.api_version = (1, 0) - # NOTE(pas-ha) force-reset global inspector session object - inspector._INSPECTOR_SESSION = None class CommonFunctionsTestCase(BaseTestCase): @@ -89,35 +145,18 @@ class CommonFunctionsTestCase(BaseTestCase): self.assertTrue(warn_mock.called) -@mock.patch('ironic.common.keystone.get_auth', - lambda _section, **kwargs: mock.sentinel.auth) -@mock.patch('ironic.common.keystone.get_session', - lambda _section, **kwargs: mock.sentinel.session) @mock.patch.object(eventlet, 'spawn_n', lambda f, *a, **kw: f(*a, **kw)) -@mock.patch.object(client.ClientV1, 'introspect') -@mock.patch.object(client.ClientV1, '__init__', return_value=None) +@mock.patch('ironic.drivers.modules.inspector._get_client', autospec=True) class InspectHardwareTestCase(BaseTestCase): - def test_ok(self, mock_init, mock_introspect): + def test_ok(self, mock_client): + mock_introspect = mock_client.return_value.introspect self.assertEqual(states.INSPECTING, self.driver.inspect.inspect_hardware(self.task)) - mock_init.assert_called_once_with( - session=mock.sentinel.session, - api_version=self.api_version, - inspector_url=None) - mock_introspect.assert_called_once_with(self.node.uuid) - - def test_url(self, mock_init, mock_introspect): - self.config(service_url='meow', group='inspector') - self.assertEqual(states.INSPECTING, - self.driver.inspect.inspect_hardware(self.task)) - mock_init.assert_called_once_with( - session=mock.sentinel.session, - api_version=self.api_version, - inspector_url='meow') mock_introspect.assert_called_once_with(self.node.uuid) @mock.patch.object(task_manager, 'acquire', autospec=True) - def test_error(self, mock_acquire, mock_init, mock_introspect): + def test_error(self, mock_acquire, mock_client): + mock_introspect = mock_client.return_value.introspect mock_introspect.side_effect = RuntimeError('boom') self.driver.inspect.inspect_hardware(self.task) mock_introspect.assert_called_once_with(self.node.uuid) @@ -125,97 +164,50 @@ class InspectHardwareTestCase(BaseTestCase): self.assertIn('boom', task.node.last_error) task.process_event.assert_called_once_with('fail') - def test_is_standalone(self, mock_init, mock_introspect): - self.config(auth_strategy='noauth') - self.assertEqual(states.INSPECTING, - self.driver.inspect.inspect_hardware(self.task)) - mock_init.assert_called_once_with( - session=None, - api_version=self.api_version, - inspector_url=None) - mock_introspect.assert_called_once_with(self.node.uuid) - -@mock.patch('ironic.common.keystone.get_auth', - lambda _section, **kwargs: mock.sentinel.auth) -@mock.patch('ironic.common.keystone.get_session', - lambda _section, **kwargs: mock.sentinel.session) -@mock.patch.object(client.ClientV1, 'get_status') -@mock.patch.object(client.ClientV1, '__init__', return_value=None) +@mock.patch('ironic.drivers.modules.inspector._get_client', autospec=True) class CheckStatusTestCase(BaseTestCase): def setUp(self): super(CheckStatusTestCase, self).setUp() self.node.provision_state = states.INSPECTING - def test_not_inspecting(self, mock_init, mock_get): + def test_not_inspecting(self, mock_client): + mock_get = mock_client.return_value.get_status self.node.provision_state = states.MANAGEABLE inspector._check_status(self.task) self.assertFalse(mock_get.called) - def test_not_inspector(self, mock_init, mock_get): + def test_not_inspector(self, mock_client): + mock_get = mock_client.return_value.get_status self.task.driver.inspect = object() inspector._check_status(self.task) self.assertFalse(mock_get.called) - def test_not_finished(self, mock_init, mock_get): + def test_not_finished(self, mock_client): + mock_get = mock_client.return_value.get_status mock_get.return_value = {} inspector._check_status(self.task) - mock_init.assert_called_once_with( - session=mock.sentinel.session, - api_version=self.api_version, - inspector_url=None) mock_get.assert_called_once_with(self.node.uuid) self.assertFalse(self.task.process_event.called) - def test_exception_ignored(self, mock_init, mock_get): + def test_exception_ignored(self, mock_client): + mock_get = mock_client.return_value.get_status mock_get.side_effect = RuntimeError('boom') inspector._check_status(self.task) - mock_init.assert_called_once_with( - session=mock.sentinel.session, - api_version=self.api_version, - inspector_url=None) mock_get.assert_called_once_with(self.node.uuid) self.assertFalse(self.task.process_event.called) - def test_status_ok(self, mock_init, mock_get): + def test_status_ok(self, mock_client): + mock_get = mock_client.return_value.get_status mock_get.return_value = {'finished': True} inspector._check_status(self.task) - mock_init.assert_called_once_with( - session=mock.sentinel.session, - api_version=self.api_version, - inspector_url=None) mock_get.assert_called_once_with(self.node.uuid) self.task.process_event.assert_called_once_with('done') - def test_status_error(self, mock_init, mock_get): + def test_status_error(self, mock_client): + mock_get = mock_client.return_value.get_status mock_get.return_value = {'error': 'boom'} inspector._check_status(self.task) - mock_init.assert_called_once_with( - session=mock.sentinel.session, - api_version=self.api_version, - inspector_url=None) mock_get.assert_called_once_with(self.node.uuid) self.task.process_event.assert_called_once_with('fail') self.assertIn('boom', self.node.last_error) - - def test_service_url(self, mock_init, mock_get): - self.config(service_url='meow', group='inspector') - mock_get.return_value = {'finished': True} - inspector._check_status(self.task) - mock_init.assert_called_once_with( - session=mock.sentinel.session, - api_version=self.api_version, - inspector_url='meow') - mock_get.assert_called_once_with(self.node.uuid) - self.task.process_event.assert_called_once_with('done') - - def test_is_standalone(self, mock_init, mock_get): - self.config(auth_strategy='noauth') - mock_get.return_value = {'finished': True} - inspector._check_status(self.task) - mock_init.assert_called_once_with( - session=None, - api_version=self.api_version, - inspector_url=None) - mock_get.assert_called_once_with(self.node.uuid) - self.task.process_event.assert_called_once_with('done') diff --git a/releasenotes/notes/deprecated-inspector-opts-b19a08339712cfd7.yaml b/releasenotes/notes/deprecated-inspector-opts-b19a08339712cfd7.yaml new file mode 100644 index 0000000000..b1603cc722 --- /dev/null +++ b/releasenotes/notes/deprecated-inspector-opts-b19a08339712cfd7.yaml @@ -0,0 +1,17 @@ +--- +deprecations: + - | + Configuration option ``[inspector]/service_url`` is deprecated + and will be ignored in the Rocky release. + Instead, use ``[inspector]/endpoint_override`` configuration option to set + specific ironic-inspector api address when automatic discovery of + inspector API endpoint from keystone catalog is not desired. + This new option has no default value (``None``) and must be set explicitly. + + - | + Relying on the value of ``[DEFAULT]/auth_strategy`` configuration option to + configure usage of standalone mode for inspector client is deprecated and + will be impossible the the Rocky release. + Instead, set ``[inspector]/auth_type`` configuration option to ``none`` and + provide the inspector API address as ``[inspector]/endpoint_override`` + configuration option. diff --git a/releasenotes/notes/keystoneauth-config-1baa45a0a2dd93b4.yaml b/releasenotes/notes/keystoneauth-config-1baa45a0a2dd93b4.yaml index d9c4a7ee06..98f687c8a0 100644 --- a/releasenotes/notes/keystoneauth-config-1baa45a0a2dd93b4.yaml +++ b/releasenotes/notes/keystoneauth-config-1baa45a0a2dd93b4.yaml @@ -8,3 +8,7 @@ features: Adds the ability to set keystoneauth settings in the ``[cinder]`` configuration section for service automatic discovery. + - | + Adds the ability to set keystoneauth settings in the + ``[inspector]`` configuration section for service automatic + discovery.