YARN-11386. Fix issue with classpath resolution (#5183)

* This PR ensures that all the special notations such as <CPS> are resolved before getting added to classpath.
apache · Dec 6, 2022 · dadd3d9 · dadd3d9
1 parent b666075
commit dadd3d9
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 29 deletions.
diff --git a/.../org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java b/.../org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
@@ -246,7 +246,7 @@ public Integer call() {
       launchContext.setCommands(newCmds);
 
       // The actual expansion of environment variables happens after calling
-      // sanitizeEnv.  This allows variables specified in NM_ADMIN_USER_ENV
+      // addConfigsToEnv.  This allows variables specified in NM_ADMIN_USER_ENV
       // to reference user or container-defined variables.
       Map<String, String> environment = launchContext.getEnvironment();
       // /////////////////////////// End of variable expansion
@@ -340,13 +340,15 @@ public Integer call() {
       try (DataOutputStream containerScriptOutStream =
                lfs.create(nmPrivateContainerScriptPath,
                    EnumSet.of(CREATE, OVERWRITE))) {
+        addConfigsToEnv(environment);
+
+        expandAllEnvironmentVars(environment, containerLogDir);
+
         // Sanitize the container's environment
         sanitizeEnv(environment, containerWorkDir, appDirs, userLocalDirs,
             containerLogDirs, localResources, nmPrivateClasspathJarDir,
             nmEnvVars);
 
-        expandAllEnvironmentVars(environment, containerLogDir);
-
         // Add these if needed after expanding so we don't expand key values.
         if (keystore != null) {
           addKeystoreVars(environment, containerWorkDir);
@@ -1641,13 +1643,35 @@ public void sanitizeEnv(Map<String, String> environment, Path pwd,
       addToEnvMap(environment, nmVars, "JVM_PID", "$$");
     }
 
+    // TODO: Remove Windows check and use this approach on all platforms after
+    // additional testing.  See YARN-358.
+    if (Shell.WINDOWS) {
+      sanitizeWindowsEnv(environment, pwd,
+          resources, nmPrivateClasspathJarDir);
+    }
+
+    // put AuxiliaryService data to environment
+    for (Map.Entry<String, ByteBuffer> meta : containerManager
+        .getAuxServiceMetaData().entrySet()) {
+      AuxiliaryServiceHelper.setServiceDataIntoEnv(
+          meta.getKey(), meta.getValue(), environment);
+      nmVars.add(AuxiliaryServiceHelper.getPrefixServiceName(meta.getKey()));
+    }
+  }
+
+  /**
+   * There are some configurations (such as {@value YarnConfiguration#NM_ADMIN_USER_ENV}) whose
+   * values need to be added to the environment variables.
+   *
+   * @param environment The environment variables map to add the configuration values to.
+   */
+  public void addConfigsToEnv(Map<String, String> environment) {
     // variables here will be forced in, even if the container has
     // specified them.  Note: we do not track these in nmVars, to
     // allow them to be ordered properly if they reference variables
     // defined by the user.
     String defEnvStr = conf.get(YarnConfiguration.DEFAULT_NM_ADMIN_USER_ENV);
-    Apps.setEnvFromInputProperty(environment,
-        YarnConfiguration.NM_ADMIN_USER_ENV, defEnvStr, conf,
+    Apps.setEnvFromInputProperty(environment, YarnConfiguration.NM_ADMIN_USER_ENV, defEnvStr, conf,
         File.pathSeparator);
 
     if (!Shell.WINDOWS) {
@@ -1658,39 +1682,21 @@ public void sanitizeEnv(Map<String, String> environment, Path pwd,
         String userPath = environment.get(Environment.PATH.name());
         environment.remove(Environment.PATH.name());
         if (userPath == null || userPath.isEmpty()) {
-          Apps.addToEnvironment(environment, Environment.PATH.name(),
-              forcePath, File.pathSeparator);
-          Apps.addToEnvironment(environment, Environment.PATH.name(),
-              "$PATH", File.pathSeparator);
+          Apps.addToEnvironment(environment, Environment.PATH.name(), forcePath,
+              File.pathSeparator);
+          Apps.addToEnvironment(environment, Environment.PATH.name(), "$PATH", File.pathSeparator);
         } else {
-          Apps.addToEnvironment(environment, Environment.PATH.name(),
-              forcePath, File.pathSeparator);
-          Apps.addToEnvironment(environment, Environment.PATH.name(),
-              userPath, File.pathSeparator);
+          Apps.addToEnvironment(environment, Environment.PATH.name(), forcePath,
+              File.pathSeparator);
+          Apps.addToEnvironment(environment, Environment.PATH.name(), userPath, File.pathSeparator);
         }
       }
     }
-
-    // TODO: Remove Windows check and use this approach on all platforms after
-    // additional testing.  See YARN-358.
-    if (Shell.WINDOWS) {
-
-      sanitizeWindowsEnv(environment, pwd,
-          resources, nmPrivateClasspathJarDir);
-    }
-    // put AuxiliaryService data to environment
-    for (Map.Entry<String, ByteBuffer> meta : containerManager
-        .getAuxServiceMetaData().entrySet()) {
-      AuxiliaryServiceHelper.setServiceDataIntoEnv(
-          meta.getKey(), meta.getValue(), environment);
-      nmVars.add(AuxiliaryServiceHelper.getPrefixServiceName(meta.getKey()));
-    }
   }
 
   private void sanitizeWindowsEnv(Map<String, String> environment, Path pwd,
       Map<Path, List<String>> resources, Path nmPrivateClasspathJarDir)
       throws IOException {
-
     String inputClassPath = environment.get(Environment.CLASSPATH.name());
 
     if (inputClassPath != null && !inputClassPath.isEmpty()) {

diff --git a/.../apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java b/.../apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java
@@ -808,6 +808,7 @@ public void handle(Event event) {
     resources.put(userjar, lpaths);
     Path nmp = new Path(testDir);
 
+    launch.addConfigsToEnv(userSetEnv);
     launch.sanitizeEnv(userSetEnv, pwd, appDirs, userLocalDirs, containerLogs,
         resources, nmp, nmEnvTrack);
     Assert.assertTrue(userSetEnv.containsKey("MALLOC_ARENA_MAX"));
@@ -864,6 +865,7 @@ public void handle(Event event) {
 
     ContainerLaunch launch = new ContainerLaunch(distContext, conf,
         dispatcher, exec, null, container, dirsHandler, containerManager);
+    launch.addConfigsToEnv(userSetEnv);
     launch.sanitizeEnv(userSetEnv, pwd, appDirs, userLocalDirs, containerLogs,
         resources, nmp, nmEnvTrack);
 
@@ -876,6 +878,7 @@ public void handle(Event event) {
     containerLaunchContext.setEnvironment(userSetEnv);
     when(container.getLaunchContext()).thenReturn(containerLaunchContext);
 
+    launch.addConfigsToEnv(userSetEnv);
     launch.sanitizeEnv(userSetEnv, pwd, appDirs, userLocalDirs, containerLogs,
         resources, nmp, nmEnvTrack);