You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by ya...@apache.org on 2026/08/18 13:52:06 UTC
(spark) branch branch-4.3 updated: [SPARK-58725][K8S] Resolve the pod before deciding whether --kill found it
This is an automated email from the ASF dual-hosted git repository.
LuciferYang pushed a commit to branch branch-4.3
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.3 by this push:
new ce42012b97fe [SPARK-58725][K8S] Resolve the pod before deciding whether --kill found it
ce42012b97fe is described below
commit ce42012b97fe07086c9356fdfd37a5dc39048845
Author: YangJie <ya...@baidu.com>
AuthorDate: Tue Aug 18 21:51:03 2026 +0800
[SPARK-58725][K8S] Resolve the pod before deciding whether --kill found it
### What changes were proposed in this pull request?
Resolve the pod before deciding whether `spark-submit --kill` found it.
`KillApplication.executeOnPod` null-checks the request handle returned by `getPod` rather than the pod it resolves to. Call `get()` on the handle and check that instead, reusing the same handle for the delete.
### Why are the changes needed?
`getPod` returns `client.pods.inNamespace(ns).withName(name)`, a fabric8 `PodResource` request handle. `BaseOperation.withName` never returns null: it throws `IllegalArgumentException("Name must be provided.")` for a null or empty name and otherwise constructs a new handle. So `Option(podToDelete).isDefined` is always true and the `printMessage("Application not found.")` branch is unreachable.
`PodResource.delete()` on a name the API server does not have sends a real DELETE. `BaseOperation.deleteAll()` catches the resulting 404 when a name is set and returns `Collections.emptyList()`, and the caller discards that list.
So `spark-submit --kill <ns>:<wrong-name> --master k8s://...` prints only the "Submitting a request to kill submission ..." banner and exits 0, which is the same output a successful kill produces. `--status` on the same submission ID does report "Application not found.", because `ListStatus.executeOnPod` calls `get()` before its null check. The two subcommands in the same file disagree about the same nonexistent pod.
The check has been unreachable since the feature was added in 2019. fabric8 4.x had the same never-null `withName` and also swallowed the 404, returning `Boolean.FALSE` instead of an empty list, and that value was discarded too.
Three points a reviewer may want to weigh, all deliberate:
Scope. The silence is specific to the API server answering 404. A 403 or 5xx still propagates a `KubernetesClientException` and a nonzero exit, so this is a wrong-name or wrong-namespace defect rather than "kill never reports failure". Standalone mode does report a nonexistent driver ("Driver ... has already finished or does not exist"), so reporting is the convention this path was missing.
The GET and DELETE are not atomic. If the pod disappears between them, fabric8 swallows the 404 and the command exits 0 having printed only the banner, which is what the old code did for a name that never existed. The race lands in the previous behavior rather than in a new error, so no retry loop is needed.
Gating on `delete()`'s returned `List[StatusDetails]` would avoid the extra GET and close that window, but `Deletable.delete()` documents that "It is not guaranteed that the returned list will contain all values marked for deletion", while `Gettable.get()` contracts "the item or null if the item doesn't exist". Depending on the first would let this bug reappear silently on a future client bump, with no unit test able to catch it.
Also note that `--kill <name>` without an `ns:` prefix, against a kube config that supplies no namespace, now throws from `get()` where it previously issued a silent no-op DELETE against a URL that could never match a pod. `ListStatus.executeOnPod` already behaves that way on the same input, so the two subcommands end up equally exposed rather than `--kill` becoming worse than `--status`.
### Does this PR introduce _any_ user-facing change?
Yes, on one path. `spark-submit --kill` against a driver pod that does not exist now prints "Application not found." instead of printing nothing. It previously exited 0 after issuing a DELETE whose 404 was swallowed. The exit code is unchanged, matching standalone mode, which also exits 0 for a nonexistent driver. No API or configuration change.
### How was this patch tested?
A test added to `K8sSubmitOpSuite`, asserting both that the message is printed and that no delete is issued.
The missing pod needs a real `PodResource` mock whose `get` is stubbed to null. An unstubbed name would make Mockito's `withName` return null, which sends the buggy check down the false branch for the wrong reason and passes without the fix.
Confirmed to fail against the unfixed tree:
```
[info] - Kill app that does not exist *** FAILED *** (7 milliseconds)
[info] org.mockito.exceptions.verification.WantedButNotInvoked: Wanted but not invoked:
[info] err.println("Application not found.");
[info] Actually, there were zero interactions with this mock.
```
(That run predated the JIRA id, so the test name in the captured output lacked the
`SPARK-58725:` prefix it carries now.)
`build/sbt -Pkubernetes 'kubernetes/testOnly org.apache.spark.deploy.k8s.submit.K8sSubmitOpSuite'`:
```
[info] - List app status (69 milliseconds)
[info] - List status for multiple apps with glob (3 milliseconds)
[info] - Kill app (11 milliseconds)
[info] - Kill app with gracePeriod (3 milliseconds)
[info] - SPARK-58725: Kill app that does not exist (2 milliseconds)
[info] - Kill multiple apps with glob without gracePeriod (3 milliseconds)
[info] Run completed in 1 second, 722 milliseconds.
[info] Total number of tests run: 6
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 6, failed 0, canceled 0, ignored 0, pending 0
```
`kubernetes/scalastyle` and `kubernetes/Test/scalastyle` report 0 errors.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
Closes #57948 from LuciferYang/fix-k8s-kill-app-not-found.
Authored-by: YangJie <ya...@baidu.com>
Signed-off-by: yangjie01 <ya...@baidu.com>
(cherry picked from commit 9d32c140e46ed3b07850e363435ec98d56761351)
Signed-off-by: yangjie01 <ya...@baidu.com>
---
.../spark/deploy/k8s/submit/K8sSubmitOps.scala | 6 +++++-
.../spark/deploy/k8s/submit/K8sSubmitOpSuite.scala | 21 ++++++++++++++++++++-
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/K8sSubmitOps.scala b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/K8sSubmitOps.scala
index bd8e0f97132d..217b236af673 100644
--- a/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/K8sSubmitOps.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/K8sSubmitOps.scala
@@ -50,7 +50,11 @@ private class KillApplication extends K8sSubmitOp {
(implicit client: KubernetesClient): Unit = {
val podToDelete = getPod(namespace, pName)
- if (Option(podToDelete).isDefined) {
+ // `getPod` returns a request handle, which is never null; only resolving it reports whether
+ // the pod exists. Without the `get()` the check below is always true, so a name that is not
+ // in the cluster would issue a delete that the API server answers with a swallowed 404 and
+ // report nothing to the user. `ListStatus.executeOnPod` resolves it the same way.
+ if (Option(podToDelete.get()).isDefined) {
getGracePeriod(sparkConf) match {
case Some(period) => podToDelete.withGracePeriod(period).delete()
case _ => podToDelete.delete()
diff --git a/resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/submit/K8sSubmitOpSuite.scala b/resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/submit/K8sSubmitOpSuite.scala
index 95a76b98b227..5cb9d30d24c9 100644
--- a/resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/submit/K8sSubmitOpSuite.scala
+++ b/resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/submit/K8sSubmitOpSuite.scala
@@ -25,7 +25,7 @@ import io.fabric8.kubernetes.api.model._
import io.fabric8.kubernetes.client.{KubernetesClient, PropagationPolicyConfigurable}
import io.fabric8.kubernetes.client.dsl.{Deletable, NamespaceListVisitFromServerGetDeleteRecreateWaitApplicable, PodResource}
import org.mockito.{ArgumentMatchers, Mock, MockitoAnnotations}
-import org.mockito.Mockito.{times, verify, when}
+import org.mockito.Mockito.{never, times, verify, when}
import org.scalatest.BeforeAndAfter
import org.apache.spark.{SparkConf, SparkFunSuite}
@@ -38,6 +38,7 @@ import org.apache.spark.scheduler.cluster.k8s.ExecutorLifecycleTestUtils.TEST_SP
class K8sSubmitOpSuite extends SparkFunSuite with BeforeAndAfter {
private val driverPodName1 = "driver1"
private val driverPodName2 = "driver2"
+ private val missingPodName = "driver3"
private val driverPod1 = buildDriverPod(driverPodName1, "1")
private val driverPod2 = buildDriverPod(driverPodName2, "2")
private val podList = List(driverPod1, driverPod2)
@@ -55,6 +56,11 @@ class K8sSubmitOpSuite extends SparkFunSuite with BeforeAndAfter {
@Mock
private var driverPodOperations2: PodResource = _
+ // The missing pod needs a real mock whose `get` returns null. `executeOnPod` resolves the
+ // handle, so a bare unstubbed name would make `withName` return null and NPE instead.
+ @Mock
+ private var missingPodOperations: PodResource = _
+
@Mock
private var kubernetesClient: KubernetesClient = _
@@ -76,10 +82,12 @@ class K8sSubmitOpSuite extends SparkFunSuite with BeforeAndAfter {
when(podOperations.inNamespace(namespace)).thenReturn(podsWithNamespace)
when(podsWithNamespace.withName(driverPodName1)).thenReturn(driverPodOperations1)
when(podsWithNamespace.withName(driverPodName2)).thenReturn(driverPodOperations2)
+ when(podsWithNamespace.withName(missingPodName)).thenReturn(missingPodOperations)
when(driverPodOperations1.get).thenReturn(driverPod1)
when(driverPodOperations1.delete()).thenReturn(Arrays.asList(new StatusDetails))
when(driverPodOperations2.get).thenReturn(driverPod2)
when(driverPodOperations2.delete()).thenReturn(Arrays.asList(new StatusDetails))
+ doReturn(null).when(missingPodOperations).get
}
test("List app status") {
@@ -120,6 +128,17 @@ class K8sSubmitOpSuite extends SparkFunSuite with BeforeAndAfter {
verify(deletable, times(1)).delete()
}
+ test("SPARK-58725: Kill app that does not exist") {
+ implicit val kubeClient: KubernetesClient = kubernetesClient
+ val killApp = new KillApplication
+ killApp.printStream = err
+ killApp.executeOnPod(missingPodName, Option(namespace), new SparkConf())
+ // scalastyle:off
+ verify(err).println(ArgumentMatchers.eq("Application not found."))
+ // scalastyle:on
+ verify(missingPodOperations, never()).delete()
+ }
+
test("Kill multiple apps with glob without gracePeriod") {
implicit val kubeClient: KubernetesClient = kubernetesClient
val killApp = new KillApplication
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org