-
Notifications
You must be signed in to change notification settings - Fork 4k
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
cli: ecs service hotswap deployment does not wait for deployment to complete #27882
Comments
Thanks for the report. Yes we definitely should fix that. We'll be happy to review your PR when it's ready. |
Thanks @pahud please find my PR now linked to this issue :) |
@pahud Can you please re-open my linked PR? It has been closed for staleness despite me refreshing the PR recently |
Because of PRs that require cli integ tests run, some PRs have the automated review failing while awaiting review (and integ tests run). We shouldn't auto-close these as stale ever, since the ball is in our court. See #27882, where we were bad. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…8481) Because of PRs that require cli integ tests run, some PRs have the automated review failing while awaiting review (and integ tests run). We shouldn't auto-close these as stale ever, since the ball is in our court. See aws#27882, where we were bad. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
Describe the bug
ECS hotswap deployments report success immediately, instead of waiting for the deployment to succeed (or fail).
Example showing complete deploy time of 16s:
I note this behaviour looks to have been the same since the hotswap was initially implemented and so any users of this feature might expect that it is behaving as expected
Expected Behavior
I expect that the CDK hotswap deployment monitors the state of the triggered deployment via the
DescribeServices
API to ensure it completes successfully before continuingCurrent Behavior
Currently the CDK pushes the ECS hotswap deployment and then immediately reports it as a success and continues.
The CDK does set up a custom waiter to await the successful deployment but the success acceptor is configured as the expression:
This doesn't wait correctly as the new
PRIMARY
deployment is first created with an intermediate state ofrunningCount: 0
anddesiredCount: 0
. It is then populated correctly with a desired and pending count as the scheduler gets to work. But in that initial zero staterunningCount < desiredCount
is false and therefore the waiter matches on it for success and continues.Reproduction Steps
Perform any ECS hotswap deployment
Possible Solution
The following waiter acceptor expression should more accurately interrogate the
DescribeServices
state. I can raise a PR if we agree this is an issue that needs to be fixed.Additional Information/Context
Running this command I observed the following deployment state changes:
New deployment created in "zero" state
Deployment gets correct counts
Deployment launches new task successfully, previous deployment scaled down
Previous deployment scaled down, moves into
DRAINING
statePrevious deployment removed
New deployment completed
CDK CLI Version
2.103.0 (build d0d7547)
Framework Version
No response
Node.js Version
18.16.0
OS
MacOS
Language
TypeScript
Language Version
4.9.5
Other information
No response
The text was updated successfully, but these errors were encountered: