perf(disruption): exclude pending pods every NodePool is incompatible with from simulations

[pfernandes21]

Summary

On delphi production ~2,200 pending pods that no NodePool can launch for (hard-pinned karpenter.sh/capacity-type=reserved while every reserved offering is exhausted) are scheduled into every consolidation candidate's simulation. Each simulation takes ~20s against a 10s CONSOLIDATION_CANDIDATE_TIMEOUT, so candidate_skips_total{reason="candidate_timed_out"} is sustained and underutilized consolidation decisions dropped to ~0 while ~300 spot GPU nodes sit Consolidatable=True.

Mechanism — disruption-only, provisioning untouched:

1. The provisioner already places each pending pod or records an error per pass. Errors are now typed at the point they arise so the provisioner can tell a pass-invariant rejection from one that hinges on this pass's cluster state:

   // scheduling: taints, base requirements, and instance-type filtering that involved no
   // volume/DRA/topology input are rejections every pass over the same NodePools repeats.
   type NodePoolIncompatibleError struct{ error }
   // The two NodePool-limit rejections in addToNewNodeClaim. A candidate's removal can lift them,
   // so they are downgraded to NodePoolIncompatibleError only when the pod would be rejected without limits.
   type NodePoolLimitError struct{ error }
   func IsIncompatibleWithAllNodePools(err error) bool  // every multierr leg is NodePoolIncompatibleError
   func (r Results) PodsIncompatibleWithAllNodePools() []*corev1.Pod

   When limits trimmed a template's instance types, NodeClaim.instanceTypesBeforeLimits keeps the full set so an instance-type failure over the trimmed set only counts if the untrimmed set rejects the pod too. Topology.Constrains(pod) (new) says whether any topology group can affect the pod; if so the instance-type failure is not classified.
2. state.Cluster:

   MarkPodSchedulingDecisions(...)  // any error now *clears* the pod's verdict
   MarkPodsUnprovisionable(pods)    // provisioner calls with results.PodsIncompatibleWithAllNodePools()
   PodUnprovisionableTime(key)      // time of the latest invariant verdict; zero if placed / other error / unknown

   Also cleared on placement, deletion, mapping cleanup and reset. Virtual buffer pods are never recorded.
3. disruption.SimulateScheduling drops pending pods whose verdict is younger than DISRUPTION_UNPROVISIONABLE_POD_TTL (new flag, default 2m, 0 disables) before appending candidate / deleting-node pods, so those remain authoritative and ordering is unchanged. No schedulability is re-derived.
4. Visibility: gauge karpenter_voluntary_disruption_simulation_pending_pods{disposition="simulated"|"excluded_unprovisionable"} and a once-per-minute Info log with a 5-pod sample.

