From fe2c6c192174c18d1d3389026e27682abafa658f Mon Sep 17 00:00:00 2001 From: Andrew Beresford Date: Sun, 28 Nov 2021 21:47:56 +0000 Subject: [PATCH 1/5] Add network modes to ExternalTaskDefinition and TaskDefinition with External compatibility --- .../aws-ecs/lib/base/task-definition.ts | 4 ++-- .../lib/external/external-task-definition.ts | 21 +++++++++++++++---- .../external/external-task-definition.test.ts | 16 +++++++++++++- .../aws-ecs/test/task-definition.test.ts | 19 ++++++++++++++++- 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts b/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts index 9521142e650c5..0d53a789d0f32 100644 --- a/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts +++ b/packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts @@ -401,8 +401,8 @@ export class TaskDefinition extends TaskDefinitionBase { throw new Error('Cannot use inference accelerators on tasks that run on Fargate'); } - if (this.isExternalCompatible && this.networkMode !== NetworkMode.BRIDGE) { - throw new Error(`External tasks can only have Bridge network mode, got: ${this.networkMode}`); + if (this.isExternalCompatible && ![NetworkMode.BRIDGE, NetworkMode.HOST, NetworkMode.NONE].includes(this.networkMode)) { + throw new Error(`External tasks can only have Bridge, Host or None network mode, got: ${this.networkMode}`); } this._executionRole = props.executionRole; diff --git a/packages/@aws-cdk/aws-ecs/lib/external/external-task-definition.ts b/packages/@aws-cdk/aws-ecs/lib/external/external-task-definition.ts index de9fa8b87e9dc..752b6148f664b 100644 --- a/packages/@aws-cdk/aws-ecs/lib/external/external-task-definition.ts +++ b/packages/@aws-cdk/aws-ecs/lib/external/external-task-definition.ts @@ -15,7 +15,14 @@ import { * The properties for a task definition run on an External cluster. */ export interface ExternalTaskDefinitionProps extends CommonTaskDefinitionProps { - + /** + * The networking mode to use for the containers in the task. + * + * With ECS Anywhere, supported modes are bridge, host and none. + * + * @default - NetworkMode.Bridge + */ + readonly networkMode?: NetworkMode; } /** @@ -38,7 +45,6 @@ export interface ExternalTaskDefinitionAttributes extends CommonTaskDefinitionAt * @resource AWS::ECS::TaskDefinition */ export class ExternalTaskDefinition extends TaskDefinition implements IExternalTaskDefinition { - /** * Imports a task definition from the specified task definition ARN. */ @@ -59,11 +65,16 @@ export class ExternalTaskDefinition extends TaskDefinition implements IExternalT return new ImportedTaskDefinition(scope, id, { taskDefinitionArn: attrs.taskDefinitionArn, compatibility: Compatibility.EXTERNAL, - networkMode: NetworkMode.BRIDGE, + networkMode: attrs.networkMode, taskRole: attrs.taskRole, }); } + /** + * The networking mode to use for the containers in the task. + */ + public readonly networkMode: NetworkMode; + /** * Constructs a new instance of the ExternalTaskDefinition class. */ @@ -71,8 +82,10 @@ export class ExternalTaskDefinition extends TaskDefinition implements IExternalT super(scope, id, { ...props, compatibility: Compatibility.EXTERNAL, - networkMode: NetworkMode.BRIDGE, + networkMode: (props.networkMode ?? NetworkMode.BRIDGE), }); + + this.networkMode = props.networkMode ?? NetworkMode.BRIDGE; } /** diff --git a/packages/@aws-cdk/aws-ecs/test/external/external-task-definition.test.ts b/packages/@aws-cdk/aws-ecs/test/external/external-task-definition.test.ts index 3b692f73fe8cf..ba47af03e92e9 100644 --- a/packages/@aws-cdk/aws-ecs/test/external/external-task-definition.test.ts +++ b/packages/@aws-cdk/aws-ecs/test/external/external-task-definition.test.ts @@ -7,6 +7,7 @@ import * as secretsmanager from '@aws-cdk/aws-secretsmanager'; import * as ssm from '@aws-cdk/aws-ssm'; import * as cdk from '@aws-cdk/core'; import * as ecs from '../../lib'; +import { NetworkMode } from '../../lib'; describe('external task definition', () => { describe('When creating an External TaskDefinition', () => { @@ -37,6 +38,7 @@ describe('external task definition', () => { ), }), family: 'ecs-tasks', + networkMode: NetworkMode.HOST, taskRole: new iam.Role(stack, 'TaskRole', { assumedBy: new iam.ServicePrincipal('ecs-tasks.amazonaws.com'), }), @@ -51,7 +53,7 @@ describe('external task definition', () => { ], }, Family: 'ecs-tasks', - NetworkMode: ecs.NetworkMode.BRIDGE, + NetworkMode: ecs.NetworkMode.HOST, RequiresCompatibilities: [ 'EXTERNAL', ], @@ -66,6 +68,18 @@ describe('external task definition', () => { }); + test('error when an invalid networkmode is set', () => { + // GIVEN + const stack = new cdk.Stack(); + + // THEN + expect(() => { + new ecs.ExternalTaskDefinition(stack, 'ExternalTaskDef', { + networkMode: NetworkMode.AWS_VPC, + }); + }).toThrow('External tasks can only have Bridge, Host or None network mode, got: awsvpc'); + }); + test('correctly sets containers', () => { // GIVEN const stack = new cdk.Stack(); diff --git a/packages/@aws-cdk/aws-ecs/test/task-definition.test.ts b/packages/@aws-cdk/aws-ecs/test/task-definition.test.ts index 07b3a8211da06..075d71054df30 100644 --- a/packages/@aws-cdk/aws-ecs/test/task-definition.test.ts +++ b/packages/@aws-cdk/aws-ecs/test/task-definition.test.ts @@ -5,7 +5,7 @@ import * as ecs from '../lib'; describe('task definition', () => { describe('When creating a new TaskDefinition', () => { - test('A task definition with both compatibilities defaults to networkmode AwsVpc', () => { + test('A task definition with EC2 and Fargate compatibilities defaults to networkmode AwsVpc', () => { // GIVEN const stack = new cdk.Stack(); @@ -23,6 +23,23 @@ describe('task definition', () => { }); + + test('A task definition with External compatibility defaults to networkmode Bridge', () => { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + new ecs.TaskDefinition(stack, 'TD', { + cpu: '512', + memoryMiB: '512', + compatibility: ecs.Compatibility.EXTERNAL, + }); + + //THEN + expect(stack).toHaveResource('AWS::ECS::TaskDefinition', { + NetworkMode: 'bridge', + }); + }); }); describe('When importing from an existing Task definition', () => { From 58ae19d0ce8ea8ea966e8ef99cbe2fbdb2992e27 Mon Sep 17 00:00:00 2001 From: Andrew Beresford Date: Wed, 1 Dec 2021 07:50:16 +0000 Subject: [PATCH 2/5] Remove unnecessary import of NetworkMode --- .../aws-ecs/test/external/external-task-definition.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-ecs/test/external/external-task-definition.test.ts b/packages/@aws-cdk/aws-ecs/test/external/external-task-definition.test.ts index ba47af03e92e9..ec4272a7a3f91 100644 --- a/packages/@aws-cdk/aws-ecs/test/external/external-task-definition.test.ts +++ b/packages/@aws-cdk/aws-ecs/test/external/external-task-definition.test.ts @@ -7,7 +7,6 @@ import * as secretsmanager from '@aws-cdk/aws-secretsmanager'; import * as ssm from '@aws-cdk/aws-ssm'; import * as cdk from '@aws-cdk/core'; import * as ecs from '../../lib'; -import { NetworkMode } from '../../lib'; describe('external task definition', () => { describe('When creating an External TaskDefinition', () => { @@ -38,7 +37,7 @@ describe('external task definition', () => { ), }), family: 'ecs-tasks', - networkMode: NetworkMode.HOST, + networkMode: ecs.NetworkMode.HOST, taskRole: new iam.Role(stack, 'TaskRole', { assumedBy: new iam.ServicePrincipal('ecs-tasks.amazonaws.com'), }), @@ -75,7 +74,7 @@ describe('external task definition', () => { // THEN expect(() => { new ecs.ExternalTaskDefinition(stack, 'ExternalTaskDef', { - networkMode: NetworkMode.AWS_VPC, + networkMode: ecs.NetworkMode.AWS_VPC, }); }).toThrow('External tasks can only have Bridge, Host or None network mode, got: awsvpc'); }); From 62e764f92ce461ac78efb60816ea094a22157351 Mon Sep 17 00:00:00 2001 From: Andrew Beresford Date: Wed, 1 Dec 2021 09:09:30 +0000 Subject: [PATCH 3/5] Update documentation for Host and None network modes with ECS Anywhere --- packages/@aws-cdk/aws-ecs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-ecs/README.md b/packages/@aws-cdk/aws-ecs/README.md index 2c4f04e64b257..a8991ccbc1f9f 100644 --- a/packages/@aws-cdk/aws-ecs/README.md +++ b/packages/@aws-cdk/aws-ecs/README.md @@ -74,7 +74,7 @@ Here are the main differences: Application/Network Load Balancers. Only the AWS log driver is supported. Many host features are not supported such as adding kernel capabilities and mounting host devices/volumes inside the container. -- **AWS ECSAnywhere**: tasks are run and managed by AWS ECS Anywhere on infrastructure owned by the customer. Only Bridge networking mode is supported. Does not support autoscaling, load balancing, cloudmap or attachment of volumes. +- **AWS ECSAnywhere**: tasks are run and managed by AWS ECS Anywhere on infrastructure owned by the customer. Bridge, Host and None networking modes are supported. Does not support autoscaling, load balancing, cloudmap or attachment of volumes. For more information on Amazon EC2 vs AWS Fargate, networking and ECS Anywhere see the AWS Documentation: [AWS Fargate](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/AWS_Fargate.html), From d0a67c830fb0346f1fedbe3bfec8af7cc26ad251 Mon Sep 17 00:00:00 2001 From: Andrew Beresford Date: Thu, 10 Mar 2022 10:16:27 +0000 Subject: [PATCH 4/5] Update aws-cdk-test for resource --- packages/@aws-cdk/aws-ecs/test/task-definition.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-ecs/test/task-definition.test.ts b/packages/@aws-cdk/aws-ecs/test/task-definition.test.ts index 6c0ae9bd658a7..befde388b9e56 100644 --- a/packages/@aws-cdk/aws-ecs/test/task-definition.test.ts +++ b/packages/@aws-cdk/aws-ecs/test/task-definition.test.ts @@ -36,7 +36,7 @@ describe('task definition', () => { }); //THEN - expect(stack).toHaveResource('AWS::ECS::TaskDefinition', { + Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', { NetworkMode: 'bridge', }); }); From 05ba07dd2f76ee57be8187545639463bced36170 Mon Sep 17 00:00:00 2001 From: Madeline Kusters Date: Wed, 23 Mar 2022 17:26:52 -0700 Subject: [PATCH 5/5] apply feedback from review --- .../aws-ecs/lib/external/external-task-definition.ts | 11 ++--------- .../test/external/external-task-definition.test.ts | 2 +- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/@aws-cdk/aws-ecs/lib/external/external-task-definition.ts b/packages/@aws-cdk/aws-ecs/lib/external/external-task-definition.ts index 752b6148f664b..ec5643c2f2a81 100644 --- a/packages/@aws-cdk/aws-ecs/lib/external/external-task-definition.ts +++ b/packages/@aws-cdk/aws-ecs/lib/external/external-task-definition.ts @@ -20,7 +20,7 @@ export interface ExternalTaskDefinitionProps extends CommonTaskDefinitionProps { * * With ECS Anywhere, supported modes are bridge, host and none. * - * @default - NetworkMode.Bridge + * @default NetworkMode.BRIDGE */ readonly networkMode?: NetworkMode; } @@ -70,11 +70,6 @@ export class ExternalTaskDefinition extends TaskDefinition implements IExternalT }); } - /** - * The networking mode to use for the containers in the task. - */ - public readonly networkMode: NetworkMode; - /** * Constructs a new instance of the ExternalTaskDefinition class. */ @@ -82,10 +77,8 @@ export class ExternalTaskDefinition extends TaskDefinition implements IExternalT super(scope, id, { ...props, compatibility: Compatibility.EXTERNAL, - networkMode: (props.networkMode ?? NetworkMode.BRIDGE), + networkMode: props.networkMode ?? NetworkMode.BRIDGE, }); - - this.networkMode = props.networkMode ?? NetworkMode.BRIDGE; } /** diff --git a/packages/@aws-cdk/aws-ecs/test/external/external-task-definition.test.ts b/packages/@aws-cdk/aws-ecs/test/external/external-task-definition.test.ts index d049cb9ad3352..92c9e5a7f5148 100644 --- a/packages/@aws-cdk/aws-ecs/test/external/external-task-definition.test.ts +++ b/packages/@aws-cdk/aws-ecs/test/external/external-task-definition.test.ts @@ -50,7 +50,7 @@ describe('external task definition', () => { ], }, Family: 'ecs-tasks', - NetworkMode: ecs.NetworkMode.HOST, + NetworkMode: 'host', RequiresCompatibilities: [ 'EXTERNAL', ],