]> review.fuel-infra Code Review - openstack-build/heat-build.git/commitdiff
heat engine : fail update immediately on resource update failure
authorSteven Hardy <shardy@redhat.com>
Wed, 20 Feb 2013 10:24:06 +0000 (10:24 +0000)
committerSteven Hardy <shardy@redhat.com>
Wed, 20 Feb 2013 15:40:22 +0000 (15:40 +0000)
When doing an update, we want to fail immediately, instead of
collecting the resource update errors and continuing.  This is
particularly important for update rollback, where we want to
declare failure ASAP to simplify and speed up the rollback

blueprint update-rollback

Change-Id: I56133254036d8aac592dcf3cda2ca7928cc02fa9

heat/engine/parser.py
heat/tests/test_parser.py

index 963e15fc15d48a1854c23481a79669a8c6329b69..b2e3af456adfbcd5e0af32c98640d8071b8d15a9 100644 (file)
@@ -283,7 +283,6 @@ class Stack(object):
             self.state_set(self.UPDATE_IN_PROGRESS, 'Stack update started')
 
         # Now make the resources match the new stack definition
-        failures = []
         with eventlet.Timeout(self.timeout_mins * 60) as tmo:
             try:
                 # First delete any resources which are not in newstack
@@ -293,8 +292,10 @@ class Stack(object):
                                      % res.name + " definition, deleting")
                         result = res.destroy()
                         if result:
-                            failures.append('Resource %s delete failed'
-                                            % res.name)
+                            logger.error("Failed to remove %s : %s" %
+                                         (res.name, result))
+                            raise exception.ResourceUpdateFailed(
+                                resource_name=res.name)
                         else:
                             del self.resources[res.name]
 
@@ -307,8 +308,10 @@ class Stack(object):
                         self[res.name] = res
                         result = self[res.name].create()
                         if result:
-                            failures.append('Resource %s create failed'
-                                            % res.name)
+                            logger.error("Failed to add %s : %s" %
+                                         (res.name, result))
+                            raise exception.ResourceUpdateFailed(
+                                resource_name=res.name)
 
                 # Now (the hard part :) update existing resources
                 # The Resource base class allows equality-test of resources,
@@ -331,8 +334,8 @@ class Stack(object):
                     # of the existing stack (which is the stack being updated)
                     old_snippet = self.resolve_runtime_data(self[res.name].t)
                     new_snippet = self.resolve_runtime_data(res.t)
-                    if old_snippet != new_snippet:
 
+                    if old_snippet != new_snippet:
                         # Can fail if underlying resource class does not
                         # implement update logic or update requires replacement
                         retval = self[res.name].update(new_snippet)
@@ -346,37 +349,35 @@ class Stack(object):
                             # Resource requires replacement for update
                             result = self[res.name].destroy()
                             if result:
-                                failures.append('Resource %s delete failed'
-                                                % res.name)
+                                logger.error("Failed to delete %s : %s" %
+                                             (res.name, result))
+                                raise exception.ResourceUpdateFailed(
+                                    resource_name=res.name)
                             else:
                                 res.stack = self
                                 self[res.name] = res
                                 result = self[res.name].create()
                                 if result:
-                                    failures.append('Resource %s create failed'
-                                                    % res.name)
+                                    logger.error("Failed to create %s : %s" %
+                                                 (res.name, result))
+                                    raise exception.ResourceUpdateFailed(
+                                        resource_name=res.name)
                         else:
