From b9a429bb2854910add8d4cf787e6ee65ebdfc9cf Mon Sep 17 00:00:00 2001 From: Jason Lowe Date: Mon, 19 Feb 2018 08:16:25 -0600 Subject: [PATCH] Revert "YARN-7677. Docker image cannot set HADOOP_CONF_DIR. Contributed by Jim Brennan" This reverts commit 8013475d447a8377b5aed858208bf8b91dd32366. --- .../org/apache/hadoop/yarn/util/Apps.java | 22 +-- .../yarn/util/AuxiliaryServiceHelper.java | 2 +- .../server/nodemanager/ContainerExecutor.java | 62 +++----- .../nodemanager/LinuxContainerExecutor.java | 8 ++ .../launcher/ContainerLaunch.java | 88 ++++-------- .../runtime/DefaultLinuxContainerRuntime.java | 6 + .../DelegatingLinuxContainerRuntime.java | 11 ++ .../runtime/DockerLinuxContainerRuntime.java | 7 + .../runtime/ContainerRuntime.java | 11 ++ .../launcher/TestContainerLaunch.java | 133 ++---------------- 10 files changed, 110 insertions(+), 240 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/Apps.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/Apps.java index 1c90d551b7b..685c6d30540 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/Apps.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/Apps.java @@ -23,7 +23,6 @@ import static org.apache.hadoop.yarn.util.StringHelper.sjoin; import java.io.File; -import java.util.ArrayList; import java.util.Iterator; import java.util.Map; import java.util.regex.Matcher; @@ -106,26 +105,7 @@ public static void setEnvFromInputString(Map env, } } } - - /** - * - * @param envString String containing env variable definitions - * @param classPathSeparator String that separates the definitions - * @return ArrayList of environment variable names - */ - public static ArrayList getEnvVarsFromInputString(String envString, - String classPathSeparator) { - ArrayList envList = new ArrayList<>(); - if (envString != null && envString.length() > 0) { - Matcher varValMatcher = VARVAL_SPLITTER.matcher(envString); - while (varValMatcher.find()) { - String envVar = varValMatcher.group(1); - envList.add(envVar); - } - } - return envList; - } - + /** * This older version of this method is kept around for compatibility * because downstream frameworks like Spark and Tez have been using it. diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/AuxiliaryServiceHelper.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/AuxiliaryServiceHelper.java index 1374d96f261..cb118f56da9 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/AuxiliaryServiceHelper.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/AuxiliaryServiceHelper.java @@ -45,7 +45,7 @@ public static void setServiceDataIntoEnv(String serviceName, Base64.encodeBase64String(byteData)); } - public static String getPrefixServiceName(String serviceName) { + private static String getPrefixServiceName(String serviceName) { return NM_AUX_SERVICE + serviceName; } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java index 01cd9922507..f4279a3b09e 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java @@ -27,7 +27,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.LinkedHashSet; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -317,15 +316,14 @@ public int reacquireContainer(ContainerReacquisitionContext ctx) * @param command the command that will be run * @param logDir the log dir to which to copy debugging information * @param user the username of the job owner - * @param nmVars the set of environment vars that are explicitly set by NM * @throws IOException if any errors happened writing to the OutputStream, * while creating symlinks */ public void writeLaunchEnv(OutputStream out, Map environment, Map> resources, List command, Path logDir, - String user, LinkedHashSet nmVars) throws IOException { + String user) throws IOException { this.writeLaunchEnv(out, environment, resources, command, logDir, user, - ContainerLaunch.CONTAINER_SCRIPT, nmVars); + ContainerLaunch.CONTAINER_SCRIPT); } /** @@ -341,15 +339,14 @@ public void writeLaunchEnv(OutputStream out, Map environment, * @param logDir the log dir to which to copy debugging information * @param user the username of the job owner * @param outFilename the path to which to write the launch environment - * @param nmVars the set of environment vars that are explicitly set by NM * @throws IOException if any errors happened writing to the OutputStream, * while creating symlinks */ @VisibleForTesting public void writeLaunchEnv(OutputStream out, Map environment, Map> resources, List command, Path logDir, - String user, String outFilename, LinkedHashSet nmVars) - throws IOException { + String user, String outFilename) throws IOException { + updateEnvForWhitelistVars(environment); ContainerLaunch.ShellScriptBuilder sb = ContainerLaunch.ShellScriptBuilder.create(); @@ -364,40 +361,8 @@ public void writeLaunchEnv(OutputStream out, Map environment, if (environment != null) { sb.echo("Setting up env variables"); - // Whitelist environment variables are treated specially. - // Only add them if they are not already defined in the environment. - // Add them using special syntax to prevent them from eclipsing - // variables that may be set explicitly in the container image (e.g, - // in a docker image). Put these before the others to ensure the - // correct expansion is used. - for(String var : whitelistVars) { - if (!environment.containsKey(var)) { - String val = getNMEnvVar(var); - if (val != null) { - sb.whitelistedEnv(var, val); - } - } - } - // Now write vars that were set explicitly by nodemanager, preserving - // the order they were written in. - for (String nmEnvVar : nmVars) { - sb.env(nmEnvVar, environment.get(nmEnvVar)); - } - // Now write the remaining environment variables. for (Map.Entry env : environment.entrySet()) { - if (!nmVars.contains(env.getKey())) { - sb.env(env.getKey(), env.getValue()); - } - } - // Add the whitelist vars to the environment. Do this after writing - // environment variables so they are not written twice. - for(String var : whitelistVars) { - if (!environment.containsKey(var)) { - String val = getNMEnvVar(var); - if (val != null) { - environment.put(var, val); - } - } + sb.env(env.getKey(), env.getValue()); } } @@ -698,6 +663,23 @@ protected boolean isContainerActive(ContainerId containerId) { } } + /** + * Propagate variables from the nodemanager's environment into the + * container's environment if unspecified by the container. + * @param env the environment to update + * @see org.apache.hadoop.yarn.conf.YarnConfiguration#NM_ENV_WHITELIST + */ + protected void updateEnvForWhitelistVars(Map env) { + for(String var : whitelistVars) { + if (!env.containsKey(var)) { + String val = getNMEnvVar(var); + if (val != null) { + env.put(var, val); + } + } + } + } + @VisibleForTesting protected String getNMEnvVar(String varname) { return System.getenv(varname); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java index 44edc21a38b..fe54e2c4e91 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java @@ -66,6 +66,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.regex.Pattern; import static org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.runtime.LinuxContainerRuntimeConstants.*; @@ -471,6 +472,13 @@ public void prepareContainer(ContainerPrepareContext ctx) throws IOException { } } + @Override + protected void updateEnvForWhitelistVars(Map env) { + if (linuxContainerRuntime.useWhitelistEnv(env)) { + super.updateEnvForWhitelistVars(env); + } + } + @Override public int launchContainer(ContainerStartContext ctx) throws IOException, ConfigurationException { 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 ca62a5c05f3..112f54a537a 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 @@ -33,9 +33,7 @@ import java.util.EnumSet; import java.util.HashMap; import java.util.List; -import java.util.LinkedHashSet; import java.util.Map; -import java.util.Set; import java.util.Map.Entry; import java.util.concurrent.Callable; import java.util.concurrent.atomic.AtomicBoolean; @@ -219,9 +217,6 @@ public Integer call() { launchContext, containerLogDir); // /////////////////////////// End of variable expansion - // Use this to track variables that are added to the environment by nm. - LinkedHashSet nmEnvVars = new LinkedHashSet(); - FileContext lfs = FileContext.getLocalFSFileContext(); Path nmPrivateContainerScriptPath = dirsHandler.getLocalPathForWrite( @@ -266,7 +261,7 @@ public Integer call() { } // Set the token location too. - addToEnvMap(environment, nmEnvVars, + environment.put( ApplicationConstants.CONTAINER_TOKEN_FILE_ENV_NAME, new Path(containerWorkDir, FINAL_CONTAINER_TOKENS_FILE).toUri().getPath()); @@ -277,15 +272,14 @@ public Integer call() { EnumSet.of(CREATE, OVERWRITE))) { // Sanitize the container's environment sanitizeEnv(environment, containerWorkDir, appDirs, userLocalDirs, - containerLogDirs, localResources, nmPrivateClasspathJarDir, - nmEnvVars); + containerLogDirs, localResources, nmPrivateClasspathJarDir); prepareContainer(localResources, containerLocalDirs); // Write out the environment exec.writeLaunchEnv(containerScriptOutStream, environment, localResources, launchContext.getCommands(), - containerLogDir, user, nmEnvVars); + containerLogDir, user); } // /////////// End of writing out container-script @@ -1177,9 +1171,6 @@ public final void stderr(Path stderrDir, String stdErrFile) throws IOException { public abstract void env(String key, String value) throws IOException; - public abstract void whitelistedEnv(String key, String value) - throws IOException; - public abstract void echo(String echoStr) throws IOException; public final void symlink(Path src, Path dst) throws IOException { @@ -1299,11 +1290,6 @@ public void env(String key, String value) throws IOException { line("export ", key, "=\"", value, "\""); } - @Override - public void whitelistedEnv(String key, String value) throws IOException { - line("export ", key, "=${", key, ":-", "\"", value, "\"}"); - } - @Override public void echo(final String echoStr) throws IOException { line("echo \"" + echoStr + "\""); @@ -1394,11 +1380,6 @@ public void env(String key, String value) throws IOException { errorCheck(); } - @Override - public void whitelistedEnv(String key, String value) throws IOException { - env(key, value); - } - @Override public void echo(final String echoStr) throws IOException { lineWithLenCheck("@echo \"", echoStr, "\""); @@ -1454,70 +1435,60 @@ private static void putEnvIfAbsent( putEnvIfNotNull(environment, variable, System.getenv(variable)); } } - - private static void addToEnvMap( - Map envMap, Set envSet, - String envName, String envValue) { - envMap.put(envName, envValue); - envSet.add(envName); - } - + public void sanitizeEnv(Map environment, Path pwd, List appDirs, List userLocalDirs, List - containerLogDirs, Map> resources, - Path nmPrivateClasspathJarDir, - Set nmVars) throws IOException { + containerLogDirs, + Map> resources, + Path nmPrivateClasspathJarDir) throws IOException { /** * Non-modifiable environment variables */ - addToEnvMap(environment, nmVars, Environment.CONTAINER_ID.name(), - container.getContainerId().toString()); + environment.put(Environment.CONTAINER_ID.name(), container + .getContainerId().toString()); - addToEnvMap(environment, nmVars, Environment.NM_PORT.name(), + environment.put(Environment.NM_PORT.name(), String.valueOf(this.context.getNodeId().getPort())); - addToEnvMap(environment, nmVars, Environment.NM_HOST.name(), - this.context.getNodeId().getHost()); + environment.put(Environment.NM_HOST.name(), this.context.getNodeId() + .getHost()); - addToEnvMap(environment, nmVars, Environment.NM_HTTP_PORT.name(), + environment.put(Environment.NM_HTTP_PORT.name(), String.valueOf(this.context.getHttpPort())); - addToEnvMap(environment, nmVars, Environment.LOCAL_DIRS.name(), + environment.put(Environment.LOCAL_DIRS.name(), StringUtils.join(",", appDirs)); - addToEnvMap(environment, nmVars, Environment.LOCAL_USER_DIRS.name(), - StringUtils.join(",", userLocalDirs)); + environment.put(Environment.LOCAL_USER_DIRS.name(), StringUtils.join(",", + userLocalDirs)); - addToEnvMap(environment, nmVars, Environment.LOG_DIRS.name(), + environment.put(Environment.LOG_DIRS.name(), StringUtils.join(",", containerLogDirs)); - addToEnvMap(environment, nmVars, Environment.USER.name(), - container.getUser()); + environment.put(Environment.USER.name(), container.getUser()); + + environment.put(Environment.LOGNAME.name(), container.getUser()); - addToEnvMap(environment, nmVars, Environment.LOGNAME.name(), - container.getUser()); - - addToEnvMap(environment, nmVars, Environment.HOME.name(), + environment.put(Environment.HOME.name(), conf.get( YarnConfiguration.NM_USER_HOME_DIR, YarnConfiguration.DEFAULT_NM_USER_HOME_DIR ) ); - - addToEnvMap(environment, nmVars, Environment.PWD.name(), pwd.toString()); + + environment.put(Environment.PWD.name(), pwd.toString()); + + putEnvIfAbsent(environment, Environment.HADOOP_CONF_DIR.name()); if (!Shell.WINDOWS) { - addToEnvMap(environment, nmVars, "JVM_PID", "$$"); + environment.put("JVM_PID", "$$"); } // variables here will be forced in, even if the container has specified them. - String nmAdminUserEnv = conf.get( - YarnConfiguration.NM_ADMIN_USER_ENV, - YarnConfiguration.DEFAULT_NM_ADMIN_USER_ENV); - Apps.setEnvFromInputString(environment, nmAdminUserEnv, File.pathSeparator); - nmVars.addAll(Apps.getEnvVarsFromInputString(nmAdminUserEnv, - File.pathSeparator)); + Apps.setEnvFromInputString(environment, conf.get( + YarnConfiguration.NM_ADMIN_USER_ENV, + YarnConfiguration.DEFAULT_NM_ADMIN_USER_ENV), File.pathSeparator); // TODO: Remove Windows check and use this approach on all platforms after // additional testing. See YARN-358. @@ -1531,7 +1502,6 @@ public void sanitizeEnv(Map environment, Path pwd, .getAuxServiceMetaData().entrySet()) { AuxiliaryServiceHelper.setServiceDataIntoEnv( meta.getKey(), meta.getValue(), environment); - nmVars.add(AuxiliaryServiceHelper.getPrefixServiceName(meta.getKey())); } } 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/linux/runtime/DefaultLinuxContainerRuntime.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DefaultLinuxContainerRuntime.java index 83380ee2d4b..b50d56c0e01 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DefaultLinuxContainerRuntime.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DefaultLinuxContainerRuntime.java @@ -37,6 +37,7 @@ import org.slf4j.LoggerFactory; import java.util.List; +import java.util.Map; import static org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.runtime.LinuxContainerRuntimeConstants.*; @@ -72,6 +73,11 @@ public void initialize(Configuration conf, Context nmContext) this.conf = conf; } + @Override + public boolean useWhitelistEnv(Map env) { + return true; + } + @Override public void prepareContainer(ContainerRuntimeContext ctx) throws ContainerExecutionException { 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/linux/runtime/DelegatingLinuxContainerRuntime.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DelegatingLinuxContainerRuntime.java index 675bffb00c2..dd10617a81c 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DelegatingLinuxContainerRuntime.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DelegatingLinuxContainerRuntime.java @@ -94,6 +94,17 @@ public void initialize(Configuration conf, Context nmContext) } } + @Override + public boolean useWhitelistEnv(Map env) { + try { + LinuxContainerRuntime runtime = pickContainerRuntime(env); + return runtime.useWhitelistEnv(env); + } catch (ContainerExecutionException e) { + LOG.debug("Unable to determine runtime"); + return false; + } + } + @VisibleForTesting LinuxContainerRuntime pickContainerRuntime( Map environment) throws ContainerExecutionException { 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/linux/runtime/DockerLinuxContainerRuntime.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DockerLinuxContainerRuntime.java index de225e6a2b1..401fc4ac128 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DockerLinuxContainerRuntime.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/linux/runtime/DockerLinuxContainerRuntime.java @@ -371,6 +371,13 @@ public Set getCapabilities() { return capabilities; } + @Override + public boolean useWhitelistEnv(Map env) { + // Avoid propagating nodemanager environment variables into the container + // so those variables can be picked up from the Docker image instead. + return false; + } + private String runDockerVolumeCommand(DockerVolumeCommand dockerVolumeCommand, Container container) throws ContainerExecutionException { try { 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/runtime/ContainerRuntime.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/runtime/ContainerRuntime.java index 7caa0edf4de..aa294fc57c2 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/runtime/ContainerRuntime.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/runtime/ContainerRuntime.java @@ -24,6 +24,8 @@ import org.apache.hadoop.classification.InterfaceStability; import org.apache.hadoop.yarn.server.nodemanager.containermanager.container.Container; +import java.util.Map; + /** * An abstraction for various container runtime implementations. Examples * include Process Tree, Docker, Appc runtimes etc. These implementations @@ -83,4 +85,13 @@ void reapContainer(ContainerRuntimeContext ctx) * and hostname */ String[] getIpAndHost(Container container) throws ContainerExecutionException; + + /** + * Whether to propagate the whitelist of environment variables from the + * nodemanager environment into the container environment. + * @param env the container's environment variables + * @return true if whitelist variables should be propagated, false otherwise + * @see org.apache.hadoop.yarn.conf.YarnConfiguration#NM_ENV_WHITELIST + */ + boolean useWhitelistEnv(Map env); } \ No newline at end of file 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 47e268cab11..5923f8ef055 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 @@ -41,7 +41,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.StringTokenizer; @@ -186,10 +185,8 @@ public void testSpecialCharSymlinks() throws IOException { DefaultContainerExecutor defaultContainerExecutor = new DefaultContainerExecutor(); defaultContainerExecutor.setConf(new YarnConfiguration()); - LinkedHashSet nmVars = new LinkedHashSet<>(); defaultContainerExecutor.writeLaunchEnv(fos, env, resources, commands, - new Path(localLogDir.getAbsolutePath()), "user", tempFile.getName(), - nmVars); + new Path(localLogDir.getAbsolutePath()), "user", tempFile.getName()); fos.flush(); fos.close(); FileUtil.setExecutable(tempFile, true); @@ -263,9 +260,8 @@ public void testInvalidSymlinkDiagnostics() throws IOException { DefaultContainerExecutor defaultContainerExecutor = new DefaultContainerExecutor(); defaultContainerExecutor.setConf(new YarnConfiguration()); - LinkedHashSet nmVars = new LinkedHashSet<>(); defaultContainerExecutor.writeLaunchEnv(fos, env, resources, commands, - new Path(localLogDir.getAbsolutePath()), "user", nmVars); + new Path(localLogDir.getAbsolutePath()), "user"); fos.flush(); fos.close(); FileUtil.setExecutable(tempFile, true); @@ -327,9 +323,8 @@ protected String getNMEnvVar(String varname) { conf.set(YarnConfiguration.NM_ENV_WHITELIST, "HADOOP_MAPRED_HOME,HADOOP_YARN_HOME"); defaultContainerExecutor.setConf(conf); - LinkedHashSet nmVars = new LinkedHashSet<>(); defaultContainerExecutor.writeLaunchEnv(fos, env, resources, commands, - new Path(localLogDir.getAbsolutePath()), "user", nmVars); + new Path(localLogDir.getAbsolutePath()), "user"); String shellContent = new String(Files.readAllBytes(Paths.get(shellFile.getAbsolutePath())), StandardCharsets.UTF_8); @@ -342,8 +337,7 @@ protected String getNMEnvVar(String varname) { Assert.assertFalse(shellContent.contains("HADOOP_HDFS_HOME")); // Available in env and in whitelist Assert.assertTrue(shellContent.contains( - "export HADOOP_YARN_HOME=${HADOOP_YARN_HOME:-\"nodemanager_yarn_home\"}" - )); + "export HADOOP_YARN_HOME=\"nodemanager_yarn_home\"")); fos.flush(); fos.close(); } @@ -378,9 +372,8 @@ protected String getNMEnvVar(String varname) { conf.set(YarnConfiguration.NM_ENV_WHITELIST, "HADOOP_MAPRED_HOME,HADOOP_YARN_HOME"); lce.setConf(conf); - LinkedHashSet nmVars = new LinkedHashSet<>(); lce.writeLaunchEnv(fos, env, resources, commands, - new Path(localLogDir.getAbsolutePath()), "user", nmVars); + new Path(localLogDir.getAbsolutePath()), "user"); String shellContent = new String(Files.readAllBytes(Paths.get(shellFile.getAbsolutePath())), StandardCharsets.UTF_8); @@ -389,106 +382,13 @@ protected String getNMEnvVar(String varname) { // Whitelisted variable overridden by container Assert.assertTrue(shellContent.contains( "export HADOOP_MAPRED_HOME=\"/opt/hadoopbuild\"")); - // Available in env but not in whitelist + // Verify no whitelisted variables inherited from NM env Assert.assertFalse(shellContent.contains("HADOOP_HDFS_HOME")); - // Available in env and in whitelist - Assert.assertTrue(shellContent.contains( - "export HADOOP_YARN_HOME=${HADOOP_YARN_HOME:-\"nodemanager_yarn_home\"}" - )); + Assert.assertFalse(shellContent.contains("HADOOP_YARN_HOME")); fos.flush(); fos.close(); } - @Test(timeout = 20000) - public void testWriteEnvOrder() throws Exception { - // Valid only for unix - assumeNotWindows(); - List commands = new ArrayList(); - - // Setup user-defined environment - Map env = new HashMap(); - env.put("USER_VAR_1", "1"); - env.put("USER_VAR_2", "2"); - env.put("NM_MODIFIED_VAR_1", "nm 1"); - env.put("NM_MODIFIED_VAR_2", "nm 2"); - - // These represent vars explicitly set by NM - LinkedHashSet trackedNmVars = new LinkedHashSet<>(); - trackedNmVars.add("NM_MODIFIED_VAR_1"); - trackedNmVars.add("NM_MODIFIED_VAR_2"); - - // Setup Nodemanager environment - final Map nmEnv = new HashMap<>(); - nmEnv.put("WHITELIST_VAR_1", "wl 1"); - nmEnv.put("WHITELIST_VAR_2", "wl 2"); - nmEnv.put("NON_WHITELIST_VAR_1", "nwl 1"); - nmEnv.put("NON_WHITELIST_VAR_2", "nwl 2"); - DefaultContainerExecutor defaultContainerExecutor = - new DefaultContainerExecutor() { - @Override - protected String getNMEnvVar(String varname) { - return nmEnv.get(varname); - } - }; - - // Setup conf with whitelisted variables - ArrayList whitelistVars = new ArrayList<>(); - whitelistVars.add("WHITELIST_VAR_1"); - whitelistVars.add("WHITELIST_VAR_2"); - YarnConfiguration conf = new YarnConfiguration(); - conf.set(YarnConfiguration.NM_ENV_WHITELIST, - whitelistVars.get(0) + "," + whitelistVars.get(1)); - - // These are in the NM env, but not in the whitelist. - ArrayList nonWhiteListEnv = new ArrayList<>(); - nonWhiteListEnv.add("NON_WHITELIST_VAR_1"); - nonWhiteListEnv.add("NON_WHITELIST_VAR_2"); - - // Write the launch script - File shellFile = Shell.appendScriptExtension(tmpDir, "hello"); - Map> resources = new HashMap>(); - FileOutputStream fos = new FileOutputStream(shellFile); - defaultContainerExecutor.setConf(conf); - defaultContainerExecutor.writeLaunchEnv(fos, env, resources, commands, - new Path(localLogDir.getAbsolutePath()), "user", trackedNmVars); - fos.flush(); - fos.close(); - - // Examine the script - String shellContent = - new String(Files.readAllBytes(Paths.get(shellFile.getAbsolutePath())), - StandardCharsets.UTF_8); - // First make sure everything is there that's supposed to be - for (String envVar : env.keySet()) { - Assert.assertTrue(shellContent.contains(envVar + "=")); - } - for (String wlVar : whitelistVars) { - Assert.assertTrue(shellContent.contains(wlVar + "=")); - } - for (String nwlVar : nonWhiteListEnv) { - Assert.assertFalse(shellContent.contains(nwlVar + "=")); - } - // Explicitly Set NM vars should be before user vars - for (String nmVar : trackedNmVars) { - for (String userVar : env.keySet()) { - // Need to skip nm vars and whitelist vars - if (!trackedNmVars.contains(userVar) && - !whitelistVars.contains(userVar)) { - Assert.assertTrue(shellContent.indexOf(nmVar + "=") < - shellContent.indexOf(userVar + "=")); - } - } - } - // Whitelisted vars should be before explicitly set NM vars - for (String wlVar : whitelistVars) { - for (String nmVar : trackedNmVars) { - Assert.assertTrue(shellContent.indexOf(wlVar + "=") < - shellContent.indexOf(nmVar + "=")); - } - } - } - - @Test (timeout = 20000) public void testInvalidEnvSyntaxDiagnostics() throws IOException { @@ -510,9 +410,8 @@ public void testInvalidEnvSyntaxDiagnostics() throws IOException { DefaultContainerExecutor defaultContainerExecutor = new DefaultContainerExecutor(); defaultContainerExecutor.setConf(new YarnConfiguration()); - LinkedHashSet nmVars = new LinkedHashSet<>(); defaultContainerExecutor.writeLaunchEnv(fos, env, resources, commands, - new Path(localLogDir.getAbsolutePath()), "user", nmVars); + new Path(localLogDir.getAbsolutePath()), "user"); fos.flush(); fos.close(); @@ -594,9 +493,8 @@ public void testContainerLaunchStdoutAndStderrDiagnostics() throws IOException { commands.add(command); ContainerExecutor exec = new DefaultContainerExecutor(); exec.setConf(new YarnConfiguration()); - LinkedHashSet nmVars = new LinkedHashSet<>(); exec.writeLaunchEnv(fos, env, resources, commands, - new Path(localLogDir.getAbsolutePath()), "user", nmVars); + new Path(localLogDir.getAbsolutePath()), "user"); fos.flush(); fos.close(); @@ -687,7 +585,7 @@ public void handle(Event event) { Path nmp = new Path(testDir); launch.sanitizeEnv(userSetEnv, pwd, appDirs, userLocalDirs, containerLogs, - resources, nmp, Collections.emptySet()); + resources, nmp); List result = getJarManifestClasspath(userSetEnv.get(Environment.CLASSPATH.name())); @@ -706,7 +604,7 @@ public void handle(Event event) { dispatcher, exec, null, container, dirsHandler, containerManager); launch.sanitizeEnv(userSetEnv, pwd, appDirs, userLocalDirs, containerLogs, - resources, nmp, Collections.emptySet()); + resources, nmp); result = getJarManifestClasspath(userSetEnv.get(Environment.CLASSPATH.name())); @@ -1630,10 +1528,9 @@ public void testDebuggingInformation() throws IOException { FileOutputStream fos = new FileOutputStream(tempFile); ContainerExecutor exec = new DefaultContainerExecutor(); exec.setConf(conf); - LinkedHashSet nmVars = new LinkedHashSet<>(); exec.writeLaunchEnv(fos, env, resources, commands, new Path(localLogDir.getAbsolutePath()), "user", - tempFile.getName(), nmVars); + tempFile.getName()); fos.flush(); fos.close(); FileUtil.setExecutable(tempFile, true); @@ -1856,9 +1753,8 @@ private void validateShellExecutorForDifferentEnvs(Map env) List commands = new ArrayList(); DefaultContainerExecutor executor = new DefaultContainerExecutor(); executor.setConf(new Configuration()); - LinkedHashSet nmVars = new LinkedHashSet<>(); executor.writeLaunchEnv(fos, env, resources, commands, - new Path(localLogDir.getAbsolutePath()), user, nmVars); + new Path(localLogDir.getAbsolutePath()), user); fos.flush(); fos.close(); @@ -1902,9 +1798,8 @@ public void testValidEnvVariableSubstitution() throws IOException { Configuration execConf = new Configuration(); execConf.setBoolean(YarnConfiguration.NM_LOG_CONTAINER_DEBUG_INFO, false); executor.setConf(execConf); - LinkedHashSet nmVars = new LinkedHashSet<>(); executor.writeLaunchEnv(fos, env, resources, commands, - new Path(localLogDir.getAbsolutePath()), user, nmVars); + new Path(localLogDir.getAbsolutePath()), user); fos.flush(); fos.close();