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
Add ambiguous selector check to HPA #112011
Add ambiguous selector check to HPA #112011
Conversation
@pbeschetnov: This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The 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. |
Hi @pbeschetnov. 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 Once the patch is verified, the new status will be reflected by the 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. |
2d50be1
to
777aa97
Compare
fbd0856
to
e6005c3
Compare
e6005c3
to
09b5a08
Compare
09b5a08
to
978e233
Compare
978e233
to
d13f021
Compare
/ok-to-test |
/retest |
1 similar comment
/retest |
/lgtm |
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
/approve
/assign @smarterclayton |
cb5eb83
to
8a1634d
Compare
8a1634d
to
caddfdd
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mwielgus, pbeschetnov, pbetkier, smarterclayton The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
1 similar comment
/retest |
/skip |
What type of PR is this?
/kind bug
What this PR does / why we need it:
The PR changes Horizontal Pod Autoscaler behavior: TL;DR we're adding logic that if more than one HPAs are operating on the same pod, we stop them.
There are cases when selected Pod sets overlap due to misconfiguration:
Example 1:
Deployment A (with controlling HPA A) has selector:
Deployment B (with controlling HPA B) has selector:
And there is a group of Pods with:
Example 2:
HPA A and HPA B point to the same deployment.
As a result:
The same Pods are actuated first by HPA A then by HPA B in an unpredictable way.
Intended behavior:
In this case the HPAs should be disabled and the problem — reported to the user through conditions.
The algorithm to determine these situations is as follows:
ScalingActive=false
and return an error.Which issue(s) this PR fixes:
Fixes #112027
Special notes for your reviewer:
The BiMultimap from package
selectors
is used to store all active HPAs' selectors and efficiently find the HPA-references to current pods.The BiMultimap operates on:
It supports methods: Put (object), PutSelector — to insert entities; and Select — to get objects by a given selector; ReverseSelect — to get selectors that point to a given object.
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: