Merge "Correctly initialize copies of stack during updating stack"

5 files changed, 212 insertions, 35 deletions

diff --git a/heat/engine/service.py b/heat/engine/service.py
index 08d65d53f..a1097db54 100644
--- a/heat/engine/service.py
+++ b/heat/engine/service.py
@@ -712,15 +712,17 @@ class EngineService(service.Service):
             raise exception.RequestLimitExceeded(
                 message=exception.StackResourceLimitExceeded.msg_fmt)
         stack_name = current_stack.name
-        convergence = current_stack.convergence
+        current_kwargs = current_stack.get_kwargs_for_cloning()
+
         common_params = api.extract_args(args)
         common_params.setdefault(rpc_api.PARAM_TIMEOUT,
                                  current_stack.timeout_mins)
         common_params.setdefault(rpc_api.PARAM_DISABLE_ROLLBACK,
                                  current_stack.disable_rollback)
+
+        current_kwargs.update(common_params)
         updated_stack = parser.Stack(cnxt, stack_name, tmpl,
-                                     convergence=convergence,
-                                     **common_params)
+                                     **current_kwargs)
         updated_stack.parameters.set_stack_id(current_stack.identifier())
 
         self._validate_deferred_auth_context(cnxt, updated_stack)
diff --git a/heat/engine/stack.py b/heat/engine/stack.py
index 7410c41f1..b5ed15163 100755
--- a/heat/engine/stack.py
+++ b/heat/engine/stack.py
@@ -388,31 +388,64 @@ class Stack(collections.Mapping):
                    username=stack.username, convergence=stack.convergence,
                    current_traversal=stack.current_traversal)
 
-    @profiler.trace('Stack.store', hide_args=False)
-    def store(self, backup=False):
-        '''
-        Store the stack in the database and return its ID
-        If self.id is set, we update the existing stack.
-        '''
-        s = {
-            'name': self._backup_name() if backup else self.name,
-            'raw_template_id': self.t.store(self.context),
+    def get_kwargs_for_cloning(self, keep_status=False, only_db=False):
+        """Get common kwargs for calling Stack() for cloning.
+
+        The point of this method is to reduce the number of places that we
+        need to update when a kwarg to Stack.__init__() is modified. It
+        is otherwise easy to forget an option and cause some unexpected
+        error if this option is lost.
+
+        Note:
+        - This doesn't return the args(name, template) but only the kwargs.
+        - We often want to start 'fresh' so don't want to maintain the old
+          status, action and status_reason.
+        - We sometimes only want the DB attributes.
+        """
+
+        stack = {
             'owner_id': self.owner_id,
             'username': self.username,
-            'tenant': self.tenant_id,
-            'action': self.action,
-            'status': self.status,
-            'status_reason': self.status_reason,
-            'timeout': self.timeout_mins,
+            'tenant_id': self.tenant_id,
+            'timeout_mins': self.timeout_mins,
             'disable_rollback': self.disable_rollback,
             'stack_user_project_id': self.stack_user_project_id,
-            'updated_at': self.updated_time,
             'user_creds_id': self.user_creds_id,
-            'backup': backup,
             'nested_depth': self.nested_depth,
             'convergence': self.convergence,
             'current_traversal': self.current_traversal,
         }
+        if keep_status:
+            stack.update({
+                'action': self.action,
+                'status': self.status,
+                'status_reason': self.status_reason})
+        if not only_db:
+            stack['parent_resource'] = self.parent_resource_name
+            stack['strict_validate'] = self.strict_validate
+
+        return stack
+
+    @profiler.trace('Stack.store', hide_args=False)
+    def store(self, backup=False):
+        '''
+        Store the stack in the database and return its ID
+        If self.id is set, we update the existing stack.
+        '''
+        s = self.get_kwargs_for_cloning(keep_status=True, only_db=True)
+        s['name'] = self._backup_name() if backup else self.name
+        s['backup'] = backup
+        s['updated_at'] = self.updated_time
+        if self.t.id is None:
+            s['raw_template_id'] = self.t.store(self.context)
+        else:
+            s['raw_template_id'] = self.t.id
+        # name inconsistencies
+        s['tenant'] = s['tenant_id']
+        del s['tenant_id']
+        s['timeout'] = s['timeout_mins']
+        del s['timeout_mins']
+
         if self.id:
             stack_object.Stack.update_by_id(self.context, self.id, s)
         else:
@@ -791,10 +824,10 @@ class Stack(collections.Mapping):
             LOG.debug('Loaded existing backup stack')
             return self.load(self.context, stack=s)
         elif create_if_missing:
+            kwargs = self.get_kwargs_for_cloning()
+            kwargs['owner_id'] = self.id
             prev = type(self)(self.context, self.name, copy.deepcopy(self.t),
-                              owner_id=self.id,
-                              user_creds_id=self.user_creds_id,
-                              convergence=self.convergence)
+                              **kwargs)
             prev.store(backup=True)
             LOG.debug('Created new backup stack')
             return prev
