Merge "Restructure the inspector module in preparation for its expansion"

8 files changed, 152 insertions, 99 deletions

diff --git a/ironic/drivers/modules/inspector/__init__.py b/ironic/drivers/modules/inspector/__init__.py
new file mode 100644
index 000000000..bb2dd43c8
--- /dev/null
+++ b/ironic/drivers/modules/inspector/__init__.py
@@ -0,0 +1,15 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from ironic.drivers.modules.inspector.interface import Inspector
+
+__all__ = ['Inspector']
diff --git a/ironic/drivers/modules/inspector/client.py b/ironic/drivers/modules/inspector/client.py
new file mode 100644
index 000000000..7e996492e
--- /dev/null
+++ b/ironic/drivers/modules/inspector/client.py
@@ -0,0 +1,57 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+"""Client helper for ironic-inspector."""
+
+from keystoneauth1 import exceptions as ks_exception
+import openstack
+
+from ironic.common import exception
+from ironic.common.i18n import _
+from ironic.common import keystone
+from ironic.conf import CONF
+
+
+_INSPECTOR_SESSION = None
+
+
+def _get_inspector_session(**kwargs):
+    global _INSPECTOR_SESSION
+    if not _INSPECTOR_SESSION:
+        if CONF.auth_strategy != 'keystone':
+            # 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."""
+    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
+    try:
+        return openstack.connection.Connection(
+            session=session,
+            oslo_conf=conf).baremetal_introspection
+    except ks_exception.DiscoveryFailure as exc:
+        raise exception.ConfigInvalid(
+            _("Could not contact ironic-inspector for version discovery: %s")
+            % exc)
diff --git a/ironic/drivers/modules/inspector.py b/ironic/drivers/modules/inspector/interface.py
index dbf171714..731029dbe 100644
--- a/ironic/drivers/modules/inspector.py
+++ b/ironic/drivers/modules/inspector/interface.py
@@ -20,13 +20,10 @@ import shlex
 from urllib import parse as urlparse
 
 import eventlet
-from keystoneauth1 import exceptions as ks_exception
-import openstack
 from oslo_log import log as logging
 
 from ironic.common import exception
 from ironic.common.i18n import _
-from ironic.common import keystone
 from ironic.common import states
 from ironic.common import utils
 from ironic.conductor import periodics
@@ -36,47 +33,14 @@ from ironic.conf import CONF
 from ironic.drivers import base
 from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules import inspect_utils
+from ironic.drivers.modules.inspector import client
 
 LOG = logging.getLogger(__name__)
 
-_INSPECTOR_SESSION = None
 # Internal field to mark whether ironic or inspector manages boot for the node
 _IRONIC_MANAGES_BOOT = 'inspector_manage_boot'
 
 
-def _get_inspector_session(**kwargs):
-    global _INSPECTOR_SESSION
-    if not _INSPECTOR_SESSION:
-        if CONF.auth_strategy != 'keystone':
-            # 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."""
-    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
-    try:
-        return openstack.connection.Connection(
-            session=session,
-            oslo_conf=conf).baremetal_introspection
-    except ks_exception.DiscoveryFailure as exc:
-        raise exception.ConfigInvalid(
-            _("Could not contact ironic-inspector for version discovery: %s")
-            % exc)
-
-
 def _get_callback_endpoint(client):
     root = CONF.inspector.callback_endpoint_override or client.get_endpoint()
     if root == 'mdns':
@@ -197,8 +161,8 @@ def _parse_kernel_params():
 def _start_managed_inspection(task):
     """Start inspection managed by ironic."""
     try:
-        client = _get_client(task.context)
-        endpoint = _get_callback_endpoint(client)
+        cli = client.get_client(task.context)
+        endpoint = _get_callback_endpoint(cli)
         params = dict(_parse_kernel_params(),
                       **{'ipa-inspection-callback-url': endpoint})
         if utils.fast_track_enabled(task.node):
@@ -208,7 +172,7 @@ def _start_managed_inspection(task):
         with cond_utils.power_state_for_network_configuration(task):
             task.driver.network.add_inspection_network(task)
         task.driver.boot.prepare_ramdisk(task, ramdisk_params=params)