-                            logger.warning("Cannot update resource %s," %
-                                           res.name + " reason %s" % retval)
-                            failures.append('Resource %s update failed'
-                                            % res.name)
-
-                # Set stack status values
-                if not failures:
-                    # flip the template & parameters to the newstack values
-                    self.t = newstack.t
-                    self.parameters = newstack.parameters
-                    template_outputs = self.t[template.OUTPUTS]
-                    self.outputs = self.resolve_static_data(template_outputs)
-                    self.dependencies = self._get_dependencies(
-                        self.resources.itervalues())
-                    self.store()
-
-                    stack_status = self.UPDATE_COMPLETE
-                    reason = 'Stack successfully updated'
-                else:
-                    stack_status = self.UPDATE_FAILED
-                    reason = ",".join(failures)
+                            logger.error("Failed to update %s" % res.name)
+                            raise exception.ResourceUpdateFailed(
+                                resource_name=res.name)
+
+                # flip the template & parameters to the newstack values
+                self.t = newstack.t
+                self.parameters = newstack.parameters
+                template_outputs = self.t[template.OUTPUTS]
+                self.outputs = self.resolve_static_data(template_outputs)
+                self.dependencies = self._get_dependencies(
+                    self.resources.itervalues())
+                self.store()
+
+                stack_status = self.UPDATE_COMPLETE
+                reason = 'Stack successfully updated'
 
             except eventlet.Timeout as t:
                 if t is tmo:
@@ -385,6 +386,9 @@ class Stack(object):
                 else:
                     # not my timeout
                     raise
+            except exception.ResourceUpdateFailed as e:
+                stack_status = self.UPDATE_FAILED
+                reason = str(e) or "Error : %s" % type(e)
 
         self.state_set(stack_status, reason)
 
index b0a899b076f563fb1ea8585adfc9ced45630d5e9..83b458cc3737a6af840f3f3733e928eb54a17496 100644 (file)
@@ -20,10 +20,10 @@ import mox
 from heat.common import context
 from heat.common import exception
 from heat.common import template_format
+from heat.engine import resource
 from heat.engine import parser
 from heat.engine import parameters
 from heat.engine import template
-from heat.engine.resource import Resource
 from heat.tests.utils import stack_delete_after
 import heat.db as db_api
 
@@ -188,8 +188,8 @@ class TemplateTest(unittest.TestCase):
                           snippet, params)
 
     def test_resource_refs(self):
-        resources = {'foo': self.m.CreateMock(Resource),
-                     'blarg': self.m.CreateMock(Resource)}
+        resources = {'foo': self.m.CreateMock(resource.Resource),
+                     'blarg': self.m.CreateMock(resource.Resource)}
         resources['foo'].FnGetRefId().AndReturn('bar')
         self.m.ReplayAll()
 
@@ -404,3 +404,209 @@ class StackTest(unittest.TestCase):
         db_s = db_api.stack_get(self.ctx, stack_id)
         self.assertNotEqual(db_s, None)
         self.assertEqual(self.stack.state, self.stack.DELETE_FAILED)
