Merge "Make tags handling more robust"

heat/engine/api.py

@@ -221,7 +221,7 @@ def format_stack(stack, preview=False, resolve_outputs=True):
         rpc_api.STACK_OWNER: stack.username,
         rpc_api.STACK_PARENT: stack.owner_id,
         rpc_api.STACK_USER_PROJECT_ID: stack.stack_user_project_id,
-        rpc_api.STACK_TAGS: stack.tags,
+        rpc_api.STACK_TAGS: stack.tags or None,
     }
 
     if not preview:

heat/engine/service.py

@@ -896,7 +896,7 @@ class EngineService(service.ServiceBase):
         # stack definition. If PARAM_EXISTING is specified, we merge
         # any environment provided into the existing one and attempt
         # to use the existing stack template, if one is not provided.
-        if args.get(rpc_api.PARAM_EXISTING):
+        if args.get(rpc_api.PARAM_EXISTING, False):
             assert template_id is None, \
                 "Cannot specify template_id with PARAM_EXISTING"
 
@@ -976,9 +976,9 @@ class EngineService(service.ServiceBase):
         common_params.setdefault(rpc_api.PARAM_CONVERGE,
                                  current_stack.converge)
 
-        if args.get(rpc_api.PARAM_EXISTING):
+        if args.get(rpc_api.PARAM_EXISTING, False):
-            common_params.setdefault(rpc_api.STACK_TAGS,
+            if rpc_api.STACK_TAGS not in common_params:
-                                     current_stack.tags)
+                common_params[rpc_api.STACK_TAGS] = current_stack.tags
         current_kwargs.update(common_params)
         updated_stack = parser.Stack(cnxt, stack_name, tmpl,
                                      **current_kwargs)

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(
+            if self.id is not None:
-                self.context, self.id)
+                tags = stack_tag_object.StackTagList.get(self.context, self.id)
-            if tags:
+                self._tags = [t.tag for t in tags] if tags else []
-                self._tags = [t.tag for t in tags]
+            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:
+        self._store_tags()
-            stack_tag_object.StackTagList.set(self.context, self.id, self.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 is already store()d in IN_PROGRESS state, so write tags now
-                stack_tag_object.StackTagList.set(self.context, self.id,
+            # otherwise new set won't appear until COMPLETE/FAILED.
-                                                  newstack.tags)
+            self._store_tags()
-            else:
-                stack_tag_object.StackTagList.delete(self.context, self.id)
 
             check_message = functools.partial(self._check_for_message,
                                               msg_queue)

heat/tests/engine/service/test_stack_update.py

@@ -396,7 +396,7 @@ resources:
         del api_args[rpc_api.STACK_TAGS]
         _, _, updated_stack = self.man._prepare_stack_updates(
             self.ctx, stk, t, {}, None, None, None, api_args, None)
-        self.assertIsNone(updated_stack.tags)
+        self.assertEqual([], updated_stack.tags)
 
     def test_stack_update_existing_registry(self):
         # Use a template with existing flag and ensure the

heat/tests/test_stack.py

@@ -1407,7 +1407,8 @@ class StackTest(common.HeatTestCase):
         self.stack.store()
         stack_id = self.stack.id
         test_stack = stack.Stack.load(self.ctx, stack_id=stack_id)
-        self.assertIsNone(test_stack.tags)
+        self.assertIsNone(test_stack._tags)
+        self.assertEqual([], test_stack.tags)
 
         self.stack = stack.Stack(self.ctx, 'stack_name', self.tmpl)
         self.stack.tags = ['tag1', 'tag2']
@@ -1424,7 +1425,7 @@ class StackTest(common.HeatTestCase):
         self.stack.store()
         stack_id = self.stack.id
         test_stack = stack.Stack.load(self.ctx, stack_id=stack_id)
-        self.assertIsNone(test_stack.tags)
+        self.assertEqual([], test_stack.tags)
 
         self.stack = stack.Stack(self.ctx, 'stack_name', self.tmpl,
                                  tags=['tag1', 'tag2'])

heat/tests/test_stack_update.py

@@ -234,7 +234,7 @@ class StackUpdateTest(common.HeatTestCase):
         self.stack.update(updated_stack)
         self.assertEqual((stack.Stack.UPDATE, stack.Stack.COMPLETE),
                          self.stack.state)
-        self.assertIsNone(self.stack.tags)
+        self.assertEqual([], self.stack.tags)
 
     def test_update_modify_ok_replace(self):
         tmpl = {'HeatTemplateFormatVersion': '2012-12-12',