From 715be7c6d8455632a09faa8242815bcfa662cc3a Mon Sep 17 00:00:00 2001 From: Daniel Neilson <53624638+ddneilson@users.noreply.github.com> Date: Mon, 10 Aug 2020 12:41:19 -0500 Subject: [PATCH] fix(deadline): add retention policy for created efs (#11) BREAKING CHANGE: databaseRemovalPolicy property of Repository has been renamed to removalPolicy.database --- integ/lib/storage-struct.ts | 6 +- .../aws-rfdk/lib/deadline/lib/repository.ts | 40 +++++- .../lib/deadline/test/repository.test.ts | 124 ++++++++++++++++-- 3 files changed, 150 insertions(+), 20 deletions(-) diff --git a/integ/lib/storage-struct.ts b/integ/lib/storage-struct.ts index 289ca6496..4991f14c4 100644 --- a/integ/lib/storage-struct.ts +++ b/integ/lib/storage-struct.ts @@ -65,6 +65,7 @@ export class StorageStruct extends Construct { deadlineEfs = new FileSystem(this, 'FileSystem', { vpc, + removalPolicy: RemovalPolicy.DESTROY, }); deadlineMountableEfs = new MountableEfs(this, { filesystem: deadlineEfs, @@ -89,7 +90,10 @@ export class StorageStruct extends Construct { logGroupProps: { logGroupPrefix: Stack.of(this).stackName + '-' + id, }, - databaseRemovalPolicy: RemovalPolicy.DESTROY, + removalPolicy: { + database: RemovalPolicy.DESTROY, + filesystem: RemovalPolicy.DESTROY, + }, }); this.docdb = ( deadlineDatabase || this.repo.node.findChild('DocumentDatabase') as DatabaseCluster ); diff --git a/packages/aws-rfdk/lib/deadline/lib/repository.ts b/packages/aws-rfdk/lib/deadline/lib/repository.ts index 7584bc937..766b2b0b0 100644 --- a/packages/aws-rfdk/lib/deadline/lib/repository.ts +++ b/packages/aws-rfdk/lib/deadline/lib/repository.ts @@ -212,6 +212,29 @@ export interface RepositoryBackupOptions { readonly databaseRetention?: Duration; } +/* + * Properties that define the removal policies of resources that are created by the Repository. These define what happens + * to the resources when the stack that defines them is destroyed. + */ +export interface RepositoryRemovalPolicies { + /** + * If this Repository is creating its own Amazon DocumentDB database, then this specifies the retention policy to + * use on the database. If the Repository is not creating a DocumentDB database, because one was given, + * then this property is ignored. + * + * @default RemovalPolicy.RETAIN + */ + readonly database?: RemovalPolicy; + + /** + * If this Repository is creating its own Amazon Elastic File System (EFS), then this specifies the retention policy to + * use on the filesystem. If the Repository is not creating an EFS, because one was given, then this property is ignored. + * + * @default RemovalPolicy.RETAIN + */ + readonly filesystem?: RemovalPolicy; +} + /** * Properties for the Deadline repository */ @@ -266,12 +289,12 @@ export interface RepositoryProps { readonly database?: DatabaseConnection; /** - * If this Repository is creating its own DocumentDB database, then this specifies the retention policy to - * use on the database. + * Define the removal policies for the resources that this Repository creates. These define what happens + * to the resoureces when the stack that defines them is destroyed. * - * @default RemovalPolicy.RETAIN + * @default RemovalPolicy.RETAIN for all resources */ - readonly databaseRemovalPolicy?: RemovalPolicy; + readonly removalPolicy?: RepositoryRemovalPolicies; /** * If this Repository is creating its own Amazon DocumentDB database, then this specifies the number of @@ -399,6 +422,12 @@ export class Repository extends Construct implements IRepository { if (props.database && props.backupOptions?.databaseRetention) { this.node.addWarning('Backup retention for database will not be applied since a database is not being created by this construct'); } + if (props.fileSystem && props.removalPolicy?.filesystem) { + this.node.addWarning('RemovalPolicy for filesystem will not be applied since a filesystem is not being created by this construct'); + } + if (props.database && props.removalPolicy?.database) { + this.node.addWarning('RemovalPolicy for database will not be applied since a database is not being created by this construct'); + } this.version = props.version; @@ -409,6 +438,7 @@ export class Repository extends Construct implements IRepository { vpcSubnets: props.vpcSubnets ?? { subnetType: SubnetType.PRIVATE }, encrypted: true, lifecyclePolicy: EfsLifecyclePolicy.AFTER_14_DAYS, + removalPolicy: props.removalPolicy?.filesystem ?? RemovalPolicy.RETAIN, }), }); @@ -427,7 +457,7 @@ export class Repository extends Construct implements IRepository { backup: { retention: props.backupOptions?.databaseRetention ?? Repository.DEFAULT_DATABASE_RETENTION_PERIOD, }, - removalPolicy: props.databaseRemovalPolicy ?? RemovalPolicy.RETAIN, + removalPolicy: props.removalPolicy?.database ?? RemovalPolicy.RETAIN, }); /* istanbul ignore next */ if (!dbCluster.secret) { diff --git a/packages/aws-rfdk/lib/deadline/test/repository.test.ts b/packages/aws-rfdk/lib/deadline/test/repository.test.ts index 11c035fd8..fdbb8c6f0 100644 --- a/packages/aws-rfdk/lib/deadline/test/repository.test.ts +++ b/packages/aws-rfdk/lib/deadline/test/repository.test.ts @@ -369,6 +369,116 @@ test('repository mounts repository filesystem', () => { expect(userData).toMatch(new RegExp(escapeTokenRegex('mountEfs.sh ${Token[TOKEN.\\d+]} /mnt/efs/fs1 rw'))); }); +test.each([ + [RemovalPolicy.DESTROY, 'Delete'], + [RemovalPolicy.RETAIN, 'Retain'], + [RemovalPolicy.SNAPSHOT, 'Snapshot'], +])('repository honors database removal policy: %p', (policy: RemovalPolicy, expected: string) => { + // WHEN + new Repository(stack, 'repositoryInstaller', { + vpc, + version: deadlineVersion, + removalPolicy: { + database: policy, + }, + }); + + // THEN + expectCDK(stack).to(haveResourceLike('AWS::DocDB::DBCluster', { + DeletionPolicy: expected, + }, ResourcePart.CompleteDefinition)); +}); + +test.each([ + [RemovalPolicy.DESTROY, 'Delete'], + [RemovalPolicy.RETAIN, 'Retain'], + [RemovalPolicy.SNAPSHOT, 'Snapshot'], +])('repository honors filesystem removal policy: %p', (policy: RemovalPolicy, expected: string) => { + // WHEN + new Repository(stack, 'repositoryInstaller', { + vpc, + version: deadlineVersion, + removalPolicy: { + filesystem: policy, + }, + }); + + // THEN + expectCDK(stack).to(haveResourceLike('AWS::EFS::FileSystem', { + DeletionPolicy: expected, + }, ResourcePart.CompleteDefinition)); +}); + +test('repository warns if removal policy for filesystem when filesystem provided', () => { + // GIVEN + const testEFS = new EfsFileSystem(stack, 'TestEfsFileSystem', { + vpc, + }); + const testFS = new MountableEfs(stack, { + filesystem: testEFS, + }); + + // WHEN + const repo = new Repository(stack, 'repositoryInstaller', { + vpc, + fileSystem: testFS, + version: deadlineVersion, + removalPolicy: { + filesystem: RemovalPolicy.DESTROY, + }, + }); + + // THEN + expect(repo.node.metadata).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + type: 'aws:cdk:warning', + data: 'RemovalPolicy for filesystem will not be applied since a filesystem is not being created by this construct', + }), + ]), + ); +}); + +test('repository warns if removal policy for database when database provided', () => { + // GIVEN + const fsDatabase = new DatabaseCluster(stack, 'TestDbCluster', { + masterUser: { + username: 'master', + }, + instanceProps: { + instanceType: InstanceType.of( + InstanceClass.R4, + InstanceSize.LARGE, + ), + vpc, + vpcSubnets: { + onePerAz: true, + subnetType: SubnetType.PRIVATE, + }, + }, + }); + + // WHEN + const repo = new Repository(stack, 'repositoryInstaller', { + vpc, + database: DatabaseConnection.forDocDB({ database: fsDatabase, login: fsDatabase.secret! }), + version: deadlineVersion, + removalPolicy: { + database: RemovalPolicy.DESTROY, + }, + }); + + // THEN + expect(repo.node.metadata).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + type: 'aws:cdk:warning', + data: 'RemovalPolicy for database will not be applied since a database is not being created by this construct', + }), + ]), + ); +}); + test('repository creates deadlineDatabase if none provided', () => { const testEFS = new EfsFileSystem(stack, 'TestEfsFileSystem', { vpc, @@ -446,20 +556,6 @@ test('repository honors database instance count', () => { })); }); -test('repository honors database removal policy', () => { - // WHEN - new Repository(stack, 'repositoryInstaller', { - vpc, - version: deadlineVersion, - databaseRemovalPolicy: RemovalPolicy.DESTROY, - }); - - // THEN - expectCDK(stack).to(haveResourceLike('AWS::DocDB::DBCluster', { - DeletionPolicy: 'Delete', - }, ResourcePart.CompleteDefinition)); -}); - test('repository honors database retention period', () => { // GIVEN const period = 20;