From 2e527e1191a2c78e508f297238310422b793be31 Mon Sep 17 00:00:00 2001
From: Thomas Herve <therve@redhat.com>
Date: Wed, 3 May 2017 16:40:28 +0200
Subject: [PATCH] Fix snapshot delete in convergence

The patch at I1cb321a3878a0abce9b41832f76bf77c25bf7cb4 properly deleted
the snapshots from the database, but as delete in convergence sets the
template to an empty template, stack.resources is empty. This works
around the problem by deleting the snapshots beforehand.

Related-Bug: #1508299
Change-Id: Id1c2c1a293fdcda07c527f29fedc00b716b303bc
(cherry picked from commit df1708b1a83f21f249aa08827fa88a25fc62c9e5)
---
 heat/engine/service.py         |  1 +
 heat/engine/stack.py           | 21 ++++++++-------------
 heat/tests/test_convg_stack.py | 32 --------------------------------
 3 files changed, 9 insertions(+), 45 deletions(-)

diff --git a/heat/engine/service.py b/heat/engine/service.py
index ab47c37771..7fbaf21b97 100644
--- a/heat/engine/service.py
+++ b/heat/engine/service.py
@@ -1387,6 +1387,7 @@ class EngineService(service.ServiceBase):
             def convergence_delete():
                 stack.thread_group_mgr = self.thread_group_mgr
                 self.worker_service.stop_all_workers(stack)
+                stack.delete_all_snapshots()
                 template = templatem.Template.create_empty_template(
                     from_template=stack.t)
                 stack.converge_stack(template=template, action=stack.DELETE)
diff --git a/heat/engine/stack.py b/heat/engine/stack.py
index 7e93a75849..30b23b369b 100644
--- a/heat/engine/stack.py
+++ b/heat/engine/stack.py
@@ -1282,14 +1282,6 @@ class Stack(collections.Mapping):
         LOG.info(_LI('convergence_dependencies: %s'),
                  self.convergence_dependencies)
 
-        # Delete all the snapshots before starting delete operation
-        if self.action == self.DELETE:
-            snapshots = snapshot_object.Snapshot.get_all(self.context,
-                                                         self.id)
-            for snapshot in snapshots:
-                self.delete_snapshot(snapshot)
-                snapshot_object.Snapshot.delete(self.context, snapshot.id)
-
         # create sync_points for resources in DB
         for rsrc_id, is_update in self.convergence_dependencies:
             sync_point.create(self.context, rsrc_id,
@@ -1770,11 +1762,7 @@ class Stack(collections.Mapping):
                                'Failed to %s : %s' % (action, failure))
                 return
 
-        snapshots = snapshot_object.Snapshot.get_all(self.context,
-                                                     self.id)
-        for snapshot in snapshots:
-            self.delete_snapshot(snapshot)
-            snapshot_object.Snapshot.delete(self.context, snapshot.id)
+        self.delete_all_snapshots()
 
         if not backup:
             try:
@@ -1886,6 +1874,13 @@ class Stack(collections.Mapping):
             pre_completion_func=save_snapshot_func)
         sus_task(timeout=self.timeout_secs())
 
+    def delete_all_snapshots(self):
+        """Remove all snapshots for this stack."""
+        snapshots = snapshot_object.Snapshot.get_all(self.context, self.id)
+        for snapshot in snapshots:
+            self.delete_snapshot(snapshot)
+            snapshot_object.Snapshot.delete(self.context, snapshot.id)
+
     @profiler.trace('Stack.delete_snapshot', hide_args=False)
     def delete_snapshot(self, snapshot):
         """Remove a snapshot from the backends."""
diff --git a/heat/tests/test_convg_stack.py b/heat/tests/test_convg_stack.py
index 003e8950bd..bcc80c4b82 100644
--- a/heat/tests/test_convg_stack.py
+++ b/heat/tests/test_convg_stack.py
@@ -21,7 +21,6 @@ from heat.engine import stack as parser
 from heat.engine import template as templatem
 from heat.objects import raw_template as raw_template_object
 from heat.objects import resource as resource_objects
-from heat.objects import snapshot as snapshot_objects
 from heat.objects import stack as stack_object
 from heat.objects import sync_point as sync_point_object
 from heat.rpc import worker_client
@@ -553,37 +552,6 @@ class StackConvergenceCreateUpdateDeleteTest(common.HeatTestCase):
         self.assertTrue(mock_syncpoint_del.called)
         self.assertTrue(mock_ccu.called)
 
-    def test_snapshot_delete(self, mock_cr):
-        tmpl = {'HeatTemplateFormatVersion': '2012-12-12',
-                'Resources': {'R1': {'Type': 'GenericResourceType'}}}
-        stack = parser.Stack(utils.dummy_context(), 'updated_time_test',
-                             templatem.Template(tmpl))
-        stack.current_traversal = 'prev_traversal'
-        stack.action, stack.status = stack.CREATE, stack.COMPLETE
-        stack.store()
-        stack.thread_group_mgr = tools.DummyThreadGroupManager()
-        snapshot_values = {
-            'stack_id': stack.id,
-            'name': 'fake_snapshot',
-            'tenant': stack.context.tenant_id,
-            'status': 'COMPLETE',
-            'data': None
-        }
-        snapshot_objects.Snapshot.create(stack.context, snapshot_values)
-
-        # Ensure that snapshot is not deleted on stack update
-        stack.converge_stack(template=stack.t, action=stack.UPDATE)
-        db_snapshot_obj = snapshot_objects.Snapshot.get_all(
-            stack.context, stack.id)
-        self.assertEqual('fake_snapshot', db_snapshot_obj[0].name)
-        self.assertEqual(stack.id, db_snapshot_obj[0].stack_id)
-
-        # Ensure that snapshot is deleted on stack delete
-        stack.converge_stack(template=stack.t, action=stack.DELETE)
-        self.assertEqual([], snapshot_objects.Snapshot.get_all(
-            stack.context, stack.id))
-        self.assertTrue(mock_cr.called)
-
 
 @mock.patch.object(parser.Stack, '_persist_state')
 class TestConvgStackStateSet(common.HeatTestCase):