What is deliberately not excluded (each has a regression test): pods rejected on NodePool limits (a candidate's removal frees limit headroom and the pod must stay in replacement sizing), ReservedOfferingError (a deferred decision; disruption simulates in fallback mode), topology / DRA / volume / minValues-dependent failures, and any error left in PodErrors by a deadline-cut Solve unless it is itself typed invariant (which does not depend on pass progress).

Unchanged: karpenter.sh/do-not-disrupt, PDB handling, IsPreempting volcano special case, pods_did_not_schedule, Go default of CONSOLIDATION_CANDIDATE_TIMEOUT.

Residual approximation, documented in unprovisionablepods.go: an excluded pod may still bind to an existing node another pod vacates (kube-scheduler decides that regardless), so a simulation may hand that room to a candidate's pods instead; if the pending pod wins, the candidate's pods return to the backlog and provisioning launches for them — the same outcome as any pod turning pending between simulation and execution.

Tests

• pkg/controllers/provisioning/scheduling/unprovisionableverdict_test.go: hard incompatibility classified; NodePool limits, topology, ReservedOfferingError not; deadline-cut Solve keeps only invariant verdicts (custom context that trips Err() after N calls to hit the skipped-retry window).
• pkg/controllers/disruption/unprovisionablepods_test.go: reserved-only pod excluded and delete still fires; TTL expiry / zero TTL / missing verdict; compatible NodePool appearing clears the exclusion; limit-blocked pod stays in replacement sizing (single candidate and two candidates removed at once, identical replacement pod sets with TTL on/off); a pod on a capped NodePool with an intrinsic incompatibility is still excluded; drift replaces the drifted node with the pod excluded; metrics.
• pkg/controllers/state/suite_test.go: verdict lifecycle (generic error clears, invariant re-marks, placement / deletion / reset clear).

Verify on delphi after rollout

• karpenter_voluntary_disruption_simulation_pending_pods{disposition="excluded_unprovisionable"} ≈ the reserved-pinned backlog (~2,000), simulated small.
• karpenter_voluntary_disruption_consolidation_candidate_skips_total{reason="candidate_timed_out"} rate → ~0.
• karpenter_voluntary_disruption_decisions_total{method=~"single|multi"} resumes while the pending queue is still ~2,000.
• Log excluding pending pods the provisioner found incompatible with every NodePool from disruption simulation at most once/min.
• Rollback knob: DISRUPTION_UNPROVISIONABLE_POD_TTL=0.

Rollout needs a follow-up monorepo PR to repin infra/kraftsman/fork and bump EXA_SCALE_IMAGE_TAG_BY_STACK (staging first).

Link to Devin session: https://app.devin.ai/sessions/ffcacff9aef74da3a29bb973f01a5872
Open in Devin Desktop: https://app.devin.ai/desktop/session/ffcacff9aef74da3a29bb973f01a5872?variant=devin
Requested by: @pfernandes21

---

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[devin-ai-integration]

Devin Review found 1 potential issue.

[devin-ai-integration]

Adversarial review of #61 — verdict: needs changes (do not merge as-is)

The mechanism (record the provisioner's latest per-pod error, drop fresh-error pods from SimulateScheduling before candidate/deleting-node pods are appended) is implemented cleanly and most of the lifecycle is right. The premise it rests on is not: a provisioning-pass error is not invariant across disruption simulations. The provisioner solves against the whole cluster in strict reserved mode; a disruption simulation solves against the cluster minus the candidates in fallback reserved mode. Two error classes flip between those two views, and I reproduced both locally on this branch (uncommitted tests, output below).

Findings by severity

HIGH — NodePool-limit errors are not simulation-invariant; the replacement can be under-sized (unprovisionablepods.go:35-41, cluster.go:511, helpers.go:82)
Scheduler.remainingResources = spec.limits − Σ capacity of the state nodes handed to the scheduler. SimulateScheduling removes the candidates from those state nodes, so a NodePool at its limit gains one node (or the candidate's CPU/memory) of headroom inside the simulation. A pending pod the provisioner rejected with node limits have been exhausted for nodepool / all available instance types exceed limits for nodepool (scheduler.go:759,764) is therefore placeable in the disruption simulation — on a new NodeClaim that the candidate's own pods share. The file header explicitly lists "to a NodePool at its limit" as a pod that "opens no NodeClaim ... in every disruption simulation alike"; that statement is false.

Repro (envtest, pkg/controllers/disruption, uncommitted): NodePool limits.nodes=2, two 32-CPU nodes, nodes[0] full, candidate nodes[1] hosts one 1-CPU pod, pending pod requests 40 CPU. Provisioner pass → error → verdict recorded. Then SimulateScheduling(candidate=nodes[1]):

baseline (TTL=0):  replacement pods=[pending candidatePod] instanceTypes=192   # pending pod placed on the replacement
PR      (TTL=2m):  replacement pods=[candidatePod]         instanceTypes=600   # replacement sized for the candidate pod only

Failure scenario in production: reserved/limited NodePool at limits, underutilized candidate R with pods C, pending pods Q blocked only by the limit. PR emits replace R with a NodeClaim sized for C. Once launched, kube-scheduler binds Q (pending longest) onto it; C is evicted, goes pending, and the provisioner now rejects C on the same limit → running workload evicted into an indefinitely pending state. Baseline packs Q and C into one adequately sized replacement (or Q into its own claim, which attribution drops). Validate re-runs the same filtered simulation, so it cannot catch this. Applies to drift (drift.go:84) and multi-node consolidation identically (more candidates → more headroom). Whether delphi's reserved NodePool carries spec.limits decides whether this fires today; the code is wrong regardless.

MEDIUM — ReservedOfferingError ("deferred") pods are recorded as unprovisionable (cluster.go:511, provisioner.go:438-452,464-469)
offeringsToReserve (nodeclaim.go) returns ReservedOfferingError when compatible reserved offerings exist but cannot be reserved in this pass — by design a deferred decision, retried next pass. The provisioner itself treats it that way (logs "deferring scheduling decision…", subtracts them from UnschedulablePodsCount), but MarkPodSchedulingDecisions receives results.PodErrors unfiltered, so the PR stamps them as "could place nowhere". Two consequences: (a) pods that are provisionable next pass (in-pass contention for N reservations marks N−1 pods per pass) are hidden from simulations for up to 2m; (b) disruption simulations run in fallback mode, so an unpinned deferred pod would have landed on on-demand/spot — including on the candidate's replacement — same under-sizing as above.

Repro (envtest, pkg/controllers/provisioning/scheduling, uncommitted; mirrors "shouldn't fallback to on-demand or spot when compatible reserved offerings are available"): 3 pods, 2 reservations of capacity 1 → pass 1 schedules one pod, the other two get ReservedOfferingError:

deferred pod paladincrimson-... unprovisionable verdict at 2026-09-03 12:45:25   # non-zero
deferred pod jawcrack-...       unprovisionable verdict at 2026-09-03 12:45:25   # non-zero

and pass 2 places one of them. Note for the delphi case: if the 2,200 pinned pods are getting ReservedOfferingError (check for the "deferring scheduling decision" log line) rather than a hard incompatibility, excluding this class from the verdict would also remove most of the PR's benefit — which is the real signal that "provisioner error" is the wrong predicate.

MEDIUM — A timed-out provisioning pass writes false verdicts (provisioner.go:429-436,464, scheduler.go Solve)
Solve has a 1m deadline. When a pod fails it records podErrors[pod] and re-queues it so it is retried after the rest of the batch has been placed (scheduler.go:527-533 — this is how inter-pod affinity within a batch, topology-spread alternation and in-pass reservation release get resolved); only a successful retry deletes the entry. If the deadline hits first, Solve breaks with the pod still in podErrors although its retry never ran. The provisioner deliberately ignores DeadlineExceeded and passes the partial PodErrors to MarkPodSchedulingDecisions, so every "errored once, not yet retried" pod gets a fresh unprovisionable verdict. Pods never popped get no verdict (fine); the half-processed ones get a wrong one.

Repro (envtest, pkg/controllers/provisioning/scheduling, uncommitted): pod A (2 CPU, popped first) with required hostname affinity to pod B (1 CPU). Without a deadline both schedule and PodErrors is empty. With a context whose Err() flips to DeadlineExceeded after N polls:

deadline after 3 Err() polls: B placed, A in PodErrors (unsatisfiable topology constraint for pod affinity ...), A's requeued retry never ran

and MarkPodSchedulingDecisions(results.PodErrors, ...) — what Schedule does at provisioner.go:464 — leaves A with a non-zero PodUnprovisionableTime. Suggest not writing verdicts at all when Solve returned context.DeadlineExceeded.

LOW — Verdict timestamp vs. state snapshot (cluster.go:496)
now is taken when MarkPodSchedulingDecisions runs, i.e. after batching + up to 1m of Solve, but the verdict describes the cluster as of pass start. Effective staleness bound is TTL + batch + solve ≈ 3m, not 2m. Not unsafe on its own (the pending backlog snapshot in PassReads is stale by the same order), but the doc/flag text should say so.

LOW — The disruption controller now mutates verdict state (provisioner.go:199-211)
SimulateScheduling → pendingPodsForPass → GetPendingPods runs Validate and calls MarkPodSchedulingDecisions for rejected pods — pre-existing, but with this PR every disruption pass refreshes an unprovisionable verdict. Harmless (those pods never enter any simulation) but "the provisioner's most recent scheduling simulation" in the docs is inaccurate.

LOW — Gauge is last-writer-wins across every simulation (unprovisionablepods.go:106-107)
Set by each candidate, validation and multi-node simulation; concurrent candidate evaluation overwrites it. Values within a pass are near-identical so this is fine, but the Help text should say "latest simulation" rather than implying a pass-level aggregate. Cardinality is 2 fixed series — no issue.

INFO — Test coverage of the dangerous cases
All envtest fixtures are delete-only (candidate pod fits an existing node), where exclusion cannot change the command. Nothing exercises replacement sizing, NodePool limits, reserved deferral, a timed-out pass, or drift/multi-node. The three repros above are ready to become regression tests once the verdict model is fixed.

Areas checked, no issue found

• Ordering in SimulateScheduling: filter runs before candidate and deleting-node pods are appended; those stay authoritative, PDB filtering (IsCurrentlyReschedulable) is unchanged; NewCandidate/karpenter.sh/do-not-disrupt and podutils.IsPreempting's Volcano carve-out are untouched.
• pods_did_not_schedule: AllNonPendingPodsScheduled already ignores IsProvisionable pods' errors, so excluding pending pods cannot change that verdict. Emptiness does not simulate pods.
• Lifecycle: Solve deletes a pod from podErrors on a later success, so a pod cannot be in both podErrors and a placement map; MarkPodSchedulingDecisions stores before it deletes anyway. Cleared by UpdatePodToNodeClaimMapping (both call sites), ClearPodSchedulingMappings (pod ready/terminal via metrics pod controller, DeletePod), and Reset (fail-open: everything simulated until the next pass). Memory is bounded by pending pods, same as the sibling maps. sync.Map use matches them.
• Clock: state.Cluster and the disruption controller share the operator clock (controllers.go:106); stored c.clock.Now() is compared against clk.Now() from the same source. time.Now() in helpers.go is only used for stage timing.
• Provisioning stall: no new verdicts → all age out after the TTL → fail-open to today's behaviour. Negative TTL rejected in options validation.
• PassReads memoizes the backlog per pass while verdicts are read live per simulation; a pod can flip between candidates within one pass, which is harmless.

Suggested direction

Make the verdict mean "structurally unplaceable", not "errored in the last pass": (1) never record a verdict on a DeadlineExceeded pass; (2) skip IsReservedOfferingError entries; (3) type the two limit errors at scheduler.go:759,764 (e.g. NodePoolLimitError) and skip them too — or, cheaper as an interim, re-admit excluded pods in SimulateScheduling whenever a candidate's NodePool has spec.limits. A more general alternative that keeps the perf win: leave excluded pods out of the full solve, then try only CanAdd of each excluded pod against the resulting NewNodeClaims so replacement sizing stays honest. Whichever is chosen, add the limit and reserved-deferral regression tests.

Verified locally on c8c991a: go test ./pkg/controllers/disruption/... ./pkg/controllers/state/... passes (envtest 1.37.0), plus the three repro tests above. No commits or pushes were made.