fix(kfoperators): don't fail gang-scheduled jobs still waiting for PodGroup admission as "operator hasn't updated the CR" - #24
Merged
pfernandes21 merged 1 commit intoSep 3, 2026
Conversation
… stale-CR check The pytorch/tensorflow/mpi plugins fail a job when status.startTime is still nil kf-operator.timeout after creation, on the assumption that a missing startTime means the training operator never saw the CR. With Volcano gang scheduling the operator deliberately withholds pod creation (and therefore startTime) until the PodGroup is admitted, writing only status.lastReconcileTime while it waits. A healthy queued gang therefore looks 'stale' and is deleted as soon as scheduler admission takes longer than the timeout. Count either timestamp as proof the operator reconciled the job; the check still fires when the operator is genuinely absent. 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:
|
jld-adriano
approved these changes
Sep 3, 2026
1 of 3 tasks
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.
Tracking issue
Related to exa-labs/monorepo#138180 (kf-operator timeout 1m → 10m on delphi).
Why are the changes needed?
The pytorch/tensorflow/mpi plugins fail a task with
kubeflow operator hasn't updated the <kind> custom resource since creation timewhenstatus.startTimeis still nilkf-operator.timeoutafter the CR was created. The check was written to detect an absent operator, and a nilstartTimeis the wrong proxy for that under gang scheduling.With Volcano gang scheduling the training operator (v1.9.0,
pkg/controller.v1/common/job.goReconcileJobs) deliberately does not create pods — and therefore never reaches the code that stampsstartTime— until the PodGroup leavesPending. While it waits it writes onlystatus.lastReconcileTime:So a healthy, queued gang looks "stale" to Flyte and is deleted as soon as scheduler admission takes longer than the timeout. The timeout can bound how long we wait for the operator; it should not bound how long the scheduler takes to admit a gang.
RCA on delphi (all read-only, Loki
flyte/kubeflow/volcanonamespaces), 6 ×e3a-i4lc-bs256gc-hn2-4x(4×8 GPU) executions killed 2026-09-02 23:16Z–2026-09-03 00:27Z, e.g.am4k45t4rg8qpcmflv9c:am4k…-fvcmtxhi-0createdPodGroup … unschedulable→ status write (lastReconcileTime), no podskubeflow operator hasn't updated the pytorch custom resource since creation time 23:16:31→ CR deletedJob … was deletedNo worker pod ever existed for these six; the "SIGTERM to healthy workers" seen on
ak5ppmf4lzktk59hbqtbattempts 0/1/2 is a different mechanism — Volcanopreemptevicting the prio -6 gang for a prio 0 task (a4bf…), which the operator then records asfailed=N— not this check. Attempts 1/2 of that execution sat queued for 71 min and 64 min withstartTimeset only because their PodGroups happened to be admitted within ~10 s, i.e. whether a gang survives the check today depends on where the scheduler is in its session loop when the CR lands. The delphi bump to 10m (exa-labs/monorepo#138180, live since 00:37Z, zero kills since) makes that race rare, but any gang whose PodGroup staysPendingpast the configured timeout — saturated queue, long sessions — is still killed for no fault of the operator.What changes were proposed in this pull request?
common.OperatorNeverReconciled(status)=status.StartTime == nil && status.LastReconcileTime == nil, used by all three plugins in place of the bareStartTime == niltest. Either timestamp proves the operator reconciled the CR; the check still fires when neither is set (operator down / not watching the namespace), which is the case it was written for. No config or behaviour change for jobs that reach pod creation.How was this patch tested?
TestOperatorNeverReconciled(common): empty status and Created-condition-only status are "never reconciled";StartTime,LastReconcileTime, or both are not.TestGetTaskPhasein pytorch/tensorflow/mpi: new case — CR created 1h ago,StartTimenil,LastReconcileTimeset →PhaseQueued, no error. Verified it fails on the pre-patch predicate (kubeflow operator hasn't updated …) and passes with the patch; the existing "operator did not modify the job" and "suspended" cases are unchanged and still pass.go test ./go/tasks/plugins/k8s/kfoperators/...,go build ./...(flyteplugins),gofmt -lclean.Roll-out note: delphi pins the flyte-binary image by digest in
infra/core/delphi/training-stack.ts(FLYTE_IMAGE_TAG), so this needs a fork image build + a monorepo bump after merge; not done here.Labels
fixed
Check all the applicable boxes
Link to Devin session: https://app.devin.ai/sessions/84936c9760074a4793d713c917952f29
Open in Devin Desktop: https://app.devin.ai/desktop/session/84936c9760074a4793d713c917952f29?variant=devin
Requested by: @jld-adriano