openstack
/
heat
Code Issues Proposed changes

Implement diffs of ResourceDefinitions 

The last remaining code using the deprecated methods in the
ResourceDefinition class to treat the definition like a CloudFormation
template snippet is the tmpl_diff in the Resource class. Replace the dict
diff with a Diff object that has an API which can be queried instead. For
backward compatibility with existing resource plugins, the current diff
objects are for now able to be treated like dicts. However, this is marked
deprecated and can be removed along with the equivalent support for
ResourceDefinitions in the future.

Change-Id: I9ef50f0d9b142d464a1b02da7a46536c532d2ebd
This commit is contained in:
Zane Bitter 2016-04-13 19:49:04 -04:00
parent d76e825b31
commit 75b56789e6
8 changed files with 230 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
heat/engine/resource.py
View File

@ -501,15 +501,7 @@ class Resource(object):
If something has been removed in after which exists in before we set it
to None.
"""
# Create a set containing the keys in both current and update template
template_keys = set(six.iterkeys(before))
template_keys.update(set(six.iterkeys(after)))
# Create a set of keys which differ (or are missing/added)
changed_keys_set = set([k for k in template_keys
if before.get(k) != after.get(k)])
return dict((k, after.get(k)) for k in changed_keys_set)
return after - before
def update_template_diff_properties(self, after_props, before_props):
"""Return changed Properties between the before and after properties.
@ -1035,13 +1027,11 @@ class Resource(object):
"""
if self._needs_update(after, before, after_props, before_props,
prev_resource, check_init_complete):
tmpl_diff = self.update_template_diff(function.resolve(after),
before)
tmpl_diff = self.update_template_diff(after.freeze(), before)
if tmpl_diff and self.needs_replace_with_tmpl_diff(tmpl_diff):
raise exception.UpdateReplace(self)
self.update_template_diff_properties(after_props,
before_props)
self.update_template_diff_properties(after_props, before_props)
return True
def _check_restricted_actions(self, actions, after, before,
@ -1140,8 +1130,7 @@ class Resource(object):
with self._action_recorder(action, exception.UpdateReplace):
after_props.validate()
tmpl_diff = self.update_template_diff(function.resolve(after),
before)
tmpl_diff = self.update_template_diff(after.freeze(), before)
if tmpl_diff and self.needs_replace_with_tmpl_diff(tmpl_diff):
raise exception.UpdateReplace(self)

90
heat/engine/rsrc_defn.py
View File

@ -31,6 +31,37 @@ __all__ = ['ResourceDefinition']
class ResourceDefinitionCore(object):
"""A definition of a resource, independent of any template format."""
class Diff(object):
"""A diff between two versions of the same resource definition."""
def __init__(self, old_defn, new_defn):
if not (isinstance(old_defn, ResourceDefinitionCore) and
isinstance(new_defn, ResourceDefinitionCore)):
raise TypeError
self.old_defn = old_defn
self.new_defn = new_defn
def properties_changed(self):
"""Return True if the resource properties have changed."""
return self.old_defn._properties != self.new_defn._properties
def metadata_changed(self):
"""Return True if the resource metadata has changed."""
return self.old_defn._metadata != self.new_defn._metadata
def update_policy_changed(self):
"""Return True if the resource update policy has changed."""
return self.old_defn._update_policy != self.new_defn._update_policy
def __bool__(self):
"""Return True if anything has changed."""
return (self.properties_changed() or
self.metadata_changed() or
self.update_policy_changed())
__nonzero__ = __bool__
DELETION_POLICIES = (
DELETE, RETAIN, SNAPSHOT,
) = (
@ -100,6 +131,9 @@ class ResourceDefinitionCore(object):
intrinsic functions). Named arguments passed to this method override
the values passed as arguments to the constructor.
"""
if getattr(self, '_frozen', False) and not overrides:
return self
def arg_item(attr_name):
name = attr_name.lstrip('_')
if name in overrides:
@ -227,6 +261,17 @@ class ResourceDefinitionCore(object):
return self._rendering
def __sub__(self, previous):
"""Calculate the difference between this definition and a previous one.
Return a Diff object that can be used to establish differences between
this definition and a previous definition of the same resource.
"""
if not isinstance(previous, ResourceDefinitionCore):
return NotImplemented
return self.Diff(previous, self)
def __eq__(self, other):
"""Compare this resource definition for equality with another.
@ -301,6 +346,51 @@ class ResourceDefinition(ResourceDefinitionCore, collections.Mapping):
'the ResourceDefinition API to work with the definition of the '
'resource instance.')
class Diff(ResourceDefinitionCore.Diff, collections.Mapping):
"""A resource definition diff that acts like a cfn template snippet.
This class exists only for backwards compatibility with existing
resource plugins and unit tests; it is deprecated and could be removed
as soon as the Ocata release. Prefer using the API directly rather than
treating the diff as a dict containing the differences between two cfn
template snippets.
"""
_deprecation_msg = (
'Reading the ResourceDefinition Diff as if it were a diff of two '
'snippets from CloudFormation templates is deprecated, and the '
'ability to treat it as such will be removed in the future. '
'Resource plugins should use the ResourceDefinition.Diff API and '
'the ResourceDefinition API to detect changes in the definition '
'and work with the new definition of the resource.')
def __contains__(self, key):
warnings.warn(self._deprecation_msg, DeprecationWarning)
if key == PROPERTIES:
return self.properties_changed()
elif key == METADATA:
return self.metadata_changed()
elif key == UPDATE_POLICY:
return self.update_policy_changed()
else:
return False
def __iter__(self):
return (k for k in _KEYS if k in self)
def __getitem__(self, key):
if key not in self:
raise KeyError
return self.new_defn.get(key)
def __len__(self):
return len(list(iter(self)))
def __repr__(self):
"""Return a string representation of the diff."""
return 'ResourceDefinition.Diff %s' % repr(dict(self))
def __eq__(self, other):
"""Compare this resource definition for equality with another.

