Github user mccheah commented on a diff in the pull request:

    https://github.com/apache/spark/pull/23174#discussion_r239702559
  
    --- Diff: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStep.scala
 ---
    @@ -87,44 +88,61 @@ private[spark] class 
BasicExecutorFeatureStep(kubernetesConf: KubernetesExecutor
         val executorCpuQuantity = new QuantityBuilder(false)
           .withAmount(executorCoresRequest)
           .build()
    -    val executorExtraClasspathEnv = executorExtraClasspath.map { cp =>
    -      new EnvVarBuilder()
    -        .withName(ENV_CLASSPATH)
    -        .withValue(cp)
    -        .build()
    -    }
    -    val executorExtraJavaOptionsEnv = kubernetesConf
    -      .get(EXECUTOR_JAVA_OPTIONS)
    -      .map { opts =>
    -        val subsOpts = Utils.substituteAppNExecIds(opts, 
kubernetesConf.appId,
    -          kubernetesConf.executorId)
    -        val delimitedOpts = Utils.splitCommandString(subsOpts)
    -        delimitedOpts.zipWithIndex.map {
    -          case (opt, index) =>
    -            new 
EnvVarBuilder().withName(s"$ENV_JAVA_OPT_PREFIX$index").withValue(opt).build()
    +
    +    val executorEnv: Seq[EnvVar] = {
    +        (Seq(
    +          (ENV_DRIVER_URL, driverUrl),
    +          (ENV_EXECUTOR_CORES, executorCores.toString),
    +          (ENV_EXECUTOR_MEMORY, executorMemoryString),
    +          (ENV_APPLICATION_ID, kubernetesConf.appId),
    +          // This is to set the SPARK_CONF_DIR to be /opt/spark/conf
    +          (ENV_SPARK_CONF_DIR, SPARK_CONF_DIR_INTERNAL),
    +          (ENV_EXECUTOR_ID, kubernetesConf.executorId)
    +        ) ++ kubernetesConf.environment).map { case (k, v) =>
    +          new EnvVarBuilder()
    +            .withName(k)
    +            .withValue(v)
    +            .build()
             }
    -      }.getOrElse(Seq.empty[EnvVar])
    -    val executorEnv = (Seq(
    -      (ENV_DRIVER_URL, driverUrl),
    -      (ENV_EXECUTOR_CORES, executorCores.toString),
    -      (ENV_EXECUTOR_MEMORY, executorMemoryString),
    -      (ENV_APPLICATION_ID, kubernetesConf.appId),
    -      // This is to set the SPARK_CONF_DIR to be /opt/spark/conf
    -      (ENV_SPARK_CONF_DIR, SPARK_CONF_DIR_INTERNAL),
    -      (ENV_EXECUTOR_ID, kubernetesConf.executorId)) ++
    -      kubernetesConf.environment)
    -      .map(env => new EnvVarBuilder()
    -        .withName(env._1)
    -        .withValue(env._2)
    -        .build()
    -      ) ++ Seq(
    -      new EnvVarBuilder()
    -        .withName(ENV_EXECUTOR_POD_IP)
    -        .withValueFrom(new EnvVarSourceBuilder()
    -          .withNewFieldRef("v1", "status.podIP")
    +      } ++ {
    +        Seq(new EnvVarBuilder()
    +          .withName(ENV_EXECUTOR_POD_IP)
    +          .withValueFrom(new EnvVarSourceBuilder()
    +            .withNewFieldRef("v1", "status.podIP")
    +            .build())
               .build())
    -        .build()
    -    ) ++ executorExtraJavaOptionsEnv ++ executorExtraClasspathEnv.toSeq
    +      } ++ {
    +        Option(secMgr.getSecretKey()).map { authSecret =>
    +          new EnvVarBuilder()
    +            .withName(SecurityManager.ENV_AUTH_SECRET)
    +            .withValue(authSecret)
    --- End diff --
    
    I filed https://issues.apache.org/jira/browse/SPARK-26301 to suggest the 
alternative scheme. Unlike SPARK-26139 this would change the functionality that 
was merged here.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to