-        client.start_introspection(task.node.uuid, manage_boot=False)
+        cli.start_introspection(task.node.uuid, manage_boot=False)
         cond_utils.node_power_action(task, states.POWER_ON)
     except Exception as exc:
         LOG.exception('Unable to start managed inspection for node %(uuid)s: '
@@ -295,7 +259,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_introspection(node_uuid)
+        client.get_client(task.context).abort_introspection(node_uuid)
 
     @periodics.node_periodic(
         purpose='checking hardware inspection status',
@@ -310,7 +274,7 @@ class Inspector(base.InspectInterface):
 def _start_inspection(node_uuid, context):
     """Call to inspector to start inspection."""
     try:
-        _get_client(context).start_introspection(node_uuid)
+        client.get_client(context).start_introspection(node_uuid)
     except Exception as exc:
         LOG.error('Error contacting ironic-inspector for inspection of node '
                   '%(node)s: %(cls)s: %(err)s',
@@ -339,7 +303,7 @@ def _check_status(task):
               task.node.uuid)
 
     try:
-        inspector_client = _get_client(task.context)
+        inspector_client = client.get_client(task.context)
         status = inspector_client.get_introspection(node.uuid)
     except Exception:
         # NOTE(dtantsur): get_status should not normally raise
diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py
index d56652b1e..27061737d 100644
--- a/ironic/tests/unit/api/controllers/v1/test_node.py
+++ b/ironic/tests/unit/api/controllers/v1/test_node.py
@@ -44,8 +44,8 @@ from ironic.common import indicator_states
 from ironic.common import policy
 from ironic.common import states
 from ironic.conductor import rpcapi
+from ironic.conf import CONF
 from ironic.drivers.modules import inspect_utils
-from ironic.drivers.modules import inspector
 from ironic import objects
 from ironic.objects import fields as obj_fields
 from ironic import tests as tests_root
@@ -54,7 +54,6 @@ from ironic.tests.unit.api import base as test_api_base
 from ironic.tests.unit.api import utils as test_api_utils
 from ironic.tests.unit.objects import utils as obj_utils
 
-CONF = inspector.CONF
 
 with open(
         os.path.join(
diff --git a/ironic/tests/unit/drivers/modules/inspector/__init__.py b/ironic/tests/unit/drivers/modules/inspector/__init__.py
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/ironic/tests/unit/drivers/modules/inspector/__init__.py
diff --git a/ironic/tests/unit/drivers/modules/inspector/test_client.py b/ironic/tests/unit/drivers/modules/inspector/test_client.py
new file mode 100644
index 000000000..08f0fcd93
--- /dev/null
+++ b/ironic/tests/unit/drivers/modules/inspector/test_client.py
@@ -0,0 +1,65 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from unittest import mock
+
+from keystoneauth1 import exceptions as ks_exception
+import openstack
+
+from ironic.common import context
+from ironic.common import exception
+from ironic.conf import CONF
+from ironic.drivers.modules.inspector import client
+from ironic.tests.unit.db import base as db_base
+
+
+@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(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
+        client._INSPECTOR_SESSION = None
+        self.context = context.RequestContext(global_request_id='global')
+
+    def test_get_client(self, mock_conn, mock_session, mock_auth):
+        client.get_client(self.context)
+        mock_conn.assert_called_once_with(
+            session=mock.sentinel.session,
+            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_conn, mock_session, mock_auth):
+        self.config(auth_strategy='noauth')
+        client.get_client(self.context)
+        self.assertEqual('none', CONF.inspector.auth_type)
+        mock_conn.assert_called_once_with(
+            session=mock.sentinel.session,
+            oslo_conf=mock.ANY)
+        self.assertEqual(1, mock_auth.call_count)
+        self.assertEqual(1, mock_session.call_count)
+
+    def test_get_client_connection_problem(
+            self, mock_conn, mock_session, mock_auth):
+        mock_conn.side_effect = ks_exception.DiscoveryFailure("")
+        self.assertRaises(exception.ConfigInvalid,
+                          client.get_client, self.context)
+        mock_conn.assert_called_once_with(
+            session=mock.sentinel.session,
+            oslo_conf=mock.ANY)
+        self.assertEqual(1, mock_auth.call_count)
+        self.assertEqual(1, mock_session.call_count)
diff --git a/ironic/tests/unit/drivers/modules/test_inspector.py b/ironic/tests/unit/drivers/modules/inspector/test_interface.py
index 75ccc3ebf..f4dbdd4b0 100644
--- a/ironic/tests/unit/drivers/modules/test_inspector.py
+++ b/ironic/tests/unit/drivers/modules/inspector/test_interface.py
@@ -13,65 +13,19 @@
 from unittest import mock
 
 import eventlet
-from keystoneauth1 import exceptions as ks_exception
-import openstack
 
-from ironic.common import context
 from ironic.common import exception
 from ironic.common import states
 from ironic.common import utils
 from ironic.conductor import task_manager
+from ironic.conf import CONF
 from ironic.drivers.modules import inspect_utils
-from ironic.drivers.modules import inspector
+from ironic.drivers.modules.inspector import client
+from ironic.drivers.modules.inspector import interface as inspector
 from ironic.drivers.modules.redfish import utils as redfish_utils
 from ironic.tests.unit.db import base as db_base
 from ironic.tests.unit.objects import utils as obj_utils
 
-CONF = inspector.CONF
-
-
-@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(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.context = context.RequestContext(global_request_id='global')
-
-    def test__get_client(self, mock_conn, mock_session, mock_auth):
-        inspector._get_client(self.context)
-        mock_conn.assert_called_once_with(
-            session=mock.sentinel.session,
-            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_conn, mock_session, mock_auth):
-        self.config(auth_strategy='noauth')
-        inspector._get_client(self.context)
-        self.assertEqual('none', inspector.CONF.inspector.auth_type)
-        mock_conn.assert_called_once_with(
-            session=mock.sentinel.session,
-            oslo_conf=mock.ANY)
-        self.assertEqual(1, mock_auth.call_count)
-        self.assertEqual(1, mock_session.call_count)
-
-    def test__get_client_connection_problem(
-            self, mock_conn, mock_session, mock_auth):
-        mock_conn.side_effect = ks_exception.DiscoveryFailure("")
-        self.assertRaises(exception.ConfigInvalid,
-                          inspector._get_client, self.context)
-        mock_conn.assert_called_once_with(
-            session=mock.sentinel.session,
-            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):
@@ -129,7 +83,7 @@ class CommonFunctionsTestCase(BaseTestCase):
 
 
 @mock.patch.object(eventlet, 'spawn_n', lambda f, *a, **kw: f(*a, **kw))
-@mock.patch('ironic.drivers.modules.inspector._get_client', autospec=True)
+@mock.patch.object(client, 'get_client', autospec=True)
 class InspectHardwareTestCase(BaseTestCase):
     def test_validate_ok(self, mock_client):
         self.iface.validate(self.task)
@@ -369,7 +323,7 @@ class InspectHardwareTestCase(BaseTestCase):
             self.task, 'power off', timeout=None)
 
 
-@mock.patch('ironic.drivers.modules.inspector._get_client', autospec=True)
+@mock.patch.object(client, 'get_client', autospec=True)
 class CheckStatusTestCase(BaseTestCase):
     def setUp(self):
         super(CheckStatusTestCase, self).setUp()
@@ -593,7 +547,7 @@ class CheckStatusTestCase(BaseTestCase):
         mock_get_data.assert_not_called()
 
 
-@mock.patch('ironic.drivers.modules.inspector._get_client', autospec=True)
+@mock.patch.object(client, 'get_client', autospec=True)
 class InspectHardwareAbortTestCase(BaseTestCase):
     def test_abort_ok(self, mock_client):
         mock_abort = mock_client.return_value.abort_introspection
diff --git a/ironic/tests/unit/drivers/modules/test_inspect_utils.py b/ironic/tests/unit/drivers/modules/test_inspect_utils.py
index 7cb451473..473b0ee7c 100644
--- a/ironic/tests/unit/drivers/modules/test_inspect_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_inspect_utils.py
@@ -23,14 +23,13 @@ from ironic.common import context as ironic_context
 from ironic.common import exception
 from ironic.common import swift
 from ironic.conductor import task_manager
+from ironic.conf import CONF
 from ironic.drivers.modules import inspect_utils as utils
-from ironic.drivers.modules import inspector
 from ironic import objects
 from ironic.tests.unit.db import base as db_base
 from ironic.tests.unit.objects import utils as obj_utils
 
 sushy = importutils.try_import('sushy')
-CONF = inspector.CONF
 
 
 @mock.patch('time.sleep', lambda sec: None)