Merge "Convergence: Handling resouce clean-up"

heat/engine/resource.py

@@ -1047,21 +1047,13 @@ class Resource(object):
 
     def delete_convergence(self, template_id, resource_data, engine_id):
         '''
-        Deletes the resource by invoking the scheduler TaskRunner
+        Destroys the resource. The destroy task is run in a scheduler
-        and it persists the resource's current_template_id to template_id and
+        TaskRunner after acquiring the lock on resource.
-        resource's requires to list of the required resource id from the
-        given resource_data and existing resource's requires.
         '''
         with self.lock(engine_id):
-            runner = scheduler.TaskRunner(self.delete)
+            runner = scheduler.TaskRunner(self.destroy)
             runner()
 
-            # update the resource db record
-            self.current_template_id = template_id
-            current_requires = {graph_key[0]
-                                for graph_key, data in resource_data.items()}
-            self.requires = (list(set(self.requires) - current_requires))
-
     @scheduler.wrappertask
     def delete(self):
         '''
@@ -1233,14 +1225,16 @@ class Resource(object):
     def unlock(self, rsrc, engine_id, atomic_key):
         if atomic_key is None:
             atomic_key = 0
-        res = rsrc.select_and_update(
+
+        updated_ok = rsrc.select_and_update(
             {'engine_id': None,
              'current_template_id': self.current_template_id,
              'updated_at': self.updated_time,
              'requires': self.requires},
             expected_engine_id=engine_id,
             atomic_key=atomic_key + 1)
-        if res != 1:
+
+        if not updated_ok:
             LOG.warn(_LW('Failed to unlock resource %s'), rsrc.name)
 
     def _resolve_attribute(self, name):

heat/engine/stack.py

@@ -1609,9 +1609,10 @@ class Stack(collections.Mapping):
         1. Delete previous raw template if stack completes successfully.
         2. Deletes all sync points. They are no longer needed after stack
            has completed/failed.
-        3. Delete the stack is the action is DELETE.
+        3. Delete the stack if the action is DELETE.
        '''
-        if self.prev_raw_template_id is not None:
+        if (self.prev_raw_template_id is not None and
+                self.status != self.FAILED):
             prev_tmpl_id = self.prev_raw_template_id
             self.prev_raw_template_id = None
             self.store()

heat/engine/worker.py

@@ -203,7 +203,7 @@ class WorkerService(service.Service):
                     cnxt, stack.id, current_traversal, reason)
                 return
 
-        graph_key = (rsrc.id, is_update)
+        graph_key = (resource_id, is_update)
         if graph_key not in graph and rsrc.replaces is not None:
             # If we are a replacement, impersonate the replaced resource for
             # the purposes of calculating whether subsequent resources are
@@ -221,7 +221,7 @@ class WorkerService(service.Service):
                     input_data if fwd else None, fwd)
 
             check_stack_complete(cnxt, rsrc.stack, current_traversal,
-                                 rsrc.id, deps, is_update)
+                                 resource_id, deps, is_update)
         except sync_point.SyncPointNotFound:
             # Reload the stack to determine the current traversal, and check
             # the SyncPoint for the current node to determine if it is ready.

heat/tests/test_engine_service.py

@@ -40,6 +40,7 @@ from heat.engine import stack_lock
 from heat.engine import template as templatem
 from heat.engine import watchrule
 from heat.objects import event as event_object
+from heat.objects import raw_template as raw_template_object
 from heat.objects import resource as resource_objects
 from heat.objects import stack as stack_object
 from heat.objects import sync_point as sync_point_object
@@ -502,18 +503,25 @@ class StackConvergenceCreateUpdateDeleteTest(common.HeatTestCase):
         stack.mark_complete(stack.current_traversal)
         self.assertTrue(stack.purge_db.called)
 
-    def test_purge_db_deletes_previous_template(self, mock_cr):
+    @mock.patch.object(raw_template_object.RawTemplate, 'delete')
+    def test_purge_db_deletes_previous_template(self, mock_tmpl_delete,
+                                                mock_cr):
         stack = tools.get_stack('test_stack', utils.dummy_context(),
                                 template=tools.string_template_five,
                                 convergence=True)
-        prev_tmpl = templatem.Template.create_empty_template()
+        stack.prev_raw_template_id = 10
-        prev_tmpl.store()
-        stack.prev_raw_template_id = prev_tmpl.id
-        stack.store()
         stack.purge_db()
-        self.assertRaises(exception.NotFound,
+        self.assertTrue(mock_tmpl_delete.called)
-                          templatem.Template.load,
+
-                          stack.context, prev_tmpl.id)
+    @mock.patch.object(raw_template_object.RawTemplate, 'delete')
+    def test_purge_db_does_not_delete_previous_template_when_stack_fails(
+            self, mock_tmpl_delete, mock_cr):
+        stack = tools.get_stack('test_stack', utils.dummy_context(),
+                                template=tools.string_template_five,
+                                convergence=True)
+        stack.status = stack.FAILED
+        stack.purge_db()
+        self.assertFalse(mock_tmpl_delete.called)
 
     def test_purge_db_deletes_sync_points(self, mock_cr):
         stack = tools.get_stack('test_stack', utils.dummy_context(),
@@ -525,17 +533,16 @@ class StackConvergenceCreateUpdateDeleteTest(common.HeatTestCase):
             stack.context, stack.id, stack.current_traversal)
         self.assertEqual(0, rows)
 
-    def test_purge_db_deletes_stack_for_deleted_stack(self, mock_cr):
+    @mock.patch.object(stack_object.Stack, 'delete')
+    def test_purge_db_deletes_stack_for_deleted_stack(self, mock_stack_delete,
+                                                      mock_cr):
         stack = tools.get_stack('test_stack', utils.dummy_context(),
                                 template=tools.string_template_five,
                                 convergence=True)
         stack.store()
         stack.state_set(stack.DELETE, stack.COMPLETE, 'test reason')
         stack.purge_db()
-        self.assertRaises(exception.NotFound,
+        self.assertTrue(mock_stack_delete.called)
-                          parser.Stack.load,
-                          stack.context, stack_id=stack.id,
-                          show_deleted=False)
 
     def test_get_best_existing_db_resource(self, mock_cr):
         stack = tools.get_stack('test_stack', utils.dummy_context(),

heat/tests/test_resource.py

@@ -1479,21 +1479,17 @@ class ResourceTest(common.HeatTestCase):
                res.name)
         self.assertEqual(msg, six.text_type(ex))
 
-    @mock.patch.object(resource.Resource, 'delete')
+    def test_delete_convergence(self):
-    def test_delete_convergence(self, mock_delete):
         tmpl = rsrc_defn.ResourceDefinition('test_res', 'Foo')
         res = generic_rsrc.GenericResource('test_res', tmpl, self.stack)
         res.requires = [1, 2]
         res._store()
+        res.destroy = mock.Mock()
         self._assert_resource_lock(res.id, None, None)
         res.delete_convergence('template_key', {(1, True): {},
                                                 (1, True): {}},
                                'engine-007')
-
+        self.assertTrue(res.destroy.called)
-        mock_delete.assert_called_once_with()
-        self.assertEqual('template_key', res.current_template_id)
-        self.assertEqual([2], res.requires)
-        self._assert_resource_lock(res.id, None, 2)
 
     def test_delete_in_progress_convergence(self):
         tmpl = rsrc_defn.ResourceDefinition('test_res', 'Foo')