feat(aws-ecs): New CDK constructs for ECS Anywhere task and service definitions #14931

hariohmprasath · 2021-06-01T00:11:17Z

Here is how the user experience will looks like, when using the new constructs for provisioning the ECS Anywhere resources:

// Stack definition
const stack = new cdk.Stack();

// ECS Task definition
const taskDefinition = new ecs.ExternalTaskDefinition(stack, 'ExternalTaskDef');

// Main container
const container = taskDefinition.addContainer('web', {
  image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
  memoryLimitMiB: 512,
});

// Port mapping
container.addPortMappings({
  containerPort: 3000,
});

// ECS Service definition
new ecs.ExternalService(stack, 'ExternalService', {
  cluster,
  taskDefinition,
});

Note: Currently ECS anywhere doesn't support autoscaling, load balancing, AWS Cloudmap discovery and attachment of volumes. So validation rules are created part of this pull request.

This is a follow up for the below PR:
#14811

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

…ith CDK * Customers can now extends the base ecs task `ecs.TaskDefinition` and service `ecs.BaseService` constructs to enable provision of these artifacts, a sample of it provided below. * I will be following up with another PR to introduce two new constructs for ECS anywhere tasks and services (like `ExternalTaskDefinition` & `ExternalTaskDefinition`) so these new constructs can be directly used for provisioning ECS anywhere resources ---- **Sample Task:** ```typescript export class ExternalTaskDefinition extends ecs.TaskDefinition implements ecs.ITaskDefinition { constructor(scope: Construct, id: string, props: ecs.CommonTaskDefinitionProps = {}) { super(scope, id, { ...props, compatibility: ecs.Compatibility.EXTERNAL }); } } ``` **Sample Service:** ```typescript export interface ExternalServiceProps extends ecs.BaseServiceOptions { readonly taskDefinition: ecs.TaskDefinition; } export class ExternalService extends ecs.BaseService implements ecs.IService { constructor(scope: Construct, id: string, props: ExternalServiceProps) { super(scope, id, { ...props, desiredCount: 0, launchType: ecs.LaunchType.EXTERNAL, }, { cluster: props.cluster.clusterName, taskDefinition: props.taskDefinition.taskDefinitionArn }, props.taskDefinition); } } ```

Initial commit with base test cases

gitpod-io · 2021-06-01T00:11:19Z

SoManyHs

Thanks for this contribution. It is recommended for such large features as this to include a proposal for a design first.

There's a few issues I have with extending the existing constructs, given how different the feature set is with ECS-A and regular ECS. Normally, I'd also want to see some integ tests in this kind of change, but I'm not really sure how we can do that in this case. @rix0rrr is there any precedent for simulating an external instance using a VM or something within the CDK test suite?

packages/@aws-cdk/aws-ecs/README.md

SoManyHs · 2021-06-11T06:50:23Z

packages/@aws-cdk/aws-ecs/lib/external/external-task-definition.ts

