Merge "Refactoring: create ironic.conductor.inspection"

4 files changed, 232 insertions, 193 deletions

diff --git a/ironic/conductor/inspection.py b/ironic/conductor/inspection.py
new file mode 100644
index 000000000..53c76e99d
--- /dev/null
+++ b/ironic/conductor/inspection.py
@@ -0,0 +1,108 @@
+#    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.
+
+"""Inspection implementation for the conductor."""
+
+from oslo_log import log
+from oslo_utils import excutils
+
+from ironic.common import exception
+from ironic.common.i18n import _
+from ironic.common import states
+from ironic.conductor import task_manager
+from ironic.conductor import utils
+
+LOG = log.getLogger(__name__)
+
+
+@task_manager.require_exclusive_lock
+def inspect_hardware(task):
+    """Initiates inspection.
+
+    :param task: a TaskManager instance with an exclusive lock
+                 on its node.
+    :raises: HardwareInspectionFailure if driver doesn't
+             return the state as states.MANAGEABLE, states.INSPECTWAIT.
+
+    """
+    node = task.node
+
+    def handle_failure(e, log_func=LOG.error):
+        utils.node_history_record(task.node, event=e,
+                                  event_type=states.INTROSPECTION,
+                                  error=True, user=task.context.user_id)
+        task.process_event('fail')
+        log_func("Failed to inspect node %(node)s: %(err)s",
+                 {'node': node.uuid, 'err': e})
+
+    # Inspection cannot start in fast-track mode, wipe token and URL.
+    utils.wipe_token_and_url(task)
+
+    try:
+        new_state = task.driver.inspect.inspect_hardware(task)
+    except exception.IronicException as e:
+        with excutils.save_and_reraise_exception():
+            error = str(e)
+            handle_failure(error)
+    except Exception as e:
+        error = (_('Unexpected exception of type %(type)s: %(msg)s') %
+                 {'type': type(e).__name__, 'msg': e})
+        handle_failure(error, log_func=LOG.exception)
+        raise exception.HardwareInspectionFailure(error=error)
+
+    if new_state == states.MANAGEABLE:
+        task.process_event('done')
+        LOG.info('Successfully inspected node %(node)s',
+                 {'node': node.uuid})
+    elif new_state == states.INSPECTWAIT:
+        task.process_event('wait')
+        LOG.info('Successfully started introspection on node %(node)s',
+                 {'node': node.uuid})
+    else:
+        error = (_("During inspection, driver returned unexpected "
+                   "state %(state)s") % {'state': new_state})
+        handle_failure(error)
+        raise exception.HardwareInspectionFailure(error=error)
+
+
+@task_manager.require_exclusive_lock
+def abort_inspection(task):
+    """Abort inspection for the node."""
+    node = task.node
+
+    try:
+        task.driver.inspect.abort(task)
+    except exception.UnsupportedDriverExtension:
+        with excutils.save_and_reraise_exception():
+            LOG.error('Inspect interface "%(intf)s" does not support abort '
+                      'operation for node %(node)s',
+                      {'intf': node.inspect_interface, 'node': node.uuid})
+    except Exception as e:
+        with excutils.save_and_reraise_exception():
+            LOG.exception('Error when aborting inspection of node %(node)s',
+                          {'node': node.uuid})
+            error = _('Failed to abort inspection: %s') % e
+            utils.node_history_record(task.node, event=error,
+                                      event_type=states.INTROSPECTION,
+                                      error=True,
+                                      user=task.context.user_id)
+            node.save()
+
+    error = _('Inspection was aborted by request.')
+    utils.node_history_record(task.node, event=error,
+                              event_type=states.INTROSPECTION,
+                              error=True,
+                              user=task.context.user_id)
+    utils.wipe_token_and_url(task)
+    task.process_event('abort')
+    LOG.info('Successfully aborted inspection of node %(node)s',
+             {'node': node.uuid})
diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 74e3192cf..bbd2355bd 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -63,6 +63,7 @@ from ironic.conductor import allocations
 from ironic.conductor import base_manager
 from ironic.conductor import cleaning
 from ironic.conductor import deployments
+from ironic.conductor import inspection
 from ironic.conductor import notification_utils as notify_utils
 from ironic.conductor import periodics
 from ironic.conductor import steps as conductor_steps
