-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
fix(cli): ecs hotswap deployment waits correctly for success or failure #28448
Changes from all commits
83c37c1
313afb9
f0280e1
d4c772d
1bef62e
7e3b63b
18fbe29
1618d84
807d834
8a9a126
e01ba9d
162139e
b16e436
85517d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -119,11 +119,11 @@ export async function isHotswappableEcsServiceChange( | |
|
||
// Step 3 - wait for the service deployments triggered in Step 2 to finish | ||
// configure a custom Waiter | ||
(sdk.ecs() as any).api.waiters.deploymentToFinish = { | ||
name: 'DeploymentToFinish', | ||
(sdk.ecs() as any).api.waiters.deploymentCompleted = { | ||
name: 'DeploymentCompleted', | ||
operation: 'describeServices', | ||
delay: 10, | ||
maxAttempts: 60, | ||
delay: 6, | ||
maxAttempts: 100, | ||
Comment on lines
+125
to
+126
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you briefly explain the reason for changing these? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The original max delay was I've shortened the delay here to make the deployment more responsive -- people are using hotswap because it's meant to be faster and the value of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I just reminded myself from my commit message for this change as well: the |
||
acceptors: [ | ||
{ | ||
matcher: 'pathAny', | ||
|
@@ -143,16 +143,26 @@ export async function isHotswappableEcsServiceChange( | |
expected: 'INACTIVE', | ||
state: 'failure', | ||
}, | ||
|
||
// failure if any services report a deployment with status FAILED | ||
{ | ||
matcher: 'path', | ||
argument: "length(services[].deployments[? rolloutState == 'FAILED'][]) > `0`", | ||
expected: true, | ||
state: 'failure', | ||
}, | ||
|
||
// wait for all services to report only a single deployment | ||
{ | ||
matcher: 'path', | ||
argument: "length(services[].deployments[? status == 'PRIMARY' && runningCount < desiredCount][]) == `0`", | ||
argument: 'length(services[? length(deployments) > `1`]) == `0`', | ||
expected: true, | ||
state: 'success', | ||
}, | ||
], | ||
}; | ||
// create a custom Waiter that uses the deploymentToFinish configuration added above | ||
const deploymentWaiter = new (AWS as any).ResourceWaiter(sdk.ecs(), 'deploymentToFinish'); | ||
// create a custom Waiter that uses the deploymentCompleted configuration added above | ||
const deploymentWaiter = new (AWS as any).ResourceWaiter(sdk.ecs(), 'deploymentCompleted'); | ||
// wait for all of the waiters to finish | ||
await Promise.all(Object.entries(servicePerClusterUpdates).map(([clusterName, serviceUpdates]) => { | ||
return deploymentWaiter.wait({ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not well versed in this arena, whats the difference between
deploymentToFinish
anddeploymentCompleted
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's purely just naming :) I describe this more in my original PR that was closed (here) but because we can now detect that the deployment failed I noticed the error message based on this naming wasn't clear what had occured
Before:
After: