kubeflow / kubeflow/trainer

Volcano PodGroup is not cleaned up when TrainJob transitions to Failed

Open
#4,033 20 comments 0 reactions 2 assignees Claimed by @daehyun-kim-94 View on GitHub
area/scheduler kind/bug
Dominant language
Go
Stars
2.2k
Forks
1.1k
Avg merge
3d 22h
Merged PRs (30d)
39

Description

## What happened?

When a Volcano-scheduled TrainJob (using `podGroupPolicy.volcano`) transitions to the `Failed`
condition, its associated `scheduling.volcano.sh/v1beta1 PodGroup` object is never cleaned up.
The PodGroup remains in the cluster indefinitely (we observed cases persisting for multiple
days), and its `minResources` continues to be reserved by the Volcano scheduler's queue capacity
accounting. This means the queue appears to have insufficient capacity for new TrainJobs even
when the underlying nodes have plenty of free GPU resources — new TrainJobs sharing the same
queue get stuck `Pending` indefinitely with `FailedScheduling: pod group is not ready`.

Root cause (traced through the reconcile loop, verified against `master` @ `c6b8472c`):

1. `TrainJobReconciler.Reconcile()` skips `reconcileObjects()` entirely once
`trainjob.IsTrainJobFinished()` is true
([trainjob_controller.go#L108-L109](https://github.com/kubeflow/trainer/blob/c6b8472ce74dec51857ad62138bbc0b5f9819f5b/pkg/controller/trainjob_controller.go#L108-L109)),
so `Volcano.Build()` is never invoked again for a Failed TrainJob.
2. `Volcano.Build()` only has logic to skip re-creating an existing PodGroup
(`if oldPodGroup != nil && !Spec.Suspend { return nil, nil }`,
[volcano.go#L146-L156](https://github.com/kubeflow/trainer/blob/c6b8472ce74dec51857ad62138bbc0b5f9819f5b/pkg/runtime/framework/plugins/volcano/volcano.go#L146-L156))
— even if it were still called, it has no mechanism to signal deletion, since its return type
(`[]apiruntime.ApplyConfiguration`) only expresses "objects to apply," not "objects to delete."
3. Volcano's `PodGroupSpec` has no TTL/expiration field of its own (unlike `batch/v1 Job`'s
`ttlSecondsAfterFinished`), so nothing in Volcano itself will clean it up either.
4. The PodGroup is a direct child (OwnerReference) of the TrainJob, not of the JobSet — so even
if the JobSet is configured with a TTL and cleans itself up (see the comment in
[jobset.go#L454-L455](https://github.com/kubeflow/trainer/blob/c6b8472ce74dec51857ad62138bbc0b5f9819f5b/pkg/runtime/framework/plugins/jobset/jobset.go#L454-L455)),
the PodGroup is unaffected.
5. No plugin in this codebase currently issues explicit `Delete()` calls on child resources
(confirmed via `grep -rn "\.Delete(ctx\|client\.Delete" pkg/runtime/framework/plugins/*/*.go`
→ zero matches); everything relies on OwnerReference cascade GC triggered by TrainJob deletion.
This appears to be an intentional design choice, which means simply "adding a delete call
somewhere" may not fit the existing architecture without discussion.
6. Additionally, even the controller's RBAC currently has no `delete` verb on
`scheduling.volcano.sh/podgroups`
([clusterrole.yaml](https://github.com/kubeflow/trainer/blob/c6b8472ce74dec51857ad62138bbc0b5f9819f5b/charts/kubeflow-trainer/templates/rbac/clusterrole.yaml),
[role.yaml](https://github.com/kubeflow/trainer/blob/c6b8472ce74dec51857ad62138bbc0b5f9819f5b/manifests/base/rbac/role.yaml)
only grant `create;get;list;patch;update;watch`) — whichever direction is chosen below will
also need an RBAC change to add `delete`.

We manually confirmed that deleting the TrainJob object itself does cascade-delete the PodGroup
immediately via OwnerReference GC — so the fix doesn't need to touch PodGroup deletion semantics,
only needs a mechanism to clean up the PodGroup once the owning TrainJob is known to be Failed
and not going to restart.

## What did you expect to happen?

The PodGroup associated with a Failed TrainJob should eventually be cleaned up (or at least have
its queue reservation released) without requiring the TrainJob object itself to be deleted, so
that queue capacity is not permanently consumed by long-dead jobs.

## Proposed directions (would like maintainer input before opening a PR)

I looked at three possible approaches and would like feedback on which fits the project's
architecture best before investing in an implementation. All three would also need the RBAC
`delete` verb addition mentioned above.

**A. New optional plugin interface** (e.g. `TerminationPlugin`) called once a TrainJob reaches a
terminal state, letting plugins clean up side resources that aren't naturally garbage collected.
Volcano would implement it to delete the PodGroup. This doesn't touch any existing interface
(similar to how `EnforcePodGroupPolicyPlugin`/`WatchExtensionPlugin` are already optional), but
it's a new public extension point, so it may need a KEP or at least design sign-off.

**B. Have Volcano implement `TrainJobStatusPlugin.Status()`** and delete the PodGroup as a side
effect when the TrainJob is Failed. Smaller diff, reuses an already-invoked-post-finish hook, but
mixes a read-oriented method with a delete side effect.

**C. Add an explicit branch in `TrainJobReconciler.Reconcile()`** that imports the Volcano
PodGroup type directly and deletes it on Failed. Smallest change, but couples the generic
TrainJob controller to a specific scheduler plugin, breaking the separation of concerns the
plugin architecture otherwise maintains.

Happy to implement whichever direction maintainers prefer, or a different one entirely.

## Environment

Kubernetes version: 1.30.x
Kubeflow Trainer version: v2.1.0 in production, reproduced against `master` @ `c6b8472c` by direct
source inspection (Volcano gang-scheduling enabled, 4-node H200 GPU cluster)
Kubeflow Python SDK version: N/A (not SDK-related)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.