3
heat/tests/autoscaling/test_heat_scaling_group.py
View File

@ -18,7 +18,6 @@ import six
from heat.common import exception
from heat.common import grouputils
from heat.common import template_format
from heat.engine import function
from heat.engine import rsrc_defn
from heat.tests.autoscaling import inline_templates
from heat.tests import common
@ -567,7 +566,7 @@ class RollingUpdatePolicyDiffTest(common.HeatTestCase):
# get the updated json snippet for the InstanceGroup resource in the
# context of the current stack
updated_grp = updated_stack['my-group']
updated_grp_json = function.resolve(updated_grp.t)
updated_grp_json = updated_grp.t.freeze()
# identify the template difference
tmpl_diff = updated_grp.update_template_diff(

3
heat/tests/autoscaling/test_scaling_group.py
View File

@ -20,7 +20,6 @@ from heat.common import exception
from heat.common import grouputils
from heat.common import template_format
from heat.engine.clients.os import nova
from heat.engine import function
from heat.engine import rsrc_defn
from heat.engine import scheduler
from heat.tests.autoscaling import inline_templates
@ -702,7 +701,7 @@ class RollingUpdatePolicyDiffTest(common.HeatTestCase):
# get the updated json snippet for the InstanceGroup resource in the
# context of the current stack
updated_grp = updated_stack['WebServerGroup']
updated_grp_json = function.resolve(updated_grp.t)
updated_grp_json = updated_grp.t.freeze()
# identify the template difference
tmpl_diff = updated_grp.update_template_diff(

1
heat/tests/openstack/heat/test_instance_group.py
View File

@ -331,7 +331,6 @@ class LoadbalancerReloadTest(common.HeatTestCase):
u'LoadBalancerPort': u'80',
u'Protocol': u'HTTP'}],
u'AvailabilityZones': ['abc', 'xyz']},
u'DeletionPolicy': 'Delete',
u'Metadata': {}}
stack = utils.parse_stack(t, params=inline_templates.as_params)

3
heat/tests/openstack/heat/test_instance_group_update_policy.py
View File

@ -17,7 +17,6 @@ import mock
from heat.common import exception
from heat.common import template_format
from heat.engine import function
from heat.engine.resources.openstack.heat import instance_group as instgrp
from heat.engine import rsrc_defn
from heat.tests import common
@ -241,7 +240,7 @@ class InstanceGroupTest(common.HeatTestCase):
# get the updated json snippet for the InstanceGroup resource in the
# context of the current stack
updated_grp = updated_stack['JobServerGroup']
updated_grp_json = function.resolve(updated_grp.t)
updated_grp_json = updated_grp.t.freeze()
# identify the template difference
tmpl_diff = updated_grp.update_template_diff(

6
heat/tests/openstack/heat/test_resource_group.py
View File

@ -19,7 +19,6 @@ import six
from heat.common import exception
from heat.common import grouputils
from heat.common import template_format
from heat.engine import function
from heat.engine.resources.openstack.heat import resource_group
from heat.engine import rsrc_defn
from heat.engine import scheduler
@ -1015,8 +1014,7 @@ class RollingUpdatePolicyDiffTest(common.HeatTestCase):
updated_stack = utils.parse_stack(updated)
updated_grp = updated_stack['group1']
updated_grp_json = function.resolve(
updated_grp.t)
updated_grp_json = updated_grp.t.freeze()
# identify the template difference
tmpl_diff = updated_grp.update_template_diff(
@ -1083,7 +1081,7 @@ class RollingUpdateTest(common.HeatTestCase):
'type': 'OverwrittenFnGetRefIdType'}})
updated_stack = utils.parse_stack(updated)
updated_grp = updated_stack['group1']
updated_grp_json = function.resolve(updated_grp.t)
updated_grp_json = updated_grp.t.freeze()
tmpl_diff = updated_grp.update_template_diff(
updated_grp_json, current_grp_json)

