Ensure that stacks can't get stuck IN_PROGRESS

Make sure that any unexpected exceptions are handled by moving the stack to
the FAILED state.

Change-Id: Ic948c2fe5baf23c9c4ced33060f672ca9c278a19
Closes-Bug: #1492433

heat/engine/stack.py

@@ -20,6 +20,7 @@ import re
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_utils import encodeutils
+from oslo_utils import excutils
 from oslo_utils import timeutils as oslo_timeutils
 from oslo_utils import uuidutils
 from osprofiler import profiler
@@ -71,6 +72,25 @@ class ForcedCancel(BaseException):
         return "Operation cancelled"
 
 
+def reset_state_on_error(func):
+    @six.wraps(func)
+    def handle_exceptions(stack, *args, **kwargs):
+        errmsg = None
+        try:
+            return func(stack, *args, **kwargs)
+        except BaseException as exc:
+            with excutils.save_and_reraise_exception():
+                errmsg = six.text_type(exc)
+                LOG.error(_LE('Unexpected exception in %(func)s: %(msg)s'),
+                          {'func': func.__name__, 'msg': errmsg})
+        finally:
+            if stack.state == stack.IN_PROGRESS:
+                stack.set_state(stack.action, stack.FAILED, errmsg)
+                assert errmsg is not None, "Returned while IN_PROGRESS"
+
+    return handle_exceptions
+
+
 @six.python_2_unicode_compatible
 class Stack(collections.Mapping):
 
@@ -793,6 +813,7 @@ class Stack(collections.Mapping):
                 r._store()
 
     @profiler.trace('Stack.create', hide_args=False)
+    @reset_state_on_error
     def create(self):
         """Create the stack and all of the resources."""
         def rollback():
@@ -894,6 +915,7 @@ class Stack(collections.Mapping):
                                            (self.status == self.FAILED))
 
     @profiler.trace('Stack.check', hide_args=False)
+    @reset_state_on_error
     def check(self):
         self.updated_time = oslo_timeutils.utcnow()
         checker = scheduler.TaskRunner(
@@ -947,6 +969,7 @@ class Stack(collections.Mapping):
             return None
 
     @profiler.trace('Stack.adopt', hide_args=False)
+    @reset_state_on_error
     def adopt(self):
         """Adopt existing resources into a new stack."""
         def rollback():
@@ -966,6 +989,7 @@ class Stack(collections.Mapping):
         creator(timeout=self.timeout_secs())
 
     @profiler.trace('Stack.update', hide_args=False)
+    @reset_state_on_error
     def update(self, newstack, event=None):
         """Update the stack.
 
@@ -1419,6 +1443,7 @@ class Stack(collections.Mapping):
         return stack_status, reason
 
     @profiler.trace('Stack.delete', hide_args=False)
+    @reset_state_on_error
     def delete(self, action=DELETE, backup=False, abandon=False):
         """Delete all of the resources, and then the stack itself.
 
@@ -1510,6 +1535,7 @@ class Stack(collections.Mapping):
             self.id = None
 
     @profiler.trace('Stack.suspend', hide_args=False)
+    @reset_state_on_error
     def suspend(self):
         """Suspend the stack.
 
@@ -1535,6 +1561,7 @@ class Stack(collections.Mapping):
         sus_task(timeout=self.timeout_secs())
 
     @profiler.trace('Stack.resume', hide_args=False)
+    @reset_state_on_error
     def resume(self):
         """Resume the stack.
 
@@ -1560,6 +1587,7 @@ class Stack(collections.Mapping):
         sus_task(timeout=self.timeout_secs())
 
     @profiler.trace('Stack.snapshot', hide_args=False)
+    @reset_state_on_error
     def snapshot(self, save_snapshot_func):
         """Snapshot the stack, invoking handle_snapshot on all resources."""
         self.updated_time = oslo_timeutils.utcnow()
@@ -1572,6 +1600,7 @@ class Stack(collections.Mapping):
         sus_task(timeout=self.timeout_secs())
 
     @profiler.trace('Stack.delete_snapshot', hide_args=False)
+    @reset_state_on_error
     def delete_snapshot(self, snapshot):
         """Remove a snapshot from the backends."""
         for name, rsrc in six.iteritems(self.resources):
@@ -1581,6 +1610,7 @@ class Stack(collections.Mapping):
                 scheduler.TaskRunner(rsrc.delete_snapshot, data)()
 
     @profiler.trace('Stack.restore', hide_args=False)
+    @reset_state_on_error
     def restore(self, snapshot):
         """Restore the given snapshot.
 

heat/tests/test_stack.py

@@ -2486,3 +2486,58 @@ class StackKwargsForCloningTest(common.HeatTestCase):
             # just make sure that the kwargs are valid
             # (no exception should be raised)
             stack.Stack(ctx, utils.random_name(), tmpl, **res)
+
+
+class ResetStateOnErrorTest(common.HeatTestCase):
+    class DummyStack(object):
+
+        (COMPLETE, IN_PROGRESS, FAILED) = range(3)
+        action = 'something'
+        state = COMPLETE
+
+        set_state = mock.MagicMock()
+
+        @stack.reset_state_on_error
+        def raise_exception(self):
+            self.state = self.IN_PROGRESS
+            raise ValueError('oops')
+
+        @stack.reset_state_on_error
+        def raise_exit_exception(self):
+            self.state = self.IN_PROGRESS
+            raise BaseException('bye')
+
+        @stack.reset_state_on_error
+        def succeed(self):
+            return 'Hello world'
+
+        @stack.reset_state_on_error
+        def fail(self):
+            self.state = self.FAILED
+            return 'Hello world'
+
+    def test_success(self):
+        dummy = self.DummyStack()
+
+        self.assertEqual('Hello world', dummy.succeed())
+        self.assertFalse(dummy.set_state.called)
+
+    def test_failure(self):
+        dummy = self.DummyStack()
+
+        self.assertEqual('Hello world', dummy.fail())
+        self.assertFalse(dummy.set_state.called)
+
+    def test_reset_state_exception(self):
+        dummy = self.DummyStack()
+
+        exc = self.assertRaises(ValueError, dummy.raise_exception)
+        self.assertIn('oops', str(exc))
+        self.assertTrue(dummy.set_state.called)
+
+    def test_reset_state_exit_exception(self):
+        dummy = self.DummyStack()
+
+        exc = self.assertRaises(BaseException, dummy.raise_exit_exception)
+        self.assertIn('bye', str(exc))
+        self.assertTrue(dummy.set_state.called)