From 761726ced6badc18b5fae47f90c1c6fc167bbe77 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Wed, 23 Apr 2014 18:58:21 -0400 Subject: [PATCH] Don't manipulate template during abandon When deleting a stack during an abandon we want to refrain from deleting the underlying resources, but there is no need to modify their templates to do so. In fact, that is highly undesirable because it leaves the system in an incoherent state, and causes problems for moving away from templates-as-dictionaries to having an internal template API. Change-Id: Iddbb3c53342da66f0edadb1e7a314c29a3eff75b --- heat/engine/parser.py | 8 ++------ heat/engine/resource.py | 13 ++++++++----- heat/engine/service.py | 4 +--- heat/engine/stack_resource.py | 7 ++----- heat/tests/test_parser.py | 17 +---------------- heat/tests/test_resource.py | 14 +++----------- heat/tests/test_stack_resource.py | 13 ++----------- 7 files changed, 19 insertions(+), 57 deletions(-) diff --git a/heat/engine/parser.py b/heat/engine/parser.py index f4b40fde93..4fb465f799 100644 --- a/heat/engine/parser.py +++ b/heat/engine/parser.py @@ -787,22 +787,18 @@ class Stack(collections.Mapping): self.clients.nova().availability_zones.list(detailed=False)] return self._zones - def set_deletion_policy(self, policy): - for res in self.resources.values(): - res.set_deletion_policy(policy) - def set_stack_user_project_id(self, project_id): self.stack_user_project_id = project_id self.store() - def get_abandon_data(self): + def prepare_abandon(self): return { 'name': self.name, 'id': self.id, 'action': self.action, 'status': self.status, 'template': self.t.t, - 'resources': dict((res.name, res.get_abandon_data()) + 'resources': dict((res.name, res.prepare_abandon()) for res in self.resources.values()) } diff --git a/heat/engine/resource.py b/heat/engine/resource.py index 7217daeea1..24eaa02629 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -149,6 +149,8 @@ class Resource(object): self.attributes_schema, self._resolve_attribute) + self.abandon_in_progress = False + if stack.id: resource = db_api.resource_get_by_name_and_stack(self.context, name, stack.id) @@ -451,10 +453,8 @@ class Resource(object): self.reparse() return self._do_action(action, self.properties.validate) - def set_deletion_policy(self, policy): - self.t['DeletionPolicy'] = policy - - def get_abandon_data(self): + def prepare_abandon(self): + self.abandon_in_progress = True return { 'name': self.name, 'resource_id': self.resource_id, @@ -673,7 +673,10 @@ class Resource(object): try: self.state_set(action, self.IN_PROGRESS) - deletion_policy = self.t.get('DeletionPolicy', DELETE) + if self.abandon_in_progress: + deletion_policy = RETAIN + else: + deletion_policy = self.t.get('DeletionPolicy', DELETE) handle_data = None if deletion_policy == DELETE: if callable(getattr(self, 'handle_delete', None)): diff --git a/heat/engine/service.py b/heat/engine/service.py index 5289dfbefd..d175bed0a4 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -748,9 +748,7 @@ class EngineService(service.Service): try: # Get stack details before deleting it. - stack_info = stack.get_abandon_data() - # Set deletion policy to 'Retain' for all resources in the stack. - stack.set_deletion_policy(resource.RETAIN) + stack_info = stack.prepare_abandon() except: with excutils.save_and_reraise_exception(): lock.release(stack.id) diff --git a/heat/engine/stack_resource.py b/heat/engine/stack_resource.py index d0bfeef570..19e4879c08 100644 --- a/heat/engine/stack_resource.py +++ b/heat/engine/stack_resource.py @@ -304,11 +304,8 @@ class StackResource(resource.Resource): return done - def set_deletion_policy(self, policy): - self.nested().set_deletion_policy(policy) - - def get_abandon_data(self): - return self.nested().get_abandon_data() + def prepare_abandon(self): + return self.nested().prepare_abandon() def get_output(self, op): ''' diff --git a/heat/tests/test_parser.py b/heat/tests/test_parser.py index a6377cfb6e..404b2afa56 100644 --- a/heat/tests/test_parser.py +++ b/heat/tests/test_parser.py @@ -1063,7 +1063,7 @@ class StackTest(HeatTestCase): self.stack = parser.Stack(self.ctx, 'stack_details_test', parser.Template(tpl)) self.stack.store() - info = self.stack.get_abandon_data() + info = self.stack.prepare_abandon() self.assertIsNone(info['action']) self.assertIn('id', info) self.assertEqual('stack_details_test', info['name']) @@ -1071,21 +1071,6 @@ class StackTest(HeatTestCase): self.assertIsNone(info['status']) self.assertEqual(tpl, info['template']) - @utils.stack_delete_after - def test_set_stack_res_deletion_policy(self): - tpl = {'Resources': - {'A': {'Type': 'GenericResourceType'}, - 'B': {'Type': 'GenericResourceType'}}} - stack = parser.Stack(self.ctx, - 'stack_details_test', - parser.Template(tpl)) - stack.store() - stack.set_deletion_policy(resource.RETAIN) - self.assertEqual(resource.RETAIN, - stack.resources['A'].t['DeletionPolicy']) - self.assertEqual(resource.RETAIN, - stack.resources['B'].t['DeletionPolicy']) - @utils.stack_delete_after def test_set_param_id(self): self.stack = parser.Stack(self.ctx, 'param_arn_test', diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py index af93ab2ddb..7b3ef6163e 100644 --- a/heat/tests/test_resource.py +++ b/heat/tests/test_resource.py @@ -148,15 +148,7 @@ class ResourceTest(HeatTestCase): self.assertEqual((res.CREATE, res.COMPLETE), res.state) self.assertEqual('wibble', res.status_reason) - def test_set_deletion_policy(self): - tmpl = {'Type': 'Foo'} - res = generic_rsrc.GenericResource('test_resource', tmpl, self.stack) - res.set_deletion_policy(resource.RETAIN) - self.assertEqual(resource.RETAIN, res.t['DeletionPolicy']) - res.set_deletion_policy(resource.DELETE) - self.assertEqual(resource.DELETE, res.t['DeletionPolicy']) - - def test_get_abandon_data(self): + def test_prepare_abandon(self): tmpl = {'Type': 'Foo'} res = generic_rsrc.GenericResource('test_resource', tmpl, self.stack) expected = { @@ -168,7 +160,7 @@ class ResourceTest(HeatTestCase): 'status': 'COMPLETE', 'type': 'Foo' } - actual = res.get_abandon_data() + actual = res.prepare_abandon() self.assertEqual(expected, actual) def test_abandon_with_resource_data(self): @@ -187,7 +179,7 @@ class ResourceTest(HeatTestCase): 'status': 'COMPLETE', 'type': 'Foo' } - actual = res.get_abandon_data() + actual = res.prepare_abandon() self.assertEqual(expected, actual) self.m.VerifyAll() diff --git a/heat/tests/test_stack_resource.py b/heat/tests/test_stack_resource.py index b126a237b8..99b27f3f1f 100644 --- a/heat/tests/test_stack_resource.py +++ b/heat/tests/test_stack_resource.py @@ -242,19 +242,10 @@ class StackResourceTest(HeatTestCase): self.assertEqual(self.stack.id, self.parent_resource.resource_id) @utils.stack_delete_after - def test_set_deletion_policy(self): + def test_prepare_abandon(self): self.parent_resource.create_with_template(self.templ, {"KeyName": "key"}) - self.stack = self.parent_resource.nested() - self.parent_resource.set_deletion_policy(resource.RETAIN) - for res in self.stack.resources.values(): - self.assertEqual(resource.RETAIN, res.t['DeletionPolicy']) - - @utils.stack_delete_after - def test_get_abandon_data(self): - self.parent_resource.create_with_template(self.templ, - {"KeyName": "key"}) - ret = self.parent_resource.get_abandon_data() + ret = self.parent_resource.prepare_abandon() # check abandoned data contains all the necessary information. # (no need to check stack/resource IDs, because they are # randomly generated uuids)