From e06903d9ab4c7fac9ae4b6faa2727f4e8ae4028e Mon Sep 17 00:00:00 2001 From: Rakesh H S Date: Mon, 14 Sep 2015 18:22:24 +0530 Subject: [PATCH] Convergence: Assign current_template_id when resource fails When a resource was being updated and a failure occurs, we are not setting the rsrc.current_template_id. The rsrc.current_template_id should also be set when resource failure occurs, since it was acted upon using the current template. Change-Id: I5dfda012f36a05691af8fc490560bf9fca043662 Closes-bug: #1495363 --- heat/engine/resource.py | 19 +++++++---- heat/tests/test_resource.py | 66 +++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 7 deletions(-) diff --git a/heat/engine/resource.py b/heat/engine/resource.py index 9e80c190f8..780be3f4f4 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -869,19 +869,24 @@ class Resource(object): resource's requires to list of the required resource id from the given resource_data and existing resource's requires. ''' - with self.lock(engine_id): - new_temp = template.Template.load(self.context, template_id) - new_res_def = new_temp.resource_definitions(self.stack)[self.name] - runner = scheduler.TaskRunner(self.update, new_res_def) - runner(timeout=timeout) - - # update the resource db record (stored in unlock) + def update_tmpl_id_and_requires(): self.current_template_id = template_id self.requires = list( set(data[u'id'] for data in resource_data.values() if data is not None) ) + with self.lock(engine_id): + new_temp = template.Template.load(self.context, template_id) + new_res_def = new_temp.resource_definitions(self.stack)[self.name] + runner = scheduler.TaskRunner(self.update, new_res_def) + try: + runner(timeout=timeout) + update_tmpl_id_and_requires() + except exception.ResourceFailure: + update_tmpl_id_and_requires() + raise + @scheduler.wrappertask def update(self, after, before=None, prev_resource=None): ''' diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py index 3c22af35a8..b3e589b5bf 100644 --- a/heat/tests/test_resource.py +++ b/heat/tests/test_resource.py @@ -1768,6 +1768,72 @@ class ResourceTest(common.HeatTestCase): # ensure requirements are not updated for failed resource self.assertEqual([1, 2], res.requires) + @mock.patch.object(resource.Resource, 'update') + def test_update_resource_convergence_failed(self, mock_update): + tmpl = rsrc_defn.ResourceDefinition('test_res', + 'ResourceWithPropsType') + res = generic_rsrc.GenericResource('test_res', tmpl, self.stack) + res.requires = [2] + res._store() + self._assert_resource_lock(res.id, None, None) + + new_temp = template.Template({ + 'HeatTemplateFormatVersion': '2012-12-12', + 'Resources': { + 'test_res': {'Type': 'ResourceWithPropsType', + 'Properties': {'Foo': 'abc'}} + }}, env=self.env) + new_temp.store() + + res_data = {(1, True): {u'id': 4, u'name': 'A', 'attrs': {}}, + (2, True): {u'id': 3, u'name': 'B', 'attrs': {}}} + exc = Exception(_('Resource update failed')) + dummy_ex = exception.ResourceFailure(exc, res, action=res.UPDATE) + mock_update.side_effect = dummy_ex + self.assertRaises(exception.ResourceFailure, + res.update_convergence, new_temp.id, res_data, + 'engine-007', 120) + + expected_rsrc_def = new_temp.resource_definitions(self.stack)[res.name] + mock_update.assert_called_once_with(expected_rsrc_def) + # check if current_template_id was updated + self.assertEqual(new_temp.id, res.current_template_id) + # check if requires was updated + self.assertItemsEqual([3, 4], res.requires) + self._assert_resource_lock(res.id, None, 2) + + @mock.patch.object(resource.Resource, 'update') + def test_update_resource_convergence_update_replace(self, mock_update): + tmpl = rsrc_defn.ResourceDefinition('test_res', + 'ResourceWithPropsType') + res = generic_rsrc.GenericResource('test_res', tmpl, self.stack) + res.requires = [2] + res._store() + self._assert_resource_lock(res.id, None, None) + + new_temp = template.Template({ + 'HeatTemplateFormatVersion': '2012-12-12', + 'Resources': { + 'test_res': {'Type': 'ResourceWithPropsType', + 'Properties': {'Foo': 'abc'}} + }}, env=self.env) + new_temp.store() + + res_data = {(1, True): {u'id': 4, u'name': 'A', 'attrs': {}}, + (2, True): {u'id': 3, u'name': 'B', 'attrs': {}}} + mock_update.side_effect = exception.UpdateReplace + self.assertRaises(exception.UpdateReplace, + res.update_convergence, new_temp.id, res_data, + 'engine-007', 120) + + expected_rsrc_def = new_temp.resource_definitions(self.stack)[res.name] + mock_update.assert_called_once_with(expected_rsrc_def) + # ensure that current_template_id was not updated + self.assertEqual(None, res.current_template_id) + # ensure that requires was not updated + self.assertItemsEqual([2], res.requires) + self._assert_resource_lock(res.id, None, 2) + @mock.patch.object(resource.scheduler.TaskRunner, '__init__', return_value=None) @mock.patch.object(resource.scheduler.TaskRunner, '__call__')