Use openstacksdk for accessing ironic-inspector

Change-Id: Ibe4f6bf0b38364b5dd214e6c7e58d45a4d71ffdf

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

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

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')),

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
+    session = _get_inspector_session()
-    if CONF.auth_strategy != 'keystone':
+    # NOTE(dtantsur): openstacksdk expects config option groups to match
-        CONF.set_override('auth_type', 'none', group='inspector')
+    # service name, but we use just "inspector".
-    service_auth = keystone.get_auth('inspector')
+    conf = dict(CONF)
-    session = _get_inspector_session(auth=service_auth)
+    conf['ironic-inspector'] = conf.pop('inspector')
-    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)
     # 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
+    return openstack.connection.Connection(
-    # 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)
+        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')
+    if not status.error and not status.is_finished:
-    finished = status.get('finished')
-    if not error and not 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')

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,
+    def test__get_client(self, mock_conn, 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(
+        mock_conn.assert_called_once_with(
             session=mock.sentinel.session,
-            api_version=self.api_version,
+            oslo_conf=mock.ANY)
-            inspector_url='inspector_url')
+        self.assertEqual(1, mock_auth.call_count)
-        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,
+    def test__get_client_standalone(self, mock_conn, 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(
+        mock_conn.assert_called_once_with(
             session=mock.sentinel.session,
-            api_version=self.api_version,
+            oslo_conf=mock.ANY)
-            inspector_url='inspector_url')
+        self.assertEqual(1, mock_auth.call_count)
-        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()
-        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 = mock_client.return_value.get_introspection
-        mock_get.return_value = {}
+        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 = mock_client.return_value.get_introspection
-        mock_get.return_value = {'finished': True}
+        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 = mock_client.return_value.get_introspection
-        mock_get.return_value = {'error': 'boom'}
+        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)

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,

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 = (

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)

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

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

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