@@ -1365,35 +1366,7 @@ class ConductorManager(base_manager.BaseConductorManager):
             return
 
         if node.provision_state == states.INSPECTWAIT:
-            try:
-                task.driver.inspect.abort(task)
-            except exception.UnsupportedDriverExtension:
-                with excutils.save_and_reraise_exception():
-                    intf_name = task.driver.inspect.__class__.__name__
-                    LOG.error('Inspect interface %(intf)s does not '
-                              'support abort operation when aborting '
-                              'inspection of node %(node)s',
-                              {'intf': intf_name, 'node': node.uuid})
-            except Exception as e:
-                with excutils.save_and_reraise_exception():
-                    LOG.exception('Error in aborting the inspection of '
-                                  'node %(node)s', {'node': node.uuid})
-                    error = _('Failed to abort inspection: %s') % e
-                    utils.node_history_record(task.node, event=error,
-                                              event_type=states.INTROSPECTION,
-                                              error=True,
-                                              user=task.context.user_id)
-                    node.save()
-            error = _('Inspection was aborted by request.')
-            utils.node_history_record(task.node, event=error,
-                                      event_type=states.INTROSPECTION,
-                                      error=True,
-                                      user=task.context.user_id)
-            utils.wipe_token_and_url(task)
-            task.process_event('abort')
-            LOG.info('Successfully aborted inspection of node %(node)s',
-                     {'node': node.uuid})
-            return
+            return inspection.abort_inspection(task)
 
     @METRICS.timer('ConductorManager._sync_power_states')
     @periodics.periodic(spacing=CONF.conductor.sync_power_state_interval,
@@ -3038,7 +3011,7 @@ class ConductorManager(base_manager.BaseConductorManager):
                 task.process_event(
                     'inspect',
                     callback=self._spawn_worker,
-                    call_args=(_do_inspect_hardware, task),
+                    call_args=(inspection.inspect_hardware, task),
                     err_handler=utils.provisioning_error_handler)
 
             except exception.InvalidState:
@@ -3858,53 +3831,3 @@ def do_sync_power_state(task, count):
             task, old_power_state)
 
     return count
