diff --git a/heat/engine/resource.py b/heat/engine/resource.py index 50b6b4cd9..e60625a1f 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -1047,21 +1047,13 @@ class Resource(object): def delete_convergence(self, template_id, resource_data, engine_id): ''' - Deletes the resource by invoking the scheduler TaskRunner - and it persists the resource's current_template_id to template_id and - resource's requires to list of the required resource id from the - given resource_data and existing resource's requires. + Destroys the resource. The destroy task is run in a scheduler + TaskRunner after acquiring the lock on resource. ''' with self.lock(engine_id): - runner = scheduler.TaskRunner(self.delete) + runner = scheduler.TaskRunner(self.destroy) runner() - # update the resource db record - self.current_template_id = template_id - current_requires = {graph_key[0] - for graph_key, data in resource_data.items()} - self.requires = (list(set(self.requires) - current_requires)) - @scheduler.wrappertask def delete(self): ''' @@ -1233,14 +1225,16 @@ class Resource(object): def unlock(self, rsrc, engine_id, atomic_key): if atomic_key is None: atomic_key = 0 - res = rsrc.select_and_update( + + updated_ok = rsrc.select_and_update( {'engine_id': None, 'current_template_id': self.current_template_id, 'updated_at': self.updated_time, 'requires': self.requires}, expected_engine_id=engine_id, atomic_key=atomic_key + 1) - if res != 1: + + if not updated_ok: LOG.warn(_LW('Failed to unlock resource %s'), rsrc.name) def _resolve_attribute(self, name): diff --git a/heat/engine/stack.py b/heat/engine/stack.py index 9cbca3b18..2128353e3 100755 --- a/heat/engine/stack.py +++ b/heat/engine/stack.py @@ -1609,9 +1609,10 @@ class Stack(collections.Mapping): 1. Delete previous raw template if stack completes successfully. 2. Deletes all sync points. They are no longer needed after stack has completed/failed. - 3. Delete the stack is the action is DELETE. + 3. Delete the stack if the action is DELETE. ''' - if self.prev_raw_template_id is not None: + if (self.prev_raw_template_id is not None and + self.status != self.FAILED): prev_tmpl_id = self.prev_raw_template_id self.prev_raw_template_id = None self.store() diff --git a/heat/engine/worker.py b/heat/engine/worker.py index 89e93846e..2291da451 100644 --- a/heat/engine/worker.py +++ b/heat/engine/worker.py @@ -203,7 +203,7 @@ class WorkerService(service.Service): cnxt, stack.id, current_traversal, reason) return - graph_key = (rsrc.id, is_update) + graph_key = (resource_id, is_update) if graph_key not in graph and rsrc.replaces is not None: # If we are a replacement, impersonate the replaced resource for # the purposes of calculating whether subsequent resources are @@ -221,7 +221,7 @@ class WorkerService(service.Service): input_data if fwd else None, fwd) check_stack_complete(cnxt, rsrc.stack, current_traversal, - rsrc.id, deps, is_update) + resource_id, deps, is_update) except sync_point.SyncPointNotFound: # Reload the stack to determine the current traversal, and check # the SyncPoint for the current node to determine if it is ready. diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py index 4c50682c7..9e8e38d5e 100644 --- a/heat/tests/test_engine_service.py +++ b/heat/tests/test_engine_service.py @@ -40,6 +40,7 @@ from heat.engine import stack_lock from heat.engine import template as templatem from heat.engine import watchrule from heat.objects import event as event_object +from heat.objects import raw_template as raw_template_object from heat.objects import resource as resource_objects from heat.objects import stack as stack_object from heat.objects import sync_point as sync_point_object @@ -502,18 +503,25 @@ class StackConvergenceCreateUpdateDeleteTest(common.HeatTestCase): stack.mark_complete(stack.current_traversal) self.assertTrue(stack.purge_db.called) - def test_purge_db_deletes_previous_template(self, mock_cr): + @mock.patch.object(raw_template_object.RawTemplate, 'delete') + def test_purge_db_deletes_previous_template(self, mock_tmpl_delete, + mock_cr): stack = tools.get_stack('test_stack', utils.dummy_context(), template=tools.string_template_five, convergence=True) - prev_tmpl = templatem.Template.create_empty_template() - prev_tmpl.store() - stack.prev_raw_template_id = prev_tmpl.id - stack.store() + stack.prev_raw_template_id = 10 stack.purge_db() - self.assertRaises(exception.NotFound, - templatem.Template.load, - stack.context, prev_tmpl.id) + self.assertTrue(mock_tmpl_delete.called) + + @mock.patch.object(raw_template_object.RawTemplate, 'delete') + def test_purge_db_does_not_delete_previous_template_when_stack_fails( + self, mock_tmpl_delete, mock_cr): + stack = tools.get_stack('test_stack', utils.dummy_context(), + template=tools.string_template_five, + convergence=True) + stack.status = stack.FAILED + stack.purge_db() + self.assertFalse(mock_tmpl_delete.called) def test_purge_db_deletes_sync_points(self, mock_cr): stack = tools.get_stack('test_stack', utils.dummy_context(), @@ -525,17 +533,16 @@ class StackConvergenceCreateUpdateDeleteTest(common.HeatTestCase): stack.context, stack.id, stack.current_traversal) self.assertEqual(0, rows) - def test_purge_db_deletes_stack_for_deleted_stack(self, mock_cr): + @mock.patch.object(stack_object.Stack, 'delete') + def test_purge_db_deletes_stack_for_deleted_stack(self, mock_stack_delete, + mock_cr): stack = tools.get_stack('test_stack', utils.dummy_context(), template=tools.string_template_five, convergence=True) stack.store() stack.state_set(stack.DELETE, stack.COMPLETE, 'test reason') stack.purge_db() - self.assertRaises(exception.NotFound, - parser.Stack.load, - stack.context, stack_id=stack.id, - show_deleted=False) + self.assertTrue(mock_stack_delete.called) def test_get_best_existing_db_resource(self, mock_cr): stack = tools.get_stack('test_stack', utils.dummy_context(), diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py index 07b044dea..6bc8ea7fd 100644 --- a/heat/tests/test_resource.py +++ b/heat/tests/test_resource.py @@ -1479,21 +1479,17 @@ class ResourceTest(common.HeatTestCase): res.name) self.assertEqual(msg, six.text_type(ex)) - @mock.patch.object(resource.Resource, 'delete') - def test_delete_convergence(self, mock_delete): + def test_delete_convergence(self): tmpl = rsrc_defn.ResourceDefinition('test_res', 'Foo') res = generic_rsrc.GenericResource('test_res', tmpl, self.stack) res.requires = [1, 2] res._store() + res.destroy = mock.Mock() self._assert_resource_lock(res.id, None, None) res.delete_convergence('template_key', {(1, True): {}, (1, True): {}}, 'engine-007') - - mock_delete.assert_called_once_with() - self.assertEqual('template_key', res.current_template_id) - self.assertEqual([2], res.requires) - self._assert_resource_lock(res.id, None, 2) + self.assertTrue(res.destroy.called) def test_delete_in_progress_convergence(self): tmpl = rsrc_defn.ResourceDefinition('test_res', 'Foo')