You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "ruanwenjun (via GitHub)" <gi...@apache.org> on 2026/09/23 02:23:26 UTC
[PR] [SPARK-59725][K8S] Create Volcano PodGroup after driver pod creation [spark]
ruanwenjun opened a new pull request, #58982:
URL: https://github.com/apache/spark/pull/58982
### What changes were proposed in this pull request?
Create the Volcano PodGroup after the driver pod by returning it from
`VolcanoFeatureStep.getAdditionalKubernetesResources()`. The existing submission
logic attaches the driver owner reference before applying the PodGroup.
### Why are the changes needed?
Currently Spark creates the PodGroup, creates the driver pod, and then updates the
PodGroup with the driver owner reference. If the submission client exits or fails
before that update completes, the PodGroup can remain without an owner reference
and cannot be garbage-collected with the driver.
Creating the PodGroup with its owner reference eliminates this window and saves
one PodGroup write. Existing cleanup of the driver on post-pod resource creation
failure is preserved.
[JIRA: SPARK-59725](https://issues.apache.org/jira/browse/SPARK-59725)
### Does this PR introduce _any_ user-facing change?
Yes. With the Volcano feature step, the PodGroup is created after the driver pod
and includes the driver owner reference on its first write. No configuration
changes are required.
### How was this patch tested?
Added regression coverage for successful submission, driver creation failure,
and PodGroup creation failure. The tests check ordering, the owner reference at
the time of the first PodGroup write, and driver cleanup. Updated existing file
and JSON template tests and the executor resource test.
Local validation passed (Java 17):
```sh
build/sbt -Pkubernetes -Pvolcano \
'kubernetes/testOnly *VolcanoFeatureStepSuite *ClientSuite' \
'kubernetes/scalastyle' 'kubernetes/Test/scalastyle'
```
All 14 tests passed, and both Scalastyle checks reported zero errors or warnings.
The three new regression cases were also run against the original implementation;
all three failed because the PodGroup was written before the driver was created.
`git diff --check` passed.
A live Volcano cluster integration test was not run.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: OpenAI Codex (GPT-6)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59725][K8S] Bind Volcano PodGroup with driver when podgroup creation [spark]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on PR #58982:
URL: https://github.com/apache/spark/pull/58982#issuecomment-5797858180
The CI error doesn't seem to be related to the changes in this PR; I'll try to fix it in #59000.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59725][K8S] Create Volcano PodGroup with driver owner reference after driver pod creation [spark]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on PR #58982:
URL: https://github.com/apache/spark/pull/58982#issuecomment-5790291959
@dongjoon-hyun PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59725][K8S] Bind Volcano PodGroup with driver when podgroup creation [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #58982:
URL: https://github.com/apache/spark/pull/58982#issuecomment-5811914463
Thank you for the PR, @ruanwenjun. However, I don't think we can accept this approach because it breaks Volcano versions that Apache Spark documents as supported.
`docs/running-on-kubernetes.md` says Spark on Volcano is supported since Volcano v1.7.0. From v1.7.0 through v1.11.x, Volcano enables the `/pods/validate` admission webhook by default. Its `validatePod` calls `checkPG(pod, pgName, true)` for any pod with the `scheduling.k8s.io/group-name` annotation, and that call rejects pod creation when the PodGroup doesn't exist yet (`isVCJob == true` returns an error even on `NotFound`). With this PR, the driver pod is created before its PodGroup, so every Volcano submission would fail on those versions. Our CI installs Volcano v1.14.2, where this check was removed, so CI doesn't catch it.
Also, this PR moves the leak window rather than closing it. If the submission client dies between the driver pod creation and the PodGroup creation, we would be left with a driver pod whose PodGroup never exists, instead of an orphaned PodGroup. `NonFatal` failures are already cleaned up by the current code.
Finally, the new integration test passes on the current `master` too, because the existing owner-reference refresh already makes the PodGroup garbage-collectable. So it doesn't cover the race described in the PR description.
Could you keep the PodGroup as a pre-resource and look for a way to address the remaining window without changing the creation order?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59725][K8S] Bind Volcano PodGroup with driver when podgroup creation [spark]
Posted by "ruanwenjun (via GitHub)" <gi...@apache.org>.
ruanwenjun commented on PR #58982:
URL: https://github.com/apache/spark/pull/58982#issuecomment-5882294810
@dongjoon-hyun Thanks for the review. You're right; I overlooked compatibility with older Volcano versions.
Spark already attempts to clean up resources on submission failures. In our environment, we have observed leftover resources when the submission process is forcibly killed, or when API server overload causes cleanup requests to fail.
Our external cleanup service watches driver pods by label and can remove leftover drivers, but it does not currently handle PodGroups. This motivated creating the PodGroup with its driver owner reference already set, so deleting the
driver would also clean up the PodGroup.
I agree that this does not eliminate all interrupted-submission cases and that we need to preserve compatibility. I haven't found a simple way to close the owner-reference window if the submission process is killed. Do you have any suggestions for addressing this within Spark?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org