diff --git a/heat/engine/service.py b/heat/engine/service.py index 41a07d3b2e..fb4b2dfa0a 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -710,15 +710,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 373f2fefcb..ba5fede427 100755 --- a/heat/engine/stack.py +++ b/heat/engine/stack.py @@ -374,31 +374,64 @@ class Stack(collections.Mapping): username=stack.username, convergence=stack.convergence, current_traversal=stack.current_traversal) + 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_id': self.tenant_id, + 'timeout_mins': self.timeout_mins, + 'disable_rollback': self.disable_rollback, + 'stack_user_project_id': self.stack_user_project_id, + 'user_creds_id': self.user_creds_id, + '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 = { - 'name': self._backup_name() if backup else self.name, - 'raw_template_id': self.t.store(self.context), - '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, - '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, - } + 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: @@ -772,10 +805,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 @@ -849,8 +882,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 cad96f533e..29f60e49d5 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 acf055b081..a8788b5f6b 100644 --- a/heat/tests/test_stack.py +++ b/heat/tests/test_stack.py @@ -1736,3 +1736,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 3904311ab6..b329ddc049 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)