-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
Automated cherry pick of #53690 #53798
Automated cherry pick of #53690 #53798
Conversation
Hi @mattjmcnaughton. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
/assign @MaciekPytel |
/lgtm |
@wojtek-t can you approve this cherry-pick? |
/ok-to-test |
@mattjmcnaughton looks like the unittests are actually failing (due to useMetricsAPI option missing in testCase in 1.7 branch). Can you take a look? Thanks. |
/retest Review the full test history for this PR. |
6 similar comments
/retest Review the full test history for this PR. |
/retest Review the full test history for this PR. |
/retest Review the full test history for this PR. |
/retest Review the full test history for this PR. |
/retest Review the full test history for this PR. |
/retest Review the full test history for this PR. |
@mattjmcnaughton @MaciekPytel - this doesn't compile:
If you want this to be cherrypicked in the next 1.7 patch release, can you please fix it by Monday? |
Fix kubernetes#53670 Fix a bug where `desiredReplicas` could be greater than `maxReplicas` if the original value for `desiredReplicas > scaleUpLimit` and `scaleUpLimit > maxReplicas`. Previously, when that happened, we would scale up to `scaleUpLimit`, and then in the next auto-scaling run, scale down to `maxReplicas`. Address this issue and introduce a regression test.
9eebd8c
to
27297ea
Compare
@MaciekPytel @wojtek-t failing unit test is now fixed, so this is ready for another look. thanks :) |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MaciekPytel, mattjmcnaughton Associated issue: 53690 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
/retest Review the full test history for this PR. |
/test all [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue. |
Cherry pick of #53690 on release-1.7.
#53690: Fix hpa scaling above max replicas w/ scaleUpLimit