diff --git a/heat/engine/resources/openstack/heat/resource_group.py b/heat/engine/resources/openstack/heat/resource_group.py index 742bb5e387..5b8b89bdf5 100644 --- a/heat/engine/resources/openstack/heat/resource_group.py +++ b/heat/engine/resources/openstack/heat/resource_group.py @@ -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) diff --git a/heat/engine/resources/openstack/heat/software_deployment.py b/heat/engine/resources/openstack/heat/software_deployment.py index 64285ed971..ba91872882 100644 --- a/heat/engine/resources/openstack/heat/software_deployment.py +++ b/heat/engine/resources/openstack/heat/software_deployment.py @@ -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() diff --git a/heat/tests/openstack/heat/test_resource_group.py b/heat/tests/openstack/heat/test_resource_group.py index 180a47d622..ef421ae4fa 100644 --- a/heat/tests/openstack/heat/test_resource_group.py +++ b/heat/tests/openstack/heat/test_resource_group.py @@ -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)