podgroup controller: don't delete a ReplicaSet's PodGroup while it still owns active pods; re-check after annotating - #22
Open
devin-ai-integration[bot] wants to merge 1 commit into
Conversation
…ill owns active pods; re-check after annotating A Deployment creates each new ReplicaSet at replicas=0 and scales it up right after, so addReplicaSet always sees a replicas=0 event for it. When the pod watch runs ahead of the ReplicaSet watch, that stale event arrives after the pod's PodGroup was created and deletes it; the pod is then annotated to a PodGroup that does not exist, the scheduler cache skips the job (nil spec), and processNextReq never revisits an annotated pod, so the pod is Pending forever with zero scheduling attempts. - addReplicaSet: on replicas=0, keep the PodGroup while the ReplicaSet still controls a pod that is neither terminating nor Succeeded/Failed (the pods the ReplicaSet controller itself counts as active). Genuine scale-downs still delete it once those pods are gone. - createNormalPodPGIfNotExist: after the annotation patch, re-Get the PodGroup from the API server (not the lister) and recreate it if it disappeared, treating AlreadyExists as success. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind bug
What this PR does / why we need it:
A pod scheduled by Volcano through a plain Deployment can end up Pending forever with zero scheduling attempts, zero events, zero scheduler log lines: it is annotated to a PodGroup that does not exist. Today this wedged the
pythia-embed-exa3-document-verifycanary ondelphi-productionfor 3+ hours and failed thedes-neuron-paritygate of exa-labs/monorepo#138491's production deploy. Every Volcano-scheduled Deployment in the cluster (dsv4-interactive, GLM, ...) is exposed on every rollout.Root cause — a watch-order race in the podgroup controller (
pkg/controllers/podgroup/pg_controller.go,pg_controller_handler.go):pg_controller.go:105-106registers the ReplicaSet informer'sAddFunc/UpdateFunc→addReplicaSet, which (before this PR) unconditionally deletedpodgroup-<RS UID>wheneverspec.replicas == 0. Pods go through a workqueue on a different goroutine (addPod→processNextReq→createNormalPodPGIfNotExist).A Deployment creates every new ReplicaSet at
replicas: 0and scales it up immediately, so the controller always sees areplicas == 0event for a ReplicaSet that is about to own pods. Normally it lands ~0.5–1 s before the pod exists. When the ReplicaSet watch lags the pod watch, it lands after the pod's PodGroup was created:(volcano-controllers log, delphi-production, 2026-09-03; create → delete 39 µs apart, then the pod is annotated to the already-deleted PodGroup.)
The wedge is permanent:
processNextReqreturns early whenpod.Annotations[volcano.sh/group-name] != ""(pg_controller.go:167), there is no podUpdateFunc, informer resync is 0, and nothing ever re-checks that the PodGroup exists. On the scheduler side a job with a nil PodGroup spec is skipped outright (pkg/scheduler/cache/cache.go:1527, "scheduling spec of Job … is nil, ignore it"), hence no attempts and noUnschedulableevent.kube_podgroup/volcano_podgroup_status_phaseforpodgroup-9e0857f9…had zero series 15:20–17:30Z while the previous canary pod's identical PodGroup showedRunning— the object was absent, not stale; this is not gang/minMemberwaiting.Fix (two independent guards):
activePodsOwnedByuses the same notion of "active" the ReplicaSet controller uses forstatus.replicas, so a real scale-down still deletes the PodGroup: the spec update arrives while the pods are live (skip), the status update arrives once they are terminating (delete).Which issue(s) this PR fixes:
Fixes the verify-canary / Volcano-Deployment "Pending with zero scheduling attempts" wedge seen on delphi-production 2026-09-03 (exa-labs/monorepo#138491 production
des-neuron-parityfailure). Companion: exa-labs/monorepo#138697 moves the verification canaries off Volcano entirely; this PR fixes the class for every other Volcano-scheduled Deployment.Special notes for your reviewer:
pg_controller_test.go):TestAddReplicaSet_ZeroReplicasOnlyDeletesPodGroupWithoutActivePods(keep for pending/running owned pod; delete for none / terminating / Succeeded+Failed / pod controlled by another RS),TestCreateNormalPodPGIfNotExist_RecreatesPodGroupDeletedDuringAnnotation(a reactor deletes the PodGroup during the annotation patch; the PodGroup must exist and be pod-owned afterwards),TestCreateNormalPodPGIfNotExist_NoRecreateWhenPodGroupSurvives(exactly one create in the happy path). The first two fail onexa/masterand pass here; full packagego test ./pkg/controllers/podgroup/...green,go vet/gofmtclean.replicas==0ReplicaSet event (cheap, cached) and one extra APIGetper newly annotated normal pod. No scheduler or CRD changes. StatefulSet handling untouched.…-pps5w. A follow-up could re-check annotated pods on a periodic resync; kept out of this PR to keep the diff to the race.Does this PR introduce a user-facing change?
Link to Devin session: https://app.devin.ai/sessions/1cdd69b71889489bb0680babf43a12a5
Open in Devin Desktop: https://app.devin.ai/desktop/session/1cdd69b71889489bb0680babf43a12a5?variant=devin
Requested by: @jld-adriano