From 2c24badd55cdf90ba79bedc6ea18516ba0b5e6ca Mon Sep 17 00:00:00 2001
From: Zane Bitter <zbitter@redhat.com>
Date: Wed, 10 Feb 2016 19:14:16 -0500
Subject: [PATCH] Always do cleanup after a failed stack update

Move the cleanup code (that sets the status to FAILED and merges the new
environment into the old one) so that it runs on any exception, including
exit exceptions (due to e.g. the thread being cancelled or the engine
restarted). In the case of the environment this isn't as good as merging
them before the update starts (since it's possible for Heat to die without
time to run exception handlers), but at least it leaves things consistent
in the orderly case.

Change-Id: Ia4d2deca393ac9e250932c5cdb3691b6ee2b3ef4
Partial-bug: #1544348
Related-bug: #1492433
---
 heat/engine/stack.py | 60 ++++++++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 25 deletions(-)

diff --git a/heat/engine/stack.py b/heat/engine/stack.py
index ac78008f94..b7d00df8f1 100644
--- a/heat/engine/stack.py
+++ b/heat/engine/stack.py
@@ -1322,6 +1322,7 @@ class Stack(collections.Mapping):
         existing_params = environment.Environment({env_fmt.PARAMETERS:
                                                   self.t.env.params})
         previous_template_id = None
+        should_rollback = False
         try:
             update_task = update.StackUpdate(
                 self, newstack, backup_stack,
@@ -1362,14 +1363,17 @@ class Stack(collections.Mapping):
         except scheduler.Timeout:
             self.status = self.FAILED
             self.status_reason = 'Timed out'
-        except (ForcedCancel, exception.ResourceFailure) as e:
+        except (ForcedCancel, Exception) as e:
             # If rollback is enabled when resource failure occurred,
             # we do another update, with the existing template,
             # so we roll back to the original state
-            if self._update_exception_handler(
-                    exc=e, action=action, update_task=update_task):
+            should_rollback = self._update_exception_handler(e, action,
+                                                             update_task)
+            if should_rollback:
                 yield self.update_task(oldstack, action=self.ROLLBACK)
-                return
+        except BaseException as e:
+            with excutils.save_and_reraise_exception():
+                self._update_exception_handler(e, action, update_task)
         else:
             LOG.debug('Deleting backup stack')
             backup_stack.delete(backup=True)
@@ -1379,29 +1383,33 @@ class Stack(collections.Mapping):
             self.t = newstack.t
             template_outputs = self.t[self.t.OUTPUTS]
             self.outputs = self.resolve_static_data(template_outputs)
+        finally:
+            if should_rollback:
+                # Already handled in rollback task
+                return
 
-        # Don't use state_set to do only one update query and avoid race
-        # condition with the COMPLETE status
-        self.action = action
+            # Don't use state_set to do only one update query and avoid race
+            # condition with the COMPLETE status
+            self.action = action
 
-        self._send_notification_and_add_event()
-        if self.status == self.FAILED:
-            # Since template was incrementally updated based on existing and
-            # new stack resources, we should have user params of both.
-            existing_params.load(newstack.t.env.user_env_as_dict())
-            self.t.env = existing_params
-            self.t.store(self.context)
-            backup_stack.t.env = existing_params
-            backup_stack.t.store(self.context)
-        self.store()
+            self._send_notification_and_add_event()
+            if self.status == self.FAILED:
+                # Since template was incrementally updated based on existing
+                # and new stack resources, we should have user params of both.
+                existing_params.load(newstack.t.env.user_env_as_dict())
+                self.t.env = existing_params
+                self.t.store(self.context)
+                backup_stack.t.env = existing_params
+                backup_stack.t.store(self.context)
+            self.store()
 
-        if previous_template_id is not None:
-            raw_template_object.RawTemplate.delete(self.context,
-                                                   previous_template_id)
+            if previous_template_id is not None:
+                raw_template_object.RawTemplate.delete(self.context,
+                                                       previous_template_id)
 
-        lifecycle_plugin_utils.do_post_ops(self.context, self,
-                                           newstack, action,
-                                           (self.status == self.FAILED))
+            lifecycle_plugin_utils.do_post_ops(self.context, self,
+                                               newstack, action,
+                                               (self.status == self.FAILED))
 
     def _update_exception_handler(self, exc, action, update_task):
         """Handle exceptions in update_task.
@@ -1419,8 +1427,10 @@ class Stack(collections.Mapping):
         if isinstance(exc, ForcedCancel):
             update_task.updater.cancel_all()
             return exc.with_rollback or not self.disable_rollback
-
-        return not self.disable_rollback
+        elif isinstance(exc, exception.ResourceFailure):
+            return not self.disable_rollback
+        else:
+            return False
 
     def _message_parser(self, message):
         if message == rpc_api.THREAD_CANCEL: