From 17b5f9ec84e8ad7f8c2b0f38b599456995d0aa75 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Tue, 17 Nov 2015 16:07:11 -0500 Subject: [PATCH] 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 | 30 ++++++++++++++++++++++ heat/tests/test_stack.py | 55 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/heat/engine/stack.py b/heat/engine/stack.py index 5eea563470..38d70390bd 100644 --- a/heat/engine/stack.py +++ b/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. diff --git a/heat/tests/test_stack.py b/heat/tests/test_stack.py index 567494548a..f89cde8abc 100644 --- a/heat/tests/test_stack.py +++ b/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)