From 147e1e6afe28e24ef7b508ca7fed0bb0a926b6f0 Mon Sep 17 00:00:00 2001 From: Thomas Herve Date: Wed, 1 May 2013 16:54:57 +0200 Subject: [PATCH] Make DeletionPolicy a resource attribute instead of a property. This improves validation of the different values, and make the Retain policy works for all resources. Fixes: bug #1160779 Change-Id: Idb0d275c44661db341f693d9d80629661d66c7ac --- heat/engine/resource.py | 19 ++++++- heat/engine/resources/s3.py | 5 -- heat/engine/resources/swift.py | 7 +-- heat/engine/service.py | 1 + heat/tests/test_s3.py | 5 +- heat/tests/test_swift.py | 5 +- heat/tests/test_validate.py | 82 +++++++++++++++++++++++++++ templates/S3_Single_Instance.template | 4 +- templates/Swift.template | 4 +- 9 files changed, 111 insertions(+), 21 deletions(-) diff --git a/heat/engine/resource.py b/heat/engine/resource.py index 460c26a8..7728df02 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -400,8 +400,22 @@ class Resource(object): def validate(self): logger.info('Validating %s' % str(self)) + self.validate_deletion_policy(self.t) return self.properties.validate() + @staticmethod + def validate_deletion_policy(template): + deletion_policy = template.get('DeletionPolicy', 'Delete') + if deletion_policy not in ('Delete', 'Retain', 'Snapshot'): + msg = 'Invalid DeletionPolicy %s' % deletion_policy + raise exception.StackValidationFailed(message=msg) + elif deletion_policy == 'Snapshot': + # Some resources will support it in the future, in which case we + # should check for the presence of a handle_snapshot method for + # example. + msg = 'Snapshot DeletionPolicy not supported' + raise exception.StackValidationFailed(message=msg) + def delete(self): ''' Delete the resource. Subclasses should provide a handle_delete() method @@ -420,8 +434,9 @@ class Resource(object): try: self.state_set(self.DELETE_IN_PROGRESS) - if callable(getattr(self, 'handle_delete', None)): - self.handle_delete() + if self.t.get('DeletionPolicy', 'Delete') == 'Delete': + if callable(getattr(self, 'handle_delete', None)): + self.handle_delete() except Exception as ex: logger.exception('Delete %s', str(self)) failure = exception.ResourceFailure(ex) diff --git a/heat/engine/resources/s3.py b/heat/engine/resources/s3.py index 57668bff..df7f2697 100644 --- a/heat/engine/resources/s3.py +++ b/heat/engine/resources/s3.py @@ -35,9 +35,6 @@ class S3Bucket(resource.Resource): 'AuthenticatedRead', 'BucketOwnerRead', 'BucketOwnerFullControl']}, - 'DeletionPolicy': { - 'Type': 'String', - 'AllowedValues': ['Delete', 'Retain']}, 'WebsiteConfiguration': {'Type': 'Map', 'Schema': website_schema}} @@ -93,8 +90,6 @@ class S3Bucket(resource.Resource): def handle_delete(self): """Perform specified delete policy""" - if self.properties['DeletionPolicy'] == 'Retain': - return logger.debug('S3Bucket delete container %s' % self.resource_id) if self.resource_id is not None: try: diff --git a/heat/engine/resources/swift.py b/heat/engine/resources/swift.py index 6d4866df..219a9042 100644 --- a/heat/engine/resources/swift.py +++ b/heat/engine/resources/swift.py @@ -29,10 +29,7 @@ class SwiftContainer(resource.Resource): 'name': {'Type': 'String'}, 'X-Container-Read': {'Type': 'String'}, 'X-Container-Write': {'Type': 'String'}, - 'X-Container-Meta': {'Type': 'Map', 'Default': {}}, - 'DeletionPolicy': { - 'Type': 'String', - 'AllowedValues': ['Delete', 'Retain']}} + 'X-Container-Meta': {'Type': 'Map', 'Default': {}}} def __init__(self, name, json_snippet, stack): super(SwiftContainer, self).__init__(name, json_snippet, stack) @@ -82,8 +79,6 @@ class SwiftContainer(resource.Resource): def handle_delete(self): """Perform specified delete policy""" - if self.properties['DeletionPolicy'] == 'Retain': - return logger.debug('SwiftContainer delete container %s' % self.resource_id) if self.resource_id is not None: try: diff --git a/heat/engine/service.py b/heat/engine/service.py index 583c7437..ac5e8a05 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -290,6 +290,7 @@ class EngineService(service.Service): props = properties.Properties(ResourceClass.properties_schema, res.get('Properties', {})) try: + ResourceClass.validate_deletion_policy(res) props.validate(with_value=False) except Exception as ex: return {'Error': str(ex)} diff --git a/heat/tests/test_s3.py b/heat/tests/test_s3.py index de4cc5ca..7bc6906a 100644 --- a/heat/tests/test_s3.py +++ b/heat/tests/test_s3.py @@ -225,12 +225,13 @@ class s3Test(HeatTestCase): self.m.ReplayAll() t = self.load_template() - properties = t['Resources']['S3Bucket']['Properties'] - properties['DeletionPolicy'] = 'Retain' + bucket = t['Resources']['S3Bucket'] + bucket['DeletionPolicy'] = 'Retain' stack = self.parse_stack(t) resource = self.create_resource(t, stack, 'S3Bucket') # if delete_container is called, mox verify will succeed resource.delete() + self.assertEqual(resource.DELETE_COMPLETE, resource.state) try: self.m.VerifyAll() diff --git a/heat/tests/test_swift.py b/heat/tests/test_swift.py index 787a9aab..da9a2d16 100644 --- a/heat/tests/test_swift.py +++ b/heat/tests/test_swift.py @@ -246,12 +246,13 @@ class swiftTest(HeatTestCase): self.m.ReplayAll() t = self.load_template() - properties = t['Resources']['SwiftContainer']['Properties'] - properties['DeletionPolicy'] = 'Retain' + container = t['Resources']['SwiftContainer'] + container['DeletionPolicy'] = 'Retain' stack = self.parse_stack(t) resource = self.create_resource(t, stack, 'SwiftContainer') # if delete_container is called, mox verify will succeed resource.delete() + self.assertEqual(resource.DELETE_COMPLETE, resource.state) try: self.m.VerifyAll() diff --git a/heat/tests/test_validate.py b/heat/tests/test_validate.py index 808d5ed5..265b6399 100644 --- a/heat/tests/test_validate.py +++ b/heat/tests/test_validate.py @@ -17,6 +17,7 @@ from heat.tests.v1_1 import fakes from heat.common import exception from heat.common import template_format from heat.engine.resources import instance as instances +from heat.engine import resources from heat.engine import service import heat.db.api as db_api from heat.engine import parser @@ -30,6 +31,7 @@ test_template_volumeattach = ''' "Resources" : { "WikiDatabase": { "Type": "AWS::EC2::Instance", + "DeletionPolicy": "Delete", "Properties": { "ImageId": "image_name", "InstanceType": "m1.large", @@ -262,11 +264,70 @@ test_template_unimplemented_property = ''' } ''' +test_template_invalid_deletion_policy = ''' +{ + "AWSTemplateFormatVersion" : "2010-09-09", + "Description" : "test.", + "Parameters" : { + + "KeyName" : { +''' + \ + '"Description" : "Name of an existing EC2' + \ + 'KeyPair to enable SSH access to the instances",' + \ + ''' + "Type" : "String" + } + }, + + "Resources" : { + "WikiDatabase": { + "Type": "AWS::EC2::Instance", + "DeletionPolicy": "Destroy", + "Properties": { + "ImageId": "image_name", + "InstanceType": "m1.large", + "KeyName": { "Ref" : "KeyName" } + } + } + } + } + ''' + +test_template_snapshot_deletion_policy = ''' +{ + "AWSTemplateFormatVersion" : "2010-09-09", + "Description" : "test.", + "Parameters" : { + + "KeyName" : { +''' + \ + '"Description" : "Name of an existing EC2' + \ + 'KeyPair to enable SSH access to the instances",' + \ + ''' + "Type" : "String" + } + }, + + "Resources" : { + "WikiDatabase": { + "Type": "AWS::EC2::Instance", + "DeletionPolicy": "Snapshot", + "Properties": { + "ImageId": "image_name", + "InstanceType": "m1.large", + "KeyName": { "Ref" : "KeyName" } + } + } + } + } + ''' + class validateTest(HeatTestCase): def setUp(self): super(validateTest, self).setUp() self.fc = fakes.FakeClient() + resources.initialise() setup_dummy_db() def test_validate_volumeattach_valid(self): @@ -373,3 +434,24 @@ class validateTest(HeatTestCase): self.assertEqual( res, {'Error': 'Property SourceDestCheck not implemented yet'}) + + def test_invalid_deletion_policy(self): + t = template_format.parse(test_template_invalid_deletion_policy) + self.m.StubOutWithMock(instances.Instance, 'nova') + instances.Instance.nova().AndReturn(self.fc) + self.m.ReplayAll() + + engine = service.EngineService('a', 't') + res = dict(engine.validate_template(None, t)) + self.assertEqual(res, {'Error': 'Invalid DeletionPolicy Destroy'}) + + def test_snapshot_deletion_policy(self): + t = template_format.parse(test_template_snapshot_deletion_policy) + self.m.StubOutWithMock(instances.Instance, 'nova') + instances.Instance.nova().AndReturn(self.fc) + self.m.ReplayAll() + + engine = service.EngineService('a', 't') + res = dict(engine.validate_template(None, t)) + self.assertEqual( + res, {'Error': 'Snapshot DeletionPolicy not supported'}) diff --git a/templates/S3_Single_Instance.template b/templates/S3_Single_Instance.template index 0c58ab6f..6519092e 100644 --- a/templates/S3_Single_Instance.template +++ b/templates/S3_Single_Instance.template @@ -6,13 +6,13 @@ "Resources" : { "S3BucketWebsite" : { "Type" : "AWS::S3::Bucket", + "DeletionPolicy" : "Delete", "Properties" : { "AccessControl" : "PublicRead", "WebsiteConfiguration" : { "IndexDocument" : "index.html", "ErrorDocument" : "error.html" - }, - "DeletionPolicy" : "Delete" + } } }, "S3Bucket" : { diff --git a/templates/Swift.template b/templates/Swift.template index 2ec17aed..74468bbf 100644 --- a/templates/Swift.template +++ b/templates/Swift.template @@ -6,13 +6,13 @@ "Resources" : { "SwiftContainerWebsite" : { "Type" : "OS::Swift::Container", + "DeletionPolicy" : "Delete", "Properties" : { "X-Container-Read" : ".r:*", "X-Container-Meta" : { "Web-Index" : "index.html", "Web-Error" : "error.html" - }, - "DeletionPolicy" : "Delete" + } } }, "SwiftContainer" : { -- 2.45.2