docs: Balanced consolidation policy RFC#2942
docs: Balanced consolidation policy RFC#2942jamesmt-aws wants to merge 3 commits intokubernetes-sigs:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jamesmt-aws The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @jamesmt-aws. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
7b4dff4 to
592c2bd
Compare
|
/easycla |
designs/balanced-consolidation.md
Outdated
|
|
||
| This exists in all consolidation modes. The cost threshold concentrates the remaining moves onto higher-impact candidates. The system self-corrects: a nearly-empty replacement scores as a trivial DELETE next cycle. Cascades terminate because each round has strictly fewer displaced nodes. | ||
|
|
||
| Configuring kube-scheduler with `MostAllocated` scoring reduces divergence. The [Workload-Aware Scheduling proposal](https://docs.google.com/document/d/1mPYqS4cFmsHPaVQDKyCz7-TKyWNJGjTaZQD3Umkvmgk) (Kepka, Feb 2026) addresses this more directly. |
There was a problem hiding this comment.
Btw this doc isn't accessible. Presumably it's private or possibly a bad link?
There was a problem hiding this comment.
let me ask around for an updated link that I can send you on Kubernetes Slack. I think there was a lot of chatter at KubeCon about the path forward, and I don't really need the link. That will come up in other ways.
Pull Request Test Coverage Report for Build 23920136107Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
| @@ -0,0 +1,442 @@ | |||
| # Balanced Consolidation: Scoring Moves by Savings and Disruption | |||
There was a problem hiding this comment.
I am super excited about this approach!
designs/balanced-consolidation.md
Outdated
| spec: | ||
| disruption: | ||
| consolidationPolicy: Balanced | ||
| consolidationThreshold: 2.0 |
There was a problem hiding this comment.
Thoughts on exposing different Enum values that codify these numbers, rather than the number itself? Also -- note that json cannot support floats in a stable way.
There was a problem hiding this comment.
good call on the float. the formally motivated values are all integers. k=1 is break-even (deletes only, no replaces). k=2 is where within-family replaces become viable, 4-step max steps to stasis. k=3 adds 8 cross-family pairs with the same 4-step number of steps to stasis. at k=4 churn chains, jump to 9 steps and the formal analysis starts arguing against k=4 (or any higher value). so the natural set is {1, 2, 3} and I'll restrict the input type.
I thought about named presets but Karpenter doesn't have an ordinal enum pattern today, and picking names that age well is hard. "Conservative/Balanced/Aggressive" reuses "Balanced" which is already the policy name. I think the integer is simpler, but we can do whatever you and the rest of the community want to do here.
There was a problem hiding this comment.
From an API design perspective, I am not sure that we need both knobs.
Right now, WhenEmpty: k=INF and WhenEmptyOrUnderutilized, k=0.
I see two approaches:
- Expand the enum that aliases other K values
- Expose a new
consolidationThresholdthat works when consolidationPolicy:WhenEmptyOrUnderutilizedand simply changes the threshold.
cc: @jmdeal, @DerekFrank curious to your thoughts.
There was a problem hiding this comment.
Yeah, I think your idea is better. The doc as-written assumes (maybe defensively) that can't justify k=2 uniquely for customers. If that's right, then we need k=3 and k=4, and then we might as well just make this parameter adjust the behavior of WhenEmptyOrUnderutilized. If we can uniquely justify k=2, then we can have a new enum.
I'm leaning towards making k a parameter of WhenEmptyOrUnderutilized based purely on this RFC. @jmdeal and @DerekFrank I'm happy to do whatever you two think is sensible, I'll try to catch up with you you today.
| 1. **Pod deletion cost** ([`controller.kubernetes.io/pod-deletion-cost`](https://kubernetes.io/docs/concepts/workloads/controllers/replicaset/#pod-deletion-cost)), divided by 2^27, range -16 to +16. Default 0. The ReplicaSet controller uses this for scale-down ordering; Karpenter reuses it as a disruption signal. | ||
| 2. **Pod priority**, divided by 2^25, range -64 to +30. Default 0. Higher-priority pods increase their node's disruption cost. | ||
|
|
||
| With neither set, per-pod disruption cost is 1.0. `EvictionCost` clamps to [-10, 10]. The scoring path clamps negative values to 0 via `max(0, EvictionCost(pod))` in the per-node formula (see [NodePool Totals](#nodepool-totals)). Other consumers of `EvictionCost` (eviction ordering) still see negatives. Scoring range per pod: [0, 10]. |
There was a problem hiding this comment.
How does a user communicate that disrupting a pod is free?
There was a problem hiding this comment.
if users want their pods to have zero disruption cost, they can set pod-deletion-cost to a large negative value. that drives EvictionCost negative, and the disruption cost in this RFC clamps the result to 0, so the pod contributes nothing to the node's total disruption cost.
the node still has a disruption cost of 1. nodes have a disruption cost independent of their pods (cordoning, draining, API calls, replacement latency). we haven't modeled this precisely and cost=1 is a placeholder. I'll make that clearer in the design. for today it eliminates a divide-by-zero that comes up if node disruption cost is zero. it could be larger if we wanted, but we don't need that yet.
so a node where every pod has negative deletion cost scores the same as an empty node. cheap to disrupt, not free. if a user wants truly zero-friction disruption at a NodePool level, they want WhenEmptyOrUnderutilized (or k=+inf)
There was a problem hiding this comment.
Got it -- so in our docs we would recommend setting it to -1 if you want to treat it as free.
6e41cd8 to
63468c3
Compare
A new consolidationPolicy: Balanced that scores each consolidation move by comparing savings and disruption as fractions of NodePool totals. Moves where disruption outweighs savings are rejected. - consolidationThreshold (integer, 1-3, default 2): a move passes when its disruption fraction is at most k times its savings fraction - Per-node disruption cost of 1.0 eliminates division-by-zero edge cases - Score-based ranking replaces disruption-only ranking when budget limits move count - Exhaustive verification across c7i/m7i/r7i confirms k=2 is the smallest value that makes within-family REPLACEs viable
63468c3 to
d89e204
Compare
|
squashed to fix EasyCLA (removed Co-Authored-By trailer that the bot couldn't resolve). no content changes beyond what was already pushed. /easycla |
Source: kubernetes-sigs#2942 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Updated the RFC based on a prototype implementation against the karpenter codebase (12 files, 15 unit tests, all passing). The implementation branch is on my fork if anyone wants to look at it. Four substantive changes:
Also: added a brief definition of cycling loops, reframed the churn chain analysis as a formula safety property (on-demand picks cheapest so chains are length 1 in practice; spot has the 15-instance guard), and did an editing pass. |
Updated based on a prototype implementation against the karpenter codebase (all tests passing). Four substantive changes: 1. Examples corrected: all worked numbers now include the 1.0 per-node disruption cost base defined in the NodePool Totals section. Scores shift (3.33 becomes 2.81, 1.67 becomes 1.50, etc.) but all approve/reject decisions hold. 2. evicted_pods clarified: reschedulable pods only. DaemonSet, mirror, and node-owned pods are excluded since they are not evicted during consolidation. 3. Event cardinality: single-node moves emit on Node + NodeClaim. Multi-node moves emit one event on NodePool, not per-candidate. 4. Totals caching: guidance on avoiding O(N) API fan-out when computing NodePool totals at scale. Candidates-only approximation is safe (conservative direction). Also: cycling loop definition, churn chain reframe (formula safety property not practical scenario), consolidateAfter wording, Orwell pass.
14a675d to
a403fee
Compare
Summary
A new
consolidationPolicy: Balancedthat scores each consolidation move by comparing savings and disruption as fractions of NodePool totals. Moves where disruption outweighs savings are rejected.consolidationThreshold(integer, 1-3, default 2): a move passes when its disruption fraction is at most k times its savings fractionRelated issues
aws#8868, aws#8536, aws#6642, aws#7146, #2319, #1019, #735, #1851, #2705, #2883, #1440, #1686, #1430, aws#5218, aws#3577