Copy correct definition to the backup stack

We were using the definition of the existing resource, immediately after
updating it in place, to copy to the backup stack. This turns out to be
incorrect, because the definition of the new resource is only copied to the
existing resource if it was actually updated. It doesn't follow that the
existing resource's definition must be the same, because it's actually the
frozen definition (using the stored property values from the DB) that's
compared with the new definition when deciding to update. The existing
definition might well be rubbish if a previous update is failed, because it
does not get updated in the existing template after the resource is updated
in place. This caused bug 1622795 when combined with the new conditionals
feature.

We were also using the wrong template to do the parsing in both cases: when
updating in-place we are passing the new definition, but we were reparsing
using the existing template. And we were effectively doing the same when
creating a new resource, because we accessed the template through the new
resource only *after* it had been moved to the existing stack. In both
cases the correct code would have been:

  definition = new_res.t.reparse(self.previous_stack,
                                 new_res.stack.t)

However, there's no actual need to reparse the resource definitions before
passing them to Template.add_resource(), because that method immediately
calls render_hot() to get back to the unparsed definition anyway. So just
don't bother. These issues have been requiring us up until now to cache the
conditionals from the new_stack in the Template, to prevent the reparsing
from looking at the wrong set of conditionals. This change relieves us of
that restriction.

Finally, change the debug error messages to distinguish this case (copying
a resource definition into the backup template) from "backing up" a
resource (moving an actual physical resource into the backup stack).

Change-Id: I7be92f2e1b812c23fa52d87c18c7f22f1be94446
Closes-Bug: #1622795
This commit is contained in:
Zane Bitter 2016-09-12 20:51:52 -04:00
parent 6d61a7a7ff
commit 7b129f666f
2 changed files with 90 additions and 8 deletions

View File

@ -129,10 +129,8 @@ class StackUpdate(object):
# can have if it was copied to backup stack
if (res_name not in
self.previous_stack.t[self.previous_stack.t.RESOURCES]):
LOG.debug("Backing up new Resource %s" % res_name)
definition = new_res.t.reparse(self.previous_stack,
new_res.stack.t)
self.previous_stack.t.add_resource(definition)
LOG.debug("Storing definition of new Resource %s", res_name)
self.previous_stack.t.add_resource(new_res.t)
self.previous_stack.t.store(self.previous_stack.context)
yield new_res.create()
@ -167,10 +165,9 @@ class StackUpdate(object):
# Save updated resource definition to backup stack
# cause it allows the backup stack resources to be
# synchronized
LOG.debug("Backing up updated Resource %s" % res_name)
definition = existing_res.t.reparse(self.previous_stack,
existing_res.stack.t)
self.previous_stack.t.add_resource(definition)
LOG.debug("Storing definition of updated Resource %s",
res_name)
self.previous_stack.t.add_resource(new_res.t)
self.previous_stack.t.store(self.previous_stack.context)
LOG.info(_LI("Resource %(res_name)s for stack "

View File

@ -160,6 +160,84 @@ outputs:
'no_prod_res']}
'''
before_rename_tmpl = '''
heat_template_version: 2016-10-14
parameters:
env_type:
default: test
type: string
conditions:
cd1: {equals : [{get_param: env_type}, "prod"]}
resources:
test:
type: OS::Heat::TestResource
properties:
value: {if: [cd1, 'prod', 'test']}
'''
after_rename_tmpl = '''
heat_template_version: 2016-10-14
parameters:
env_type:
default: prod
type: string
conditions:
cd2: {equals : [{get_param: env_type}, "prod"]}
resources:
test:
type: OS::Heat::TestResource
properties:
value: {if: [cd2, 'prod', 'test']}
test2:
type: OS::Heat::TestResource
properties:
value: {if: [cd2, 'prod', 'test']}
'''
fail_rename_tmpl = '''
heat_template_version: 2016-10-14
parameters:
env_type:
default: prod
type: string
conditions:
cd3: {equals : [{get_param: env_type}, "prod"]}
resources:
test:
type: OS::Heat::TestResource
properties:
value: {if: [cd3, 'prod', 'test']}
test2:
type: OS::Heat::TestResource
properties:
value: {if: [cd3, 'prod', 'test']}
test_fail:
type: OS::Heat::TestResource
properties:
fail: True
depends_on: [test, test2]
'''
recover_rename_tmpl = '''
heat_template_version: 2016-10-14
parameters:
env_type:
default: prod
type: string
conditions:
cd3: {equals : [{get_param: env_type}, "prod"]}
resources:
test2:
type: OS::Heat::TestResource
properties:
value: {if: [cd3, 'prod', 'test']}
test_fail:
type: OS::Heat::TestResource
properties:
fail: False
depends_on: [test2]
'''
class CreateUpdateResConditionTest(functional_base.FunctionalTestsBase):
@ -363,3 +441,10 @@ class CreateUpdateResConditionTest(functional_base.FunctionalTestsBase):
resources = self.client.resources.list(stack_identifier)
self.res_assert_for_test(resources)
self.output_assert_for_test(stack_identifier)
def test_condition_rename(self):
stack_identifier = self.stack_create(template=before_rename_tmpl)
self.update_stack(stack_identifier, template=after_rename_tmpl)
self.update_stack(stack_identifier, template=fail_rename_tmpl,
expected_status='UPDATE_FAILED')
self.update_stack(stack_identifier, template=recover_rename_tmpl)