-
Notifications
You must be signed in to change notification settings - Fork 4k
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
leave a buffer of underutilized nodes when scaling down #5611
Conversation
Appreciate the PR (though I am not sure I fully understand the problem yet) 👍
This is expected. CA doesn't support |
I know that it's not supported, that's why I made this PR to make it somewhat supported, it's not perfect but might be good enough ... I'll have to test it in our clusters to know more, but wanted to share the approach in case anyone else finds it useful or has inout on how to make it better. |
/assign @MaciekPytel |
A better version of this would calculate the "free cpu" on all nodes and then leave "10% of capacity free" instead of this crude node math. |
3d49328
to
5be1790
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: grosser The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
this worked but was not reliable enough (sometimes leaves half a node empty, sometimes full nodes) and cost accounting was not great either (cannot differentiate between bad binpacking and intentional gaps) |
What type of PR is this?
/kind feature
What this PR does / why we need it:
... so allow users to opt-in to empty space
This is not perfect since it will not create "new empty space" by scaling up, but I think it's a good step forward and allows us to fix some edge-cases that capacity buffer does not solve.
Which issue(s) this PR fixes:
Fixes #5377
Special notes for your reviewer:
Does this PR introduce a user-facing change?