Skip to content

Commit 78ec6f7

Browse files
committed
[SPARK-55173][K8S][TESTS] Improve K8s IT to use TestConstants consistently and match syntax for extensibility
### What changes were proposed in this pull request? This PR aims to improve K8s integration tests by refactoring - To re-use `TestConstants` consistently, e.g., `"minikube"` -> `BACKEND_MINIKUBE` and `"docker-desktop"` -> `BACKEND_DOCKER_DESKTOP`. - To use `match` syntax (instead of `if` statement) for extensibility. ```scala - val storageClassName = if (testBackend == MinikubeTestBackend) "standard" else "hostpath" - val hostname = if (testBackend == MinikubeTestBackend) "minikube" else "docker-desktop" + val (storageClassName, hostname) = testBackend match { + case MinikubeTestBackend => ("standard", BACKEND_MINIKUBE) + case DockerForDesktopBackend => ("hostpath", BACKEND_DOCKER_DESKTOP) + case _ => ("hostpath", BACKEND_DOCKER_DESKTOP) + } ``` ### Why are the changes needed? This helps us maintain the test backend consistently and add a new backend easily. ### Does this PR introduce _any_ user-facing change? No, this is a test-only change. ### How was this patch tested? Pass the CIs with the existing test cases. Manually check the previous code patterns like the following. **BEFORE** ``` $ git grep 'testBackend ==' | wc -l 4 ``` ``` $ git grep "docker-desktop" | grep scala resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/PVTestsSuite.scala: val hostname = if (testBackend == MinikubeTestBackend) "minikube" else "docker-desktop" resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/TestConstants.scala: val BACKEND_DOCKER_DESKTOP = "docker-desktop" ``` **AFTER** ``` $ git grep 'testBackend ==' | wc -l 0 ``` ``` $ git grep "docker-desktop" | grep scala resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/TestConstants.scala: val BACKEND_DOCKER_DESKTOP = "docker-desktop" ``` ### Was this patch authored or co-authored using generative AI tooling? No. Closes #53956 from dongjoon-hyun/SPARK-55173. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
1 parent 03dedbb commit 78ec6f7

File tree

3 files changed

+17
-5
lines changed

3 files changed

+17
-5
lines changed

resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/PVTestsSuite.scala

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,19 @@ import org.scalatest.concurrent.{Eventually, PatienceConfiguration}
2323
import org.scalatest.time.{Milliseconds, Span}
2424

2525
import org.apache.spark.deploy.k8s.integrationtest.KubernetesSuite._
26+
import org.apache.spark.deploy.k8s.integrationtest.TestConstants._
27+
import org.apache.spark.deploy.k8s.integrationtest.backend.docker.DockerForDesktopBackend
2628
import org.apache.spark.deploy.k8s.integrationtest.backend.minikube.MinikubeTestBackend
2729

2830
private[spark] trait PVTestsSuite { k8sSuite: KubernetesSuite =>
2931
import PVTestsSuite._
3032

3133
private def setupLocalStorage(): Unit = {
32-
val storageClassName = if (testBackend == MinikubeTestBackend) "standard" else "hostpath"
33-
val hostname = if (testBackend == MinikubeTestBackend) "minikube" else "docker-desktop"
34+
val (storageClassName, hostname) = testBackend match {
35+
case MinikubeTestBackend => ("standard", BACKEND_MINIKUBE)
36+
case DockerForDesktopBackend => ("hostpath", BACKEND_DOCKER_DESKTOP)
37+
case _ => ("hostpath", BACKEND_DOCKER_DESKTOP)
38+
}
3439
val pvBuilder = new PersistentVolumeBuilder()
3540
.withKind("PersistentVolume")
3641
.withApiVersion("v1")

resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolumeSuite.scala

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,10 @@ private[spark] trait VolumeSuite { k8sSuite: KubernetesSuite =>
9999
}
100100

101101
test("A driver-only Spark job with an OnDemand PVC volume", k8sTestTag) {
102-
val storageClassName = if (testBackend == MinikubeTestBackend) "standard" else "hostpath"
102+
val storageClassName = testBackend match {
103+
case MinikubeTestBackend => "standard"
104+
case _ => "hostpath"
105+
}
103106
val DRIVER_PREFIX = "spark.kubernetes.driver.volumes.persistentVolumeClaim"
104107
sparkAppConf
105108
.set("spark.kubernetes.driver.master", "local[10]")
@@ -148,7 +151,10 @@ private[spark] trait VolumeSuite { k8sSuite: KubernetesSuite =>
148151
}
149152

150153
test("A Spark job with two executors with OnDemand PVC volumes", k8sTestTag) {
151-
val storageClassName = if (testBackend == MinikubeTestBackend) "standard" else "hostpath"
154+
val storageClassName = testBackend match {
155+
case MinikubeTestBackend => "standard"
156+
case _ => "hostpath"
157+
}
152158
val EXECUTOR_PREFIX = "spark.kubernetes.executor.volumes.persistentVolumeClaim"
153159
sparkAppConf
154160
.set("spark.executor.instances", "2")

resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/backend/minikube/Minikube.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package org.apache.spark.deploy.k8s.integrationtest.backend.minikube
1919
import io.fabric8.kubernetes.client.{Config, KubernetesClient, KubernetesClientBuilder}
2020

2121
import org.apache.spark.deploy.k8s.integrationtest.ProcessUtils
22+
import org.apache.spark.deploy.k8s.integrationtest.TestConstants._
2223
import org.apache.spark.internal.Logging
2324

2425
// TODO support windows
@@ -57,7 +58,7 @@ private[spark] object Minikube extends Logging {
5758
"non-numeric suffix is intentionally dropped)")
5859
}
5960

60-
new KubernetesClientBuilder().withConfig(Config.autoConfigure("minikube")).build()
61+
new KubernetesClientBuilder().withConfig(Config.autoConfigure(BACKEND_MINIKUBE)).build()
6162
}
6263

6364
def getMinikubeStatus(): MinikubeStatus.Value = {

0 commit comments

Comments
 (0)