@@ -868,8 +901,10 @@ class Stack(collections.Mapping):
         if action == self.UPDATE:
             # Oldstack is useless when the action is not UPDATE , so we don't
             # need to build it, this can avoid some unexpected errors.
+            kwargs = self.get_kwargs_for_cloning()
             oldstack = Stack(self.context, self.name, copy.deepcopy(self.t),
-                             convergence=self.convergence)
+                             **kwargs)
+
         backup_stack = self._backup_stack()
         try:
             update_task = update.StackUpdate(
diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py
index 9ee5d7ba1..19045985e 100644
--- a/heat/tests/test_engine_service.py
+++ b/heat/tests/test_engine_service.py
@@ -1016,6 +1016,7 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase):
         template = '{ "Template": "data" }'
         old_stack = get_wordpress_stack(stack_name, self.ctx)
         sid = old_stack.store()
+        old_stack.set_stack_user_project_id('1234')
         s = stack_object.Stack.get_by_id(self.ctx, sid)
 
         stack = get_wordpress_stack(stack_name, self.ctx)
@@ -1027,8 +1028,18 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase):
         environment.Environment(params).AndReturn(stack.env)
         parser.Stack(self.ctx, stack.name,
                      stack.t,
-                     timeout_mins=60, disable_rollback=True,
-                     convergence=False).AndReturn(stack)
+                     convergence=False,
+                     current_traversal=None,
+                     disable_rollback=True,
+                     nested_depth=0,
+                     owner_id=None,
+                     parent_resource=None,
+                     stack_user_project_id='1234',
+                     strict_validate=True,
+                     tenant_id='test_tenant_id',
+                     timeout_mins=60,
+                     user_creds_id=u'1',
+                     username='test_username').AndReturn(stack)
 
         self.m.StubOutWithMock(stack, 'validate')
         stack.validate().AndReturn(None)
@@ -1061,6 +1072,7 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase):
 
         old_stack = get_wordpress_stack_no_params(stack_name, self.ctx)
         sid = old_stack.store()
+        old_stack.set_stack_user_project_id('1234')
         s = stack_object.Stack.get_by_id(self.ctx, sid)
 
         t = template_format.parse(wp_template_no_default)
@@ -1076,8 +1088,14 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase):
         environment.Environment(no_params).AndReturn(old_stack.env)
         parser.Stack(self.ctx, stack.name,
                      stack.t,
-                     timeout_mins=60, disable_rollback=True,
-                     convergence=False).AndReturn(stack)
+                     convergence=False, current_traversal=None,
+                     disable_rollback=True, nested_depth=0,
+                     owner_id=None, parent_resource=None,
+                     stack_user_project_id='1234',
+                     strict_validate=True,
+                     tenant_id='test_tenant_id', timeout_mins=60,
+                     user_creds_id=u'1',
+                     username='test_username').AndReturn(stack)
 
         self.m.StubOutWithMock(stack, 'validate')
         stack.validate().AndReturn(None)
@@ -1110,6 +1128,7 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase):
         old_stack.timeout_mins = 1
         old_stack.disable_rollback = False
         sid = old_stack.store()
+        old_stack.set_stack_user_project_id('1234')
         s = stack_object.Stack.get_by_id(self.ctx, sid)
 
         stack = get_wordpress_stack(stack_name, self.ctx)
