From edcfd4527ca93acdf54403aafaa070b17aff5dd0 Mon Sep 17 00:00:00 2001 From: Bikas Saha Date: Fri, 19 Apr 2013 19:29:22 +0000 Subject: [PATCH] YARN-593. container launch on Windows does not correctly populate classpath with new process's environment variables and localized resources (Chris Nauroth via bikas) git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1469998 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-yarn-project/CHANGES.txt | 4 + .../launcher/ContainerLaunch.java | 81 +++++++++++++++---- 2 files changed, 70 insertions(+), 15 deletions(-) diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index b5a6ab6aa7..9a0e020822 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -37,6 +37,10 @@ Trunk - Unreleased YARN-493. Fixed some shell related flaws in YARN on Windows. (Chris Nauroth via vinodkv) + YARN-593. container launch on Windows does not correctly populate + classpath with new process's environment variables and localized resources + (Chris Nauroth via bikas) + BREAKDOWN OF HADOOP-8562 SUBTASKS YARN-158. Yarn creating package-info.java must not depend on sh. 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 b61e071609..d48a4b7541 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 @@ -28,6 +28,7 @@ import java.io.OutputStream; import java.io.PrintStream; import java.util.ArrayList; import java.util.EnumSet; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -211,7 +212,7 @@ public class ContainerLaunch implements Callable { FINAL_CONTAINER_TOKENS_FILE).toUri().getPath()); // Sanitize the container's environment - sanitizeEnv(environment, containerWorkDir, appDirs); + sanitizeEnv(environment, containerWorkDir, appDirs, localResources); // Write out the environment writeLaunchEnv(containerScriptOutStream, environment, localResources, @@ -506,9 +507,17 @@ public class ContainerLaunch implements Callable { @Override protected void link(Path src, Path dst) throws IOException { - line(String.format("@%s symlink \"%s\" \"%s\"", Shell.WINUTILS, - new File(dst.toString()).getPath(), - new File(src.toUri().getPath()).getPath())); + File srcFile = new File(src.toUri().getPath()); + String srcFileStr = srcFile.getPath(); + String dstFileStr = new File(dst.toString()).getPath(); + // If not on Java7+ on Windows, then copy file instead of symlinking. + // See also FileUtil#symLink for full explanation. + if (!Shell.isJava7OrAbove() && srcFile.isFile()) { + line(String.format("@copy \"%s\" \"%s\"", srcFileStr, dstFileStr)); + } else { + line(String.format("@%s symlink \"%s\" \"%s\"", Shell.WINUTILS, + dstFileStr, srcFileStr)); + } } @Override @@ -532,7 +541,8 @@ public class ContainerLaunch implements Callable { } public void sanitizeEnv(Map environment, - Path pwd, List appDirs) throws IOException { + Path pwd, List appDirs, Map> resources) + throws IOException { /** * Non-modifiable environment variables */ @@ -566,16 +576,6 @@ public class ContainerLaunch implements Callable { environment.put("JVM_PID", "$$"); } - // TODO: Remove Windows check and use this approach on all platforms after - // additional testing. See YARN-358. - if (Shell.WINDOWS) { - String inputClassPath = environment.get(Environment.CLASSPATH.name()); - if (inputClassPath != null && !inputClassPath.isEmpty()) { - environment.put(Environment.CLASSPATH.name(), - FileUtil.createJarWithClassPath(inputClassPath, pwd)); - } - } - /** * Modifiable environment variables */ @@ -594,6 +594,57 @@ public class ContainerLaunch implements Callable { YarnConfiguration.NM_ADMIN_USER_ENV, YarnConfiguration.DEFAULT_NM_ADMIN_USER_ENV) ); + + // TODO: Remove Windows check and use this approach on all platforms after + // additional testing. See YARN-358. + if (Shell.WINDOWS) { + String inputClassPath = environment.get(Environment.CLASSPATH.name()); + if (inputClassPath != null && !inputClassPath.isEmpty()) { + StringBuilder newClassPath = new StringBuilder(inputClassPath); + + // Localized resources do not exist at the desired paths yet, because the + // container launch script has not run to create symlinks yet. This + // means that FileUtil.createJarWithClassPath can't automatically expand + // wildcards to separate classpath entries for each file in the manifest. + // To resolve this, append classpath entries explicitly for each + // resource. + for (Map.Entry> entry : resources.entrySet()) { + boolean targetIsDirectory = new File(entry.getKey().toUri().getPath()) + .isDirectory(); + + for (String linkName : entry.getValue()) { + // Append resource. + newClassPath.append(File.pathSeparator).append(pwd.toString()) + .append(Path.SEPARATOR).append(linkName); + + // FileUtil.createJarWithClassPath must use File.toURI to convert + // each file to a URI to write into the manifest's classpath. For + // directories, the classpath must have a trailing '/', but + // File.toURI only appends the trailing '/' if it is a directory that + // already exists. To resolve this, add the classpath entries with + // explicit trailing '/' here for any localized resource that targets + // a directory. Then, FileUtil.createJarWithClassPath will guarantee + // that the resulting entry in the manifest's classpath will have a + // trailing '/', and thus refer to a directory instead of a file. + if (targetIsDirectory) { + newClassPath.append(Path.SEPARATOR); + } + } + } + + // When the container launches, it takes the parent process's environment + // and then adds/overwrites with the entries from the container launch + // context. Do the same thing here for correct substitution of + // environment variables in the classpath jar manifest. + Map mergedEnv = new HashMap( + System.getenv()); + mergedEnv.putAll(environment); + + String classPathJar = FileUtil.createJarWithClassPath( + newClassPath.toString(), pwd, mergedEnv); + environment.put(Environment.CLASSPATH.name(), classPathJar); + } + } } static void writeLaunchEnv(OutputStream out,