From dadd3d9138d2e90a1bb0463e4053d76afc4c8854 Mon Sep 17 00:00:00 2001 From: Gautham B A Date: Tue, 6 Dec 2022 16:32:26 +0530 Subject: [PATCH] YARN-11386. Fix issue with classpath resolution (#5183) * This PR ensures that all the special notations such as are resolved before getting added to classpath. --- .../launcher/ContainerLaunch.java | 64 ++++++++++--------- .../launcher/TestContainerLaunch.java | 3 + 2 files changed, 38 insertions(+), 29 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java index 0acc4de470..7d91e5d395 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/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 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 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 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 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 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 meta : containerManager - .getAuxServiceMetaData().entrySet()) { - AuxiliaryServiceHelper.setServiceDataIntoEnv( - meta.getKey(), meta.getValue(), environment); - nmVars.add(AuxiliaryServiceHelper.getPrefixServiceName(meta.getKey())); - } } private void sanitizeWindowsEnv(Map environment, Path pwd, Map> resources, Path nmPrivateClasspathJarDir) throws IOException { - String inputClassPath = environment.get(Environment.CLASSPATH.name()); if (inputClassPath != null && !inputClassPath.isEmpty()) { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java index 6b0732b4e5..ed22254906 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunch.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/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);