feat(ecs-patterns): support runtimePlatform for fargate patterns #20756

michaeldrey · 2022-06-15T21:20:13Z

feat(aws-ecs-patterns): support runtime platform for fargate patterns

Using the implementation of runtimePlatform from this previous commit, I have added this functionality to the L3 constructs for the fargate patterns. The request asked for this feature to be added to QueueProcessingFargateService and NetworkLoadBalancedFargateService, however I felt it should be added to the rest of the Fargate patterns.

I have added integration tests for each pattern with the runtimePlatform property and have added a section to the readme about how to use it.

closes #18462

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

This PR adds new unconventional dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

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

gitpod-io · 2022-06-15T21:20:20Z

…dk into adding_runtimePlatform

comcalvi

This property (along with all the others that these props share, like cpu) should be extracted into its own level of props that they all inherit, instead of having to just add this prop everywhere. @rix0rrr thoughts on this?

packages/@aws-cdk/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts

.../@aws-cdk/aws-ecs-patterns/lib/fargate/application-multiple-target-groups-fargate-service.ts

packages/@aws-cdk/aws-ecs-patterns/lib/fargate/network-load-balanced-fargate-service.ts

...ages/@aws-cdk/aws-ecs-patterns/lib/fargate/network-multiple-target-groups-fargate-service.ts

packages/@aws-cdk/aws-ecs-patterns/lib/fargate/queue-processing-fargate-service.ts

packages/@aws-cdk/aws-ecs-patterns/lib/fargate/scheduled-fargate-task.ts

Pull request has been modified.

michaeldrey · 2022-06-16T23:06:07Z

@comcalvi That makes sense. I can do that in this PR (imo scope creep this out a bunch), or create a new issue/PR for it.

Looking at the props, aside from scheduled-fargate-task, they're all the same with the exception of a few have the additional securityGroups prop. Condensing them into a base interface that they inherit will make all props consistent across the services as they each end up calling FargateService

scheduled-fargate-task already inherits from scheduled-ecs-task base so I don't believe that needs to be included (but i could be wrong)

rix0rrr · 2022-06-17T18:09:00Z

packages/@aws-cdk/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts

