openstack/heat
Code Issues Proposed changes

Make ResourceDefinition round-trip stable to avoid extra writes

Browse Source 
The part of a ResourceDefinition that lists explicit dependencies was not
round-trip stable. As a result, when we copied a new resource definition
into the existing template during a stack update, we would end up rewriting
the template unnecesarily (i.e. even though we check for changes) every
time if depends_on was not specified in the resource originally. At the end
of each update, we write the new template to the DB in its entirety, which
removes these extra lines again, ensuring that we will experience the same
problem on every update. This was causing a *lot* of unnecessary writes.

This change ensures that the definition remains stable across a round-trip,
so that no unnecessary changes appear in the template.

Change-Id: If7292e49755db0153d7d0db9f7d3875fa9c1d408
Closes-Bug: #1494108
This commit is contained in:
Zane Bitter 2015-09-10 17:13:33 -04:00
parent e7b89a485f
commit a69431ab6c
7 changed files with 22 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
heat/engine/cfn/template.py
View File

@ -144,9 +144,7 @@ class CfnTemplate(template.Template):
data = self.parse(stack, snippet)
depends = data.get(RES_DEPENDS_ON)
if not depends:
depends = []
elif isinstance(depends, six.string_types):
if isinstance(depends, six.string_types):
depends = [depends]
kwargs = {

4
heat/engine/hot/template.py
View File

@ -247,9 +247,7 @@ class HOTemplate20130523(template.Template):
data = self.parse(stack, snippet)
depends = data.get(RES_DEPENDS_ON)
if not depends:
depends = []
elif isinstance(depends, six.string_types):
if isinstance(depends, six.string_types):
depends = [depends]
kwargs = {

13
heat/engine/rsrc_defn.py
View File

@ -55,7 +55,6 @@ class ResourceDefinitionCore(object):
:param update_policy: A dictionary of supplied update policies
:param description: A string describing the resource
"""
depends = depends or []
self.name = name
self.resource_type = resource_type
self.description = description or ''
@ -80,10 +79,11 @@ class ResourceDefinitionCore(object):
function.Function))
self._hash ^= _hash_data(metadata)
assert isinstance(depends, (collections.Sequence,
function.Function))
assert not isinstance(depends, six.string_types)
self._hash ^= _hash_data(depends)
if depends is not None:
assert isinstance(depends, (collections.Sequence,
function.Function))
assert not isinstance(depends, six.string_types)
self._hash ^= _hash_data(depends)
if deletion_policy is not None:
assert deletion_policy in self.DELETION_POLICIES
@ -170,7 +170,8 @@ class ResourceDefinitionCore(object):
True),
function.dependencies(data, datapath))
return itertools.chain((get_resource(dep) for dep in self._depends),
explicit_depends = [] if self._depends is None else self._depends
return itertools.chain((get_resource(dep) for dep in explicit_depends),
strict_func_deps(self._properties,
path(PROPERTIES)),
strict_func_deps(self._metadata,

2
heat/tests/test_hot.py
View File

@ -992,6 +992,8 @@ class HOTemplateTest(common.HeatTestCase):
deletion_policy: Retain
update_policy:
foo: bar
resource2:
type: AWS::EC2::Instance
''')
source = template.Template(hot_tpl)
empty = template.Template(copy.deepcopy(hot_tpl_empty))

6
heat/tests/test_resource_group.py
View File

@ -218,14 +218,12 @@ class ResourceGroupTest(common.HeatTestCase):
"heat_template_version": "2015-04-30",
"resources": {
"0": {
"depends_on": [],
"type": "OverwrittenFnGetRefIdType",
"properties": {
"foo": "bar"
}
},
"1": {
"depends_on": [],
"type": "OverwrittenFnGetRefIdType",
"properties": {
"foo": "baz"
@ -234,7 +232,6 @@ class ResourceGroupTest(common.HeatTestCase):
}
}
resource_def = {
"depends_on": [],
"type": "OverwrittenFnGetRefIdType",
"properties": {
"foo": "baz"
@ -252,14 +249,12 @@ class ResourceGroupTest(common.HeatTestCase):
"heat_template_version": "2015-04-30",
"resources": {
"0": {
"depends_on": [],
"type": "OverwrittenFnGetRefIdType",
"properties": {
"foo": "bar"
}
},
"1": {
"depends_on": [],
"type": "OverwrittenFnGetRefIdType",
"properties": {
"foo": "bar"
@ -268,7 +263,6 @@ class ResourceGroupTest(common.HeatTestCase):
}
}
resource_def = {
"depends_on": [],
"type": "OverwrittenFnGetRefIdType",
"properties": {
"foo": "baz"

9
heat/tests/test_rsrc_defn.py
View File

@ -144,6 +144,15 @@ class ResourceDefinitionTest(common.HeatTestCase):
self.assertEqual(expected_hot, rd.render_hot())
def test_render_hot_empty(self):
rd = rsrc_defn.ResourceDefinition('rsrc', 'SomeType')
expected_hot = {
'type': 'SomeType',
}
self.assertEqual(expected_hot, rd.render_hot())
def test_template_equality(self):
class FakeStack(object):
def __init__(self, params):

2
heat/tests/test_template.py
View File

@ -905,6 +905,8 @@ class TemplateTest(common.HeatTestCase):
DeletionPolicy: Retain
UpdatePolicy:
foo: bar
resource2:
Type: AWS::EC2::Instance
''')
source = template.Template(cfn_tpl)
empty = template.Template(copy.deepcopy(empty_template))