From 97df8bb6ca82b2a225226340b118a93bf1cf1120 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Wed, 5 Sep 2018 19:25:52 -0400 Subject: [PATCH] Improve best existing resource selection Rank all existing versions of a resource in a convergence stack to improve the likelihood that we find the best one to update. This allows us to roll back to the original version of a resource (or even attempt an in-place update of it) when replacing it has failed. Previously this only worked during automatic rollback; on subsequent updates we would always work on the failed replacement (which inevitably meant attempting another replacement in almost all cases). Change-Id: Ia231fae85d1ddb9fc7b7de4e82cec0c0e0fd06b7 Story: #2003579 Task: 24881 --- heat/engine/stack.py | 44 +++++++++++-------- heat/tests/test_convg_stack.py | 40 ++++++++--------- .../functional/test_create_update.py | 30 +++++++++++-- ...k-failed-replacement-08ebb9271617fe9d.yaml | 11 +++++ 4 files changed, 84 insertions(+), 41 deletions(-) create mode 100644 releasenotes/notes/manual-rollback-failed-replacement-08ebb9271617fe9d.yaml diff --git a/heat/engine/stack.py b/heat/engine/stack.py index 8314814af1..15e995566e 100644 --- a/heat/engine/stack.py +++ b/heat/engine/stack.py @@ -42,6 +42,7 @@ from heat.engine import parent_rsrc from heat.engine import resource from heat.engine import resources from heat.engine import scheduler +from heat.engine import status from heat.engine import stk_defn from heat.engine import sync_point from heat.engine import template as tmpl @@ -1462,26 +1463,33 @@ class Stack(collections.Mapping): self.converge_stack(rollback_tmpl, action=self.ROLLBACK) def _get_best_existing_rsrc_db(self, rsrc_name): - candidate = None if self.ext_rsrcs_db: - for ext_rsrc in self.ext_rsrcs_db.values(): - if ext_rsrc.name != rsrc_name: - continue - if ext_rsrc.current_template_id == self.t.id: - # Rollback where the previous resource still exists - candidate = ext_rsrc - break - elif (ext_rsrc.current_template_id == - self.prev_raw_template_id): - # Current resource is otherwise a good candidate - candidate = ext_rsrc - elif candidate is None: - # In multiple concurrent updates, if candidate is not - # found in current/previous template, it could be found - # in old tmpl. - candidate = ext_rsrc + def suitability(ext_rsrc): + score = 0 - return candidate + if ext_rsrc.status == status.ResourceStatus.FAILED: + score -= 30 + if ext_rsrc.action == status.ResourceStatus.DELETE: + score -= 50 + if ext_rsrc.replaced_by: + score -= 1 + if ext_rsrc.current_template_id == self.prev_raw_template_id: + # Current resource + score += 5 + if ext_rsrc.current_template_id == self.t.id: + # Rolling back to previous resource + score += 10 + + return score, ext_rsrc.updated_at + + candidates = sorted((r for r in self.ext_rsrcs_db.values() + if r.name == rsrc_name), + key=suitability, + reverse=True) + if candidates: + return candidates[0] + + return None def _update_or_store_resources(self): self.ext_rsrcs_db = self.db_active_resources_get() diff --git a/heat/tests/test_convg_stack.py b/heat/tests/test_convg_stack.py index f1d1d5e9a2..a642b51cc9 100644 --- a/heat/tests/test_convg_stack.py +++ b/heat/tests/test_convg_stack.py @@ -16,7 +16,6 @@ from oslo_config import cfg from heat.common import template_format from heat.engine import environment -from heat.engine import resource as res from heat.engine import stack as parser from heat.engine import template as templatem from heat.objects import raw_template as raw_template_object @@ -426,38 +425,39 @@ class StackConvergenceCreateUpdateDeleteTest(common.HeatTestCase): stack = tools.get_stack('test_stack', utils.dummy_context(), template=tools.string_template_five, convergence=True) - stack.store() stack.prev_raw_template_id = 2 stack.t.id = 3 - dummy_res = stack.resources['A'] - a_res_2 = res.Resource('A', dummy_res.t, stack) - a_res_2.current_template_id = 2 - a_res_2.id = 2 - a_res_3 = res.Resource('A', dummy_res.t, stack) - a_res_3.current_template_id = 3 - a_res_3.id = 3 - a_res_1 = res.Resource('A', dummy_res.t, stack) - a_res_1.current_template_id = 1 - a_res_1.id = 1 - existing_res = {2: a_res_2, - 3: a_res_3, - 1: a_res_1} + + def db_resource(current_template_id): + db_res = resource_objects.Resource(stack.context) + db_res['id'] = current_template_id + db_res['name'] = 'A' + db_res['current_template_id'] = current_template_id + db_res['action'] = 'CREATE' + db_res['status'] = 'COMPLETE' + db_res['updated_at'] = None + db_res['replaced_by'] = None + return db_res + + a_res_2 = db_resource(2) + a_res_3 = db_resource(3) + a_res_1 = db_resource(1) + existing_res = {a_res_2.id: a_res_2, + a_res_3.id: a_res_3, + a_res_1.id: a_res_1} stack.ext_rsrcs_db = existing_res best_res = stack._get_best_existing_rsrc_db('A') # should return resource with template id 3 which is current template self.assertEqual(a_res_3.id, best_res.id) # no resource with current template id as 3 - existing_res = {1: a_res_1, - 2: a_res_2} - stack.ext_rsrcs_db = existing_res + del existing_res[3] best_res = stack._get_best_existing_rsrc_db('A') # should return resource with template id 2 which is prev template self.assertEqual(a_res_2.id, best_res.id) # no resource with current template id as 3 or 2 - existing_res = {1: a_res_1} - stack.ext_rsrcs_db = existing_res + del existing_res[2] best_res = stack._get_best_existing_rsrc_db('A') # should return resource with template id 1 existing in DB self.assertEqual(a_res_1.id, best_res.id) diff --git a/heat_integrationtests/functional/test_create_update.py b/heat_integrationtests/functional/test_create_update.py index b5793685f2..a84dcde0e3 100644 --- a/heat_integrationtests/functional/test_create_update.py +++ b/heat_integrationtests/functional/test_create_update.py @@ -68,9 +68,8 @@ def _change_rsrc_properties(template, rsrcs, values): for rsrc_name in rsrcs: rsrc_prop = modified_template['resources'][ rsrc_name]['properties'] - for prop in rsrc_prop: - if prop in values: - rsrc_prop[prop] = values[prop] + for prop, new_val in values.items(): + rsrc_prop[prop] = new_val return modified_template @@ -280,6 +279,31 @@ resources: self.assertEqual(updated_resources, self.list_resources(stack_identifier)) + @test.requires_convergence + def test_stack_update_replace_manual_rollback(self): + template = _change_rsrc_properties(test_template_one_resource, + ['test1'], + {'update_replace_value': '1'}) + stack_identifier = self.stack_create(template=template) + original_resource_id = self.get_physical_resource_id(stack_identifier, + 'test1') + + tmpl_update = _change_rsrc_properties(test_template_one_resource, + ['test1'], + {'update_replace_value': '2', + 'fail': True}) + # Update with bad template, we should fail + self.update_stack(stack_identifier, tmpl_update, + expected_status='UPDATE_FAILED', + disable_rollback=True) + # Manually roll back to previous template + self.update_stack(stack_identifier, template) + final_resource_id = self.get_physical_resource_id(stack_identifier, + 'test1') + # Original resource was good, and replacement was never created, so it + # should be kept. + self.assertEqual(original_resource_id, final_resource_id) + def test_stack_update_provider(self): template = _change_rsrc_properties( test_template_one_resource, ['test1'], diff --git a/releasenotes/notes/manual-rollback-failed-replacement-08ebb9271617fe9d.yaml b/releasenotes/notes/manual-rollback-failed-replacement-08ebb9271617fe9d.yaml new file mode 100644 index 0000000000..9cfebdcf96 --- /dev/null +++ b/releasenotes/notes/manual-rollback-failed-replacement-08ebb9271617fe9d.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + Heat can now perform a stack update to roll back to a previous version of a + resource after a previous attempt to create a replacement for it failed + (provided that convergence is enabled). This allows the user to recover a + stack where a resource has been inadvertantly replaced with a definition + than can never succeed because it conflicts with the original. Previously + this required automatic rollback to be enabled, or the user had to update + the stack with a non-conflicting definition before rolling back to the + original.