From d6774c294d8f097154165ff76ba54bef7401475c Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 9 Jul 2019 13:45:28 +0200 Subject: [PATCH] Use openstacksdk for accessing ironic-inspector Change-Id: Ibe4f6bf0b38364b5dd214e6c7e58d45a4d71ffdf --- doc/source/admin/inspection.rst | 4 - driver-requirements.txt | 1 - ironic/conf/inspector.py | 11 --- ironic/drivers/modules/inspector.py | 65 ++++++--------- .../unit/drivers/modules/test_inspector.py | 81 +++++++------------ ironic/tests/unit/drivers/test_generic.py | 1 - .../drivers/third_party_driver_mock_specs.py | 16 ---- .../unit/drivers/third_party_driver_mocks.py | 11 --- lower-constraints.txt | 2 +- ...cated-inspector-opts-0520b08dbcd10681.yaml | 8 ++ requirements.txt | 2 +- 11 files changed, 65 insertions(+), 137 deletions(-) create mode 100644 releasenotes/notes/deprecated-inspector-opts-0520b08dbcd10681.yaml diff --git a/doc/source/admin/inspection.rst b/doc/source/admin/inspection.rst index 3c03406ab8..39ad9cdf03 100644 --- a/doc/source/admin/inspection.rst +++ b/doc/source/admin/inspection.rst @@ -86,9 +86,6 @@ enabled to use it: [DEFAULT] enabled_inspect_interfaces = inspector,no-inspect -You must additionally install python-ironic-inspector-client_ to use -this functionality. - If the ironic-inspector service is not registered in the service catalog, set the following option: @@ -106,5 +103,4 @@ configuration file must be set:: keep_ports = present .. _ironic-inspector: https://pypi.org/project/ironic-inspector -.. _python-ironic-inspector-client: https://pypi.org/project/python-ironic-inspector-client .. _python-ironicclient: https://pypi.org/project/python-ironicclient diff --git a/driver-requirements.txt b/driver-requirements.txt index f95fb560a4..c56f3d0adb 100644 --- a/driver-requirements.txt +++ b/driver-requirements.txt @@ -6,7 +6,6 @@ # These are available on pypi proliantutils>=2.7.0 pysnmp>=4.3.0,<5.0.0 -python-ironic-inspector-client>=1.5.0 python-scciclient>=0.8.0 python-dracclient>=3.0.0,<4.0.0 python-xclarityclient>=0.1.6 diff --git a/ironic/conf/inspector.py b/ironic/conf/inspector.py index d449f1f2e9..da63687b4d 100644 --- a/ironic/conf/inspector.py +++ b/ironic/conf/inspector.py @@ -18,17 +18,6 @@ from ironic.common.i18n import _ from ironic.conf import auth opts = [ - cfg.BoolOpt('enabled', default=False, - help=_('This option has no affect since the classic drivers ' - 'removal.'), - deprecated_for_removal=True), - 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, help=_('period (in seconds) to check status of nodes ' 'on inspection')), diff --git a/ironic/drivers/modules/inspector.py b/ironic/drivers/modules/inspector.py index 658d973ab8..5774079300 100644 --- a/ironic/drivers/modules/inspector.py +++ b/ironic/drivers/modules/inspector.py @@ -17,8 +17,8 @@ Modules required to work with ironic_inspector: import eventlet from futurist import periodics +import openstack from oslo_log import log as logging -from oslo_utils import importutils from ironic.common import exception from ironic.common.i18n import _ @@ -31,53 +31,40 @@ from ironic.drivers import base LOG = logging.getLogger(__name__) -client = importutils.try_import('ironic_inspector_client') - - -INSPECTOR_API_VERSION = (1, 3) - _INSPECTOR_SESSION = None def _get_inspector_session(**kwargs): global _INSPECTOR_SESSION if not _INSPECTOR_SESSION: - _INSPECTOR_SESSION = keystone.get_session('inspector', **kwargs) + if CONF.auth_strategy == 'noauth': + # NOTE(dtantsur): using set_default instead of set_override because + # the native keystoneauth option must have priority. + CONF.set_default('auth_type', 'none', group='inspector') + service_auth = keystone.get_auth('inspector') + _INSPECTOR_SESSION = keystone.get_session('inspector', + auth=service_auth, + **kwargs) return _INSPECTOR_SESSION 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 - inspector_url = keystone.get_endpoint('inspector', session=session, - **adapter_params) + session = _get_inspector_session() + # NOTE(dtantsur): openstacksdk expects config option groups to match + # service name, but we use just "inspector". + conf = dict(CONF) + conf['ironic-inspector'] = conf.pop('inspector') # 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, - session=session, - inspector_url=inspector_url) + return openstack.connection.Connection( + session=session, + oslo_conf=conf).baremetal_introspection class Inspector(base.InspectInterface): """In-band inspection via ironic-inspector project.""" - def __init__(self): - if not client: - raise exception.DriverLoadError( - _('python-ironic-inspector-client Python module not found')) - def get_properties(self): """Return the properties of the interface. @@ -122,7 +109,7 @@ class Inspector(base.InspectInterface): node_uuid = task.node.uuid LOG.debug('Aborting inspection for node %(uuid)s using ' 'ironic-inspector', {'uuid': node_uuid}) - _get_client(task.context).abort(node_uuid) + _get_client(task.context).abort_introspection(node_uuid) @periodics.periodic(spacing=CONF.inspector.status_check_period) def _periodic_check_result(self, manager, context): @@ -144,7 +131,7 @@ class Inspector(base.InspectInterface): def _start_inspection(node_uuid, context): """Call to inspector to start inspection.""" try: - _get_client(context).introspect(node_uuid) + _get_client(context).start_introspection(node_uuid) except Exception as exc: LOG.exception('Exception during contacting ironic-inspector ' 'for inspection of node %(node)s: %(err)s', @@ -173,7 +160,7 @@ def _check_status(task): task.node.uuid) try: - status = _get_client(task.context).get_status(node.uuid) + status = _get_client(task.context).get_introspection(node.uuid) except Exception: # NOTE(dtantsur): get_status should not normally raise # let's assume it's a transient failure and retry later @@ -182,9 +169,7 @@ def _check_status(task): node.uuid) return - error = status.get('error') - finished = status.get('finished') - if not error and not finished: + if not status.error and not status.is_finished: return # If the inspection has finished or failed, we need to update the node, so @@ -192,12 +177,12 @@ def _check_status(task): task.upgrade_lock() node = task.node - if error: + if status.error: LOG.error('Inspection failed for node %(uuid)s with error: %(err)s', - {'uuid': node.uuid, 'err': error}) + {'uuid': node.uuid, 'err': status.error}) node.last_error = (_('ironic-inspector inspection failed: %s') - % error) + % status.error) task.process_event('fail') - elif finished: + elif status.is_finished: LOG.info('Inspection finished successfully for node %s', node.uuid) task.process_event('done') diff --git a/ironic/tests/unit/drivers/modules/test_inspector.py b/ironic/tests/unit/drivers/modules/test_inspector.py index ae5f1e8aab..2b33211010 100644 --- a/ironic/tests/unit/drivers/modules/test_inspector.py +++ b/ironic/tests/unit/drivers/modules/test_inspector.py @@ -11,8 +11,8 @@ # under the License. import eventlet -import ironic_inspector_client as client import mock +import openstack from ironic.common import context from ironic.common import states @@ -22,67 +22,41 @@ from ironic.tests.unit.db import base as db_base from ironic.tests.unit.objects import utils as obj_utils -@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) +@mock.patch.object(openstack.connection, 'Connection', autospec=True) 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, 3) 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' + def test__get_client(self, mock_conn, mock_session, mock_auth): inspector._get_client(self.context) - mock_init.assert_called_once_with( + mock_conn.assert_called_once_with( session=mock.sentinel.session, - api_version=self.api_version, - inspector_url='inspector_url') - self.assertEqual(0, mock_sauth.call_count) + oslo_conf=mock.ANY) + self.assertEqual(1, mock_auth.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): + def test__get_client_standalone(self, mock_conn, mock_session, mock_auth): 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( + mock_conn.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) + oslo_conf=mock.ANY) + self.assertEqual(1, mock_auth.call_count) self.assertEqual(1, mock_session.call_count) class BaseTestCase(db_base.DbTestCase): def setUp(self): super(BaseTestCase, self).setUp() - self.config(enabled=True, group='inspector') self.node = obj_utils.get_test_node(self.context, inspect_interface='inspector') self.iface = inspector.Inspector() @@ -91,7 +65,6 @@ class BaseTestCase(db_base.DbTestCase): self.task.shared = False self.task.node = self.node self.task.driver = mock.Mock(spec=['inspect'], inspect=self.iface) - self.api_version = (1, 0) class CommonFunctionsTestCase(BaseTestCase): @@ -107,14 +80,14 @@ class CommonFunctionsTestCase(BaseTestCase): @mock.patch('ironic.drivers.modules.inspector._get_client', autospec=True) class InspectHardwareTestCase(BaseTestCase): def test_ok(self, mock_client): - mock_introspect = mock_client.return_value.introspect + mock_introspect = mock_client.return_value.start_introspection self.assertEqual(states.INSPECTWAIT, self.iface.inspect_hardware(self.task)) mock_introspect.assert_called_once_with(self.node.uuid) @mock.patch.object(task_manager, 'acquire', autospec=True) def test_error(self, mock_acquire, mock_client): - mock_introspect = mock_client.return_value.introspect + mock_introspect = mock_client.return_value.start_introspection mock_introspect.side_effect = RuntimeError('boom') self.iface.inspect_hardware(self.task) mock_introspect.assert_called_once_with(self.node.uuid) @@ -130,47 +103,53 @@ class CheckStatusTestCase(BaseTestCase): self.node.provision_state = states.INSPECTWAIT def test_not_inspecting(self, mock_client): - mock_get = mock_client.return_value.get_status + mock_get = mock_client.return_value.get_introspection self.node.provision_state = states.MANAGEABLE inspector._check_status(self.task) self.assertFalse(mock_get.called) def test_not_check_inspecting(self, mock_client): - mock_get = mock_client.return_value.get_status + mock_get = mock_client.return_value.get_introspection self.node.provision_state = states.INSPECTING inspector._check_status(self.task) self.assertFalse(mock_get.called) def test_not_inspector(self, mock_client): - mock_get = mock_client.return_value.get_status + mock_get = mock_client.return_value.get_introspection self.task.driver.inspect = object() inspector._check_status(self.task) self.assertFalse(mock_get.called) def test_not_finished(self, mock_client): - mock_get = mock_client.return_value.get_status - mock_get.return_value = {} + mock_get = mock_client.return_value.get_introspection + mock_get.return_value = mock.Mock(is_finished=False, + error=None, + spec=['is_finished', 'error']) inspector._check_status(self.task) mock_get.assert_called_once_with(self.node.uuid) self.assertFalse(self.task.process_event.called) def test_exception_ignored(self, mock_client): - mock_get = mock_client.return_value.get_status + mock_get = mock_client.return_value.get_introspection mock_get.side_effect = RuntimeError('boom') inspector._check_status(self.task) mock_get.assert_called_once_with(self.node.uuid) self.assertFalse(self.task.process_event.called) def test_status_ok(self, mock_client): - mock_get = mock_client.return_value.get_status - mock_get.return_value = {'finished': True} + mock_get = mock_client.return_value.get_introspection + mock_get.return_value = mock.Mock(is_finished=True, + error=None, + spec=['is_finished', 'error']) inspector._check_status(self.task) mock_get.assert_called_once_with(self.node.uuid) self.task.process_event.assert_called_once_with('done') def test_status_error(self, mock_client): - mock_get = mock_client.return_value.get_status - mock_get.return_value = {'error': 'boom'} + mock_get = mock_client.return_value.get_introspection + mock_get.return_value = mock.Mock(is_finished=True, + error='boom', + spec=['is_finished', 'error']) inspector._check_status(self.task) mock_get.assert_called_once_with(self.node.uuid) self.task.process_event.assert_called_once_with('fail') @@ -180,12 +159,12 @@ class CheckStatusTestCase(BaseTestCase): @mock.patch('ironic.drivers.modules.inspector._get_client', autospec=True) class InspectHardwareAbortTestCase(BaseTestCase): def test_abort_ok(self, mock_client): - mock_abort = mock_client.return_value.abort + mock_abort = mock_client.return_value.abort_introspection self.iface.abort(self.task) mock_abort.assert_called_once_with(self.node.uuid) def test_abort_error(self, mock_client): - mock_abort = mock_client.return_value.abort + mock_abort = mock_client.return_value.abort_introspection mock_abort.side_effect = RuntimeError('boom') self.assertRaises(RuntimeError, self.iface.abort, self.task) mock_abort.assert_called_once_with(self.node.uuid) diff --git a/ironic/tests/unit/drivers/test_generic.py b/ironic/tests/unit/drivers/test_generic.py index 6eef22d4e6..c8475aad68 100644 --- a/ironic/tests/unit/drivers/test_generic.py +++ b/ironic/tests/unit/drivers/test_generic.py @@ -36,7 +36,6 @@ class ManualManagementHardwareTestCase(db_base.DbTestCase): self.config(enabled_hardware_types=['manual-management'], enabled_power_interfaces=['fake'], enabled_management_interfaces=['noop', 'fake']) - self.config(enabled=True, group='inspector') def test_default_interfaces(self): node = obj_utils.create_test_node(self.context, diff --git a/ironic/tests/unit/drivers/third_party_driver_mock_specs.py b/ironic/tests/unit/drivers/third_party_driver_mock_specs.py index da51d5059b..db8adc5296 100644 --- a/ironic/tests/unit/drivers/third_party_driver_mock_specs.py +++ b/ironic/tests/unit/drivers/third_party_driver_mock_specs.py @@ -40,22 +40,6 @@ DRACCLIENT_CONSTANTS_REBOOT_REQUIRED_MOD_SPEC = ( 'false' ) -# ironic_inspector -IRONIC_INSPECTOR_CLIENT_SPEC = ( - 'ClientV1', -) - - -class InspectorClientV1Specs(object): - def __init__(self, session, inspector_url, api_version): - pass - - def introspect(self, uuid): - pass - - def get_status(self, uuid): - pass - # proliantutils PROLIANTUTILS_SPEC = ( diff --git a/ironic/tests/unit/drivers/third_party_driver_mocks.py b/ironic/tests/unit/drivers/third_party_driver_mocks.py index b11fa0e696..9c351b8a72 100644 --- a/ironic/tests/unit/drivers/third_party_driver_mocks.py +++ b/ironic/tests/unit/drivers/third_party_driver_mocks.py @@ -163,17 +163,6 @@ irmc_boot.check_share_fs_mounted_patcher = mock.patch( irmc_boot.check_share_fs_mounted_patcher.return_value = None -ironic_inspector_client = importutils.try_import('ironic_inspector_client') -if not ironic_inspector_client: - ironic_inspector_client = mock.MagicMock( - spec_set=mock_specs.IRONIC_INSPECTOR_CLIENT_SPEC) - ironic_inspector_client.ClientV1 = mock_specs.InspectorClientV1Specs - sys.modules['ironic_inspector_client'] = ironic_inspector_client - if 'ironic.drivers.modules.inspector' in sys.modules: - six.moves.reload_module( - sys.modules['ironic.drivers.modules.inspector']) - - class MockKwargsException(Exception): def __init__(self, *args, **kwargs): super(MockKwargsException, self).__init__(*args) diff --git a/lower-constraints.txt b/lower-constraints.txt index 7241fd3a52..77ddd3e26a 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -19,7 +19,7 @@ keystoneauth1==3.11.0 keystonemiddleware==4.17.0 mock==3.0.0 openstackdocstheme==1.20.0 -openstacksdk==0.25.0 +openstacksdk==0.31.2 os-api-ref==1.4.0 os-traits==0.4.0 oslo.concurrency==3.26.0 diff --git a/releasenotes/notes/deprecated-inspector-opts-0520b08dbcd10681.yaml b/releasenotes/notes/deprecated-inspector-opts-0520b08dbcd10681.yaml new file mode 100644 index 0000000000..da30701a0d --- /dev/null +++ b/releasenotes/notes/deprecated-inspector-opts-0520b08dbcd10681.yaml @@ -0,0 +1,8 @@ +--- +upgrade: + - | + The deprecated configuration options ``enabled`` and ``service_url`` from + the ``inspector`` section have been removed. + - | + The python-ironic-inspector-client package is no longer required for the + ``inspector`` inspect interface (openstacksdk is used instead). diff --git a/requirements.txt b/requirements.txt index 1ac808ad49..e543ea71d6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -47,4 +47,4 @@ jsonschema>=2.6.0 # MIT psutil>=3.2.2 # BSD futurist>=1.2.0 # Apache-2.0 tooz>=1.58.0 # Apache-2.0 -openstacksdk>=0.25.0 # Apache-2.0 +openstacksdk>=0.31.2 # Apache-2.0