refactoring update_task

Function update_task will be too complex if we add another exception inside,
but this is required by bug 1475057.
Require small refactoring.
Change-Id: Idec60343b2b859b466514400de9bb1ce22879d96
Partial-Bug: #1475057

heat/engine/stack.py

@@ -1156,37 +1156,25 @@ class Stack(collections.Mapping):
                         yield
                     else:
                         message = event.wait()
-                        if message == rpc_api.THREAD_CANCEL:
-                            raise ForcedCancel()
+                        self._message_parser(message)
             finally:
                 self.reset_dependencies()
 
             if action in (self.UPDATE, self.RESTORE, self.ROLLBACK):
-                reason = 'Stack %s completed successfully' % action
-            stack_status = self.COMPLETE
+                self.status_reason = 'Stack %s completed successfully' % action
+            self.status = self.COMPLETE
 
         except scheduler.Timeout:
-            stack_status = self.FAILED
-            reason = 'Timed out'
-        except ForcedCancel as e:
-            reason = six.text_type(e)
-
-            stack_status = self.FAILED
-            if action == self.UPDATE:
-                update_task.updater.cancel_all()
+            self.status = self.FAILED
+            self.status_reason = 'Timed out'
+        except (ForcedCancel, exception.ResourceFailure) 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):
                 yield self.update_task(oldstack, action=self.ROLLBACK)
                 return
-
-        except exception.ResourceFailure as e:
-            reason = six.text_type(e)
-
-            stack_status = self.FAILED
-            if action == self.UPDATE:
-                # If rollback is enabled, we do another update, with the
-                # existing template, so we roll back to the original state
-                if not self.disable_rollback:
-                    yield self.update_task(oldstack, action=self.ROLLBACK)
-                    return
         else:
             LOG.debug('Deleting backup stack')
             backup_stack.delete(backup=True)
@@ -1199,8 +1187,6 @@ class Stack(collections.Mapping):
         # Don't use state_set to do only one update query and avoid race
         # condition with the COMPLETE status
         self.action = action
-        self.status = stack_status
-        self.status_reason = reason
 
         notification.send(self)
         self._add_event(self.action, self.status, self.status_reason)
@@ -1210,6 +1196,22 @@ class Stack(collections.Mapping):
                                            newstack, action,
                                            (self.status == self.FAILED))
 
+    def _update_exception_handler(self, exc, action, update_task):
+        require_rollback = False
+        self.status_reason = six.text_type(exc)
+        self.status = self.FAILED
+        if action == self.UPDATE:
+            if not self.disable_rollback:
+                require_rollback = True
+            if isinstance(exc, ForcedCancel):
+                update_task.updater.cancel_all()
+                require_rollback = True
+        return require_rollback
+
+    def _message_parser(self, message):
+        if message == rpc_api.THREAD_CANCEL:
+            raise ForcedCancel()
+
     def _delete_backup_stack(self, stack):
         # Delete resources in the backup stack referred to by 'stack'
 

heat/tests/test_stack.py

@@ -2140,6 +2140,48 @@ class StackTest(common.HeatTestCase):
                                                None)
         self.assertEqual(expected_message, six.text_type(expected_exception))
 
+    def update_exception_handler(self, exc, action=stack.Stack.UPDATE,
+                                 disable_rollback=False):
+        tmpl = template.Template({
+            'HeatTemplateFormatVersion': '2012-12-12',
+            'Resources': {
+                'foo': {'Type': 'GenericResourceType'}
+            }
+        })
+        update_task = mock.MagicMock()
+
+        self.stack = stack.Stack(utils.dummy_context(),
+                                 'test_stack',
+                                 tmpl,
+                                 disable_rollback=disable_rollback)
+        self.stack.store()
+        self.m.ReplayAll()
+        res = self.stack._update_exception_handler(
+            exc=exc, action=action, update_task=update_task)
+        if isinstance(exc, exception.ResourceFailure):
+            if disable_rollback:
+                self.assertFalse(res)
+            else:
+                self.assertTrue(res)
+        elif isinstance(exc, stack.ForcedCancel):
+            update_task.updater.cancel_all.assert_called_once_with()
+            self.assertTrue(res)
+        self.m.VerifyAll()
+
+    def test_update_exception_handler_resource_failure_no_rollback(self):
+        reason = 'something strange happened'
+        exc = exception.ResourceFailure(reason, None, action='UPDATE')
+        self.update_exception_handler(exc, disable_rollback=True)
+
+    def test_update_exception_handler_resource_failure_rollback(self):
+        reason = 'something strange happened'
+        exc = exception.ResourceFailure(reason, None, action='UPDATE')
+        self.update_exception_handler(exc, disable_rollback=False)
+
+    def test_update_exception_handler_force_cancel(self):
+        exc = stack.ForcedCancel()
+        self.update_exception_handler(exc, disable_rollback=False)
+
 
 class StackKwargsForCloningTest(common.HeatTestCase):
     scenarios = [