-
Notifications
You must be signed in to change notification settings - Fork 154
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
Introduce service stable logic to ensure ECS workload available #4548
Conversation
Signed-off-by: khanhtc1202 <[email protected]>
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #4548 +/- ##
==========================================
- Coverage 29.97% 29.95% -0.03%
==========================================
Files 220 220
Lines 25851 25872 +21
==========================================
Hits 7749 7749
- Misses 17455 17476 +21
Partials 647 647
☔ View full report in Codecov by Sentry. |
Signed-off-by: khanhtc1202 <[email protected]>
Signed-off-by: khanhtc1202 <[email protected]>
Signed-off-by: khanhtc1202 <[email protected]>
@kentakozuka plz |
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.
Nice :)
Left a comment.
// Retry wait service stable check. | ||
retryServiceStable = 40 | ||
// Interval wait service stable check. | ||
retryServiceStableInterval = 15 * time.Second |
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.
Can you tell me why this timeout is 10mins(40 * 15sec)?
The default deployment timeout is now 6 hours.
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.
This follows the AWS cli implementation, where they use the above numbers. Of course, we can wait up to 6 hours, but I just feel like it's overdoing.
https://awscli.amazonaws.com/v2/documentation/api/latest/reference/ecs/wait/services-stable.html#description
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.
OK 👍 Thanks
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.
Super!
Let's go 🚀
…-cd#4548) * Introduce service stable logic to ensure ECS workload available Signed-off-by: khanhtc1202 <[email protected]> * Reimplement WaitServiceStable logic Signed-off-by: khanhtc1202 <[email protected]> * Update waiting logic Signed-off-by: khanhtc1202 <[email protected]> * Update service stable condition Signed-off-by: khanhtc1202 <[email protected]> --------- Signed-off-by: khanhtc1202 <[email protected]> Signed-off-by: sZma5a <[email protected]>
…-cd#4548) * Introduce service stable logic to ensure ECS workload available Signed-off-by: khanhtc1202 <[email protected]> * Reimplement WaitServiceStable logic Signed-off-by: khanhtc1202 <[email protected]> * Update waiting logic Signed-off-by: khanhtc1202 <[email protected]> * Update service stable condition Signed-off-by: khanhtc1202 <[email protected]> --------- Signed-off-by: khanhtc1202 <[email protected]> Signed-off-by: moko-poi <[email protected]>
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #4478
Does this PR introduce a user-facing change?: