Don't load nested stack to get ResourceGroup blacklist
In ResourceGroup, we (unwisely in retrospect) allow blacklisting of resources by reference ID (i.e. physical resource ID in most cases). In order to both determine whether a removal policy entry was an existing resource name and (if not) to find the resource by physical ID, we would always load the nested stack into memory. To avoid this, unconditionally create an output in the nested stack that returns a mapping of all the resource reference IDs and use it for finding a resource with a matching reference ID. Only if the output is not available (because the nested stack hasn't been updated since prior to the output being defined), fall back to the old way of doing it. Avoid looking up IDs for names that already appear in the blacklist (because they were blacklisted in a previous update and have been removed from the stack.) Since this is more expensive in time (though hopefully cheaper in space), update the blacklist only once on resource creation and whenever it actually changes during an update. This is accomplished by moving the update to a separate function that is called explicitly, rather than making it a side-effect of getting the current blacklist (which occurs up to 4 times in each create/update). Change-Id: Ic32d7d99bad06f92b3d2919d58cd1f9128bcfe83 Partial-Bug: #1731349
This commit is contained in:
parent
9f9605d927
commit
32ec5141de
|
@ -303,7 +303,7 @@ class ResourceGroup(stack_resource.StackResource):
|
|||
if not self.get_size():
|
||||
return
|
||||
|
||||
first_name = next(self._resource_names(update_rsrc_data=False))
|
||||
first_name = next(self._resource_names())
|
||||
test_tmpl = self._assemble_nested([first_name],
|
||||
include_all=True)
|
||||
res_def = next(six.itervalues(test_tmpl.resource_definitions(None)))
|
||||
|
@ -330,44 +330,69 @@ class ResourceGroup(stack_resource.StackResource):
|
|||
else:
|
||||
return []
|
||||
|
||||
def _name_blacklist(self, update_rsrc_data=True):
|
||||
"""Resolve the remove_policies to names for removal."""
|
||||
|
||||
nested = self.nested()
|
||||
|
||||
# To avoid reusing names after removal, we store a comma-separated
|
||||
# blacklist in the resource data - in cases where you want to
|
||||
# overwrite the stored data, removal_policies_mode: update can be used
|
||||
current_blacklist = self._current_blacklist()
|
||||
p_mode = self.properties[self.REMOVAL_POLICIES_MODE]
|
||||
if p_mode == self.REMOVAL_POLICY_UPDATE:
|
||||
new_blacklist = []
|
||||
else:
|
||||
new_blacklist = current_blacklist
|
||||
def _get_new_blacklist_entries(self, properties, current_blacklist):
|
||||
insp = grouputils.GroupInspector.from_parent_resource(self)
|
||||
|
||||
# Now we iterate over the removal policies, and update the blacklist
|
||||
# with any additional names
|
||||
rsrc_names = set(new_blacklist)
|
||||
|
||||
for r in self.properties[self.REMOVAL_POLICIES]:
|
||||
for r in properties.get(self.REMOVAL_POLICIES, []):
|
||||
if self.REMOVAL_RSRC_LIST in r:
|
||||
# Tolerate string or int list values
|
||||
for n in r[self.REMOVAL_RSRC_LIST]:
|
||||
str_n = six.text_type(n)
|
||||
if not nested or str_n in nested:
|
||||
rsrc_names.add(str_n)
|
||||
continue
|
||||
rsrc = nested.resource_by_refid(str_n)
|
||||
if rsrc:
|
||||
rsrc_names.add(rsrc.name)
|
||||
if (str_n in current_blacklist or
|
||||
self.resource_id is None or
|
||||
str_n in insp.member_names(include_failed=True)):
|
||||
yield str_n
|
||||
elif isinstance(n, six.string_types):
|
||||
try:
|
||||
refids = self.get_output(self.REFS_MAP)
|
||||
except (exception.NotFound,
|
||||
exception.TemplateOutputError) as op_err:
|
||||
LOG.debug('Falling back to resource_by_refid() '
|
||||
' due to %s', op_err)
|
||||
rsrc = self.nested().resource_by_refid(n)
|
||||
if rsrc is not None:
|
||||
yield rsrc.name
|
||||
else:
|
||||
if refids is not None:
|
||||
for name, refid in refids.items():
|
||||
if refid == n:
|
||||
yield name
|
||||
break
|
||||
|
||||
# Clear output cache from prior to stack update, so we don't get
|
||||
# outdated values after stack update.
|
||||
self._outputs = None
|
||||
|
||||
def _update_name_blacklist(self, properties):
|
||||
"""Resolve the remove_policies to names for removal."""
|
||||
# To avoid reusing names after removal, we store a comma-separated
|
||||
# blacklist in the resource data - in cases where you want to
|
||||
# overwrite the stored data, removal_policies_mode: update can be used
|
||||
curr_bl = set(self._current_blacklist())
|
||||
p_mode = properties.get(self.REMOVAL_POLICIES_MODE,
|
||||
self.REMOVAL_POLICY_APPEND)
|
||||
if p_mode == self.REMOVAL_POLICY_UPDATE:
|
||||
init_bl = set()
|
||||
else:
|
||||
init_bl = curr_bl
|
||||
updated_bl = init_bl | set(self._get_new_blacklist_entries(properties,
|
||||
curr_bl))
|
||||
|
||||
# If the blacklist has changed, update the resource data
|
||||
if update_rsrc_data and rsrc_names != set(current_blacklist):
|
||||
self.data_set('name_blacklist', ','.join(rsrc_names))
|
||||
return rsrc_names
|
||||
if updated_bl != curr_bl:
|
||||
self.data_set('name_blacklist', ','.join(sorted(updated_bl)))
|
||||
|
||||
def _resource_names(self, size=None, update_rsrc_data=True):
|
||||
name_blacklist = self._name_blacklist(update_rsrc_data)
|
||||
def _name_blacklist(self):
|
||||
"""Get the list of resource names to blacklist."""
|
||||
bl = set(self._current_blacklist())
|
||||
if self.resource_id is None:
|
||||
bl |= set(self._get_new_blacklist_entries(self.properties, bl))
|
||||
return bl
|
||||
|
||||
def _resource_names(self, size=None):
|
||||
name_blacklist = self._name_blacklist()
|
||||
if size is None:
|
||||
size = self.get_size()
|
||||
|
||||
|
@ -385,6 +410,7 @@ class ResourceGroup(stack_resource.StackResource):
|
|||
return len(self._name_blacklist() & set(existing_members))
|
||||
|
||||
def handle_create(self):
|
||||
self._update_name_blacklist(self.properties)
|
||||
if self.update_policy.get(self.BATCH_CREATE):
|
||||
batch_create = self.update_policy[self.BATCH_CREATE]
|
||||
max_batch_size = batch_create[self.MAX_BATCH_SIZE]
|
||||
|
@ -443,6 +469,7 @@ class ResourceGroup(stack_resource.StackResource):
|
|||
checkers = []
|
||||
self.properties = json_snippet.properties(self.properties_schema,
|
||||
self.context)
|
||||
self._update_name_blacklist(self.properties)
|
||||
if prop_diff and self.res_def_changed(prop_diff):
|
||||
updaters = self._try_rolling_update()
|
||||
if updaters:
|
||||
|
@ -540,12 +567,13 @@ class ResourceGroup(stack_resource.StackResource):
|
|||
|
||||
if key == self.REFS:
|
||||
value = [get_res_fn(r) for r in resource_names]
|
||||
elif key == self.REFS_MAP:
|
||||
value = {r: get_res_fn(r) for r in resource_names}
|
||||
|
||||
if value is not None:
|
||||
yield output.OutputDefinition(output_name, value)
|
||||
|
||||
value = {r: get_res_fn(r) for r in resource_names}
|
||||
yield output.OutputDefinition(self.REFS_MAP, value)
|
||||
|
||||
def build_resource_definition(self, res_name, res_defn):
|
||||
res_def = copy.deepcopy(res_defn)
|
||||
|
||||
|
|
|
@ -701,6 +701,9 @@ class SoftwareDeploymentGroup(resource_group.ResourceGroup):
|
|||
def res_def_changed(self, prop_diff):
|
||||
return True
|
||||
|
||||
def _update_name_blacklist(self, properties):
|
||||
pass
|
||||
|
||||
def _name_blacklist(self):
|
||||
return set()
|
||||
|
||||
|
|
|
@ -174,6 +174,15 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
"Foo": "Bar"
|
||||
}
|
||||
}
|
||||
},
|
||||
"outputs": {
|
||||
"refs_map": {
|
||||
"value": {
|
||||
"0": {"get_resource": "0"},
|
||||
"1": {"get_resource": "1"},
|
||||
"2": {"get_resource": "2"},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -211,6 +220,13 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
}
|
||||
},
|
||||
"outputs": {
|
||||
"refs_map": {
|
||||
"value": {
|
||||
"0": {"get_resource": "0"},
|
||||
"1": {"get_resource": "1"},
|
||||
"2": {"get_resource": "2"},
|
||||
}
|
||||
},
|
||||
"foo": {
|
||||
"value": [
|
||||
{"get_attr": ["0", "foo"]},
|
||||
|
@ -238,6 +254,13 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
"type": "OverwrittenFnGetRefIdType",
|
||||
"properties": {}
|
||||
}
|
||||
},
|
||||
"outputs": {
|
||||
"refs_map": {
|
||||
"value": {
|
||||
"0": {"get_resource": "0"},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
self.assertEqual(expect, resg._assemble_nested(['0']).t)
|
||||
|
@ -253,6 +276,7 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
resg = resource_group.ResourceGroup('test', snip, stack)
|
||||
expect = {
|
||||
"heat_template_version": "2015-04-30",
|
||||
"outputs": {"refs_map": {"value": {}}},
|
||||
}
|
||||
self.assertEqual(expect, resg._assemble_nested([]).t)
|
||||
|
||||
|
@ -278,6 +302,13 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
'role': 'webserver'
|
||||
}
|
||||
}
|
||||
},
|
||||
"outputs": {
|
||||
"refs_map": {
|
||||
"value": {
|
||||
"0": {"get_resource": "0"},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
self.assertEqual(expect, resg._assemble_nested(['0']).t)
|
||||
|
@ -298,6 +329,14 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
"foo": "baz"
|
||||
}
|
||||
}
|
||||
},
|
||||
"outputs": {
|
||||
"refs_map": {
|
||||
"value": {
|
||||
"0": {"get_resource": "0"},
|
||||
"1": {"get_resource": "1"},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
resource_def = rsrc_defn.ResourceDefinition(
|
||||
|
@ -332,6 +371,12 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
}
|
||||
},
|
||||
"outputs": {
|
||||
"refs_map": {
|
||||
"value": {
|
||||
"0": {"get_resource": "0"},
|
||||
"1": {"get_resource": "1"},
|
||||
}
|
||||
},
|
||||
"bar": {
|
||||
"value": [
|
||||
{"get_attr": ["0", "bar"]},
|
||||
|
@ -371,6 +416,14 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
"foo": "bar"
|
||||
}
|
||||
}
|
||||
},
|
||||
"outputs": {
|
||||
"refs_map": {
|
||||
"value": {
|
||||
"0": {"get_resource": "0"},
|
||||
"1": {"get_resource": "1"},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -404,6 +457,14 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
"foo": "bar"
|
||||
}
|
||||
}
|
||||
},
|
||||
"outputs": {
|
||||
"refs_map": {
|
||||
"value": {
|
||||
"0": {"get_resource": "0"},
|
||||
"1": {"get_resource": "1"},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
resource_def = rsrc_defn.ResourceDefinition(
|
||||
|
@ -449,6 +510,14 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
"type": "OverwrittenFnGetRefIdType",
|
||||
"properties": {}
|
||||
}
|
||||
},
|
||||
"outputs": {
|
||||
"refs_map": {
|
||||
"value": {
|
||||
"0": {"get_resource": "0"},
|
||||
"1": {"get_resource": "1"},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
self.assertEqual(expected, nested_tmpl.t)
|
||||
|
@ -488,6 +557,15 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
]
|
||||
}
|
||||
}
|
||||
},
|
||||
"outputs": {
|
||||
"refs_map": {
|
||||
"value": {
|
||||
"0": {"get_resource": "0"},
|
||||
"1": {"get_resource": "1"},
|
||||
"2": {"get_resource": "2"},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
nested = resg._assemble_nested(['0', '1', '2']).t
|
||||
|
@ -514,6 +592,13 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
]
|
||||
}
|
||||
}
|
||||
},
|
||||
"outputs": {
|
||||
"refs_map": {
|
||||
"value": {
|
||||
"0": {"get_resource": "0"},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
nested = resg._assemble_nested(['0']).t
|
||||
|
@ -543,6 +628,13 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
]
|
||||
}
|
||||
}
|
||||
},
|
||||
"outputs": {
|
||||
"refs_map": {
|
||||
"value": {
|
||||
"0": {"get_resource": "0"},
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
nested = resg._assemble_nested(['0']).t
|
||||
|
@ -710,7 +802,7 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
resgrp._assemble_nested = mock.Mock(return_value='tmpl')
|
||||
resgrp.properties.data[resgrp.COUNT] = 2
|
||||
self.patchobject(scheduler.TaskRunner, 'start')
|
||||
resgrp.handle_update(snip, None, None)
|
||||
resgrp.handle_update(snip, mock.Mock(), {})
|
||||
self.assertTrue(resgrp._assemble_nested.called)
|
||||
|
||||
def test_handle_delete(self):
|
||||
|
@ -728,7 +820,7 @@ class ResourceGroupTest(common.HeatTestCase):
|
|||
resgrp._assemble_nested = mock.Mock(return_value=None)
|
||||
resgrp.properties.data[resgrp.COUNT] = 5
|
||||
self.patchobject(scheduler.TaskRunner, 'start')
|
||||
resgrp.handle_update(snip, None, None)
|
||||
resgrp.handle_update(snip, mock.Mock(), {})
|
||||
self.assertTrue(resgrp._assemble_nested.called)
|
||||
|
||||
|
||||
|
@ -741,48 +833,63 @@ class ResourceGroupBlackList(common.HeatTestCase):
|
|||
# 4) resource_list (refid) not in nested()
|
||||
# 5) resource_list in nested() -> saved
|
||||
# 6) resource_list (refid) in nested() -> saved
|
||||
# 7) resource_list (refid) in nested(), update -> saved
|
||||
# 8) resource_list, update -> saved
|
||||
# 9) resource_list (refid) in nested(), grouputils fallback -> saved
|
||||
# A) resource_list (refid) in nested(), update, grouputils -> saved
|
||||
scenarios = [
|
||||
('1', dict(data_in=None, rm_list=[],
|
||||
nested_rsrcs=[], expected=[],
|
||||
saved=False, rm_mode='append')),
|
||||
saved=False, fallback=False, rm_mode='append')),
|
||||
('2', dict(data_in='0,1,2', rm_list=[],
|
||||
nested_rsrcs=[], expected=['0', '1', '2'],
|
||||
saved=False, rm_mode='append')),
|
||||
saved=False, fallback=False, rm_mode='append')),
|
||||
('3', dict(data_in='1,3', rm_list=['6'],
|
||||
nested_rsrcs=['0', '1', '3'],
|
||||
expected=['1', '3'],
|
||||
saved=False, rm_mode='append')),
|
||||
saved=False, fallback=False, rm_mode='append')),
|
||||
('4', dict(data_in='0,1', rm_list=['id-7'],
|
||||
nested_rsrcs=['0', '1', '3'],
|
||||
expected=['0', '1'],
|
||||
saved=False, rm_mode='append')),
|
||||
saved=False, fallback=False, rm_mode='append')),
|
||||
('5', dict(data_in='0,1', rm_list=['3'],
|
||||
nested_rsrcs=['0', '1', '3'],
|
||||
expected=['0', '1', '3'],
|
||||
saved=True, rm_mode='append')),
|
||||
saved=True, fallback=False, rm_mode='append')),
|
||||
('6', dict(data_in='0,1', rm_list=['id-3'],
|
||||
nested_rsrcs=['0', '1', '3'],
|
||||
expected=['0', '1', '3'],
|
||||
saved=True, rm_mode='append')),
|
||||
saved=True, fallback=False, rm_mode='append')),
|
||||
('7', dict(data_in='0,1', rm_list=['id-3'],
|
||||
nested_rsrcs=['0', '1', '3'],
|
||||
expected=['3'],
|
||||
saved=True, rm_mode='update')),
|
||||
saved=True, fallback=False, rm_mode='update')),
|
||||
('8', dict(data_in='1', rm_list=[],
|
||||
nested_rsrcs=['0', '1', '2'],
|
||||
expected=[],
|
||||
saved=True, rm_mode='update')),
|
||||
saved=True, fallback=False, rm_mode='update')),
|
||||
('9', dict(data_in='0,1', rm_list=['id-3'],
|
||||
nested_rsrcs=['0', '1', '3'],
|
||||
expected=['0', '1', '3'],
|
||||
saved=True, fallback=True, rm_mode='append')),
|
||||
('A', dict(data_in='0,1', rm_list=['id-3'],
|
||||
nested_rsrcs=['0', '1', '3'],
|
||||
expected=['3'],
|
||||
saved=True, fallback=True, rm_mode='update')),
|
||||
]
|
||||
|
||||
def test_blacklist(self):
|
||||
stack = utils.parse_stack(template)
|
||||
resg = stack['group1']
|
||||
|
||||
if self.data_in is not None:
|
||||
resg.resource_id = 'foo'
|
||||
|
||||
# mock properties
|
||||
resg.properties = mock.MagicMock()
|
||||
properties = mock.MagicMock()
|
||||
p_data = {'removal_policies': [{'resource_list': self.rm_list}],
|
||||
'removal_policies_mode': self.rm_mode}
|
||||
resg.properties.__getitem__.side_effect = p_data.__getitem__
|
||||
properties.get.side_effect = p_data.get
|
||||
|
||||
# mock data get/set
|
||||
resg.data = mock.Mock()
|
||||
|
@ -790,28 +897,41 @@ class ResourceGroupBlackList(common.HeatTestCase):
|
|||
resg.data_set = mock.Mock()
|
||||
|
||||
# mock nested access
|
||||
def stack_contains(name):
|
||||
return name in self.nested_rsrcs
|
||||
mock_inspect = mock.Mock()
|
||||
self.patchobject(grouputils.GroupInspector, 'from_parent_resource',
|
||||
return_value=mock_inspect)
|
||||
mock_inspect.member_names.return_value = self.nested_rsrcs
|
||||
|
||||
def by_refid(name):
|
||||
rid = name.replace('id-', '')
|
||||
if rid not in self.nested_rsrcs:
|
||||
return None
|
||||
res = mock.Mock()
|
||||
res.name = rid
|
||||
return res
|
||||
if not self.fallback:
|
||||
refs_map = {n: 'id-%s' % n for n in self.nested_rsrcs}
|
||||
resg.get_output = mock.Mock(return_value=refs_map)
|
||||
else:
|
||||
resg.get_output = mock.Mock(side_effect=exception.NotFound)
|
||||
|
||||
nested = mock.MagicMock()
|
||||
nested.__contains__.side_effect = stack_contains
|
||||
nested.__iter__.side_effect = iter(self.nested_rsrcs)
|
||||
nested.resource_by_refid.side_effect = by_refid
|
||||
resg.nested = mock.Mock(return_value=nested)
|
||||
def stack_contains(name):
|
||||
return name in self.nested_rsrcs
|
||||
|
||||
blacklist = resg._name_blacklist()
|
||||
self.assertEqual(set(self.expected), blacklist)
|
||||
def by_refid(name):
|
||||
rid = name.replace('id-', '')
|
||||
if rid not in self.nested_rsrcs:
|
||||
return None
|
||||
res = mock.Mock()
|
||||
res.name = rid
|
||||
return res
|
||||
|
||||
nested = mock.MagicMock()
|
||||
nested.__contains__.side_effect = stack_contains
|
||||
nested.__iter__.side_effect = iter(self.nested_rsrcs)
|
||||
nested.resource_by_refid.side_effect = by_refid
|
||||
resg.nested = mock.Mock(return_value=nested)
|
||||
|
||||
resg._update_name_blacklist(properties)
|
||||
if self.saved:
|
||||
resg.data_set.assert_called_once_with('name_blacklist',
|
||||
','.join(blacklist))
|
||||
','.join(self.expected))
|
||||
else:
|
||||
resg.data_set.assert_not_called()
|
||||
self.assertEqual(set(self.expected), resg._name_blacklist())
|
||||
|
||||
|
||||
class ResourceGroupEmptyParams(common.HeatTestCase):
|
||||
|
@ -1261,12 +1381,15 @@ class RollingUpdatePolicyDiffTest(common.HeatTestCase):
|
|||
tmpl_diff = updated_grp.update_template_diff(
|
||||
updated_grp_json, current_grp_json)
|
||||
self.assertTrue(tmpl_diff.update_policy_changed())
|
||||
prop_diff = current_grp.update_template_diff_properties(
|
||||
updated_grp.properties,
|
||||
current_grp.properties)
|
||||
|
||||
# test application of the new update policy in handle_update
|
||||
current_grp._try_rolling_update = mock.Mock()
|
||||
current_grp._assemble_nested_for_size = mock.Mock()
|
||||
self.patchobject(scheduler.TaskRunner, 'start')
|
||||
current_grp.handle_update(updated_grp_json, tmpl_diff, None)
|
||||
current_grp.handle_update(updated_grp_json, tmpl_diff, prop_diff)
|
||||
self.assertEqual(updated_grp_json._update_policy or {},
|
||||
current_grp.update_policy.data)
|
||||
|
||||
|
|
Loading…
Reference in New Issue