From 050f50b95b39bfb3004e4e9db5aeccca2dea3785 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Wed, 28 Aug 2024 15:34:11 +0900 Subject: [PATCH] change properties to flat --- packages/aws-cdk-lib/aws-ecs/README.md | 21 ++--- .../aws-ecs/lib/container-definition.ts | 81 ++++++++++--------- .../aws-ecs/lib/firelens-log-router.ts | 4 +- .../aws-ecs/test/container-definition.test.ts | 65 +++++++++------ .../aws-ecs/test/firelens-log-driver.test.ts | 8 +- 5 files changed, 93 insertions(+), 86 deletions(-) diff --git a/packages/aws-cdk-lib/aws-ecs/README.md b/packages/aws-cdk-lib/aws-ecs/README.md index 23778541c5250..0efa1354df967 100644 --- a/packages/aws-cdk-lib/aws-ecs/README.md +++ b/packages/aws-cdk-lib/aws-ecs/README.md @@ -621,28 +621,17 @@ taskDefinition.addContainer('container', { ### Restart policy -To set a restart policy for the container, use the `restartPolicy`. +To enable a restart policy for the container, set `enableRestartPolicy` to true and also specify +`restartIgnoredExitCodes` and `restartAttemptPeriod` if necessary. ```ts declare const taskDefinition: ecs.TaskDefinition; taskDefinition.addContainer('container', { image: ecs.ContainerImage.fromRegistry("amazon/amazon-ecs-sample"), - restartPolicy: { - ignoredExitCodes: [1, 2, 3], - restartAttemptPeriod: cdk.Duration.seconds(360), - }, -}); -``` - -You can also just enable the restart policy with default settings. - -```ts -declare const taskDefinition: ecs.TaskDefinition; - -taskDefinition.addContainer('container', { - image: ecs.ContainerImage.fromRegistry("amazon/amazon-ecs-sample"), - restartPolicy: {}, + enableRestartPolicy: true, + restartIgnoredExitCodes: [1, 2, 3], + restartAttemptPeriod: cdk.Duration.seconds(360), }); ``` diff --git a/packages/aws-cdk-lib/aws-ecs/lib/container-definition.ts b/packages/aws-cdk-lib/aws-ecs/lib/container-definition.ts index d37b5ca4b178d..314c9eba394cb 100644 --- a/packages/aws-cdk-lib/aws-ecs/lib/container-definition.ts +++ b/packages/aws-cdk-lib/aws-ecs/lib/container-definition.ts @@ -386,16 +386,42 @@ export interface ContainerDefinitionOptions { readonly ulimits?: Ulimit[]; /** - * The restart policy for a container. + * Enable a restart policy for a container. * * When you set up a restart policy, Amazon ECS can restart the container without needing to replace the task. * * You can't enable a restart policy for a Firelens log router container. * - * @default - No restart policy. + * @default false * @see https://docs.aws.amazon.com/AmazonECS/latest/developerguide/container-restart-policy.html */ - readonly restartPolicy?: RestartPolicy; + readonly enableRestartPolicy?: boolean; + + /** + * A list of exit codes that Amazon ECS will ignore and not attempt a restart on. + * + * This property is only used if `enableRestartPolicy` is set to true. + * + * You can specify a maximum of 50 container exit codes. + * + * @default - No exit codes are ignored. + */ + readonly restartIgnoredExitCodes?: number[]; + + /** + * A period of time that the container must run for before a restart can be attempted. + * + * A container can be restarted only once every `restartAttemptPeriod` seconds. + * If a container isn't able to run for this time period and exits early, it will not be restarted. + * + * This property is only used if `enableRestartPolicy` is set to true. + * + * You can set a minimum `restartAttemptPeriod` of 60 seconds and a maximum `restartAttemptPeriod` + * of 1800 seconds. + * + * @default - Duration.seconds(300) if `enableRestartPolicy` is true, otherwise no period. + */ + readonly restartAttemptPeriod?: cdk.Duration; } /** @@ -885,7 +911,8 @@ export class ContainerDefinition extends Construct { resourceRequirements: (!this.props.gpuCount && this.inferenceAcceleratorResources.length == 0 ) ? undefined : renderResourceRequirements(this.props.gpuCount, this.inferenceAcceleratorResources), systemControls: this.props.systemControls && renderSystemControls(this.props.systemControls), - restartPolicy: this.props.restartPolicy && renderRestartPolicy(this.props.restartPolicy), + restartPolicy: this.props.enableRestartPolicy ? + renderRestartPolicy(this.props.restartIgnoredExitCodes, this.props.restartAttemptPeriod) : undefined, }; } } @@ -1509,45 +1536,19 @@ function renderSystemControls(systemControls: SystemControl[]): CfnTaskDefinitio })); } -/** - * The restart policy for a container - */ -export interface RestartPolicy { - /** - * A list of exit codes that Amazon ECS will ignore and not attempt a restart on. - * - * You can specify a maximum of 50 container exit codes. - * - * @default - No exit codes are ignored. - */ - readonly ignoredExitCodes?: number[]; - - /** - * A period of time that the container must run for before a restart can be attempted. - * - * A container can be restarted only once every `restartAttemptPeriod` seconds. - * If a container isn't able to run for this time period and exits early, it will not be restarted. - * - * You can set a minimum `restartAttemptPeriod` of 60 seconds and a maximum `restartAttemptPeriod` - * of 1800 seconds. - * - * @default Duration.seconds(300) - */ - readonly restartAttemptPeriod?: cdk.Duration; -} - -function renderRestartPolicy(restartPolicy: RestartPolicy): CfnTaskDefinition.RestartPolicy { - if (restartPolicy.ignoredExitCodes && restartPolicy.ignoredExitCodes.length > 50) { - throw new Error(`Only up to 50 can be specified for ignoredExitCodes, got: ${restartPolicy.ignoredExitCodes.length}`); +function renderRestartPolicy( + restartIgnoredExitCodes?: number[], + restartAttemptPeriod?: cdk.Duration, +): CfnTaskDefinition.RestartPolicy { + if (restartIgnoredExitCodes && restartIgnoredExitCodes.length > 50) { + throw new Error(`Only up to 50 can be specified for restartIgnoredExitCodes, got: ${restartIgnoredExitCodes.length}`); } - if (restartPolicy.restartAttemptPeriod - && (restartPolicy.restartAttemptPeriod.toSeconds() < 60 || restartPolicy.restartAttemptPeriod.toSeconds() > 1800) - ) { - throw new Error(`The restartAttemptPeriod must be between 60 seconds and 1800 seconds, got ${restartPolicy.restartAttemptPeriod.toSeconds()} seconds`); + if (restartAttemptPeriod && (restartAttemptPeriod.toSeconds() < 60 || restartAttemptPeriod.toSeconds() > 1800)) { + throw new Error(`The restartAttemptPeriod must be between 60 seconds and 1800 seconds, got ${restartAttemptPeriod.toSeconds()} seconds`); } return { Enabled: true, - IgnoredExitCodes: restartPolicy.ignoredExitCodes, - RestartAttemptPeriod: restartPolicy.restartAttemptPeriod?.toSeconds(), + IgnoredExitCodes: restartIgnoredExitCodes, + RestartAttemptPeriod: restartAttemptPeriod?.toSeconds(), }; } diff --git a/packages/aws-cdk-lib/aws-ecs/lib/firelens-log-router.ts b/packages/aws-cdk-lib/aws-ecs/lib/firelens-log-router.ts index 440e2e3553d91..fb7a264cf0a47 100644 --- a/packages/aws-cdk-lib/aws-ecs/lib/firelens-log-router.ts +++ b/packages/aws-cdk-lib/aws-ecs/lib/firelens-log-router.ts @@ -223,8 +223,8 @@ export class FirelensLogRouter extends ContainerDefinition { constructor(scope: Construct, id: string, props: FirelensLogRouterProps) { super(scope, id, props); - if (props.restartPolicy) { - throw new Error('Firelens log router container cannot have restartPolicy'); + if (props.enableRestartPolicy) { + throw new Error('Firelens log router container cannot enable restart policy'); } const options = props.firelensConfig.options; diff --git a/packages/aws-cdk-lib/aws-ecs/test/container-definition.test.ts b/packages/aws-cdk-lib/aws-ecs/test/container-definition.test.ts index e7d9dea86e828..c485c566912b4 100644 --- a/packages/aws-cdk-lib/aws-ecs/test/container-definition.test.ts +++ b/packages/aws-cdk-lib/aws-ecs/test/container-definition.test.ts @@ -2768,17 +2768,15 @@ describe('container definition', () => { new ecs.ContainerDefinition(stack, 'Container', { image: ecs.ContainerImage.fromRegistry('/aws/aws-example-app'), taskDefinition, - restartPolicy: { - ignoredExitCodes: [1, 2, 3], - restartAttemptPeriod: cdk.Duration.seconds(360), - }, + enableRestartPolicy: true, + restartIgnoredExitCodes: [1, 2, 3], + restartAttemptPeriod: cdk.Duration.seconds(360), }); // THEN Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', { ContainerDefinitions: [ { - Essential: true, Image: '/aws/aws-example-app', Name: 'Container', RestartPolicy: { @@ -2791,7 +2789,7 @@ describe('container definition', () => { }); }); - test('can specify restart policy with an empty object', () => { + test('ignore restart policy when enableRestartPolicy is set to false', () => { // GIVEN const stack = new cdk.Stack(); const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef'); @@ -2800,42 +2798,65 @@ describe('container definition', () => { new ecs.ContainerDefinition(stack, 'Container', { image: ecs.ContainerImage.fromRegistry('/aws/aws-example-app'), taskDefinition, - restartPolicy: {}, + enableRestartPolicy: false, + restartIgnoredExitCodes: [1, 2, 3], + restartAttemptPeriod: cdk.Duration.seconds(360), }); // THEN Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', { ContainerDefinitions: [ { - Essential: true, Image: '/aws/aws-example-app', Name: 'Container', - RestartPolicy: { - Enabled: true, - }, + RestartPolicy: Match.absent(), + }, + ], + }); + }); + + test('ignore restart policy when enableRestartPolicy is not specified', () => { + // GIVEN + const stack = new cdk.Stack(); + const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef'); + + // WHEN + new ecs.ContainerDefinition(stack, 'Container', { + image: ecs.ContainerImage.fromRegistry('/aws/aws-example-app'), + taskDefinition, + restartIgnoredExitCodes: [1, 2, 3], + restartAttemptPeriod: cdk.Duration.seconds(360), + }); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', { + ContainerDefinitions: [ + { + Image: '/aws/aws-example-app', + Name: 'Container', + RestartPolicy: Match.absent(), }, ], }); }); - test('throws when there are more than 50 in ignoredExitCodes', () => { + test('throws when there are more than 50 in restartIgnoredExitCodes', () => { // GIVEN const stack = new cdk.Stack(); const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef'); // WHEN - const ignoredExitCodes = Array.from({ length: 51 }, (_, i) => i); + const restartIgnoredExitCodes = Array.from({ length: 51 }, (_, i) => i); // THEN expect(() => { new ecs.ContainerDefinition(stack, 'Container', { image: ecs.ContainerImage.fromRegistry('/aws/aws-example-app'), taskDefinition, - restartPolicy: { - ignoredExitCodes, - }, + enableRestartPolicy: true, + restartIgnoredExitCodes, }); - }).toThrow(/Only up to 50 can be specified for ignoredExitCodes, got: 51/); + }).toThrow(/Only up to 50 can be specified for restartIgnoredExitCodes, got: 51/); }); test('throws when restartAttemptPeriod is greater than 1800 seconds', () => { @@ -2848,9 +2869,8 @@ describe('container definition', () => { new ecs.ContainerDefinition(stack, 'Container', { image: ecs.ContainerImage.fromRegistry('/aws/aws-example-app'), taskDefinition, - restartPolicy: { - restartAttemptPeriod: cdk.Duration.seconds(1801), - }, + enableRestartPolicy: true, + restartAttemptPeriod: cdk.Duration.seconds(1801), }); }).toThrow(/The restartAttemptPeriod must be between 60 seconds and 1800 seconds, got 1801 seconds/); }); @@ -2865,9 +2885,8 @@ describe('container definition', () => { new ecs.ContainerDefinition(stack, 'Container', { image: ecs.ContainerImage.fromRegistry('/aws/aws-example-app'), taskDefinition, - restartPolicy: { - restartAttemptPeriod: cdk.Duration.seconds(59), - }, + enableRestartPolicy: true, + restartAttemptPeriod: cdk.Duration.seconds(59), }); }).toThrow(/The restartAttemptPeriod must be between 60 seconds and 1800 seconds, got 59 seconds/); }); diff --git a/packages/aws-cdk-lib/aws-ecs/test/firelens-log-driver.test.ts b/packages/aws-cdk-lib/aws-ecs/test/firelens-log-driver.test.ts index af8891a51b7ec..6377a62b50281 100644 --- a/packages/aws-cdk-lib/aws-ecs/test/firelens-log-driver.test.ts +++ b/packages/aws-cdk-lib/aws-ecs/test/firelens-log-driver.test.ts @@ -444,7 +444,7 @@ describe('firelens log driver', () => { }); }); - test('throws when restartPolicy is set', () => { + test('throws when restart policy is enabled', () => { // THEN expect(() => { td.addFirelensLogRouter('log_router', { @@ -452,11 +452,9 @@ describe('firelens log driver', () => { firelensConfig: { type: ecs.FirelensLogRouterType.FLUENTBIT, }, - restartPolicy: { - ignoredExitCodes: [1, 2, 3], - }, + enableRestartPolicy: true, }); - }).toThrow(/Firelens log router container cannot have restartPolicy/); + }).toThrow(/Firelens log router container cannot enable restart policy/); }); }); });