From 120cd9eae6ffe61e001787eba64b8e25e0c1bef4 Mon Sep 17 00:00:00 2001 From: Eric Badger Date: Thu, 6 Aug 2020 19:40:33 +0000 Subject: [PATCH] YARN-7677. Docker image cannot set HADOOP_CONF_DIR. Contributed by Jim Brennan. --- .../org/apache/hadoop/yarn/util/Apps.java | 22 ++- .../yarn/util/AuxiliaryServiceHelper.java | 2 +- .../server/nodemanager/ContainerExecutor.java | 79 +++++++---- .../nodemanager/DockerContainerExecutor.java | 77 ---------- .../nodemanager/LinuxContainerExecutor.java | 7 - .../launcher/ContainerLaunch.java | 91 ++++++++---- .../runtime/DefaultLinuxContainerRuntime.java | 6 - .../DelegatingLinuxContainerRuntime.java | 11 -- .../runtime/DockerLinuxContainerRuntime.java | 7 - .../runtime/ContainerRuntime.java | 11 -- .../launcher/TestContainerLaunch.java | 134 ++++++++++++++++-- 11 files changed, 255 insertions(+), 192 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 9235e7dcaa8..aa629acce2a 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,6 +23,7 @@ import static org.apache.hadoop.yarn.util.StringHelper.join; 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; @@ -104,7 +105,26 @@ public class Apps { } } } - + + /** + * + * @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 cb118f56da9..1374d96f261 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 class AuxiliaryServiceHelper { Base64.encodeBase64String(byteData)); } - private static String getPrefixServiceName(String serviceName) { + public 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 376e1b232de..34fafccc92c 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,12 +27,15 @@ import java.net.UnknownHostException; 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; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock; import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock; + +import org.apache.hadoop.yarn.api.ApplicationConstants; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -93,14 +96,21 @@ public abstract class ContainerExecutor implements Configurable { private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); private final ReadLock readLock = lock.readLock(); private final WriteLock writeLock = lock.writeLock(); - private String[] whitelistVars; + private List whitelistVars = new ArrayList<>(); @Override public void setConf(Configuration conf) { this.conf = conf; if (conf != null) { - whitelistVars = conf.get(YarnConfiguration.NM_ENV_WHITELIST, - YarnConfiguration.DEFAULT_NM_ENV_WHITELIST).split(","); + whitelistVars.clear(); + whitelistVars.addAll( + Arrays.asList(conf.get(YarnConfiguration.NM_ENV_WHITELIST, + YarnConfiguration.DEFAULT_NM_ENV_WHITELIST).split(","))); + // Force HADOOP_CONF_DIR into whitelistVars + String confDir = ApplicationConstants.Environment.HADOOP_CONF_DIR.key(); + if (!whitelistVars.contains(confDir)) { + whitelistVars.add(confDir); + } } } @@ -297,14 +307,15 @@ public abstract class ContainerExecutor implements Configurable { * @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) throws IOException { + String user, LinkedHashSet nmVars) throws IOException { this.writeLaunchEnv(out, environment, resources, command, logDir, user, - ContainerLaunch.CONTAINER_SCRIPT); + ContainerLaunch.CONTAINER_SCRIPT, nmVars); } /** @@ -320,14 +331,15 @@ public abstract class ContainerExecutor implements Configurable { * @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) throws IOException { - updateEnvForWhitelistVars(environment); + String user, String outFilename, LinkedHashSet nmVars) + throws IOException { ContainerLaunch.ShellScriptBuilder sb = ContainerLaunch.ShellScriptBuilder.create(); @@ -342,9 +354,41 @@ public abstract class ContainerExecutor implements Configurable { 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 : - sb.orderEnvByDependencies(environment).entrySet()) { - sb.env(env.getKey(), env.getValue()); + sb.orderEnvByDependencies(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); + } + } } } @@ -645,23 +689,6 @@ public abstract class ContainerExecutor implements Configurable { } } - /** - * 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/DockerContainerExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DockerContainerExecutor.java index 897e253ed63..9e1b6f8661b 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DockerContainerExecutor.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/DockerContainerExecutor.java @@ -23,21 +23,16 @@ import static org.apache.hadoop.fs.CreateFlag.OVERWRITE; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.ByteArrayOutputStream; import java.io.DataOutputStream; import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; -import java.io.OutputStream; import java.io.PrintStream; import java.net.InetSocketAddress; import java.util.ArrayList; import java.util.Collections; import java.util.EnumSet; -import java.util.HashSet; import java.util.List; -import java.util.Map; -import java.util.Set; import java.util.regex.Pattern; import org.apache.commons.lang.math.RandomUtils; @@ -50,7 +45,6 @@ import org.apache.hadoop.io.IOUtils; import org.apache.hadoop.util.Shell; import org.apache.hadoop.util.Shell.ShellCommandExecutor; import org.apache.hadoop.util.StringUtils; -import org.apache.hadoop.yarn.api.ApplicationConstants; import org.apache.hadoop.yarn.api.records.ContainerId; import org.apache.hadoop.yarn.conf.YarnConfiguration; import org.apache.hadoop.yarn.factory.providers.RecordFactoryProvider; @@ -331,77 +325,6 @@ public class DockerContainerExecutor extends ContainerExecutor { return 0; } - /** - * Filter the environment variables that may conflict with the ones set in - * the docker image and write them out to an OutputStream. - * @throws IOException if there's an issue writing out the launch file. - */ - @Override - public void writeLaunchEnv(OutputStream out, Map environment, - Map> resources, List command, Path logDir, - String user) throws IOException { - ContainerLaunch.ShellScriptBuilder sb = - ContainerLaunch.ShellScriptBuilder.create(); - - //Remove environments that may conflict with the ones in Docker image. - Set exclusionSet = new HashSet<>(); - exclusionSet.add(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME); - exclusionSet.add(ApplicationConstants.Environment.HADOOP_YARN_HOME.name()); - exclusionSet.add(ApplicationConstants.Environment.HADOOP_COMMON_HOME.name()); - exclusionSet.add(ApplicationConstants.Environment.HADOOP_HDFS_HOME.name()); - exclusionSet.add(ApplicationConstants.Environment.HADOOP_CONF_DIR.name()); - exclusionSet.add(ApplicationConstants.Environment.JAVA_HOME.name()); - - if (environment != null) { - for (Map.Entry env : environment.entrySet()) { - if (!exclusionSet.contains(env.getKey())) { - sb.env(env.getKey(), env.getValue()); - } - } - } - if (resources != null) { - for (Map.Entry> entry : resources.entrySet()) { - for (String linkName : entry.getValue()) { - if (new Path(linkName).getName().equals(WILDCARD)) { - // If this is a wildcarded path, link to everything in the - // directory from the working directory - for (File wildLink : readDirAsUser(user, entry.getKey())) { - sb.symlink(new Path(wildLink.toString()), - new Path(wildLink.getName())); - } - } else { - sb.symlink(entry.getKey(), new Path(linkName)); - } - } - } - } - - // dump debugging information if configured - if (getConf() != null && getConf().getBoolean( - YarnConfiguration.NM_LOG_CONTAINER_DEBUG_INFO, - YarnConfiguration.DEFAULT_NM_LOG_CONTAINER_DEBUG_INFO)) { - sb.copyDebugInformation(new Path(ContainerLaunch.CONTAINER_SCRIPT), - new Path(logDir, ContainerLaunch.CONTAINER_SCRIPT)); - sb.listDebugInformation(new Path(logDir, DIRECTORY_CONTENTS)); - } - - sb.command(command); - - try (PrintStream pout = new PrintStream(out, false, "UTF-8")) { - sb.write(pout); - } - - if (LOG.isDebugEnabled()) { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - - try (PrintStream ps = new PrintStream(baos, false, "UTF-8")) { - sb.write(ps); - } - - LOG.debug("Script: " + baos.toString("UTF-8")); - } - } - private boolean saneDockerImage(String containerImageName) { return dockerImagePattern.matcher(containerImageName).matches(); } 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 36dca6d1526..d8b9263feb2 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 @@ -448,13 +448,6 @@ public class LinuxContainerExecutor extends ContainerExecutor { tokenFileName, localDirs, super.getConf()); } - @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 465ac82c076..41e72f4c7a3 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 @@ -38,6 +38,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; +import java.util.LinkedHashSet; import java.util.Map; import java.util.Map.Entry; import java.util.Set; @@ -214,6 +215,9 @@ public class ContainerLaunch implements Callable { } // /////////////////////////// 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( @@ -261,7 +265,7 @@ public class ContainerLaunch implements Callable { appDirs.add(new Path(appsdir, appIdStr)); } // Set the token location too. - environment.put( + addToEnvMap(environment, nmEnvVars, ApplicationConstants.CONTAINER_TOKEN_FILE_ENV_NAME, new Path(containerWorkDir, FINAL_CONTAINER_TOKENS_FILE).toUri().getPath()); @@ -272,13 +276,14 @@ public class ContainerLaunch implements Callable { EnumSet.of(CREATE, OVERWRITE))) { // Sanitize the container's environment sanitizeEnv(environment, containerWorkDir, appDirs, userLocalDirs, - containerLogDirs, - localResources, nmPrivateClasspathJarDir); + containerLogDirs, localResources, nmPrivateClasspathJarDir, + nmEnvVars); + // Write out the environment exec.writeLaunchEnv(containerScriptOutStream, environment, localResources, launchContext.getCommands(), - containerLogDir, user); + containerLogDir, user, nmEnvVars); } // /////////// End of writing out container-script @@ -1038,6 +1043,9 @@ public class ContainerLaunch implements Callable { 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 { @@ -1223,6 +1231,11 @@ public class ContainerLaunch implements Callable { 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 + "\""); @@ -1386,6 +1399,11 @@ public class ContainerLaunch implements Callable { 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, "\""); @@ -1481,63 +1499,73 @@ public class ContainerLaunch implements Callable { 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) throws IOException { + containerLogDirs, Map> resources, + Path nmPrivateClasspathJarDir, + Set nmVars) throws IOException { /** * Non-modifiable environment variables */ - environment.put(Environment.CONTAINER_ID.name(), container - .getContainerId().toString()); + addToEnvMap(environment, nmVars, Environment.CONTAINER_ID.name(), + container.getContainerId().toString()); - environment.put(Environment.NM_PORT.name(), + addToEnvMap(environment, nmVars, Environment.NM_PORT.name(), String.valueOf(this.context.getNodeId().getPort())); - environment.put(Environment.NM_HOST.name(), this.context.getNodeId() - .getHost()); + addToEnvMap(environment, nmVars, Environment.NM_HOST.name(), + this.context.getNodeId().getHost()); - environment.put(Environment.NM_HTTP_PORT.name(), + addToEnvMap(environment, nmVars, Environment.NM_HTTP_PORT.name(), String.valueOf(this.context.getHttpPort())); - environment.put(Environment.LOCAL_DIRS.name(), + addToEnvMap(environment, nmVars, Environment.LOCAL_DIRS.name(), StringUtils.join(",", appDirs)); - environment.put(Environment.LOCAL_USER_DIRS.name(), StringUtils.join(",", - userLocalDirs)); + addToEnvMap(environment, nmVars, Environment.LOCAL_USER_DIRS.name(), + StringUtils.join(",", userLocalDirs)); - environment.put(Environment.LOG_DIRS.name(), + addToEnvMap(environment, nmVars, Environment.LOG_DIRS.name(), StringUtils.join(",", containerLogDirs)); - environment.put(Environment.USER.name(), container.getUser()); - - environment.put(Environment.LOGNAME.name(), container.getUser()); + addToEnvMap(environment, nmVars, Environment.USER.name(), + container.getUser()); - environment.put(Environment.HOME.name(), + addToEnvMap(environment, nmVars, Environment.LOGNAME.name(), + container.getUser()); + + addToEnvMap(environment, nmVars, Environment.HOME.name(), conf.get( YarnConfiguration.NM_USER_HOME_DIR, YarnConfiguration.DEFAULT_NM_USER_HOME_DIR ) ); - - environment.put(Environment.PWD.name(), pwd.toString()); - - putEnvIfAbsent(environment, Environment.HADOOP_CONF_DIR.name()); - environment.put(Environment.LOCALIZATION_COUNTERS.name(), + addToEnvMap(environment, nmVars, Environment.PWD.name(), pwd.toString()); + + addToEnvMap(environment, nmVars, Environment.LOCALIZATION_COUNTERS.name(), container.localizationCountersAsString()); if (!Shell.WINDOWS) { - environment.put("JVM_PID", "$$"); + addToEnvMap(environment, nmVars, "JVM_PID", "$$"); } // variables here will be forced in, even if the container has specified them. - Apps.setEnvFromInputString(environment, conf.get( - YarnConfiguration.NM_ADMIN_USER_ENV, - YarnConfiguration.DEFAULT_NM_ADMIN_USER_ENV), File.pathSeparator); + 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)); // TODO: Remove Windows check and use this approach on all platforms after // additional testing. See YARN-358. @@ -1640,6 +1668,7 @@ public class ContainerLaunch implements Callable { .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 e9c58b83470..6c3ae853aec 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 @@ -36,7 +36,6 @@ import org.slf4j.Logger; 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,11 +71,6 @@ public class DefaultLinuxContainerRuntime implements LinuxContainerRuntime { 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 be973a590a3..f1f4451beec 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 @@ -84,17 +84,6 @@ public class DelegatingLinuxContainerRuntime implements LinuxContainerRuntime { } } - @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 5e3e15ca78c..30a1e0552ec 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 @@ -307,13 +307,6 @@ public class DockerLinuxContainerRuntime implements LinuxContainerRuntime { 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; - } - @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/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 aa294fc57c2..7caa0edf4de 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,8 +24,6 @@ import org.apache.hadoop.classification.InterfaceAudience; 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 @@ -85,13 +83,4 @@ public interface ContainerRuntime { * 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 c7914c5c172..8f10153a6b3 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 @@ -48,6 +48,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -194,9 +195,10 @@ public class TestContainerLaunch extends BaseContainerManagerTest { DefaultContainerExecutor executor = new DefaultContainerExecutor(); executor.setConf(conf); + LinkedHashSet nmVars = new LinkedHashSet<>(); executor.writeLaunchEnv(fos, env, resources, commands, new Path(localLogDir.getAbsolutePath()), "user", - tempFile.getName()); + tempFile.getName(), nmVars); fos.flush(); fos.close(); FileUtil.setExecutable(tempFile, true); @@ -269,8 +271,9 @@ public class TestContainerLaunch extends BaseContainerManagerTest { } DefaultContainerExecutor executor = new DefaultContainerExecutor(); executor.setConf(conf); + LinkedHashSet nmVars = new LinkedHashSet<>(); executor.writeLaunchEnv(fos, env, resources, commands, - new Path(localLogDir.getAbsolutePath()), "user"); + new Path(localLogDir.getAbsolutePath()), "user", nmVars); fos.flush(); fos.close(); FileUtil.setExecutable(tempFile, true); @@ -332,8 +335,9 @@ public class TestContainerLaunch extends BaseContainerManagerTest { 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"); + new Path(localLogDir.getAbsolutePath()), "user", nmVars); String shellContent = new String(Files.readAllBytes(Paths.get(shellFile.getAbsolutePath())), StandardCharsets.UTF_8); @@ -346,7 +350,8 @@ public class TestContainerLaunch extends BaseContainerManagerTest { Assert.assertFalse(shellContent.contains("HADOOP_HDFS_HOME")); // Available in env and in whitelist Assert.assertTrue(shellContent.contains( - "export HADOOP_YARN_HOME=\"nodemanager_yarn_home\"")); + "export HADOOP_YARN_HOME=${HADOOP_YARN_HOME:-\"nodemanager_yarn_home\"}" + )); fos.flush(); fos.close(); } @@ -381,8 +386,9 @@ public class TestContainerLaunch extends BaseContainerManagerTest { 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"); + new Path(localLogDir.getAbsolutePath()), "user", nmVars); String shellContent = new String(Files.readAllBytes(Paths.get(shellFile.getAbsolutePath())), StandardCharsets.UTF_8); @@ -391,13 +397,106 @@ public class TestContainerLaunch extends BaseContainerManagerTest { // Whitelisted variable overridden by container Assert.assertTrue(shellContent.contains( "export HADOOP_MAPRED_HOME=\"/opt/hadoopbuild\"")); - // Verify no whitelisted variables inherited from NM env + // Available in env but not in whitelist Assert.assertFalse(shellContent.contains("HADOOP_HDFS_HOME")); - Assert.assertFalse(shellContent.contains("HADOOP_YARN_HOME")); + // Available in env and in whitelist + Assert.assertTrue(shellContent.contains( + "export HADOOP_YARN_HOME=${HADOOP_YARN_HOME:-\"nodemanager_yarn_home\"}" + )); fos.flush(); fos.close(); } + @Test(timeout = 20000) + public void testWriteEnvOrder() throws Exception { + // Valid only for unix + assumeFalse(Shell.WINDOWS); + 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 { @@ -418,8 +517,9 @@ public class TestContainerLaunch extends BaseContainerManagerTest { List commands = new ArrayList(); DefaultContainerExecutor executor = new DefaultContainerExecutor(); executor.setConf(conf); + LinkedHashSet nmVars = new LinkedHashSet<>(); executor.writeLaunchEnv(fos, env, resources, commands, - new Path(localLogDir.getAbsolutePath()), "user"); + new Path(localLogDir.getAbsolutePath()), "user", nmVars); fos.flush(); fos.close(); @@ -501,8 +601,9 @@ public class TestContainerLaunch extends BaseContainerManagerTest { commands.add(command); ContainerExecutor exec = new DefaultContainerExecutor(); exec.setConf(conf); + LinkedHashSet nmVars = new LinkedHashSet<>(); exec.writeLaunchEnv(fos, env, resources, commands, - new Path(localLogDir.getAbsolutePath()), "user"); + new Path(localLogDir.getAbsolutePath()), "user", nmVars); fos.flush(); fos.close(); @@ -593,7 +694,7 @@ public class TestContainerLaunch extends BaseContainerManagerTest { Path nmp = new Path(testDir); launch.sanitizeEnv(userSetEnv, pwd, appDirs, userLocalDirs, containerLogs, - resources, nmp); + resources, nmp, Collections.emptySet()); List result = getJarManifestClasspath(userSetEnv.get(Environment.CLASSPATH.name())); @@ -612,7 +713,7 @@ public class TestContainerLaunch extends BaseContainerManagerTest { dispatcher, exec, null, container, dirsHandler, containerManager); launch.sanitizeEnv(userSetEnv, pwd, appDirs, userLocalDirs, containerLogs, - resources, nmp); + resources, nmp, Collections.emptySet()); result = getJarManifestClasspath(userSetEnv.get(Environment.CLASSPATH.name())); @@ -1537,9 +1638,10 @@ public class TestContainerLaunch extends BaseContainerManagerTest { 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()); + tempFile.getName(), nmVars); fos.flush(); fos.close(); FileUtil.setExecutable(tempFile, true); @@ -1771,8 +1873,9 @@ public class TestContainerLaunch extends BaseContainerManagerTest { 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); + new Path(localLogDir.getAbsolutePath()), user, nmVars); fos.flush(); fos.close(); @@ -1814,8 +1917,9 @@ public class TestContainerLaunch extends BaseContainerManagerTest { 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); + new Path(localLogDir.getAbsolutePath()), user, nmVars); fos.flush(); fos.close(); @@ -2104,6 +2208,8 @@ public class TestContainerLaunch extends BaseContainerManagerTest { throws IOException {} @Override public void env(String key, String value) throws IOException {} + @Override public void whitelistedEnv(String key, String value) + throws IOException {} @Override public void copyDebugInformation(Path src, Path dst) throws IOException {} @Override public void command(List command)