Merge "Set node last_error in TaskManager"

ironic/conductor/task_manager.py

@@ -94,6 +94,7 @@ raised in the background thread.):
 
 """
 
+import futurist
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_utils import excutils
@@ -103,6 +104,8 @@ import six
 
 from ironic.common import driver_factory
 from ironic.common import exception
+from ironic.common.i18n import _
+from ironic.common.i18n import _LE
 from ironic.common.i18n import _LW
 from ironic.common import states
 from ironic import objects
@@ -319,9 +322,35 @@ class TaskManager(object):
         self.portgroups = None
         self.fsm = None
 
-    def _thread_release_resources(self, t):
-        """Thread.link() callback to release resources."""
-        self.release_resources()
+    def _write_exception(self, future):
+        """Set node last_error if exception raised in thread."""
+        node = self.node
+        # do not rewrite existing error
+        if node and node.last_error is None:
+            method = self._spawn_args[0].__name__
+            try:
+                exc = future.exception()
+            except futurist.CancelledError:
+                LOG.exception(_LE("Execution of %(method)s for node %(node)s "
+                                  "was canceled."), {'method': method,
+                                                     'node': node.uuid})
+            else:
+                if exc is not None:
+                    msg = _("Async execution of %(method)s failed with error: "
+                            "%(error)s") % {'method': method,
+                                            'error': six.text_type(exc)}
+                    node.last_error = msg
+                    try:
+                        node.save()
+                    except exception.NodeNotFound:
+                        pass
+
+    def _thread_release_resources(self, fut):
+        """Thread callback to release resources."""
+        try:
+            self._write_exception(fut)
+        finally:
+            self.release_resources()
 
     def process_event(self, event, callback=None, call_args=None,
                       call_kwargs=None, err_handler=None, target_state=None):

ironic/tests/unit/conductor/test_task_manager.py

@@ -17,6 +17,7 @@
 
 """Tests for :class:`ironic.conductor.task_manager`."""
 
+import futurist
 import mock
 from oslo_utils import uuidutils
 
@@ -658,3 +659,51 @@ class ExclusiveLockDecoratorTestCase(tests_base.TestCase):
                           _req_excl_lock_method,
                           *self.args_task_second,
                           **self.kwargs)
+
+
+class ThreadExceptionTestCase(tests_base.TestCase):
+    def setUp(self):
+        super(ThreadExceptionTestCase, self).setUp()
+        self.node = mock.Mock(spec=objects.Node)
+        self.node.last_error = None
+        self.task = mock.Mock(spec=task_manager.TaskManager)
+        self.task.node = self.node
+        self.task._write_exception = task_manager.TaskManager._write_exception
+        self.future_mock = mock.Mock(spec_set=['exception'])
+
+        def async_method_foo():
+            pass
+
+        self.task._spawn_args = (async_method_foo,)
+
+    def test_set_node_last_error(self):
+        self.future_mock.exception.return_value = Exception('fiasco')
+        self.task._write_exception(self.task, self.future_mock)
+        self.node.save.assert_called_once_with()
+        self.assertIn('fiasco', self.node.last_error)
+        self.assertIn('async_method_foo', self.node.last_error)
+
+    def test_set_node_last_error_exists(self):
+        self.future_mock.exception.return_value = Exception('fiasco')
+        self.node.last_error = 'oops'
+        self.task._write_exception(self.task, self.future_mock)
+        self.assertFalse(self.node.save.called)
+        self.assertFalse(self.future_mock.exception.called)
+        self.assertEqual('oops', self.node.last_error)
+
+    def test_set_node_last_error_no_error(self):
+        self.future_mock.exception.return_value = None
+        self.task._write_exception(self.task, self.future_mock)
+        self.assertFalse(self.node.save.called)
+        self.future_mock.exception.assert_called_once_with()
+        self.assertIsNone(self.node.last_error)
+
+    @mock.patch.object(task_manager.LOG, 'exception', spec_set=True,
+                       autospec=True)
+    def test_set_node_last_error_cancelled(self, log_mock):
+        self.future_mock.exception.side_effect = futurist.CancelledError()
+        self.task._write_exception(self.task, self.future_mock)
+        self.assertFalse(self.node.save.called)
+        self.future_mock.exception.assert_called_once_with()
+        self.assertIsNone(self.node.last_error)
+        self.assertTrue(log_mock.called)