Use adapters for inspectorclient
Inspector-client is a bit lacking behind other clients, as it does not have Adapter-based SessionClient and thus does not support all adapter-related options. That's why we construct a session and an adapter from config section, use adapter to resolve inspector API from service catalog (or return the fixed endpoint_override one) and then pass the session and inspector API endpoint to client. This patch also deprecates `[inspector]service_url` in favor of `[inspector]endpoint_override`. As a side-effect, addressig inspector service now supports both regions and interfaces to specify entry in service catalog. Also, inspectorclient calls are now being made with the user token (wrapped with a service token) when there is a token in the task's request context. Change-Id: I21836e712fa9764468ac2654525554b5b4f03741 Partial-Bug: #1699547
This commit is contained in:
Pavlo Shchelokovskyy
parent
3e84bdb6db
commit
303ac3f835
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 = <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>
|
||||
|
||||
|
|
@ -1918,8 +1934,24 @@
|
|||
# Deprecated group/name - [inspector]/tenant_name
|
||||
#project_name = <None>
|
||||
|
||||
# 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 = <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-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 = <None>
|
||||
|
||||
# period (in seconds) to check status of nodes on inspection
|
||||
|
|
@ -1951,6 +1983,15 @@
|
|||
# Deprecated group/name - [inspector]/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>
|
||||
|
||||
|
||||
[ipmi]
|
||||
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Reference in New Issue