Revert "Use a list of environment variables for JVM options."

This reverts commit 34d7af2.

Author: ash211

resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/kubernetes/constants.scala

@@ -69,7 +69,6 @@ package object constants {
   private[spark] val ENV_MOUNTED_FILES_DIR = "SPARK_MOUNTED_FILES_DIR"
   private[spark] val ENV_PYSPARK_FILES = "PYSPARK_FILES"
   private[spark] val ENV_PYSPARK_PRIMARY = "PYSPARK_PRIMARY"
-  private[spark] val ENV_JAVA_OPT_PREFIX = "SPARK_JAVA_OPT_"
 
   // Bootstrapping dependencies with the init-container
   private[spark] val INIT_CONTAINER_ANNOTATION = "pod.beta.kubernetes.io/init-containers"

resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/kubernetes/submit/Client.scala

@@ -18,10 +18,9 @@ package org.apache.spark.deploy.kubernetes.submit
 
 import java.util.{Collections, UUID}
 
-import io.fabric8.kubernetes.api.model.{ContainerBuilder, EnvVar, EnvVarBuilder, OwnerReferenceBuilder, PodBuilder}
+import io.fabric8.kubernetes.api.model.{ContainerBuilder, OwnerReferenceBuilder, PodBuilder}
 import io.fabric8.kubernetes.client.KubernetesClient
 import scala.collection.mutable
-import scala.collection.JavaConverters._
 
 import org.apache.spark.SparkConf
 import org.apache.spark.deploy.kubernetes.config._
@@ -93,21 +92,18 @@ private[spark] class Client(
       currentDriverSpec = nextStep.configureDriver(currentDriverSpec)
     }
     val resolvedDriverJavaOpts = currentDriverSpec
-        .driverSparkConf
-        // We don't need this anymore since we just set the JVM options on the environment
-        .remove(org.apache.spark.internal.config.DRIVER_JAVA_OPTIONS)
-        .getAll
-        .map {
-          case (confKey, confValue) => s"-D$confKey=$confValue"
-        } ++ driverJavaOptions.map(Utils.splitCommandString).getOrElse(Seq.empty)
-    val driverJavaOptsEnvs: Seq[EnvVar] = resolvedDriverJavaOpts.zipWithIndex.map {
-      case (option, index) => new EnvVarBuilder()
-          .withName(s"$ENV_JAVA_OPT_PREFIX$index")
-          .withValue(option)
-          .build()
-    }
+      .driverSparkConf
+      // We don't need this anymore since we just set the JVM options on the environment
+      .remove(org.apache.spark.internal.config.DRIVER_JAVA_OPTIONS)
+      .getAll
+      .map {
+        case (confKey, confValue) => s"-D$confKey=$confValue"
+      }.mkString(" ") + driverJavaOptions.map(" " + _).getOrElse("")
     val resolvedDriverContainer = new ContainerBuilder(currentDriverSpec.driverContainer)
-      .addAllToEnv(driverJavaOptsEnvs.asJava)
+      .addNewEnv()
+        .withName(ENV_DRIVER_JAVA_OPTS)
+        .withValue(resolvedDriverJavaOpts)
+        .endEnv()
       .build()
     val resolvedDriverPod = new PodBuilder(currentDriverSpec.driverPod)
       .editSpec()

resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/kubernetes/submit/ClientSuite.scala

@@ -135,7 +135,7 @@ class ClientSuite extends SparkFunSuite with BeforeAndAfter {
         .set("spark.logConf", "true")
         .set(
           org.apache.spark.internal.config.DRIVER_JAVA_OPTIONS,
-          "-XX:+HeapDumpOnOutOfMemoryError -XX:+PrintGCDetails")
+          "-XX:+|-HeapDumpOnOutOfMemoryError")
     val submissionClient = new Client(
         submissionSteps,
         sparkConf,
@@ -147,22 +147,15 @@ class ClientSuite extends SparkFunSuite with BeforeAndAfter {
     val createdPod = createdPodArgumentCaptor.getValue
     val driverContainer = Iterables.getOnlyElement(createdPod.getSpec.getContainers)
     assert(driverContainer.getName === SecondTestConfigurationStep.containerName)
-    val driverJvmOptsEnvs = driverContainer.getEnv.asScala.filter { env =>
-      env.getName.startsWith(ENV_JAVA_OPT_PREFIX)
-    }.sortBy(_.getName)
-    assert(driverJvmOptsEnvs.size === 4)
-
-    val expectedJvmOptsValues = Seq(
-        "-Dspark.logConf=true",
+    val driverJvmOptsEnv = Iterables.getOnlyElement(driverContainer.getEnv)
+    assert(driverJvmOptsEnv.getName === ENV_DRIVER_JAVA_OPTS)
+    val driverJvmOpts = driverJvmOptsEnv.getValue.split(" ").toSet
+    assert(driverJvmOpts.contains("-Dspark.logConf=true"))
+    assert(driverJvmOpts.contains(
         s"-D${SecondTestConfigurationStep.sparkConfKey}=" +
-            s"${SecondTestConfigurationStep.sparkConfValue}",
-        s"-XX:+HeapDumpOnOutOfMemoryError",
-        s"-XX:+PrintGCDetails")
-    driverJvmOptsEnvs.zip(expectedJvmOptsValues).zipWithIndex.foreach {
-      case ((resolvedEnv, expectedJvmOpt), index) =>
-        assert(resolvedEnv.getName === s"$ENV_JAVA_OPT_PREFIX$index")
-        assert(resolvedEnv.getValue === expectedJvmOpt)
-    }
+          SecondTestConfigurationStep.sparkConfValue))
+    assert(driverJvmOpts.contains(
+        "-XX:+|-HeapDumpOnOutOfMemoryError"))
   }
 
   test("Waiting for app completion should stall on the watcher") {
@@ -218,8 +211,8 @@ private object SecondTestConfigurationStep extends DriverConfigurationStep {
   override def configureDriver(driverSpec: KubernetesDriverSpec): KubernetesDriverSpec = {
     val modifiedPod = new PodBuilder(driverSpec.driverPod)
       .editMetadata()
-        .addToAnnotations(annotationKey, annotationValue)
-        .endMetadata()
+      .addToAnnotations(annotationKey, annotationValue)
+      .endMetadata()
       .build()
     val resolvedSparkConf = driverSpec.driverSparkConf.clone().set(sparkConfKey, sparkConfValue)
     val modifiedContainer = new ContainerBuilder(driverSpec.driverContainer)

resource-managers/kubernetes/docker-minimal-bundle/src/main/docker/driver-py/Dockerfile

@@ -39,10 +39,10 @@ ENV PYSPARK_DRIVER_PYTHON python
 ENV PYTHONPATH ${SPARK_HOME}/python/:${SPARK_HOME}/python/lib/py4j-0.10.4-src.zip:${PYTHONPATH}
 
 CMD SPARK_CLASSPATH="${SPARK_HOME}/jars/*" && \
-    env | grep SPARK_JAVA_OPT_ | sed 's/[^=]*=\(.*\)/\1/g' > /tmp/java_opts.txt && \
-    readarray -t SPARK_DRIVER_JAVA_OPTS < /tmp/java_opts.txt && \
     if ! [ -z ${SPARK_MOUNTED_CLASSPATH+x} ]; then SPARK_CLASSPATH="$SPARK_MOUNTED_CLASSPATH:$SPARK_CLASSPATH"; fi && \
     if ! [ -z ${SPARK_SUBMIT_EXTRA_CLASSPATH+x} ]; then SPARK_CLASSPATH="$SPARK_SUBMIT_EXTRA_CLASSPATH:$SPARK_CLASSPATH"; fi && \
     if ! [ -z ${SPARK_EXTRA_CLASSPATH+x} ]; then SPARK_CLASSPATH="$SPARK_EXTRA_CLASSPATH:$SPARK_CLASSPATH"; fi && \
     if ! [ -z ${SPARK_MOUNTED_FILES_DIR} ]; then cp -R "$SPARK_MOUNTED_FILES_DIR/." .; fi && \
-    ${JAVA_HOME}/bin/java "${SPARK_DRIVER_JAVA_OPTS[@]}" -cp $SPARK_CLASSPATH -Xms$SPARK_DRIVER_MEMORY -Xmx$SPARK_DRIVER_MEMORY $SPARK_DRIVER_CLASS $PYSPARK_PRIMARY $PYSPARK_FILES $SPARK_DRIVER_ARGS
+    ${JAVA_HOME}/bin/java $SPARK_DRIVER_JAVA_OPTS -cp $SPARK_CLASSPATH \
+    -Xms$SPARK_DRIVER_MEMORY -Xmx$SPARK_DRIVER_MEMORY \
+    $SPARK_DRIVER_CLASS $PYSPARK_PRIMARY $PYSPARK_FILES $SPARK_DRIVER_ARGS

resource-managers/kubernetes/docker-minimal-bundle/src/main/docker/driver/Dockerfile

@@ -24,10 +24,8 @@ FROM spark-base
 COPY examples /opt/spark/examples
 
 CMD SPARK_CLASSPATH="${SPARK_HOME}/jars/*" && \
-    env | grep SPARK_JAVA_OPT_ | sed 's/[^=]*=\(.*\)/\1/g' > /tmp/java_opts.txt && \
-    readarray -t SPARK_DRIVER_JAVA_OPTS < /tmp/java_opts.txt && \
     if ! [ -z ${SPARK_MOUNTED_CLASSPATH+x} ]; then SPARK_CLASSPATH="$SPARK_MOUNTED_CLASSPATH:$SPARK_CLASSPATH"; fi && \
     if ! [ -z ${SPARK_SUBMIT_EXTRA_CLASSPATH+x} ]; then SPARK_CLASSPATH="$SPARK_SUBMIT_EXTRA_CLASSPATH:$SPARK_CLASSPATH"; fi && \
     if ! [ -z ${SPARK_EXTRA_CLASSPATH+x} ]; then SPARK_CLASSPATH="$SPARK_EXTRA_CLASSPATH:$SPARK_CLASSPATH"; fi && \
-    if ! [ -z ${SPARK_MOUNTED_FILES_DIR+x} ]; then cp -R "$SPARK_MOUNTED_FILES_DIR/." .; fi && \
-    ${JAVA_HOME}/bin/java "${SPARK_DRIVER_JAVA_OPTS[@]}" -cp $SPARK_CLASSPATH -Xms$SPARK_DRIVER_MEMORY -Xmx$SPARK_DRIVER_MEMORY $SPARK_DRIVER_CLASS $SPARK_DRIVER_ARGS
+    if ! [ -z ${SPARK_MOUNTED_FILES_DIR} ]; then cp -R "$SPARK_MOUNTED_FILES_DIR/." .; fi && \
+    ${JAVA_HOME}/bin/java $SPARK_DRIVER_JAVA_OPTS -cp $SPARK_CLASSPATH -Xms$SPARK_DRIVER_MEMORY -Xmx$SPARK_DRIVER_MEMORY $SPARK_DRIVER_CLASS $SPARK_DRIVER_ARGS

resource-managers/kubernetes/docker-minimal-bundle/src/main/docker/spark-base/Dockerfile

@@ -24,8 +24,6 @@ RUN apk upgrade --no-cache && \
     apk add --no-cache bash tini && \
     mkdir -p /opt/spark && \
     touch /opt/spark/RELEASE && \
-    rm /bin/sh && \
-    ln -sv /bin/bash /bin/sh && \
     chgrp root /etc/passwd && chmod ug+rw /etc/passwd
 
 COPY jars /opt/spark/jars

resource-managers/kubernetes/integration-tests-spark-jobs/src/main/scala/org/apache/spark/deploy/kubernetes/integrationtest/jobs/JavaOptionsTest.scala

@@ -16,9 +16,9 @@
  */
 package org.apache.spark.deploy.kubernetes.integrationtest
 
-import java.io.{File, FileOutputStream}
+import java.io.File
 import java.nio.file.Paths
-import java.util.{Properties, UUID}
+import java.util.UUID
 
 import com.google.common.base.Charsets
 import com.google.common.io.Files
@@ -226,26 +226,6 @@ private[spark] class KubernetesSuite extends SparkFunSuite with BeforeAndAfter {
         Seq.empty[String])
   }
 
-  test("Setting JVM options on the driver and executors with spaces.") {
-    assume(testBackend.name == MINIKUBE_TEST_BACKEND)
-    launchStagingServer(SSLOptions(), None)
-    val driverJvmOptionsFile = storeJvmOptionsInTempFile(
-        Map("simpleDriverConf" -> "simpleDriverConfValue",
-          "driverconfwithspaces" -> "driver conf with spaces value"),
-        "driver-jvm-options.properties",
-        "JVM options that should be set on the driver.")
-    sparkConf.set(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS,
-        "-DsimpleDriverConf=simpleDriverConfValue" +
-            " -Ddriverconfwithspaces='driver conf with spaces value'")
-    sparkConf.set("spark.files", driverJvmOptionsFile.getAbsolutePath)
-    runSparkApplicationAndVerifyCompletion(
-        JavaMainAppResource(SUBMITTER_LOCAL_MAIN_APP_RESOURCE),
-        JAVA_OPTIONS_MAIN_CLASS,
-        Seq(s"All expected JVM options were present on the driver and executors."),
-        Array(driverJvmOptionsFile.getName),
-        Seq.empty[String])
-  }
-
   test("Use a very long application name.") {
     assume(testBackend.name == MINIKUBE_TEST_BACKEND)
 
@@ -359,20 +339,6 @@ private[spark] class KubernetesSuite extends SparkFunSuite with BeforeAndAfter {
       }
     }
   }
-
-  private def storeJvmOptionsInTempFile(
-      options: Map[String, String],
-      propertiesFileName: String,
-      comments: String): File = {
-    val tempDir = Utils.createTempDir()
-    val propertiesFile = new File(tempDir, propertiesFileName)
-    val properties = new Properties()
-    options.foreach { case (propKey, propValue) => properties.setProperty(propKey, propValue) }
-    Utils.tryWithResource(new FileOutputStream(propertiesFile)) { os =>
-      properties.store(os, comments)
-    }
-    propertiesFile
-  }
 }
 
 private[spark] object KubernetesSuite {
@@ -402,8 +368,6 @@ private[spark] object KubernetesSuite {
     ".integrationtest.jobs.FileExistenceTest"
   val GROUP_BY_MAIN_CLASS = "org.apache.spark.deploy.kubernetes" +
     ".integrationtest.jobs.GroupByTest"
-  val JAVA_OPTIONS_MAIN_CLASS = "org.apache.spark.deploy.kubernetes" +
-    ".integrationtest.jobs.JavaOptionsTest"
   val TEST_EXISTENCE_FILE_CONTENTS = "contents"
 
   case object ShuffleNotReadyException extends Exception