From 78b7a471c2167326cb611f498eb875a04a1eef72 Mon Sep 17 00:00:00 2001
From: Zane Bitter <zbitter@redhat.com>
Date: Wed, 9 Oct 2019 16:36:43 -0400
Subject: Make tags handling more robust

Avoid loading the tags from the DB and then re-saving every time
the stack is stored when the stack has tags. Avoid attempting to
lazy-load the tags from the DB multiple times when there are no tags.
Avoid lazy-loading the existing tags on an update when we intend to
overwrite them anyway. Avoid writing the same set of tags multiple
times.

e.g. in a legacy update, previously we rewrote the current set of tags
when changing the state to IN_PROGRESS, then wrote the new set of tags
regardless of whether they had changed, then wrote the new set of tags
again when the update completed. In a convergence update we also did
three writes but in a different order, so that the new tags were written
every time. With this change we write the new set of tags only once.

This could also have prevented stacks with tags from being updated from
legacy to convergence, because the (unchanged) tag list would get
rewritten from inside a DB transaction. This is not expected so the
stack_tags_set() API does not pass subtransactions=True when creating a
transaction, which would cause a DB error.

Change-Id: Ia52818cfc9479d5fa6e3b236988694f47998acda
Task: 37001
---
 heat/engine/stack.py | 40 +++++++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 17 deletions(-)

(limited to 'heat/engine/stack.py')

diff --git a/heat/engine/stack.py b/heat/engine/stack.py
index f0887bbd6..f91f8d09f 100644
--- a/heat/engine/stack.py
+++ b/heat/engine/stack.py
@@ -173,6 +173,7 @@ class Stack(collections.Mapping):
         self._access_allowed_handlers = {}
         self._db_resources = None
         self._tags = tags
+        self._tags_stored = False
         self.adopt_stack_data = adopt_stack_data
         self.stack_user_project_id = stack_user_project_id
         self.created_time = created_time
@@ -182,7 +183,6 @@ class Stack(collections.Mapping):
         self.nested_depth = nested_depth
         self.convergence = convergence
         self.current_traversal = current_traversal
-        self.tags = tags
         self.prev_raw_template_id = prev_raw_template_id
         self.current_deps = current_deps
         self._worker_client = None
@@ -224,14 +224,17 @@ class Stack(collections.Mapping):
     @property
     def tags(self):
         if self._tags is None:
-            tags = stack_tag_object.StackTagList.get(
-                self.context, self.id)
-            if tags:
-                self._tags = [t.tag for t in tags]
+            if self.id is not None:
+                tags = stack_tag_object.StackTagList.get(self.context, self.id)
+                self._tags = [t.tag for t in tags] if tags else []
+            else:
+                self._tags = []
+            self._tags_stored = True
         return self._tags
 
     @tags.setter
     def tags(self, value):
+        self._tags_stored = (value == self._tags)
         self._tags = value
 
     @property
@@ -721,13 +724,23 @@ class Stack(collections.Mapping):
             self.id = new_s.id
             self.created_time = new_s.created_at
 
-        if self.tags:
-            stack_tag_object.StackTagList.set(self.context, self.id, self.tags)
+        self._store_tags()
 
         self._set_param_stackid()
 
         return self.id
 
+    def _store_tags(self):
+        if (self._tags is not None and
+                not self._tags_stored and
+                self.id is not None):
+            tags = self._tags
+            if tags:
+                stack_tag_object.StackTagList.set(self.context, self.id, tags)
+            else:
+                stack_tag_object.StackTagList.delete(self.context, self.id)
+            self._tags_stored = True
+
     def _backup_name(self):
         return '%s*' % self.name
 
@@ -1362,11 +1375,6 @@ class Stack(collections.Mapping):
             self._set_param_stackid()
 
             self.tags = new_stack.tags
-            if new_stack.tags:
-                stack_tag_object.StackTagList.set(self.context, self.id,
-                                                  new_stack.tags)
-            else:
-                stack_tag_object.StackTagList.delete(self.context, self.id)
 
         self.action = action
         self.status = self.IN_PROGRESS
@@ -1661,11 +1669,9 @@ class Stack(collections.Mapping):
             self._set_param_stackid()
 
             self.tags = newstack.tags
-            if newstack.tags:
-                stack_tag_object.StackTagList.set(self.context, self.id,
-                                                  newstack.tags)
-            else:
-                stack_tag_object.StackTagList.delete(self.context, self.id)
+            # Stack is already store()d in IN_PROGRESS state, so write tags now
+            # otherwise new set won't appear until COMPLETE/FAILED.
+            self._store_tags()
 
             check_message = functools.partial(self._check_for_message,
                                               msg_queue)
-- 
cgit v1.2.1