131
heat/tests/test_rsrc_defn.py
View File

@ -186,6 +186,76 @@ class ResourceDefinitionTest(common.HeatTestCase):
self.assertNotEqual(hash(rd1), hash(rd2))
class ResourceDefinitionDiffTest(common.HeatTestCase):
def test_properties_diff(self):
before = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'Foo': 'blarg'},
update_policy={'baz': 'quux'},
metadata={'baz': 'quux'})
after = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'Foo': 'wibble'},
update_policy={'baz': 'quux'},
metadata={'baz': 'quux'})
diff = after - before
self.assertTrue(diff.properties_changed())
self.assertFalse(diff.update_policy_changed())
self.assertFalse(diff.metadata_changed())
self.assertTrue(diff)
def test_update_policy_diff(self):
before = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'baz': 'quux'},
update_policy={'Foo': 'blarg'},
metadata={'baz': 'quux'})
after = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'baz': 'quux'},
update_policy={'Foo': 'wibble'},
metadata={'baz': 'quux'})
diff = after - before
self.assertFalse(diff.properties_changed())
self.assertTrue(diff.update_policy_changed())
self.assertFalse(diff.metadata_changed())
self.assertTrue(diff)
def test_metadata_diff(self):
before = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'baz': 'quux'},
update_policy={'baz': 'quux'},
metadata={'Foo': 'blarg'})
after = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'baz': 'quux'},
update_policy={'baz': 'quux'},
metadata={'Foo': 'wibble'})
diff = after - before
self.assertFalse(diff.properties_changed())
self.assertFalse(diff.update_policy_changed())
self.assertTrue(diff.metadata_changed())
self.assertTrue(diff)
def test_no_diff(self):
before = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'Foo': 'blarg'},
update_policy={'bar': 'quux'},
metadata={'baz': 'wibble'},
depends=['other_resource'],
deletion_policy='Delete')
after = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'Foo': 'blarg'},
update_policy={'bar': 'quux'},
metadata={'baz': 'wibble'},
depends=['other_other_resource'],
deletion_policy='Retain')
diff = after - before
self.assertFalse(diff.properties_changed())
self.assertFalse(diff.update_policy_changed())
self.assertFalse(diff.metadata_changed())
self.assertFalse(diff)
class ResourceDefinitionSnippetTest(common.HeatTestCase):
scenarios = [
('type',
@ -226,3 +296,64 @@ class ResourceDefinitionSnippetTest(common.HeatTestCase):
self.assertTrue(ws)
for warn in ws:
self.assertTrue(issubclass(warn.category, DeprecationWarning))
class ResourceDefinitionDiffSnippetTest(common.HeatTestCase):
def test_properties_diff(self):
before = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'Foo': 'blarg'},
update_policy={'baz': 'quux'},
metadata={'baz': 'quux'})
after = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'Foo': 'wibble'},
update_policy={'baz': 'quux'},
metadata={'baz': 'quux'})
diff = after - before
self.assertEqual({'Properties': after['Properties']}, dict(diff))
def test_update_policy_diff(self):
before = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'baz': 'quux'},
update_policy={'Foo': 'blarg'},
metadata={'baz': 'quux'})
after = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'baz': 'quux'},
update_policy={'Foo': 'wibble'},
metadata={'baz': 'quux'})
diff = after - before
self.assertEqual({'UpdatePolicy': after['UpdatePolicy']}, dict(diff))
def test_metadata_diff(self):
before = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'baz': 'quux'},
update_policy={'baz': 'quux'},
metadata={'Foo': 'blarg'})
after = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'baz': 'quux'},
update_policy={'baz': 'quux'},
metadata={'Foo': 'wibble'})
diff = after - before
self.assertEqual({'Metadata': after['Metadata']}, dict(diff))
def test_all_diff(self):
before = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'Foo': 'blarg'},
update_policy={'bar': 'quux'},
metadata={'baz': 'wibble'},
depends=['other_resource'],
deletion_policy='Delete')
after = rsrc_defn.ResourceDefinition('rsrc', 'SomeType',
properties={'Foo': 'wibble'},
update_policy={'bar': 'blarg'},
metadata={'baz': 'quux'},
depends=['other_other_resource'],
deletion_policy='Retain')
diff = after - before
self.assertEqual({'Properties': after['Properties'],
'UpdatePolicy': after['UpdatePolicy'],
'Metadata': after['Metadata']},
dict(diff))