[SPARK-53176][DEPLOY] Spark launcher should respect --load-spark-defaults

launcher/src/main/java/org/apache/spark/launcher/AbstractCommandBuilder.java

@@ -49,6 +49,7 @@ abstract class AbstractCommandBuilder {
   String master;
   String remote;
   protected String propertiesFile;
+  protected boolean loadSparkDefaults;
   final List<String> appArgs;
   final List<String> jars;
   final List<String> files;
@@ -362,21 +363,35 @@ Map<String, String> getEffectiveConfig() throws IOException {
   }
 
   /**
-   * Loads the configuration file for the application, if it exists. This is either the
-   * user-specified properties file, or the spark-defaults.conf file under the Spark configuration
-   * directory.
+   * Load the configuration file(s) for the application - from the user-specified properties
+   * file, and/or the spark-defaults.conf file under the Spark configuration directory, if exists.
+   * Configurations from user-specified properties file take precedence over spark-defaults.conf.
    */
   private Properties loadPropertiesFile() throws IOException {
+    Properties defaultsProps = new Properties();
+    if (propertiesFile == null || loadSparkDefaults) {
+      defaultsProps = loadPropertiesFile(new File(getConfDir(), DEFAULT_PROPERTIES_FILE));
+    }
+
     Properties props = new Properties();
-    File propsFile;
     if (propertiesFile != null) {
-      propsFile = new File(propertiesFile);
+      File propsFile = new File(propertiesFile);
       checkArgument(propsFile.isFile(), "Invalid properties file '%s'.", propertiesFile);
-    } else {
-      propsFile = new File(getConfDir(), DEFAULT_PROPERTIES_FILE);
+      props = loadPropertiesFile(propsFile);
+    }
+
+    for (Map.Entry<Object, Object> entry : props.entrySet()) {
+      if (!defaultsProps.containsKey(entry.getKey())) {
+        defaultsProps.put(entry.getKey(), entry.getValue());
+      }
     }
 
-    if (propsFile.isFile()) {
+    return defaultsProps;
+  }
+
+  private Properties loadPropertiesFile(File propsFile) throws IOException {
+    Properties props = new Properties();
+    if (propsFile != null && propsFile.isFile()) {
       try (InputStreamReader isr = new InputStreamReader(
           new FileInputStream(propsFile), StandardCharsets.UTF_8)) {
         props.load(isr);

launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java

@@ -251,6 +251,10 @@ List<String> buildSparkSubmitArgs(boolean includeRemote) {
       args.add(propertiesFile);
     }
 
+    if (loadSparkDefaults) {
+      args.add(parser.LOAD_SPARK_DEFAULTS);
+    }
+
     if (isExample) {
       jars.addAll(findExamplesJars());
     }
@@ -550,6 +554,7 @@ protected boolean handle(String opt, String value) {
         }
         case DEPLOY_MODE -> deployMode = value;
         case PROPERTIES_FILE -> propertiesFile = value;
+        case LOAD_SPARK_DEFAULTS -> loadSparkDefaults = true;
         case DRIVER_MEMORY -> conf.put(SparkLauncher.DRIVER_MEMORY, value);
         case DRIVER_JAVA_OPTIONS -> conf.put(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS, value);
         case DRIVER_LIBRARY_PATH -> conf.put(SparkLauncher.DRIVER_EXTRA_LIBRARY_PATH, value);

launcher/src/test/java/org/apache/spark/launcher/SparkSubmitCommandBuilderSuite.java

@@ -56,16 +56,18 @@ public static void cleanUp() throws Exception {
 
   @Test
   public void testDriverCmdBuilder() throws Exception {
-    testCmdBuilder(true, null);
-    testCmdBuilder(true, dummyPropsFile);
-    testCmdBuilder(true, connectPropsFile);
+    testCmdBuilder(true, null, false);
+    testCmdBuilder(true, dummyPropsFile, false);
+    testCmdBuilder(true, dummyPropsFile, true);
+    testCmdBuilder(true, connectPropsFile, false);
   }
 
   @Test
   public void testClusterCmdBuilder() throws Exception {
-    testCmdBuilder(false, null);
-    testCmdBuilder(false, dummyPropsFile);
-    testCmdBuilder(false, connectPropsFile);
+    testCmdBuilder(true, null, false);
+    testCmdBuilder(true, dummyPropsFile, false);
+    testCmdBuilder(true, dummyPropsFile, true);
+    testCmdBuilder(true, connectPropsFile, false);
   }
 
   @Test
@@ -317,13 +319,15 @@ public void testIsClientMode() {
     assertTrue(builder.isClientMode(userProps));
   }
 
-  private void testCmdBuilder(boolean isDriver, File propertiesFile) throws Exception {
+  private void testCmdBuilder(
+      boolean isDriver, File propertiesFile, boolean loadSparkDefaults) throws Exception {
     final String DRIVER_DEFAULT_PARAM = "-Ddriver-default";
     final String DRIVER_EXTRA_PARAM = "-Ddriver-extra";
     String deployMode = isDriver ? "client" : "cluster";
 
     SparkSubmitCommandBuilder launcher =
       newCommandBuilder(Collections.emptyList());
+    launcher.loadSparkDefaults = loadSparkDefaults;
     launcher.childEnv.put(CommandBuilderUtils.ENV_SPARK_HOME,
       System.getProperty("spark.test.home"));
     launcher.master = "yarn";
@@ -334,11 +338,10 @@ private void testCmdBuilder(boolean isDriver, File propertiesFile) throws Except
     launcher.appArgs.add("foo");
     launcher.appArgs.add("bar");
     launcher.conf.put("spark.foo", "foo");
-    // either set the property through "--conf" or through default property file
-    if (propertiesFile == null) {
-      launcher.childEnv.put("SPARK_CONF_DIR", System.getProperty("spark.test.home")
-          + "/launcher/src/test/resources");
-    } else {
+    launcher.childEnv.put("SPARK_CONF_DIR", System.getProperty("spark.test.home")
+        + "/launcher/src/test/resources");
+    // either set the property through "--conf" or through properties file(s)
+    if (propertiesFile != null) {
       launcher.setPropertiesFile(propertiesFile.getAbsolutePath());
       launcher.conf.put(SparkLauncher.DRIVER_MEMORY, "1g");
       launcher.conf.put(SparkLauncher.DRIVER_EXTRA_CLASSPATH, "/driver");
@@ -369,8 +372,13 @@ private void testCmdBuilder(boolean isDriver, File propertiesFile) throws Except
     String[] cp = findArgValue(cmd, "-cp").split(Pattern.quote(File.pathSeparator));
     if (isDriver) {
       assertTrue(contains("/driver", cp), "Driver classpath should contain provided entry.");
+      if (propertiesFile == null || loadSparkDefaults) {
+        assertTrue(contains("/driver-default", cp),
+          "Driver classpath should contain provided entry.");
+      }
     } else {
       assertFalse(contains("/driver", cp), "Driver classpath should not be in command.");
+      assertFalse(contains("/driver-default", cp), "Driver classpath should not be in command.");
     }
 
     String libPath = env.get(CommandBuilderUtils.getLibPathEnvName());

launcher/src/test/resources/spark-defaults.conf

@@ -16,6 +16,7 @@
 #
 
 spark.driver.memory=1g
+spark.driver.defaultExtraClassPath=/driver-default
 spark.driver.extraClassPath=/driver
 spark.driver.defaultJavaOptions=-Ddriver-default
 spark.driver.extraJavaOptions=-Ddriver-extra