Add set_spawn_error_hook to TaskManager

This patch is adding a way to create a hook on the TaskManager that gets
called upon an exception being raised. The task.spawn_after() do not
raise any exception making it impossible to add some logic around it in
case something goes bad with the method being executed by the greenthread.

Partial-Bug: #1331494
Change-Id: I5755df359a6e8678b64d4d59c25a9192f575b13d

ironic/conductor/task_manager.py

@@ -65,8 +65,24 @@ Common use of this is within the Manager like so:
                          utils.node_power_action, task, task.node,
                          new_state)
 
-All exceptions that occur in the current greenthread as part of the spawn
-handling are re-raised.
+All exceptions that occur in the current greenthread as part of the
+spawn handling are re-raised. In cases where it's impossible to handle
+the re-raised exception by wrapping the "with task_manager.acquire()"
+with a try..exception block (like the API cases where we return after
+the spawn_after()) the task allows you to set a hook to execute custom
+code when the spawned task generates an exception:
+
+    def on_error(e):
+        if isinstance(e, Exception):
+            ...
+
+    with task_manager.acquire(context, node_id) as task:
+        <do some work>
+        task.set_spawn_error_hook(on_error)
+        task.spawn_after(self._spawn_worker,
+                         utils.node_power_action, task, task.node,
+                         new_state)
+
 """
 
 from oslo.config import cfg
@@ -77,6 +93,10 @@ from ironic.common import driver_factory
 from ironic.common import exception
 from ironic.db import api as dbapi
 from ironic import objects
+from ironic.openstack.common.gettextutils import _LW
+from ironic.openstack.common import log as logging
+
+LOG = logging.getLogger(__name__)
 
 CONF = cfg.CONF
 
@@ -142,6 +162,7 @@ class TaskManager(object):
 
         self._dbapi = dbapi.get_instance()
         self._spawn_method = None
+        self._on_error_method = None
 
         self.context = context
         self.node = None
@@ -165,6 +186,22 @@ class TaskManager(object):
         self._spawn_args = args
         self._spawn_kwargs = kwargs
 
+    def set_spawn_error_hook(self, _on_error_method, *args, **kwargs):
+        """Create a hook that gets called when the task generates an exception.
+
+        Create a hook that gets called upon an exception being raised
+        from the spawned task.
+
+        :param _on_error_method: a callable object, it's first parameter
+            should accept the Exception object that was raised.
+        :param *args: additional args passed to the callable object.
+        :param **kwargs: additional kwargs passed to the callable object.
+
+        """
+        self._on_error_method = _on_error_method
+        self._on_error_args = args
+        self._on_error_kwargs = kwargs
+
     def release_resources(self):
         """Unlock a node and release resources.
 
@@ -216,8 +253,19 @@ class TaskManager(object):
                 # Don't unlock! The unlock will occur when the
                 # thread finshes.
                 return
-            except Exception:
+            except Exception as e:
                 with excutils.save_and_reraise_exception():
+                    try:
+                        # Execute the on_error hook if set
+                        if self._on_error_method:
+                            self._on_error_method(e, *self._on_error_args,
+                                                  **self._on_error_kwargs)
+                    except Exception:
+                        LOG.warning(_LW("Task's on_error hook failed to "
+                                        "call %(method)s on node %(node)s"),
+                                    {'method': self._on_error_method.__name__,
+                                    'node': self.node.uuid})
+
                     if thread is not None:
                         # This means the link() failed for some
                         # reason. Nuke the thread.

ironic/tests/conductor/test_task_manager.py

@@ -333,6 +333,51 @@ class TaskManagerTestCase(tests_base.TestCase):
         thread_mock.cancel.assert_called_once_with()
         task_release_mock.assert_called_once_with()
 
+    def test_spawn_after_on_error_hook(self, get_ports_mock, get_driver_mock,
+                                       reserve_mock, release_mock,
+                                       node_get_mock):
+        expected_exception = exception.IronicException('foo')
+        spawn_mock = mock.Mock(side_effect=expected_exception)
+        task_release_mock = mock.Mock()
+        on_error_handler = mock.Mock()
+
+        def _test_it():
+            with task_manager.TaskManager(self.context, 'node-id') as task:
+                task.set_spawn_error_hook(on_error_handler, 'fake-argument')
+                task.spawn_after(spawn_mock, 1, 2, foo='bar', cat='meow')
+                task.release_resources = task_release_mock
+
+        self.assertRaises(exception.IronicException, _test_it)
+
+        spawn_mock.assert_called_once_with(1, 2, foo='bar', cat='meow')
+        task_release_mock.assert_called_once_with()
+        on_error_handler.assert_called_once_with(expected_exception,
+                                                 'fake-argument')
+
+    def test_spawn_after_on_error_hook_exception(self, get_ports_mock,
+                                                 get_driver_mock, reserve_mock,
+                                                 release_mock, node_get_mock):
+        expected_exception = exception.IronicException('foo')
+        spawn_mock = mock.Mock(side_effect=expected_exception)
+        task_release_mock = mock.Mock()
+        # Raise an exception within the on_error handler
+        on_error_handler = mock.Mock(side_effect=Exception('unexpected'))
+        on_error_handler.__name__ = 'foo_method'
+
+        def _test_it():
+            with task_manager.TaskManager(self.context, 'node-id') as task:
+                task.set_spawn_error_hook(on_error_handler, 'fake-argument')
+                task.spawn_after(spawn_mock, 1, 2, foo='bar', cat='meow')
+                task.release_resources = task_release_mock
+
+        # Make sure the original exception is the one raised
+        self.assertRaises(exception.IronicException, _test_it)
+
+        spawn_mock.assert_called_once_with(1, 2, foo='bar', cat='meow')
+        task_release_mock.assert_called_once_with()
+        on_error_handler.assert_called_once_with(expected_exception,
+                                                 'fake-argument')
+
 
 @task_manager.require_exclusive_lock
 def _req_excl_lock_method(*args, **kwargs):