-
-
-@task_manager.require_exclusive_lock
-def _do_inspect_hardware(task):
-    """Initiates inspection.
-
-    :param task: a TaskManager instance with an exclusive lock
-                 on its node.
-    :raises: HardwareInspectionFailure if driver doesn't
-             return the state as states.MANAGEABLE, states.INSPECTWAIT.
-
-    """
-    node = task.node
-
-    def handle_failure(e, log_func=LOG.error):
-        utils.node_history_record(task.node, event=e,
-                                  event_type=states.INTROSPECTION,
-                                  error=True, user=task.context.user_id)
-        task.process_event('fail')
-        log_func("Failed to inspect node %(node)s: %(err)s",
-                 {'node': node.uuid, 'err': e})
-
-    # Inspection cannot start in fast-track mode, wipe token and URL.
-    utils.wipe_token_and_url(task)
-
-    try:
-        new_state = task.driver.inspect.inspect_hardware(task)
-    except exception.IronicException as e:
-        with excutils.save_and_reraise_exception():
-            error = str(e)
-            handle_failure(error)
-    except Exception as e:
-        error = (_('Unexpected exception of type %(type)s: %(msg)s') %
-                 {'type': type(e).__name__, 'msg': e})
-        handle_failure(error, log_func=LOG.exception)
-        raise exception.HardwareInspectionFailure(error=error)
-
-    if new_state == states.MANAGEABLE:
-        task.process_event('done')
-        LOG.info('Successfully inspected node %(node)s',
-                 {'node': node.uuid})
-    elif new_state == states.INSPECTWAIT:
-        task.process_event('wait')
-        LOG.info('Successfully started introspection on node %(node)s',
-                 {'node': node.uuid})
-    else:
-        error = (_("During inspection, driver returned unexpected "
-                   "state %(state)s") % {'state': new_state})
-        handle_failure(error)
-        raise exception.HardwareInspectionFailure(error=error)
diff --git a/ironic/tests/unit/conductor/test_inspection.py b/ironic/tests/unit/conductor/test_inspection.py
new file mode 100644
index 000000000..c64b883e4
--- /dev/null
+++ b/ironic/tests/unit/conductor/test_inspection.py
@@ -0,0 +1,118 @@
+#    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 ironic.common import exception
+from ironic.common import states
+from ironic.conductor import inspection
+from ironic.conductor import task_manager
+from ironic.tests.unit.db import base as db_base
+from ironic.tests.unit.objects import utils as obj_utils
+
+
+@mock.patch('ironic.drivers.modules.fake.FakeInspect.inspect_hardware',
+            autospec=True)
+class TestInspectHardware(db_base.DbTestCase):
+
+    def test_inspect_hardware_ok(self, mock_inspect):
+        node = obj_utils.create_test_node(
+            self.context, driver='fake-hardware',
+            provision_state=states.INSPECTING,
+            driver_internal_info={'agent_url': 'url',
+                                  'agent_secret_token': 'token'})
+        task = task_manager.TaskManager(self.context, node.uuid)
+        mock_inspect.return_value = states.MANAGEABLE
+        inspection.inspect_hardware(task)
+        node.refresh()
+        self.assertEqual(states.MANAGEABLE, node.provision_state)
+        self.assertEqual(states.NOSTATE, node.target_provision_state)
+        self.assertIsNone(node.last_error)
+        mock_inspect.assert_called_once_with(task.driver.inspect, task)
+        task.node.refresh()
+        self.assertNotIn('agent_url', task.node.driver_internal_info)
+        self.assertNotIn('agent_secret_token', task.node.driver_internal_info)
+
+    def test_inspect_hardware_return_inspecting(self, mock_inspect):
+        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
+                                          provision_state=states.INSPECTING)
+        task = task_manager.TaskManager(self.context, node.uuid)
+        mock_inspect.return_value = states.INSPECTING
+        self.assertRaises(exception.HardwareInspectionFailure,
+                          inspection.inspect_hardware, task)
+
+        node.refresh()
+        self.assertIn('driver returned unexpected state', node.last_error)
+        self.assertEqual(states.INSPECTFAIL, node.provision_state)
+        self.assertEqual(states.MANAGEABLE, node.target_provision_state)
+        mock_inspect.assert_called_once_with(task.driver.inspect, task)
+
+    def test_inspect_hardware_return_inspect_wait(self, mock_inspect):
+        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
+                                          provision_state=states.INSPECTING)
+        task = task_manager.TaskManager(self.context, node.uuid)
+        mock_inspect.return_value = states.INSPECTWAIT
+        inspection.inspect_hardware(task)
+        node.refresh()
+        self.assertEqual(states.INSPECTWAIT, node.provision_state)
+        self.assertEqual(states.MANAGEABLE, node.target_provision_state)
+        self.assertIsNone(node.last_error)
+        mock_inspect.assert_called_once_with(task.driver.inspect, task)
+
+    @mock.patch.object(inspection, 'LOG', autospec=True)
+    def test_inspect_hardware_return_other_state(self, log_mock, mock_inspect):
+        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
+                                          provision_state=states.INSPECTING)
+        task = task_manager.TaskManager(self.context, node.uuid)
+        mock_inspect.return_value = None
+        self.assertRaises(exception.HardwareInspectionFailure,
+                          inspection.inspect_hardware, task)
+        node.refresh()
+        self.assertEqual(states.INSPECTFAIL, node.provision_state)
+        self.assertEqual(states.MANAGEABLE, node.target_provision_state)
+        self.assertIsNotNone(node.last_error)
+        mock_inspect.assert_called_once_with(task.driver.inspect, task)
+        self.assertTrue(log_mock.error.called)
+
+    def test_inspect_hardware_raises_error(self, mock_inspect):
+        mock_inspect.side_effect = exception.HardwareInspectionFailure('test')
+        state = states.MANAGEABLE
+        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
+                                          provision_state=states.INSPECTING,
+                                          target_provision_state=state)
+        task = task_manager.TaskManager(self.context, node.uuid)
+
+        self.assertRaisesRegex(exception.HardwareInspectionFailure, '^test$',
+                               inspection.inspect_hardware, task)
+        node.refresh()
+        self.assertEqual(states.INSPECTFAIL, node.provision_state)
+        self.assertEqual(states.MANAGEABLE, node.target_provision_state)
+        self.assertEqual('test', node.last_error)
+        self.assertTrue(mock_inspect.called)
+
+    def test_inspect_hardware_unexpected_error(self, mock_inspect):
+        mock_inspect.side_effect = RuntimeError('x')
+        state = states.MANAGEABLE
+        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
+                                          provision_state=states.INSPECTING,
+                                          target_provision_state=state)
+        task = task_manager.TaskManager(self.context, node.uuid)
+
+        self.assertRaisesRegex(exception.HardwareInspectionFailure,
+                               'Unexpected exception of type RuntimeError: x',
+                               inspection.inspect_hardware, task)
+        node.refresh()
+        self.assertEqual(states.INSPECTFAIL, node.provision_state)
+        self.assertEqual(states.MANAGEABLE, node.target_provision_state)
+        self.assertEqual('Unexpected exception of type RuntimeError: x',
+                         node.last_error)
+        self.assertTrue(mock_inspect.called)
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index 6a6f7e08f..7278486a5 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -46,6 +46,7 @@ from ironic.common import nova
 from ironic.common import states
 from ironic.conductor import cleaning
 from ironic.conductor import deployments
