Redistribute unclaimed quota to similar node groups on scale-up#9830
Redistribute unclaimed quota to similar node groups on scale-up#9830rrangith wants to merge 1 commit into
Conversation
|
/assign @norbertcyran |
Make BalanceSimilarNodeGroups quota-aware so a scale-up claims the full collective quota across similar node groups in a single loop, instead of being capped to the best option's quota up front. Previously prepareScaleUp called applyLimits, which capped the total scale-up to the single best-option group's quota before balancing. With per-zone CapacityQuotas this produced suboptimal results (e.g. zones with quota 5/1/1 yielded ~(2,1,1) instead of the optimal (5,1,1)), and the outcome depended on which group the expander happened to pick. Now balanceScaleUps computes each target group's quota headroom (in nodes) via a read-only Tracker.CheckQuota and passes it to BalanceScaleUpBetweenGroups as a maxAddByGroup map. Each group's effective max is lowered to min(MaxSize, currentSize+headroom), so balancing fills groups up to their quota and redistributes the remainder, all in one loop. The balancing algorithm itself is unchanged. capScaleUpsByQuota is kept as the safety net that commits quota and caps the residual for shared quotas (where the read-only per-group headroom over-counts). A nil maxAddByGroup map preserves the original MaxSize-only behavior, so configurations without quotas are unaffected.
dd1d427 to
cd8ac79
Compare
|
@rrangith: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
|
/triage accepted |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jackfrancis, rrangith The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/kind feature Thanks for the review! I'll change it to a feature based on the issue's label |
|
/remove-kind bug |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Followup to #9494, which made similar-node-group balancing respect resource quotas but enforced them after balancing, capping the total scale-up to the best-option group's quota up front and dropping any unclaimed capacity. That left scale-ups suboptimal and order-dependent: with CapacityQuotas of 5/1/1 across three zones and a need for 9 nodes, the result was roughly (2,1,1) or (1,0,0) depending on which group the expander picked, instead of the optimal (5,1,1) achievable in one loop.
This PR makes BalanceSimilarNodeGroups quota-aware so a scale-up claims the full collective quota across similar node groups in a single loop, instead of being capped to the best option's quota up front.
Previously prepareScaleUp called applyLimits, which capped the total scale-up to the single best-option group's quota before balancing. With per-zone CapacityQuotas this produced suboptimal results (e.g. zones with quota 5/1/1 yielded ~(2,1,1) instead of the optimal (5,1,1)), and the outcome depended on which group the expander happened to pick.
Now balanceScaleUps computes each target group's quota headroom (in nodes) via a read-only Tracker.CheckQuota and passes it to BalanceScaleUpBetweenGroups as a maxAddByGroup map. Each group's effective max is lowered to min(MaxSize, currentSize+headroom), so balancing fills groups up to their quota and redistributes the remainder, all in one loop. The balancing algorithm itself is unchanged. capScaleUpsByQuota is kept as the safety net that commits quota and caps the residual for shared quotas (where the read-only per-group headroom over-counts).
A nil maxAddByGroup map preserves the original MaxSize-only behavior, so configurations without quotas are unaffected.
Which issue(s) this PR fixes:
Fixes #9567
Special notes for your reviewer:
For testing, I deployed this change to an AWS cluster. I had 3 similar node groups with 1 node each. One nodegroup had a quota of 5, and the other nodegroups had a quota of 2.
I created 9 new pods that each needed their own node and got the following scale-up plan:
Final scale-up plan: [{asg-A 1->2 (max: 2)} {asg-B 1->2 (max: 2)} {asg-C 1->5 (max: 5)}](real AutoScalingGroup names have been redacted)We can see that all quotas were fully consumed and a total of 6 nodes were scaled up. Before this PR, we would have only gotten up to 4 (asg-C's remaining quota) new nodes.
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: