Skip to content

[Bugfix] scheduler: defer gang pin until hard spread filtering - #867

Draft
robin-inferact wants to merge 1 commit into
ome-projects:mainfrom
robin-inferact:codex/gangpack-topology-spread-aware
Draft

[Bugfix] scheduler: defer gang pin until hard spread filtering#867
robin-inferact wants to merge 1 commit into
ome-projects:mainfrom
robin-inferact:codex/gangpack-topology-spread-aware

Conversation

@robin-inferact

Copy link
Copy Markdown
Contributor

What this PR does

  • For an unplaced gang with an explicit hard (DoNotSchedule) topology-spread constraint, have PreFilter expose the union of all domains that can hold the complete gang instead of pinning one domain before the regular scheduler filters run.
  • Make unconstrained gang members wait for the unplaced hard-spread anchor, so a worker cannot decide the domain before its leader's constraint is evaluated.
  • Atomically validate and pin the selected node's domain in Reserve, then use the existing reservation handoff and Permit gate.
  • Preserve GangPack's best-fit preference in PreScore across only the domains that survived normal filtering.
  • Cover same-key rack spreading, split-key cube spreading, and node-level hostname spreading without a failed-domain unwind.

Soft ScheduleAnyway constraints retain the current packing behavior. The change deliberately does not parse or reimplement PodTopologySpread semantics; the built-in plugin remains authoritative.

Why we need it

GangPack currently picks and reserves one co-location domain in PreFilter. A later framework filter can veto every node in that domain even when a different whole-gang-feasible domain would pass. The existing PostFilter records the first domain as failed, but the same pending gang may receive no prompt second scheduling cycle.

This is visible with OME's own required replica spreading. In a two-rack Kind test, replica 0 occupied rack8, replica 1 initially best-fit-pinned rack8, and PodTopologySpread correctly rejected it. Rack9 remained feasible, but the gang waited for InstanceReadyTimeout and Pod recreation before retrying there.

Deferring only the irreversible domain commitment lets the scheduler compose normally:

  1. GangPack proves full-gang capacity in every candidate co-location domain.
  2. PodTopologySpread and all other configured filters evaluate the candidate nodes.
  3. The selected node determines the gang domain.
  4. Reserve claims the remaining gang capacity there before another scheduling cycle begins.

This also works when the spread key differs from the gang key, including a coarser failure domain or node-level kubernetes.io/hostname spreading.

How to test

cd scheduler
KUBEBUILDER_ASSETS="$(setup-envtest use 1.35.x! -p path)" go test ./...

cd ..
go test ./pkg/controller/v1beta1/workload/ops

The external OME + real-SMG Kind harness also passed with the scheduler requeue workaround disabled:

OME_MULTINODE_REF=codex/gangpack-topology-spread-aware \
OME_SCHEDULER_REQUEUE_WORKAROUND=false \
make -C tests/ome-smg multinode

Observed result: leadersRequiringExplicitRequeue: 0, zero backend-unavailable responses, successful compatible and incompatible zero-surge rollouts, successful whole-gang worker restart, and successful same-domain gang admission.

Checklist

  • Tests added/updated
  • Docs updated
  • make test passes locally (focused root tests plus the complete scheduler module and external Kind suite were run)

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added documentation Documentation changes helm Helm chart changes controller Controller changes tests Test changes labels Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

controller Controller changes documentation Documentation changes helm Helm chart changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant