bug(webhook): Volcano priorityClassName validation exits early, skipping later ReplicatedJobs
- Dominant language
- Go
- Stars
- 2.2k
- Forks
- 1.1k
- Avg merge
- 3d 22h
- Merged PRs (30d)
- 39
Description
### What happened?
`Volcano.Validate` iterates `jobSetSpec.ReplicatedJobs` to check each job's `priorityClassName`:
https://github.com/kubeflow/trainer/blob/master/pkg/runtime/framework/plugins/volcano/volcano.go#L104-L125
When a `ReplicatedJob` uses a reserved priority class (`system-cluster-critical` or `system-node-critical`), the loop body does `return nil, allErrs` instead of `continue`. This exits `Validate()` entirely, so any `ReplicatedJob` listed after the one with the reserved priority class is never checked.
### Reproduction
Given a `ClusterTrainingRuntime`/`TrainingRuntime` with Volcano gang scheduling and JobSet `ReplicatedJobs` in this order:
1. `dataset-initializer` with `priorityClassName: system-cluster-critical`
2. `node` with `priorityClassName: some-typo-priority-class` (does not exist as a `PriorityClass` object)
The webhook admits the TrainJob without error, because hitting job 1's reserved priority class returns out of the function before job 2 is ever checked. The invalid `priorityClassName` on job 2 then propagates into the JobSet/PodGroup and only surfaces as a pod scheduling failure at runtime, instead of being rejected at admission time.
### What did you expect to happen?
Each `ReplicatedJob` should be validated independently. A reserved priority class on one job should only skip validation for that job, not short-circuit validation for the rest of the JobSet.
### Proposed fix
Change the early `return nil, allErrs` to `continue` so the loop proceeds to the next `ReplicatedJob` instead of exiting the function. The existing test suite (`volcano_test.go`) only covers single-`ReplicatedJob` scenarios, so this also needs table-driven tests with multiple `ReplicatedJobs` covering: a reserved-priority job followed by an invalid one (should still error on the invalid one), and multiple reserved-priority jobs together (should produce no errors).
I'd like to work on this — please assign it to me.
Contributor guide
Assessment
This issue has not been assessed yet.