Merge "Ensure that stacks can't get stuck IN_PROGRESS" into stable/kilo

2 files changed, 87 insertions, 0 deletions

diff --git a/heat/engine/stack.py b/heat/engine/stack.py
index ea1ee9ae9..e7547ecf1 100755
--- a/heat/engine/stack.py
+++ b/heat/engine/stack.py
@@ -21,6 +21,7 @@ import warnings
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_utils import encodeutils
+from oslo_utils import excutils
 from osprofiler import profiler
 import six
 
@@ -62,6 +63,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
+
+
 class Stack(collections.Mapping):
 
     ACTIONS = (
@@ -693,6 +713,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.
@@ -780,6 +801,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 = datetime.datetime.utcnow()
         checker = scheduler.TaskRunner(self.stack_task, self.CHECK,
@@ -829,6 +851,7 @@ class Stack(collections.Mapping):
             return None
 
     @profiler.trace('Stack.adopt', hide_args=False)
+    @reset_state_on_error
     def adopt(self):
         '''
         Adopt a stack (create stack with all the existing resources).
@@ -849,6 +872,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):
         '''
         Compare the current stack with newstack,
@@ -1105,6 +1129,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.
@@ -1192,6 +1217,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, which invokes handle_suspend for all stack resources
@@ -1213,6 +1239,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, which invokes handle_resume for all stack resources
@@ -1234,6 +1261,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):
         '''Snapshot the stack, invoking handle_snapshot on all resources.'''
         self.updated_time = datetime.datetime.utcnow()
@@ -1243,6 +1271,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):
@@ -1252,6 +1281,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, invoking handle_restore on all resources.
diff --git a/heat/tests/test_stack.py b/heat/tests/test_stack.py
index 03ab86417..1b2c0ff2c 100644
--- a/heat/tests/test_stack.py
+++ b/heat/tests/test_stack.py
@@ -1864,3 +1864,60 @@ 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
+
+        @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()
+        dummy.set_state = mock.MagicMock()
+
+        self.assertEqual('Hello world', dummy.succeed())
+        self.assertFalse(dummy.set_state.called)
+
+    def test_failure(self):
+        dummy = self.DummyStack()
+        dummy.set_state = mock.MagicMock()
+
+        self.assertEqual('Hello world', dummy.fail())
+        self.assertFalse(dummy.set_state.called)
+
+    def test_reset_state_exception(self):
+        dummy = self.DummyStack()
+        dummy.set_state = mock.MagicMock()
+
+        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()
+        dummy.set_state = mock.MagicMock()
+
+        exc = self.assertRaises(BaseException, dummy.raise_exit_exception)
+        self.assertIn('bye', str(exc))
+        self.assertTrue(dummy.set_state.called)