fix(cli): fix ecs hotswap deployment waiting conditions#27943
fix(cli): fix ecs hotswap deployment waiting conditions#27943tomwwright wants to merge 10 commits intoaws:mainfrom
Conversation
aws-cdk-automation
left a comment
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
f4a94bd to
39531ba
Compare
|
Exemption Request Fix is for CLI so snapshots are not relevant When possible may I please have integration tests run on this PR? Thank you 🙇 |
25d8e0c to
9e6ae94
Compare
|
This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
new deployments are initially in an intermediate state with 0 counts which the waiter may match and return on immediately this uses a more reliable property that signals deployment completion
the FAILED intermediate state is short faster polling ensures responsive deployments
aa6968f to
ad6d8c9
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error. |
|
The pull request linter fails with the following errors: PRs must pass status checks before we can provide a meaningful review. If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing |
|
Can I please get this re-opened? It is not an abandoned PR as evidenced by the recent updates. If possible, I would prefer to avoid creating a new PR, to avoid the noise/effort as well as the fact it appears PRs are being handled roughly in creation order |
…re (#28448) Reraising aws/aws-cdk#27943 as it was incorrectly closed as stale. See original PR for details. Closes aws/aws-cdk#27882. See linked issue for further details. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Closes #27882. See linked issue for further details.
Changes 📝
rolloutStateinstead of task countsrolloutStateto support other deployment typesAvailability of⚠️
rolloutStatepropertyNotably the documentation describes that rolloutState is only provided for services using the rolling update.
Relying on
rolloutStatewould effectively exclude ECS Service hotswap deployment support for other deployment types. To avoid this, I've opted to match for success on the presence of a single deployment -- my observation of the deployment state (see linked issue) makes me feel this will suffice.Note there is no test added for other deployment types
Support for failed deployments using
rolloutState⛔The
rolloutStateproperty is used to detect failure if it is present -- this means that non-rolling update ecs services will be unable to detect failure (parity with where we were before anyway)I've added tests and another waiter condition so that hotswap deployments detect that the deployment has failed. At this stage the error message for this is a bit obtuse -- I figure someone can improve the message in a follow-up PR if users find it confusing. Example:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license