+from ironic.conductor import inspection
 from ironic.conductor import manager
 from ironic.conductor import notification_utils
 from ironic.conductor import steps as conductor_steps
@@ -6461,77 +6462,6 @@ class ManagerSyncLocalStateTestCase(mgr_utils.CommonMixIn, db_base.DbTestCase):
 @mgr_utils.mock_record_keepalive
 class NodeInspectHardware(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
 
-    @mock.patch('ironic.drivers.modules.fake.FakeInspect.inspect_hardware',
-                autospec=True)
-    def test_inspect_hardware_ok(self, mock_inspect):
-        self._start_service()
-        node = obj_utils.create_test_node(
-            self.context, driver='fake-hardware',
-            provision_state=states.INSPECTING,
-            driver_internal_info={'agent_url': 'url',
-                                  'agent_secret_token': 'token'})
-        task = task_manager.TaskManager(self.context, node.uuid)
-        mock_inspect.return_value = states.MANAGEABLE
-        manager._do_inspect_hardware(task)
-        node.refresh()
-        self.assertEqual(states.MANAGEABLE, node.provision_state)
-        self.assertEqual(states.NOSTATE, node.target_provision_state)
-        self.assertIsNone(node.last_error)
-        mock_inspect.assert_called_once_with(task.driver.inspect, task)
-        task.node.refresh()
-        self.assertNotIn('agent_url', task.node.driver_internal_info)
-        self.assertNotIn('agent_secret_token', task.node.driver_internal_info)
-
-    @mock.patch('ironic.drivers.modules.fake.FakeInspect.inspect_hardware',
-                autospec=True)
-    def test_inspect_hardware_return_inspecting(self, mock_inspect):
-        self._start_service()
-        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
-                                          provision_state=states.INSPECTING)
-        task = task_manager.TaskManager(self.context, node.uuid)
-        mock_inspect.return_value = states.INSPECTING
-        self.assertRaises(exception.HardwareInspectionFailure,
-                          manager._do_inspect_hardware, task)
-
-        node.refresh()
-        self.assertIn('driver returned unexpected state', node.last_error)
-        self.assertEqual(states.INSPECTFAIL, node.provision_state)
-        self.assertEqual(states.MANAGEABLE, node.target_provision_state)
-        mock_inspect.assert_called_once_with(task.driver.inspect, task)
-
-    @mock.patch('ironic.drivers.modules.fake.FakeInspect.inspect_hardware',
-                autospec=True)
-    def test_inspect_hardware_return_inspect_wait(self, mock_inspect):
-        self._start_service()
-        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
-                                          provision_state=states.INSPECTING)
-        task = task_manager.TaskManager(self.context, node.uuid)
-        mock_inspect.return_value = states.INSPECTWAIT
-        manager._do_inspect_hardware(task)
-        node.refresh()
-        self.assertEqual(states.INSPECTWAIT, node.provision_state)
-        self.assertEqual(states.MANAGEABLE, node.target_provision_state)
-        self.assertIsNone(node.last_error)
-        mock_inspect.assert_called_once_with(task.driver.inspect, task)
-
-    @mock.patch.object(manager, 'LOG', autospec=True)
-    @mock.patch('ironic.drivers.modules.fake.FakeInspect.inspect_hardware',
-                autospec=True)
-    def test_inspect_hardware_return_other_state(self, mock_inspect, log_mock):
-        self._start_service()
-        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
-                                          provision_state=states.INSPECTING)
-        task = task_manager.TaskManager(self.context, node.uuid)
-        mock_inspect.return_value = None
-        self.assertRaises(exception.HardwareInspectionFailure,
-                          manager._do_inspect_hardware, task)
-        node.refresh()
-        self.assertEqual(states.INSPECTFAIL, node.provision_state)
-        self.assertEqual(states.MANAGEABLE, node.target_provision_state)
-        self.assertIsNotNone(node.last_error)
-        mock_inspect.assert_called_once_with(task.driver.inspect, task)
-        self.assertTrue(log_mock.error.called)
-
     def test__check_inspect_wait_timeouts(self):
         self._start_service()
         CONF.set_override('inspect_wait_timeout', 1, group='conductor')