+  /**
+   * Constructs a new instance of the ExternalTaskDefinition class.
+   */
+  constructor(scope: Construct, id: string, props: ExternalTaskDefinitionProps = {}) {


I wonder if it would be helpful to include the necessary IAM permissions here by default?

The above mentioned permissions are set part of ECS anywhere cluster setup, but for running a task or service we don't need any explicit permission

SoManyHs · 2021-06-11T23:15:10Z

packages/@aws-cdk/aws-ecs/lib/external/external-service.ts

+ *
+ * @resource AWS::ECS::Service
+ */
+export class ExternalService extends BaseService implements IExternalService {


This is tricky -- if we want extend from BaseService, we should probably also add more validations for all the features that aren't supported. I see the validation for CloudMapOptions not being supported, but we'll also need to add them for the following properties/methods from BaseService:

enableExecuteCommand

capacityProviderSrategies

attachToApplicationTargetGroup

attachToClassicLB

loadBalancerTarget

registerLoadBalancerTargets

configureAwsVpcNetworkingWithSecurityGroups

autoScaleTaskCount

networkConfiguration

associateCloudMapOptions

enableCloudMap

...and possibly others that I missed.

This seems like the majority of the BaseService API surface area, so we might need to ask whether or not it makes sense to extend from the BaseService class or not. If there are plans to include the things currently not supported (as enumerated in the considerations for using ECS-A), then perhaps it would. @uttarasridhar do you happen to know what the roadmap for ECS-A is?

Thanks for pointing out the service, I have taken care of everything except attachToClassicLB, as this method is already deprecated.

packages/@aws-cdk/aws-ecs/lib/external/external-task-definition.ts

Co-authored-by: Hsing-Hui Hsu <hsinghui@amazon.com>

Pull request has been modified.

hariohmprasath · 2021-06-17T16:54:58Z

Thanks for this contribution. It is recommended for such large features as this to include a proposal for a design first.

There's a few issues I have with extending the existing constructs, given how different the feature set is with ECS-A and regular ECS. Normally, I'd also want to see some integ tests in this kind of change, but I'm not really sure how we can do that in this case. @rix0rrr is there any precedent for simulating an external instance using a VM or something within the CDK test suite?

Thanks and appreciate all the support. I had a design discussion regarding this with Uttara and Nathan, part of the discussion we went with a separate construct rather than extending the current one. But I agree with you in the future I will submit a design proposal first, before working on the code changes.

Integration tests might be tricky as we would need something like Vagrant with docker support running to execute these test cases. I am open for suggestions if you have any thoughts please let me know happy to brainstorm.

nathanpeck · 2021-07-01T15:26:55Z

packages/@aws-cdk/aws-ecs/lib/external/external-service.ts

+   * Overriden method to throw error as `autoScaleTaskCount` is not supported for external service
+   */
+  public autoScaleTaskCount(_props: appscaling.EnableScalingProps): ScalableTaskCount {
+    throw new Error ('Autoscaling not supported for external service');


I was just checking under the list of considerations in the docs here: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs-anywhere.html

It appears that autoscaling is indeed supported (or at least it is not documented as an unsupported feature for ECS Anywhere). I'm confirming with the team and will attempt to setup autoscaling in my personal test environment for ECS Anywhere, and report back

Thanks @nathanpeck, Based on my conversation with specialist & product team ECS anywhere doesn't know how to add additional nodes when it needs to scale out. So the conclusion was drawn based on this. But I am happy to hear back from you on this.

This autoscaling is not for autoscaling the underlying nodes. It is for autoscaling out the number of containers on the existing nodes. So autoscaling is supported and works on ECS Anywhere as well.

hariohmprasath · 2021-07-08T07:39:12Z

Hi @uttarasridhar , @bvtujo,
Just checking to see whether you had a chance to review this PR, let me know if you have any feedback.

Thanks

packages/@aws-cdk/aws-ecs/README.md

bvtujo · 2021-07-09T16:47:47Z

packages/@aws-cdk/aws-ecs/lib/external/external-service.ts

+  /**
+   * The security groups to associate with the service. If you do not specify a security group, the default security group for the VPC is used.
+   *
+   * This property is only used for tasks that use the awsvpc network mode.


I thought bridge was the only allowed network mode--how does this prop interact with that requirement?

Also, does this create a new security group, or use the default for the VPC?

Good catch, securityGroups can be associated to a task in general doesn't need to depend on the network type. So removed the comment from the code.

packages/@aws-cdk/aws-ecs/lib/external/external-task-definition.ts

Co-authored-by: Austin Ely <austiely@amazon.com>

Pull request has been modified.

mergify · 2021-07-12T21:44:01Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Pull request has been modified.

mergify · 2021-07-13T03:21:46Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2021-07-13T03:59:57Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: bc8f7ce
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2021-07-13T04:00:23Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

…efinitions (aws#14931) Here is how the user experience will looks like, when using the new constructs for provisioning the `ECS Anywhere` resources: ```typescript // Stack definition const stack = new cdk.Stack(); // ECS Task definition const taskDefinition = new ecs.ExternalTaskDefinition(stack, 'ExternalTaskDef'); // Main container const container = taskDefinition.addContainer('web', { image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'), memoryLimitMiB: 512, }); // Port mapping container.addPortMappings({ containerPort: 3000, }); // ECS Service definition new ecs.ExternalService(stack, 'ExternalService', { cluster, taskDefinition, }); ``` > Note: Currently ECS anywhere doesn't support autoscaling, load balancing, `AWS Cloudmap` discovery and attachment of volumes. So validation rules are created part of this pull request. **This is a follow up for the below PR:** [aws#14811 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

hariohmprasath and others added 8 commits May 20, 2021 18:33

Fix eslint errors

3414e39

Merge branch 'master' into master

4b0c602

Merge branch 'aws:master' into master

f025350

New CDK construct for defining ECS Anywhere tasks and services

df0b1d1

Initial commit with base test cases

Merge branch 'master' of https://github.com/hariohmprasath/aws-cdk

2bf08b4

Additional test cases and validations for external service

619bb4f

Updates to README.md

f2fff77

peterwoodworth assigned madeline-k, uttarasridhar and SoManyHs Jun 3, 2021

SoManyHs added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label Jun 9, 2021

Merge branch 'master' into master

f4a5cb7

SoManyHs previously requested changes Jun 11, 2021

View reviewed changes

hariohmprasath and others added 4 commits June 12, 2021 02:16

Update packages/@aws-cdk/aws-ecs/README.md

e4d9718

Co-authored-by: Hsing-Hui Hsu <hsinghui@amazon.com>

Update packages/@aws-cdk/aws-ecs/README.md

63db7f9

Co-authored-by: Hsing-Hui Hsu <hsinghui@amazon.com>

Update packages/@aws-cdk/aws-ecs/README.md

97faf08

Co-authored-by: Hsing-Hui Hsu <hsinghui@amazon.com>

Update packages/@aws-cdk/aws-ecs/README.md

7e233d0

Co-authored-by: Hsing-Hui Hsu <hsinghui@amazon.com>

hariohmprasath added 2 commits June 15, 2021 15:35

Incorporating code review comments

cf7908b

Removed commented code

5e9c338

nathanpeck previously requested changes Jul 1, 2021

View reviewed changes

bvtujo reviewed Jul 9, 2021

View reviewed changes

hariohmprasath and others added 4 commits July 9, 2021 13:56

Update packages/@aws-cdk/aws-ecs/README.md

982a652

Co-authored-by: Austin Ely <austiely@amazon.com>

Update packages/@aws-cdk/aws-ecs/README.md

b9addad

Co-authored-by: Austin Ely <austiely@amazon.com>

Update packages/@aws-cdk/aws-ecs/README.md

42f928d

Co-authored-by: Austin Ely <austiely@amazon.com>

Update packages/@aws-cdk/aws-ecs/README.md

2137b35

Co-authored-by: Austin Ely <austiely@amazon.com>

Update packages/@aws-cdk/aws-ecs/README.md

43e7127

Co-authored-by: Austin Ely <austiely@amazon.com>

hariohmprasath added 2 commits July 9, 2021 14:50

Code review feedback to remove misguided comment

8671167

Default networkMode set as Bridge

1fea040

bvtujo previously approved these changes Jul 12, 2021

View reviewed changes

Merge branch 'master' into master

f98da31

hariohmprasath added 2 commits July 12, 2021 16:52

Fix build errors

e4a1e6d

Merge branch 'master' of https://github.com/hariohmprasath/aws-cdk

027d685

bvtujo approved these changes Jul 13, 2021

View reviewed changes

Merge branch 'master' into master

bc8f7ce

mergify bot merged commit 3592b26 into aws:master Jul 13, 2021

mazyu36 mentioned this pull request Sep 9, 2024

fix(ecs): ECS exec cannot be enabled for ECS Anywhere (ecs.ExternalService) #31374

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(aws-ecs): New CDK constructs for ECS Anywhere task and service definitions #14931

feat(aws-ecs): New CDK constructs for ECS Anywhere task and service definitions #14931

hariohmprasath commented Jun 1, 2021 •

edited

Loading

gitpod-io bot commented Jun 1, 2021

SoManyHs left a comment

SoManyHs Jun 11, 2021

hariohmprasath Jun 17, 2021

SoManyHs Jun 11, 2021

hariohmprasath Jun 17, 2021

hariohmprasath commented Jun 17, 2021

nathanpeck Jul 1, 2021

hariohmprasath Jul 1, 2021

nathanpeck Jul 9, 2021

hariohmprasath commented Jul 8, 2021

bvtujo Jul 9, 2021

hariohmprasath Jul 9, 2021

mergify bot commented Jul 12, 2021

mergify bot commented Jul 13, 2021

aws-cdk-automation commented Jul 13, 2021

mergify bot commented Jul 13, 2021

feat(aws-ecs): New CDK constructs for ECS Anywhere task and service definitions #14931

feat(aws-ecs): New CDK constructs for ECS Anywhere task and service definitions #14931

Conversation

hariohmprasath commented Jun 1, 2021 • edited Loading

gitpod-io bot commented Jun 1, 2021

SoManyHs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hariohmprasath commented Jun 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hariohmprasath commented Jul 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Jul 12, 2021

mergify bot commented Jul 13, 2021

aws-cdk-automation commented Jul 13, 2021

AWS CodeBuild CI Report

mergify bot commented Jul 13, 2021

hariohmprasath commented Jun 1, 2021 •

edited

Loading