Do not hide unexpected exceptions in inspection code

We have to at least provide a traceback for Python stdlib exceptions.
Also provide a more meaningful error message to a user in last_error.

Related-Bug: #1550328
Change-Id: I7cbb4bc36b5524c902b587f68212439e8f2d45df

ironic/conductor/manager.py

@@ -2572,19 +2572,23 @@ def _do_inspect_hardware(task):
     """
     node = task.node
 
-    def handle_failure(e):
+    def handle_failure(e, log_func=LOG.error):
         node.last_error = e
         task.process_event('fail')
-        LOG.error(_LE("Failed to inspect node %(node)s: %(err)s"),
-                  {'node': node.uuid, 'err': e})
+        log_func(_LE("Failed to inspect node %(node)s: %(err)s"),
+                 {'node': node.uuid, 'err': e})
 
     try:
         new_state = task.driver.inspect.inspect_hardware(task)
-
-    except Exception as e:
+    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')

ironic/tests/unit/conductor/test_manager.py

@@ -4512,12 +4512,32 @@ class NodeInspectHardware(mgr_utils.ServiceSetUpMixin,
                                           target_provision_state=state)
         task = task_manager.TaskManager(self.context, node.uuid)
 
-        self.assertRaises(exception.HardwareInspectionFailure,
-                          manager._do_inspect_hardware, task)
+        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.assertIsNotNone(node.last_error)
+        self.assertEqual('test', node.last_error)
+        self.assertTrue(mock_inspect.called)
+
+    @mock.patch('ironic.drivers.modules.fake.FakeInspect.inspect_hardware')
+    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',
+                                          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)
 
 

releasenotes/notes/inspection-logging-e1172f549ef80b04.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - Correctly handle unexpected exceptions during inspection. Return more
+    detailed error message to a user and log the traceback.