openstack
/
heat
Code Issues Proposed changes

Don't RESOLVE translate in resource init 

We can completely remove 'RESOLVE' translations involving client
lookups in resource init. As we are doing translation before create
and update, it seems we can avoid it.

However, we would need other translations, like 'REPLACE' for property
vaidations(ex. required properties that would be replaced by deprecated
properties). This would be done only when strict_validate=True to avoid
this with template-validate.

Change-Id: Ie80bdd10726a8fc8a8787b78db93acffd333137f
Closes-Bug: #1554927
Partial-Bug: #1554380
This commit is contained in:
Rabi Mishra 2016-03-09 11:33:49 +05:30 committed by Kanagaraj Manickam
parent b86ef096f8
commit 0bf9463bfc
8 changed files with 64 additions and 102 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
heat/engine/resource.py
View File

@ -192,7 +192,9 @@ class Resource(object):
self.context = stack.context
self.name = name
self.t = definition
self.reparse()
# Only translate in cases where strict_validate is True
self.reparse(translate=self.stack.strict_validate,
client_resolve=False)
self.update_policy = self.t.update_policy(self.update_policy_schema,
self.context)
self.attributes_schema.update(self.base_attributes_schema)
@ -229,8 +231,6 @@ class Resource(object):
resource = stack.db_resource_get(name)
if resource:
self._load_data(resource)
else:
self.translate_properties(self.properties)
else:
self.action = stack.cache_data[name]['action']
self.status = stack.cache_data[name]['status']
@ -331,9 +331,17 @@ class Resource(object):
{'status': self.COMPLETE, 'replaced_by': self.replaced_by})
return new_rs.id
def reparse(self):
def reparse(self, translate=True, client_resolve=True):
"""Reparse the resource properties.
Optional translate flag for property translation and
client_resolve flag for resolving properties by doing
client lookup.
"""
self.properties = self.t.properties(self.properties_schema,
self.context)
if translate:
self.translate_properties(self.properties, client_resolve)
def __eq__(self, other):
"""Allow == comparison of two resources."""
@ -802,9 +810,10 @@ class Resource(object):
# Re-resolve the template, since if the resource Ref's
# the StackId pseudo parameter, it will change after
# the parser.Stack is stored (which is after the resources
# are __init__'d, but before they are create()'d)
# are __init__'d, but before they are create()'d). We also
# do client lookups for RESOLVE translation rules here.
self.reparse()
self.translate_properties(self.properties)
self._update_stored_properties()
def pause():
@ -898,11 +907,17 @@ class Resource(object):
def translation_rules(self, properties):
"""Return specified rules for resource."""
def translate_properties(self, properties):
"""Translates old properties to new ones."""
def translate_properties(self, properties,
client_resolve=True):
"""Translates properties with resource specific rules.
The properties parameter is a properties object and the
optional client_resolve flag is to specify whether to
do 'RESOLVE' translation with client lookup.
"""
rules = self.translation_rules(properties) or []
for rule in rules:
rule.execute_rule()
rule.execute_rule(client_resolve)
def _get_resource_info(self, resource_data):
if not resource_data:
@ -1119,10 +1134,8 @@ class Resource(object):
yield self.action_handler_task(action,
args=[after, tmpl_diff,
prop_diff])
self.t = after
self.reparse()
self.translate_properties(self.properties)
self._update_stored_properties()
except exception.ResourceActionRestricted as ae:

4
heat/engine/resources/openstack/heat/none_resource.py
View File

@ -36,8 +36,10 @@ class NoneResource(resource.Resource):
prev_resource, check_init_complete=True):
return False
def reparse(self):
def reparse(self, translate=True, client_resolve=True):
self.properties = properties.Properties(schema={}, data={})
if translate:
self.translate_properties(self.properties, client_resolve)
def handle_create(self):
self.resource_id_set(six.text_type(uuid.uuid4()))

4
heat/engine/translation.py
View File

@ -101,7 +101,7 @@ class TranslationRule(object):
raise ValueError(_('client_plugin and finder should be specified '
'for Resolve rule'))
def execute_rule(self):
def execute_rule(self, client_resolve=True):
try:
(source_key, source_data) = self._get_data_from_source_path(
self.source_path)
@ -130,7 +130,7 @@ class TranslationRule(object):
elif self.rule == TranslationRule.REPLACE:
self._exec_replace(source_key, source_data,
value_key, value_data, value)
elif self.rule == TranslationRule.RESOLVE:
elif self.rule == TranslationRule.RESOLVE and client_resolve:
self._exec_resolve(source_key, source_data)
elif self.rule == TranslationRule.DELETE:
self._exec_delete(source_key, source_data, value)