+
+    @stack_delete_after
+    def test_update_badstate(self):
+        self.stack = parser.Stack(self.ctx, 'test_stack', parser.Template({}),
+                                  state=parser.Stack.CREATE_FAILED)
+        stack_id = self.stack.store()
+        self.assertEqual(self.stack.state, parser.Stack.CREATE_FAILED)
+        self.stack.update({})
+        self.assertEqual(self.stack.state, parser.Stack.UPDATE_FAILED)
+
+    @stack_delete_after
+    def test_update_add(self):
+        tmpl = {'Resources': {'AResource': {'Type': 'GenericResourceType'}}}
+
+        self.stack = parser.Stack(self.ctx, 'update_test_stack',
+                                  template.Template(tmpl))
+        self.stack.store()
+        self.stack.create()
+        self.assertEqual(self.stack.state, parser.Stack.CREATE_COMPLETE)
+
+        tmpl2 = {'Resources': {
+                 'AResource': {'Type': 'GenericResourceType'},
+                 'BResource': {'Type': 'GenericResourceType'}}}
+        updated_stack = parser.Stack(self.ctx, 'updated_stack',
+                                     template.Template(tmpl2))
+        self.stack.update(updated_stack)
+        self.assertEqual(self.stack.state, parser.Stack.UPDATE_COMPLETE)
+        self.assertTrue('BResource' in self.stack)
+
+    @stack_delete_after
+    def test_update_remove(self):
+        tmpl = {'Resources': {
+                'AResource': {'Type': 'GenericResourceType'},
+                'BResource': {'Type': 'GenericResourceType'}}}
+
+        self.stack = parser.Stack(self.ctx, 'update_test_stack',
+                                  template.Template(tmpl))
+        self.stack.store()
+        self.stack.create()
+        self.assertEqual(self.stack.state, parser.Stack.CREATE_COMPLETE)
+
+        tmpl2 = {'Resources': {'AResource': {'Type': 'GenericResourceType'}}}
+
+        updated_stack = parser.Stack(self.ctx, 'updated_stack',
+                                     template.Template(tmpl2))
+        self.stack.update(updated_stack)
+        self.assertEqual(self.stack.state, parser.Stack.UPDATE_COMPLETE)
+        self.assertFalse('BResource' in self.stack)
+
+    @stack_delete_after
+    def test_update_description(self):
+        tmpl = {'Description': 'ATemplate',
+                'Resources': {'AResource': {'Type': 'GenericResourceType'}}}
+
+        self.stack = parser.Stack(self.ctx, 'update_test_stack',
+                                  template.Template(tmpl))
+        self.stack.store()
+        self.stack.create()
+        self.assertEqual(self.stack.state, parser.Stack.CREATE_COMPLETE)
+
+        tmpl2 = {'Description': 'BTemplate',
+                 'Resources': {'AResource': {'Type': 'GenericResourceType'}}}
+
+        updated_stack = parser.Stack(self.ctx, 'updated_stack',
+                                     template.Template(tmpl2))
+        self.stack.update(updated_stack)
+        self.assertEqual(self.stack.state, parser.Stack.UPDATE_COMPLETE)
+        self.assertEqual(self.stack.t[template.DESCRIPTION], 'BTemplate')
+
+    @stack_delete_after
+    def test_update_modify_ok_replace(self):
+        # patch in a dummy property schema for GenericResource
+        dummy_schema = {'Foo': {'Type': 'String'}}
+        resource.GenericResource.properties_schema = dummy_schema
+
+        tmpl = {'Resources': {'AResource': {'Type': 'GenericResourceType',
+                                            'Properties': {'Foo': 'abc'}}}}
+
+        self.stack = parser.Stack(self.ctx, 'update_test_stack',
+                                  template.Template(tmpl))
+        self.stack.store()
+        self.stack.create()
+        self.assertEqual(self.stack.state, parser.Stack.CREATE_COMPLETE)
+
+        tmpl2 = {'Resources': {'AResource': {'Type': 'GenericResourceType',
+                                             'Properties': {'Foo': 'xyz'}}}}
+
+        updated_stack = parser.Stack(self.ctx, 'updated_stack',
+                                     template.Template(tmpl2))
+        # patch in a dummy handle_update
+        self.m.StubOutWithMock(resource.GenericResource, 'handle_update')
+        resource.GenericResource.handle_update(
+            tmpl2['Resources']['AResource']).AndReturn(
+                resource.Resource.UPDATE_REPLACE)
+        self.m.ReplayAll()
+
+        self.stack.update(updated_stack)
+        self.assertEqual(self.stack.state, parser.Stack.UPDATE_COMPLETE)
+        self.assertEqual(self.stack['AResource'].properties['Foo'], 'xyz')
+        self.m.VerifyAll()
+
+    @stack_delete_after
+    def test_update_modify_update_failed(self):
+        # patch in a dummy property schema for GenericResource
+        dummy_schema = {'Foo': {'Type': 'String'}}
+        resource.GenericResource.properties_schema = dummy_schema
+
+        tmpl = {'Resources': {'AResource': {'Type': 'GenericResourceType',
+                                            'Properties': {'Foo': 'abc'}}}}
+
+        self.stack = parser.Stack(self.ctx, 'update_test_stack',
+                                  template.Template(tmpl))
+        self.stack.store()
+        self.stack.create()
+        self.assertEqual(self.stack.state, parser.Stack.CREATE_COMPLETE)
+
+        tmpl2 = {'Resources': {'AResource': {'Type': 'GenericResourceType',
+                                             'Properties': {'Foo': 'xyz'}}}}
+
+        updated_stack = parser.Stack(self.ctx, 'updated_stack',
+                                     template.Template(tmpl2))
+
+        # patch in a dummy handle_update
+        self.m.StubOutWithMock(resource.GenericResource, 'handle_update')
+        resource.GenericResource.handle_update(
+            tmpl2['Resources']['AResource']).AndReturn(
+                resource.Resource.UPDATE_FAILED)
+        self.m.ReplayAll()
+
+        self.stack.update(updated_stack)
+        self.assertEqual(self.stack.state, parser.Stack.UPDATE_FAILED)
+        self.m.VerifyAll()
+
+    @stack_delete_after
+    def test_update_modify_replace_failed_delete(self):
+        # patch in a dummy property schema for GenericResource
+        dummy_schema = {'Foo': {'Type': 'String'}}
+        resource.GenericResource.properties_schema = dummy_schema
+
+        tmpl = {'Resources': {'AResource': {'Type': 'GenericResourceType',
+                                            'Properties': {'Foo': 'abc'}}}}
+
+        self.stack = parser.Stack(self.ctx, 'update_test_stack',
+                                  template.Template(tmpl))
+        self.stack.store()
+        self.stack.create()
+        self.assertEqual(self.stack.state, parser.Stack.CREATE_COMPLETE)
+
+        tmpl2 = {'Resources': {'AResource': {'Type': 'GenericResourceType',
+                                             'Properties': {'Foo': 'xyz'}}}}
+
+        updated_stack = parser.Stack(self.ctx, 'updated_stack',
+                                     template.Template(tmpl2))
+
+        # patch in a dummy handle_update
+        self.m.StubOutWithMock(resource.GenericResource, 'handle_update')
+        resource.GenericResource.handle_update(
+            tmpl2['Resources']['AResource']).AndReturn(
+                resource.Resource.UPDATE_REPLACE)
+
+        # make the update fail deleting the existing resource
+        self.m.StubOutWithMock(resource.Resource, 'destroy')
+        resource.Resource.destroy().AndReturn("Error")
+        self.m.ReplayAll()
+
+        self.stack.update(updated_stack)
+        self.assertEqual(self.stack.state, parser.Stack.UPDATE_FAILED)
+        self.m.VerifyAll()
+        # Unset here so destroy() is not stubbed for stack.delete cleanup
+        self.m.UnsetStubs()
+
+    @stack_delete_after
+    def test_update_modify_replace_failed_create(self):
+        # patch in a dummy property schema for GenericResource
+        dummy_schema = {'Foo': {'Type': 'String'}}
+        resource.GenericResource.properties_schema = dummy_schema
+
+        tmpl = {'Resources': {'AResource': {'Type': 'GenericResourceType',
+                                            'Properties': {'Foo': 'abc'}}}}
+
+        self.stack = parser.Stack(self.ctx, 'update_test_stack',
+                                  template.Template(tmpl))
+        self.stack.store()
+        self.stack.create()
+        self.assertEqual(self.stack.state, parser.Stack.CREATE_COMPLETE)
+
+        tmpl2 = {'Resources': {'AResource': {'Type': 'GenericResourceType',
+                                             'Properties': {'Foo': 'xyz'}}}}
+
+        updated_stack = parser.Stack(self.ctx, 'updated_stack',
+                                     template.Template(tmpl2))
+
+        # patch in a dummy handle_update
+        self.m.StubOutWithMock(resource.GenericResource, 'handle_update')
+        resource.GenericResource.handle_update(
+            tmpl2['Resources']['AResource']).AndReturn(
+                resource.Resource.UPDATE_REPLACE)
+
+        # patch in a dummy handle_create making the replace fail creating
+        self.m.StubOutWithMock(resource.GenericResource, 'handle_create')
+        resource.GenericResource.handle_create().AndRaise(Exception)
+        self.m.ReplayAll()
+
+        self.stack.update(updated_stack)
+        self.assertEqual(self.stack.state, parser.Stack.UPDATE_FAILED)
+        self.m.VerifyAll()