@@ -1121,8 +1140,14 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase):
         environment.Environment(params).AndReturn(stack.env)
         parser.Stack(self.ctx, stack.name,
                      stack.t,
-                     timeout_mins=1, disable_rollback=False,
-                     convergence=False).AndReturn(stack)
+                     convergence=False, current_traversal=None,
+                     disable_rollback=False, nested_depth=0,
+                     owner_id=None, parent_resource=None,
+                     stack_user_project_id='1234',
+                     strict_validate=True,
+                     tenant_id='test_tenant_id', timeout_mins=1,
+                     user_creds_id=u'1',
+                     username='test_username').AndReturn(stack)
 
         self.m.StubOutWithMock(stack, 'validate')
         stack.validate().AndReturn(None)
@@ -1189,6 +1214,7 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase):
 
         old_stack = parser.Stack(self.ctx, stack_name, template)
         sid = old_stack.store()
+        old_stack.set_stack_user_project_id('1234')
         s = stack_object.Stack.get_by_id(self.ctx, sid)
 
         stack = parser.Stack(self.ctx, stack_name, template)
@@ -1200,8 +1226,13 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase):
         environment.Environment(params).AndReturn(stack.env)
         parser.Stack(self.ctx, stack.name,
                      stack.t,
-                     timeout_mins=60, disable_rollback=True,
-                     convergence=False).AndReturn(stack)
+                     convergence=False, current_traversal=None,
+                     disable_rollback=True, nested_depth=0,
+                     owner_id=None, parent_resource=None,
+                     stack_user_project_id='1234', strict_validate=True,
+                     tenant_id='test_tenant_id',
+                     timeout_mins=60, user_creds_id=u'1',
+                     username='test_username').AndReturn(stack)
 
         self.m.StubOutWithMock(stack, 'validate')
         stack.validate().AndReturn(None)
@@ -1313,6 +1344,7 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase):
         old_stack = get_wordpress_stack(stack_name, self.ctx)
         old_stack.store()
         sid = old_stack.store()
+        old_stack.set_stack_user_project_id('1234')
         s = stack_object.Stack.get_by_id(self.ctx, sid)
         stack = get_wordpress_stack(stack_name, self.ctx)
 
@@ -1323,8 +1355,13 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase):
         environment.Environment(params).AndReturn(stack.env)
         parser.Stack(self.ctx, stack.name,
                      stack.t,
-                     timeout_mins=60, disable_rollback=True,
-                     convergence=False).AndReturn(stack)
+                     convergence=False, current_traversal=None,
+                     disable_rollback=True, nested_depth=0,
+                     owner_id=None, parent_resource=None,
+                     stack_user_project_id='1234', strict_validate=True,
+                     tenant_id='test_tenant_id',
+                     timeout_mins=60, user_creds_id=u'1',
+                     username='test_username').AndReturn(stack)
 
         self.m.StubOutWithMock(stack, 'validate')
         stack.validate().AndRaise(exception.StackValidationFailed(
@@ -1367,6 +1404,7 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase):
         old_stack['WebServer'].requires_deferred_auth = True
 
         sid = old_stack.store()
+        old_stack.set_stack_user_project_id('1234')
         s = stack_object.Stack.get_by_id(self.ctx, sid)
 
         self.ctx = utils.dummy_context(password=None)
@@ -1382,8 +1420,18 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase):
         environment.Environment(params).AndReturn(old_stack.env)
         parser.Stack(self.ctx, old_stack.name,
                      old_stack.t,
-                     timeout_mins=60, disable_rollback=True,
-                     convergence=False).AndReturn(old_stack)
+                     convergence=False,
+                     current_traversal=None,
+                     disable_rollback=True,
+                     nested_depth=0,
+                     owner_id=None,
+                     parent_resource=None,
+                     stack_user_project_id='1234',
+                     strict_validate=True,
+                     tenant_id='test_tenant_id',
+                     timeout_mins=60,
+                     user_creds_id=u'1',
+                     username='test_username').AndReturn(old_stack)
 
         self.m.ReplayAll()
 