30
heat/tests/openstack/neutron/test_neutron_floating_ip.py
View File

@ -182,12 +182,6 @@ class NeutronFloatingIPTest(common.HeatTestCase):
'abcd1234',
cmd_resource=None,
).MultipleTimes().AndReturn('abcd1234')
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'subnet',
'sub1234',
cmd_resource=None,
).MultipleTimes().AndReturn('sub1234')
neutronclient.Client.create_floatingip({
'floatingip': {'floating_network_id': u'abcd1234'}
}).AndReturn({'floatingip': {
@ -247,18 +241,6 @@ class NeutronFloatingIPTest(common.HeatTestCase):
self.m.VerifyAll()
def test_FnGetRefId(self):
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'network',
'abcd1234',
cmd_resource=None,
).MultipleTimes().AndReturn('abcd1234')
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'subnet',
'sub1234',
cmd_resource=None,
).MultipleTimes().AndReturn('sub1234')
self.m.ReplayAll()
t = template_format.parse(neutron_floating_template)
stack = utils.parse_stack(t)
@ -480,12 +462,6 @@ class NeutronFloatingIPTest(common.HeatTestCase):
self.m.VerifyAll()
def _test_floating_dependancy(self):
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'network',
'abcd1234',
cmd_resource=None,
).MultipleTimes().AndReturn('abcd1234')
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'router',
@ -540,12 +516,6 @@ class NeutronFloatingIPTest(common.HeatTestCase):
'abcd1234',
cmd_resource=None,
).MultipleTimes().AndReturn('abcd1234')
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'subnet',
'sub1234',
cmd_resource=None,
).MultipleTimes().AndReturn('sub1234')
self.stub_NetworkConstraint_validate()
neutronclient.Client.create_floatingip({
'floatingip': {'floating_network_id': u'abcd1234',

51
heat/tests/openstack/neutron/test_neutron_router.py
View File

@ -178,23 +178,7 @@ class NeutronRouterTest(common.HeatTestCase):
self.assertEqual([u'792ff887-6c85-4a56-b518-23f24fa65581'],
rsrc.properties['l3_agent_ids'])
def _test_validate(self):
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'network',
'net1234',
cmd_resource=None,
).MultipleTimes().AndReturn('net1234')
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'subnet',
'sub1234',
cmd_resource=None,
).MultipleTimes().AndReturn('sub1234')
self.m.ReplayAll()
def test_router_validate_distribute_l3_agents(self):
self._test_validate()
t = template_format.parse(neutron_template)
props = t['resources']['router']['properties']
@ -217,7 +201,6 @@ class NeutronRouterTest(common.HeatTestCase):
self.m.VerifyAll()
def test_router_validate_l3_agents(self):
self._test_validate()
t = template_format.parse(neutron_template)
props = t['resources']['router']['properties']
@ -233,7 +216,6 @@ class NeutronRouterTest(common.HeatTestCase):
self.m.VerifyAll()
def test_router_validate_ha_distribute(self):
self._test_validate()
t = template_format.parse(neutron_template)
props = t['resources']['router']['properties']
@ -249,7 +231,6 @@ class NeutronRouterTest(common.HeatTestCase):
self.m.VerifyAll()
def test_router_validate_ha_l3_agents(self):
self._test_validate()
t = template_format.parse(neutron_template)
props = t['resources']['router']['properties']
# test non ha can not specify more than one l3 agent id
@ -491,18 +472,6 @@ class NeutronRouterTest(common.HeatTestCase):
self._test_router_interface(resolve_router=False)
def _test_router_interface(self, resolve_router=True):
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'router',
'3e46229d-8fce-4733-819a-b5fe630550f8',
cmd_resource=None,
).AndReturn('3e46229d-8fce-4733-819a-b5fe630550f8')
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'subnet',
'91e47a57-7508-46fe-afc9-fc454e8580e1',
cmd_resource=None,
).AndReturn('91e47a57-7508-46fe-afc9-fc454e8580e1')
neutronclient.Client.add_interface_router(
'3e46229d-8fce-4733-819a-b5fe630550f8',
{'subnet_id': '91e47a57-7508-46fe-afc9-fc454e8580e1'}
@ -764,18 +733,6 @@ class NeutronRouterTest(common.HeatTestCase):
'public',
cmd_resource=None,
).AndReturn('fc68ea2c-b60b-4b4f-bd82-94ec81110766')
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'subnet',
'sub1234',
cmd_resource=None,
).AndReturn('sub1234')
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'network',
'fc68ea2c-b60b-4b4f-bd82-94ec81110766',
cmd_resource=None,
).AndReturn('fc68ea2c-b60b-4b4f-bd82-94ec81110766')
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'subnet',
@ -873,13 +830,6 @@ class NeutronRouterTest(common.HeatTestCase):
'public',
cmd_resource=None,
).AndReturn('fc68ea2c-b60b-4b4f-bd82-94ec81110766')
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'network',
'fc68ea2c-b60b-4b4f-bd82-94ec81110766',
cmd_resource=None,
).AndReturn('fc68ea2c-b60b-4b4f-bd82-94ec81110766')
neutronclient.Client.create_router({
"router": {
"name": "Test Router",
@ -960,7 +910,6 @@ class NeutronRouterTest(common.HeatTestCase):
'91e47a57-7508-46fe-afc9-fc454e8580e1',
cmd_resource=None,
).AndReturn('91e47a57-7508-46fe-afc9-fc454e8580e1')
neutronclient.Client.update_router(
'3e46229d-8fce-4733-819a-b5fe630550f8',
{'router': {

6
heat/tests/openstack/neutron/test_neutron_subnet.py
View File

@ -644,12 +644,6 @@ class NeutronSubnetTest(common.HeatTestCase):
"supported for ipv4.", six.text_type(ex))
def test_validate_both_subnetpool_cidr(self):
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'subnetpool',
'new_pool',
cmd_resource=None,
).AndReturn('new_pool')
neutronV20.find_resourceid_by_name_or_id(
mox.IsA(neutronclient.Client),
'subnetpool',

7
heat/tests/test_resource.py
View File

@ -43,6 +43,7 @@ from heat.engine import rsrc_defn
from heat.engine import scheduler
from heat.engine import stack as parser
from heat.engine import template
from heat.engine import translation
from heat.objects import resource as resource_objects
from heat.objects import resource_data as resource_data_object
from heat.tests import common
@ -163,11 +164,12 @@ class ResourceTest(common.HeatTestCase):
self.assertEqual('Resource name may not contain "/"',
six.text_type(ex))
@mock.patch.object(translation.TranslationRule, '_exec_resolve')
@mock.patch.object(parser.Stack, 'db_resource_get')
@mock.patch.object(resource.Resource, '_load_data')
@mock.patch.object(resource.Resource, 'translate_properties')
def test_stack_resources(self, mock_translate, mock_load,
mock_db_get):
mock_db_get, mock_resolve):
tpl = {'HeatTemplateFormatVersion': '2012-12-12',
'Resources':
{'A': {'Type': 'ResourceWithPropsType',
@ -183,11 +185,14 @@ class ResourceTest(common.HeatTestCase):
self.assertEqual(0, mock_load.call_count)
# set stack._resources = None to reload the resources
# and set strict_validate = False
stack._resources = None
stack.strict_validate = False
mock_db_get.return_value = mock.Mock()
self.assertEqual(1, len(stack.resources))
self.assertEqual(1, mock_translate.call_count)
self.assertEqual(1, mock_load.call_count)
self.assertEqual(0, mock_resolve.call_count)
def test_resource_new_stack_not_stored(self):
snippet = rsrc_defn.ResourceDefinition('aresource',

29
heat/tests/test_translation_rule.py
View File

@ -614,6 +614,35 @@ class TestTranslationRule(common.HeatTestCase):
rule.execute_rule()
self.assertEqual(data, props.data)
def test_resolve_rule_other_with_get_attr(self):
client_plugin, schema = self._test_resolve_rule()
class DummyStack(dict):
pass
class rsrc(object):
pass
stack = DummyStack(another_res=rsrc())
attr_func = cfn_funcs.GetAtt(stack, 'Fn::GetAtt',
['another_res', 'name'])
data = {'far': attr_func}
props = properties.Properties(schema, data)
rule = translation.TranslationRule(
props,
translation.TranslationRule.RESOLVE,
['far'],
client_plugin=client_plugin,
finder='find_name_id')
rule.execute_rule(client_resolve=False)
self.assertEqual(data, props.data)
mock_getatt = self.patchobject(attr_func, 'result',
return_value='rose')
rule.execute_rule()
self.assertEqual('pink', props.get('far'))
self.assertEqual(1, mock_getatt.call_count)
def test_resolve_rule_other_with_entity(self):
client_plugin, schema = self._test_resolve_rule()
data = {'far': 'one'}