This is an automated email from the ASF dual-hosted git repository.
LuciferYang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 9d32c140e46e [SPARK-58725][K8S] Resolve the pod before deciding
whether --kill found it
9d32c140e46e is described below
commit 9d32c140e46ed3b07850e363435ec98d56761351
Author: YangJie <[email protected]>
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 <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
---
.../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: [email protected]
For additional commands, e-mail: [email protected]
