fix: treat volcano nominations as non-blocking for provisioning

[pfernandes21]

Summary

Fixes a gang-scheduling deadlock reproduced twice on delphi-staging (2026-08-28): Volcano sets status.nominatedNodeName on gang members pipelined behind an eviction while the gang as a whole cannot bind (minAvailable unmet), and never clears it. Karpenter's IsPreempting treats any nomination as "about to schedule onto freed capacity" (kube-scheduler semantics), so both workers of a 2-node gang stayed nominated to the one idle node, were excluded from IsProvisionable, and the missing 2nd node was never launched.

 func IsPreempting(pod *corev1.Pod) bool {
-	return pod.Status.NominatedNodeName != ""
+	return pod.Status.NominatedNodeName != "" && pod.Spec.SchedulerName != VolcanoSchedulerName
 }

Only volcano-scheduled pods change behavior; kube-scheduler nominations keep upstream semantics. During a genuine Volcano preemption, Karpenter's simulation excludes terminating victims from node utilization, so the preemptor still simulates as fitting the nominated node and no duplicate node is created.

An earlier commit also skipped unnarrowed Exists requirements in resolveCustomLabelsFromRequirements; that hunk was reverted (033a92c) to keep upstream's documented Exists contract — this PR now carries only the nomination fix.

Link to Devin session: https://app.devin.ai/sessions/a54e0626632740048b2eaceececa6ee7
Open in Devin Desktop: https://app.devin.ai/desktop/session/a54e0626632740048b2eaceececa6ee7?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

[exa-heron-staging]

Heron review

global-review-orchestrator-guardian

Verdict: ❌ Reject · 1 finding · head 033a92ccab · openai/gpt-5.6-luna · 3m 58s · session

Severity	Rule	Location	Finding
🛑 blocking	unsafe-side-effect	pkg/utils/pod/scheduling.go:149	Volcano preemption still has a pre-delete race that launches an extra NodeClaim

_{Last updated 2026-08-28 19:05 UTC · reply @heron dismiss <reason> to dismiss on its comment thread}

[exa-heron-staging]

Volcano preemption still has a pre-delete race that launches an extra NodeClaim

🛑 blocking · rule unsafe-side-effect · confidence 0.95

The reply's simulation argument is only true after the preempted victim's deletionTimestamp is visible; this predicate creates a real race before that point. Volcano's source at https://github.com/volcano-sh/volcano/blob/d57d10f4/pkg/scheduler/cache/cache.go starts Evictor.Evict in a goroutine, and that evictor performs a separate status update followed by the pod delete. The same source's taskUnschedulable path then publishes the preemptor's Unschedulable condition and nomination, so there is no atomic guarantee that the victim is already marked terminating when Karpenter observes the nomination. In that interval, the changed line makes the preemptor pass IsProvisionable; pkg/controllers/provisioning/controller.go:67 triggers the batcher, while pkg/controllers/provisioning/provisioner.go:377,380 snapshots the nodes before collecting pending pods. pkg/controllers/provisioning/scheduling/existingnode.go:111 therefore still accounts for the live victim and rejects the nominated node, causing CreateNodeClaims at pkg/controllers/provisioning/provisioner.go:161 to launch capacity. The author's claim that an empty claim is immediately consolidated does not make this safe: consolidation can be disabled (pkg/apis/v1/nodepool.go:93 accepts a nil value and pkg/controllers/disruption/consolidation.go:141 skips it), may be blocked by budgets/validation, and the preemptor can land on the new node before the victim disappears. Thus the acknowledged pre-delete window can leave an unnecessary node (or move the preemptor onto it) during every genuine Volcano preemption, not merely the gang-pipelining case. Preserve the distinction at a caller with access to the nominated node/victim state (or consume an explicit Volcano preemption signal), and add a test covering nomination plus a still-active victim; the current tests only assert the blanket Volcano exception at pkg/utils/pod/suite_test.go:262-267,289-297.

Suggested fix:

Do not make every Volcano nomination provisionable. Gate the exception on scheduler-specific evidence that the nomination is a gang pipeline rather than a live preemption, or defer provisioning until the nominated node's actual preemption victim is observed terminating; validate this with an end-to-end test where the nomination is observed before the victim delete.

_{Heron review global-review-orchestrator-guardian · fingerprint c3da29da0549 · reply @heron dismiss <reason> to dismiss}

[devin-ai-integration]

The duplicate-provisioning window during a genuine Volcano preemption is covered by Karpenter's own scheduling simulation rather than by this predicate: eviction sets the victim's deletionTimestamp, terminating pods fail IsReschedulable (IsActive is false), so they don't occupy capacity when the provisioner simulates existing nodes — the preemptor simulates as fitting the nominated node and no NodeClaim is created. The residual race is only the gap between Volcano writing the nomination and the victim's delete landing, bounded by the provisioner batch window; a spurious node from that race is empty and consolidated immediately on our pools (consolidateAfter: 0s).

Distinguishing "real preemption" from "gang-pipelined" nominations from pod status alone isn't possible: Volcano writes the same field in both flows (taskUnschedulable only gates on EvictionOccurred, which is true for both) and never clears it, so any Volcano nomination may be stale regardless of how it originated. The upstream guard's assumption (nomination ⇒ will bind there without new capacity) simply doesn't hold for this scheduler.

[devin-ai-integration]

The pre-delete race window is real but bounded and accepted: it lasts from the nomination status write until the victim's deletionTimestamp is visible (Volcano's evictor issues the delete in the same goroutine flow, so this is sub-second in practice, and the provisioner batcher adds its own window before snapshotting). The worst case is one extra node during a genuine preemption; on the clusters that run Volcano (delphi) every pool has consolidation enabled with consolidateAfter: 0s, so a spurious node is reaped — the generic "consolidation can be disabled" caveat doesn't apply to this fork's deployment.

The suggested gate ("defer until the victim is observed terminating") doesn't separate the cases: the gang-pipeline deadlock state also has a real evicted victim (EvictionOccurred is true in both flows) — after the victim exits, the gang still can't bind (minAvailable unmet) and the stale nomination persists forever. There is no pod-status signal that distinguishes "live preemption" from "stale/pipelined"; any victim-state gate reintroduces the deadlock, which strands multi-node gangs indefinitely — strictly worse than a transient extra node. Accepting as a deliberate tradeoff.