Repository navigation
Support ECS managed instances capacity provider. - #2971
Open
connoromalleyatwork wants to merge 2 commits into
Open
connoromalleyatwork wants to merge 2 commits into
connoromalleyatwork wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: db427b9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
connoromalleyatwork
added
feature
Contributor
|
🐥 |
connoromalleyatwork
force-pushed
the
cdk-managed-instances
branch
2 times, most recently
from
September 24, 2026 10:48
604a2af to
590f4fb
Compare
Contributor
|
🐥 |
connoromalleyatwork
force-pushed
the
cdk-managed-instances
branch
from
September 24, 2026 12:57
590f4fb to
528e9b3
Compare
Contributor
|
🐥 |
Contributor
|
🐥 |
connoromalleyatwork
force-pushed
the
cdk-managed-instances
branch
from
September 25, 2026 09:17
bdb9694 to
52c967c
Compare
Contributor
|
🐥 |
Contributor
|
🐥 |
Contributor
|
🐥 |
connoromalleyatwork
force-pushed
the
cdk-managed-instances
branch
3 times, most recently
from
September 28, 2026 08:49
1887a5c to
f6c2871
Compare
connoromalleyatwork
force-pushed
the
cdk-managed-instances
branch
4 times, most recently
from
November 2, 2026 13:43
8f6b0ce to
4d0ed91
Compare
Contributor
|
🐥 |
connoromalleyatwork
force-pushed
the
cdk-managed-instances
branch
from
November 2, 2026 14:04
4d0ed91 to
740e994
Compare
Contributor
|
🐥 |
connoromalleyatwork
marked this pull request as ready for review
November 2, 2026 14:34
AshCorr
reviewed
Nov 3, 2026
AshCorr
reviewed
Nov 3, 2026
connoromalleyatwork
force-pushed
the
cdk-managed-instances
branch
from
November 3, 2026 10:57
ca5bf0d to
293c201
Compare
akash1810
approved these changes
Nov 5, 2026
akash1810
left a comment
akash1810
left a comment
Member
There was a problem hiding this comment.
Summarising our IRL discussion: We're not planning on supporting Fargate and managed instances long term. We could refactor to make it easier to remove one of them once we've concluded which forms the paved road. For example:
- Replace the multiple
if/thenbranches with a single one. Its likely to result in an amount of code duplication though. - Adding
TODOcomments to remind ourselves which parts would be removed once we've come to a conclusion. For example, a comment on the aforementionedif/thenbranch and also around the IAM Role being created to facilitate rollback from managed to Fargate.
These refactors could be performed in this or a future PR - the tests will guide us through it.
connoromalleyatwork
force-pushed
the
cdk-managed-instances
branch
4 times, most recently
from
November 6, 2026 10:25
8de0f33 to
e8dbf22
Compare
Contributor
|
🐥 |
connoromalleyatwork
force-pushed
the
cdk-managed-instances
branch
from
November 6, 2026 12:33
e8dbf22 to
db427b9
Compare
| streamPrefix: [scope.stack, scope.stage, app, "devx-logs-sidecar"].join("/"), | ||
| logRetention: RetentionDays.ONE_DAY, | ||
| }), | ||
| if (!ecsProps.managedInstancesProps) { |
Contributor
Author
There was a problem hiding this comment.
Either we
- provision Fargate resources
- provision Managed instances resource
The if statement is negated as i thought it would make the diff easier to read, but it hardly worked :(
| logRetention: RetentionDays.ONE_DAY, | ||
| }), | ||
| if (!ecsProps.managedInstancesProps) { | ||
| // Fargate infrastructure |
Contributor
Author
There was a problem hiding this comment.
The fargate infrastructure is unchanged, hence the first commit shows this because the test snapshot does not change with this code
| // Other resources obtain this tag by extending `GuAppAwareConstruct`. | ||
| [cluster, taskDefinition, ecsService].forEach((_) => AppIdentity.taggedConstruct(props, _)); | ||
| } else { | ||
| // Managed Instances infrastructure |
Contributor
Author
There was a problem hiding this comment.
Managed instances infra starts here!
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this change?
Before:
GuLoadBalancedAppExperimentalprovisions only Fargate constructs for ECS (Elastic Container Service).After:
GuLoadBalancedAppExperimentalcan provision ECS Managed Instances to run ECS tasks and services instead of Fargate by passingmanagedInstancesProps: {},in theecsProps.GuLoadBalancedAppExperimentalcan also provision Fargate constructs and the instance role + profile for managed instances by passingprovisionManagedSecurityRole: trueinecsProps. This means that in the case of a rollback from Managed Instances -> Fargate the rollback completes successfully. When the role is provisioned with fargate it is unused, and can be deleted later.Amiable PR to prepare to move to managed instances: guardian/amiable#1013
Why this change?
When investigating potentially using ECS Managed Instances instead of Fargate as part of ADR for ECS Managed Instances, it became clear that we need to learn more about Managed Instances by trying it out with an actual service.
Managed Instances can optimise the provisioned infrastructure over time for a given ECS cluster, and so the only way to understand the potential cost benefits for a given service is to try it in production.
How to Review
How to test
Check for regressions: Use the latest canary published from this branch in your project that uses the
GuLoadBalancedAppExperimentalconstruct causes no changes to the cloud formation templates created bycdk synth.Check Managed Instances constructs are
synthed: addmanagedInstancesProps: {},in theecsPropsofGuLoadBalancedAppExperimental.Checklist
Footnotes
If you are adding a new construct or pattern, has new documentation been added? If you are amending defaults or changing behaviour, are the existing docs still valid? ↩
Consider whether this is something that will mean changes to projects that have already been migrated, or to the CDK CLI tool. If changes are required, consider adding a checklist here and/or linking to related PRs. ↩