+  /**
+   * The runtime platform of the task definition
+   *
+   * @default undefined If not specified, `operatingSystemFamily` is LINUX and `cpuArchitecture` is X86_64


Do not use the value undefined in describing @default values. This is part of the language specification. Instead, use a dash (-) as the first word and describe the behavior of the infrastructure the construct exhibits when the value is not supplied (See Contributing Guide, Defaults).

Pull request has been modified.

comcalvi

This would be a good case for a separate PR. Feel free to assign the new PR to me, and please drop a comment on this PR linking the new one. Thanks!

packages/@aws-cdk/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts

…d-balanced-fargate-service.ts Co-authored-by: Calvin Combs <66279577+comcalvi@users.noreply.github.com>

Pull request has been modified.

aws-cdk-automation · 2022-07-19T01:18:32Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 8a54ac0
Result: SUCCEEDED
Build Logs (available for 30 days)

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

TheRealAmazonKendra · 2022-08-02T17:04:59Z

@michaeldrey I can't quite tell here if the items Calvin wanted extracted out into a separate PR have been done. Can you let me know and also let me know if this is ready for another round of reviews?

michaeldrey · 2022-08-03T04:57:04Z

@TheRealAmazonKendra they haven't yet, ill work on getting a PR created for it this week

TheRealAmazonKendra

Just assigning this to myself and putting this into changes requested so I get the alert when change have been made

michaeldrey · 2022-08-10T01:19:29Z

New PR opened with this change + the shared props

…untimePlatform (#21529) Stemming from #20756, currently there are a lot of duplicated Fargate props throughout the library ie `cpu`, `memory`. This can make adding Fargate properties tedious as you have to add them into every Fargate type. This PR introduces `FargateServiceBaseProps` which will hold all of the common Fargate properties and allows all Fargate variations to inherit from. As an example, I have added in `runtimePlatform` to all Fargate services. I still need to make a small README update, but want to get this pushed up for thoughts/suggestions/feedback. closes #20756 #18462 ---- ### All Submissions: * [x ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

…untimePlatform (aws#21529) Stemming from aws#20756, currently there are a lot of duplicated Fargate props throughout the library ie `cpu`, `memory`. This can make adding Fargate properties tedious as you have to add them into every Fargate type. This PR introduces `FargateServiceBaseProps` which will hold all of the common Fargate properties and allows all Fargate variations to inherit from. As an example, I have added in `runtimePlatform` to all Fargate services. I still need to make a small README update, but want to get this pushed up for thoughts/suggestions/feedback. closes aws#20756 aws#18462 ---- ### All Submissions: * [x ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Mike Dreyfus added 4 commits June 15, 2022 09:49

initial commit

5b63e19

fixed integration tests

8b1f264

fixed tests and added new integ tests

54a064e

fixed integ tests; updated README

b5adca8

Merge branch 'main' into adding_runtimePlatform

ef322e8

github-actions bot added effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 labels Jun 15, 2022

aws-cdk-automation requested a review from a team June 15, 2022 21:21

Mike Dreyfus and others added 11 commits June 15, 2022 15:28

minor fix to test

cda0002

Merge branch 'adding_runtimePlatform' of github.com:michaeldrey/aws-c…

1d6a943

…dk into adding_runtimePlatform

initial commit

b339aa6

fixed integration tests

8df930b

fixed tests and added new integ tests

32154d2

fixed integ tests; updated README

554891b

minor fix to test

7779ae2

fixing integ test

286cb9b

Merge branch 'adding_runtimePlatform' of github.com:michaeldrey/aws-c…

766d217

…dk into adding_runtimePlatform

removed bad test

063fee8

Merge branch 'main' into adding_runtimePlatform

b05a363

comcalvi previously requested changes Jun 16, 2022

View reviewed changes

updates from comcalvi

cdeeab5

rix0rrr changed the title ~~feat(aws-ecs-patterns): support runtimePlatform for fargate patterns~~ feat(ecs-patterns): support runtimePlatform for fargate patterns Jun 17, 2022

rix0rrr previously requested changes Jun 17, 2022

View reviewed changes

updated comments to spec

529bcb1

Merge branch 'main' into adding_runtimePlatform

20bb332

Merge branch 'main' into adding_runtimePlatform

b584384

comcalvi previously requested changes Jun 28, 2022

View reviewed changes

packages/@aws-cdk/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts Outdated Show resolved Hide resolved

michaeldrey and others added 2 commits July 5, 2022 11:34

Update packages/@aws-cdk/aws-ecs-patterns/lib/fargate/application-loa…

030f612

…d-balanced-fargate-service.ts Co-authored-by: Calvin Combs <66279577+comcalvi@users.noreply.github.com>

Merge branch 'main' into adding_runtimePlatform

ade4fb3

Merge branch 'main' into adding_runtimePlatform

8a54ac0

TheRealAmazonKendra self-assigned this Aug 3, 2022

TheRealAmazonKendra requested changes Aug 3, 2022

View reviewed changes

michaeldrey mentioned this pull request Aug 10, 2022

feat(ecs-patterns): refactor fargate interfaces and add support for runtimePlatform #21529

Merged

3 tasks

michaeldrey closed this Aug 10, 2022

michaeldrey deleted the adding_runtimePlatform branch August 16, 2022 06:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ecs-patterns): support runtimePlatform for fargate patterns #20756

feat(ecs-patterns): support runtimePlatform for fargate patterns #20756

michaeldrey commented Jun 15, 2022 •

edited by comcalvi

gitpod-io bot commented Jun 15, 2022

comcalvi left a comment

michaeldrey commented Jun 16, 2022

rix0rrr Jun 17, 2022

comcalvi left a comment

aws-cdk-automation commented Jul 19, 2022

TheRealAmazonKendra commented Aug 2, 2022

michaeldrey commented Aug 3, 2022

TheRealAmazonKendra left a comment

michaeldrey commented Aug 10, 2022

feat(ecs-patterns): support runtimePlatform for fargate patterns #20756

feat(ecs-patterns): support runtimePlatform for fargate patterns #20756

Conversation

michaeldrey commented Jun 15, 2022 • edited by comcalvi

All Submissions:

Adding new Unconventional Dependencies:

New Features

gitpod-io bot commented Jun 15, 2022

comcalvi left a comment

Choose a reason for hiding this comment

michaeldrey commented Jun 16, 2022

rix0rrr Jun 17, 2022

Choose a reason for hiding this comment

comcalvi left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Jul 19, 2022

AWS CodeBuild CI Report

TheRealAmazonKendra commented Aug 2, 2022

michaeldrey commented Aug 3, 2022

TheRealAmazonKendra left a comment

Choose a reason for hiding this comment

michaeldrey commented Aug 10, 2022

michaeldrey commented Jun 15, 2022 •

edited by comcalvi