-
Notifications
You must be signed in to change notification settings - Fork 729
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
If HPA is set, it uses HPA minReplicas when scaling up the canary #1253
If HPA is set, it uses HPA minReplicas when scaling up the canary #1253
Conversation
1ecbba4
to
835fa22
Compare
Codecov Report
@@ Coverage Diff @@
## main #1253 +/- ##
==========================================
- Coverage 54.74% 54.68% -0.06%
==========================================
Files 81 81
Lines 6949 6965 +16
==========================================
+ Hits 3804 3809 +5
- Misses 2550 2559 +9
- Partials 595 597 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
thanks for raising this PR @andylibrian 🙇
Thanks for the review, @aryan9600. I have added the v2 HPA support as you suggested. Do you have any other comment? |
72f7295
to
3df93b8
Compare
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.
could you please make this change and also modify the commit title to be something like "use min replicas set by autoscaler in ScaleFromZero
, if autoscaler is specified"?
066b314
to
1c33544
Compare
…specified Without this, the canary replicas are updated twice: to 1 replica then after a few seconds to the value of HPA minReplicas. In some cases, when updated to 1 replica (before updated by HPA controller to the minReplicas), it's considered ready: 1 of 1 (readyThreshold 100%), and the canary weight is advanced to receive traffic with less capacity than expected. Co-Authored-By: Joshua Gibeon <[email protected]> Co-authored-by: Sanskar Jaiswal <[email protected]> Signed-off-by: Andy Librian <[email protected]>
1c33544
to
8b11551
Compare
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.
lgtm! thanks @andylibrian 🙇
Without this, the canary replicas are updated twice: to 1 replica then after a few seconds to the value of HPA minReplicas.
In some cases, when updated to 1 replica (before updated by HPA controller to the minReplicas), it's considered ready: 1 of 1 (readyThreshold 100%), and the canary weight is advanced to receive traffic with less capacity than expected.
Similar PR previously: #1110
Signed-off-by: Andy Librian [email protected]