diff --git a/heat/tests/test_stack.py b/heat/tests/test_stack.py
index 4795e0f2a..d6a9a9a88 100644
--- a/heat/tests/test_stack.py
+++ b/heat/tests/test_stack.py
@@ -1773,3 +1773,45 @@ class StackTest(common.HeatTestCase):
 
         self.assertEqual(
             'foo', self.stack.resources['A'].properties['a_string'])
+
+
+class StackKwargsForCloningTest(common.HeatTestCase):
+    scenarios = [
+        ('default', dict(keep_status=False, only_db=False,
+                         not_included=['action', 'status', 'status_reason'])),
+        ('only_db', dict(keep_status=False, only_db=True,
+                         not_included=['action', 'status', 'status_reason',
+                                       'parent_resource',
+                                       'strict_validate'])),
+        ('keep_status', dict(keep_status=True, only_db=False,
+                             not_included=[])),
+        ('status_db', dict(keep_status=True, only_db=True,
+                           not_included=['parent_resource',
+                                         'strict_validate'])),
+    ]
+
+    def test_kwargs(self):
+        tmpl = template.Template(copy.deepcopy(empty_template))
+        ctx = utils.dummy_context()
+        test_data = dict(action='x', status='y',
+                         status_reason='z', timeout_mins=33,
+                         disable_rollback=True, parent_resource='fred',
+                         owner_id=32, stack_user_project_id=569,
+                         user_creds_id=123, tenant_id='some-uuid',
+                         username='jo', nested_depth=3,
+                         strict_validate=True, convergence=False,
+                         current_traversal=45)
+        self.stack = stack.Stack(ctx, utils.random_name(), tmpl,
+                                 **test_data)
+        res = self.stack.get_kwargs_for_cloning(keep_status=self.keep_status,
+                                                only_db=self.only_db)
+        for key in self.not_included:
+            self.assertNotIn(key, res)
+
+        for key in test_data:
+            if key not in self.not_included:
+                self.assertEqual(test_data[key], res[key])
+
+        # just make sure that the kwargs are valid
+        # (no exception should be raised)
+        stack.Stack(ctx, utils.random_name(), tmpl, **res)
diff --git a/heat_integrationtests/functional/test_update.py b/heat_integrationtests/functional/test_update.py
index 3904311ab..b329ddc04 100644
--- a/heat_integrationtests/functional/test_update.py
+++ b/heat_integrationtests/functional/test_update.py
@@ -49,6 +49,26 @@ resources:
         type: My::RandomString
 '''
 
+    update_userdata_template = '''
+heat_template_version: 2014-10-16
+parameters:
+  flavor:
+    type: string
+  user_data:
+    type: string
+  image:
+    type: string
+
+resources:
+  server:
+    type: OS::Nova::Server
+    properties:
+      image: {get_param: image}
+      flavor: {get_param: flavor}
+      user_data_format: SOFTWARE_CONFIG
+      user_data: {get_param: user_data}
+'''
+
     def setUp(self):
         super(UpdateStackTest, self).setUp()
         self.client = self.orchestration_client
@@ -188,3 +208,33 @@ resources:
                                   'random2': 'OS::Heat::RandomString'}
             self.assertEqual(provider_resources,
                              self.list_resources(provider_identifier))
+
+    def test_stack_update_with_replacing_userdata(self):
+        """Confirm that we can update userdata of instance during updating
+        stack by the user of member role.
+
+        Make sure that a resource that inherites from StackUser can be deleted
+        during updating stack.
+        """
+        if not self.conf.minimal_image_ref:
+            raise self.skipException("No minimal image configured to test")
+        if not self.conf.minimal_instance_type:
+            raise self.skipException("No flavor configured to test")
+
+        parms = {'flavor': self.conf.minimal_instance_type,
+                 'image': self.conf.minimal_image_ref,
+                 'user_data': ''}
+        name = self._stack_rand_name()
+
+        stack_identifier = self.stack_create(
+            stack_name=name,
+            template=self.update_userdata_template,
+            parameters=parms
+        )
+
+        parms_updated = parms
+        parms_updated['user_data'] = 'two'
+        self.update_stack(
+            stack_identifier,
+            template=self.update_userdata_template,
+            parameters=parms_updated)