@@ -6609,46 +6539,6 @@ class NodeInspectHardware(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
     def test_inspect_hardware_power_validate_fail(self, mock_validate):
         self._test_inspect_hardware_validate_fail(mock_validate)
 
-    @mock.patch('ironic.drivers.modules.fake.FakeInspect.inspect_hardware',
-                autospec=True)
-    def test_inspect_hardware_raises_error(self, mock_inspect):
-        self._start_service()
-        mock_inspect.side_effect = exception.HardwareInspectionFailure('test')
-        state = states.MANAGEABLE
-        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
-                                          provision_state=states.INSPECTING,
-                                          target_provision_state=state)
-        task = task_manager.TaskManager(self.context, node.uuid)
-
-        self.assertRaisesRegex(exception.HardwareInspectionFailure, '^test$',
-                               manager._do_inspect_hardware, task)
-        node.refresh()
-        self.assertEqual(states.INSPECTFAIL, node.provision_state)
-        self.assertEqual(states.MANAGEABLE, node.target_provision_state)
-        self.assertEqual('test', node.last_error)
-        self.assertTrue(mock_inspect.called)
-
-    @mock.patch('ironic.drivers.modules.fake.FakeInspect.inspect_hardware',
-                autospec=True)
-    def test_inspect_hardware_unexpected_error(self, mock_inspect):
-        self._start_service()
-        mock_inspect.side_effect = RuntimeError('x')
-        state = states.MANAGEABLE
-        node = obj_utils.create_test_node(self.context, driver='fake-hardware',
-                                          provision_state=states.INSPECTING,
-                                          target_provision_state=state)
-        task = task_manager.TaskManager(self.context, node.uuid)
-
-        self.assertRaisesRegex(exception.HardwareInspectionFailure,
-                               'Unexpected exception of type RuntimeError: x',
-                               manager._do_inspect_hardware, task)
-        node.refresh()
-        self.assertEqual(states.INSPECTFAIL, node.provision_state)
-        self.assertEqual(states.MANAGEABLE, node.target_provision_state)
-        self.assertEqual('Unexpected exception of type RuntimeError: x',
-                         node.last_error)
-        self.assertTrue(mock_inspect.called)
-
 
 @mock.patch.object(conductor_utils, 'node_history_record',
                    mock.Mock(spec=conductor_utils.node_history_record))
@@ -8247,7 +8137,7 @@ class NodeTraitsTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
 class DoNodeInspectAbortTestCase(mgr_utils.CommonMixIn,
                                  mgr_utils.ServiceSetUpMixin,
                                  db_base.DbTestCase):
-    @mock.patch.object(manager, 'LOG', autospec=True)
+    @mock.patch.object(inspection, 'LOG', autospec=True)
     @mock.patch('ironic.drivers.modules.fake.FakeInspect.abort', autospec=True)
     @mock.patch('ironic.conductor.task_manager.acquire', autospec=True)
     def test_do_inspect_abort_interface_not_support(self, mock_acquire,
@@ -8268,7 +8158,7 @@ class DoNodeInspectAbortTestCase(mgr_utils.CommonMixIn,
                          exc.exc_info[0])
         self.assertTrue(mock_log.error.called)
 
-    @mock.patch.object(manager, 'LOG', autospec=True)
+    @mock.patch.object(inspection, 'LOG', autospec=True)
     @mock.patch('ironic.drivers.modules.fake.FakeInspect.abort', autospec=True)
     @mock.patch('ironic.conductor.task_manager.acquire', autospec=True)
     def test_do_inspect_abort_interface_return_failed(self, mock_acquire,