[spark] branch master updated: [SPARK-39546][K8S] Support `ports` definition in executor pod template

[do...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

dongjoon 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 e64262f417b [SPARK-39546][K8S] Support `ports` definition in executor pod template
e64262f417b is described below

commit e64262f417bf381bdc664dfd1cbcfaa5aa7221fe
Author: yilun.fyl <yi...@alibaba-inc.com>
AuthorDate: Thu Sep 8 10:54:11 2022 -0700

    [SPARK-39546][K8S] Support `ports` definition in executor pod template
    
    ### What changes were proposed in this pull request?
    This pr changes the fabric8 k8s method used in BasicExecutorFeatureStep about setting blockmanager ports. Because It will override port definitions in executor pod template file.
    
    ### Why are the changes needed?
    We use spark.kubernetes.executor.podTemplateFile to specify executor pod definition. However, we cannot set port definition to pod. For example, this is my executorPodTemplate.yaml
    ```
    apiversion: v1
    kind: Pod
    spec:
      containers:
        - name: spark-kubernetes-executor
          ports:
            - containerPort: 8090
              name: jmx-exporter
              protocol: TCP
    ```
    When executors start, the pod definitions only show default blockmanager port.
    ```
          ports:
            - containerPort: 7079
              name: blockmanager
              protocol: TCP
    ```
    It looks like a bug, because the BasicExecutorFeatureStep uses withPorts to override all port definitions.
    
    ### Does this PR introduce _any_ user-facing change?
    No
    
    ### How was this patch tested?
    Add unit test
    
    Closes #37803 from fanyilun/executor-ports.
    
    Authored-by: yilun.fyl <yi...@alibaba-inc.com>
    Signed-off-by: Dongjoon Hyun <do...@apache.org>
---
 .../deploy/k8s/features/BasicExecutorFeatureStep.scala      |  2 +-
 .../deploy/k8s/features/BasicExecutorFeatureStepSuite.scala | 13 +++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStep.scala b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStep.scala
index 08d8c29b230..0b0bbc30ba4 100644
--- a/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStep.scala
+++ b/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStep.scala
@@ -201,7 +201,7 @@ private[spark] class BasicExecutorFeatureStep(
         .withValue(Utils.getCurrentUserName())
         .endEnv()
       .addAllToEnv(executorEnv.asJava)
-      .withPorts(requiredPorts.asJava)
+      .addAllToPorts(requiredPorts.asJava)
       .addToArgs("executor")
       .build()
     val executorContainerWithConfVolume = if (disableConfigMap) {
diff --git a/resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStepSuite.scala b/resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStepSuite.scala
index 84cd70c4653..32897014931 100644
--- a/resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStepSuite.scala
+++ b/resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStepSuite.scala
@@ -510,6 +510,19 @@ class BasicExecutorFeatureStepSuite extends SparkFunSuite with BeforeAndAfter {
     assert(mem === s"${expected}Mi")
   }
 
+  test("SPARK-39546: Support ports definition in executor pod template") {
+    val baseDriverPod = SparkPod.initialPod()
+    val ports = new ContainerPortBuilder()
+      .withName("port-from-template")
+      .withContainerPort(1000)
+      .build()
+    baseDriverPod.container.setPorts(Seq(ports).asJava)
+    val step1 = new BasicExecutorFeatureStep(newExecutorConf(), new SecurityManager(baseConf),
+      defaultProfile)
+    val podConfigured1 = step1.configurePod(baseDriverPod)
+    // port-from-template should exist after step1
+    assert(podConfigured1.container.getPorts.contains(ports))
+  }
 
   // There is always exactly one controller reference, and it points to the driver pod.
   private def checkOwnerReferences(executor: Pod, driverPodUid: String): Unit = {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org