-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat(ecs): add L2 support for native ECS blue/green deployments #35179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
packages/aws-cdk-lib/aws-ecs/lib/alternate-target-configuration.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk-lib/aws-ecs/lib/alternate-target-configuration.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk-lib/aws-ecs/lib/alternate-target-configuration.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk-lib/aws-ecs/lib/alternate-target-configuration.ts
Outdated
Show resolved
Hide resolved
packages/aws-cdk-lib/aws-ecs/lib/deployment-lifecycle-hook-target.ts
Outdated
Show resolved
Hide resolved
7393b3c
to
4ea208b
Compare
Hi @aemada-aws Thank you for the review. I just restored everything from #35061 and made minimal necessary changes to avoid potential regression. Let me know if there's any concerns. |
packages/aws-cdk-lib/aws-ecs/lib/alternate-target-configuration.ts
Outdated
Show resolved
Hide resolved
…ments - Restore optional alternateTarget parameter to private attachToELBv2 method - Remove unnecessary attachToELBv2WithAlternateTarget method (overcautious JSII workaround) - Clean up unused resources variable in alternate-target-configuration.ts - Simplify loadBalancerTarget method to directly call attachToELBv2 Since attachToELBv2 is private, JSII doesn't process it for cross-language bindings, making the optional parameter addition safe without breaking changes. Addresses reviewer feedback from PR aws#35179.
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
@Mergifyio refresh |
✅ Pull request refreshed |
✅ Branch has been successfully updated |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Reason for this change
This PR reverts the revert commit
86caea16c4
("Revert "revert(ecs): add support for native blue/green deployments in ECS L2 (#35061) (#35170)") without changing the git history and mitigate the JSII compatibility issues with minimal changes.The original PR #35061 added support for native blue/green deployments in ECS L2 but was reverted due to JSII compatibility issues reported in #35167. The issue was that adding an optional
alternateTarget
parameter to theattachToELBv2
method caused JSII breaking changes, as JSII treats any method signature change as breaking because it affects generated bindings in other languages (Python, Java, C#, Go).Description of changes
This PR restores the blue/green deployment functionality from PR #35061 with minimal changes to address the reviewer feedback:
attachToELBv2
is a private method, JSII doesn't process it for cross-language bindings, so adding an optional parameter is safealternateTarget
parameter back to the privateattachToELBv2
methodattachToELBv2WithAlternateTarget
method that was created due to overcautiousness about JSII compatibilityresources
variable inalternate-target-configuration.ts
(leftover from previous PR)Key changes:
attachToELBv2(targetGroup, containerName, containerPort, alternateTarget?)
- restored to original approach with optional parameter (safe for private methods)loadBalancerTarget
method - directly callsattachToELBv2
with the optional parameterDescribe any new or updated permissions being added
No new permissions are being added. This change restores the original functionality with a simplified implementation based on reviewer feedback.
Description of how you validated changes
Test results:
alternate-target-configuration.test.ts
: 6/6 tests passedbase-service.test.ts
: 25/25 tests passedChecklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license