From 340a5fab93c63cb7912d29dc4bfc0d37b663178b Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <divius.inside@gmail.com>
Date: Wed, 12 Oct 2016 16:58:32 +0200
Subject: [PATCH] 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 | 14 ++++++----
ironic/tests/unit/conductor/test_manager.py | 26 ++++++++++++++++---
.../inspection-logging-e1172f549ef80b04.yaml | 4 +++
3 files changed, 36 insertions(+), 8 deletions(-)
create mode 100644 releasenotes/notes/inspection-logging-e1172f549ef80b04.yaml
diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 9160147a61..a686cd3efb 100644
--- a/ironic/conductor/manager.py
+++ b/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')
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index 17499ade67..20d522b827 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/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)
diff --git a/releasenotes/notes/inspection-logging-e1172f549ef80b04.yaml b/releasenotes/notes/inspection-logging-e1172f549ef80b04.yaml
new file mode 100644
index 0000000000..588e58371a
--- /dev/null
+++ b/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.