From dcc454b4a05b3b7f3276143cf7693729c3256d5f Mon Sep 17 00:00:00 2001 From: Sidharta S Date: Mon, 2 Oct 2017 19:04:49 -0700 Subject: [PATCH] YARN-7226. Whitelisted variables do not support delayed variable expansion. Contributed by Jason Lowe (cherry picked from commit 7eb8499996869cdb63743f1c9eca2ba91d57ad08) --- .../server/nodemanager/ContainerExecutor.java | 44 +++++++---- .../nodemanager/LinuxContainerExecutor.java | 8 ++ .../launcher/ContainerLaunch.java | 25 ------- .../runtime/DefaultLinuxContainerRuntime.java | 6 ++ .../DelegatingLinuxContainerRuntime.java | 11 +++ .../runtime/DockerLinuxContainerRuntime.java | 7 ++ .../runtime/ContainerRuntime.java | 11 +++ .../launcher/TestContainerLaunch.java | 73 +++++++++++++++++-- 8 files changed, 141 insertions(+), 44 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/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 5fd059d50a3..0b4dbc9778a 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 @@ -26,10 +26,8 @@ import java.net.InetAddress; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -95,10 +93,15 @@ 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; @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(","); + } } @Override @@ -331,6 +334,8 @@ public abstract class ContainerExecutor implements Configurable { public void writeLaunchEnv(OutputStream out, Map environment, Map> resources, List command, Path logDir, String user, String outFilename) throws IOException { + updateEnvForWhitelistVars(environment); + ContainerLaunch.ShellScriptBuilder sb = ContainerLaunch.ShellScriptBuilder.create(); @@ -341,22 +346,11 @@ public abstract class ContainerExecutor implements Configurable { sb.stdout(logDir, CONTAINER_PRE_LAUNCH_STDOUT); sb.stderr(logDir, CONTAINER_PRE_LAUNCH_STDERR); - Set whitelist = new HashSet<>(); - - String[] nmWhiteList = conf.get(YarnConfiguration.NM_ENV_WHITELIST, - YarnConfiguration.DEFAULT_NM_ENV_WHITELIST).split(","); - for (String param : nmWhiteList) { - whitelist.add(param); - } if (environment != null) { sb.echo("Setting up env variables"); for (Map.Entry env : environment.entrySet()) { - if (!whitelist.contains(env.getKey())) { - sb.env(env.getKey(), env.getValue()); - } else { - sb.whitelistedEnv(env.getKey(), env.getValue()); - } + sb.env(env.getKey(), env.getValue()); } } @@ -657,6 +651,28 @@ 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); + } + /** * Mark the container as active. * 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 9e004e7721d..9f1ec609a8c 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 @@ -62,6 +62,7 @@ import java.net.InetSocketAddress; 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.*; @@ -441,6 +442,13 @@ public class LinuxContainerExecutor extends ContainerExecutor { } } + @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 f929dfc5910..f63b1b8adbf 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 @@ -89,7 +89,6 @@ import org.apache.hadoop.yarn.util.AuxiliaryServiceHelper; import com.google.common.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.apache.hadoop.yarn.util.ConverterUtils; public class ContainerLaunch implements Callable { @@ -1022,8 +1021,6 @@ public class ContainerLaunch implements Callable { public abstract void command(List command) throws IOException; - public abstract void whitelistedEnv(String key, String value) throws IOException; - protected static final String ENV_PRELAUNCH_STDOUT = "PRELAUNCH_OUT"; protected static final String ENV_PRELAUNCH_STDERR = "PRELAUNCH_ERR"; @@ -1162,11 +1159,6 @@ public class ContainerLaunch implements Callable { line("exec /bin/bash -c \"", StringUtils.join(" ", command), "\""); } - @Override - public void whitelistedEnv(String key, String value) throws IOException { - line("export ", key, "=${", key, ":-", "\"", value, "\"}"); - } - @Override public void setStdOut(final Path stdout) throws IOException { line("export ", ENV_PRELAUNCH_STDOUT, "=\"", stdout.toString(), "\""); @@ -1262,12 +1254,6 @@ public class ContainerLaunch implements Callable { errorCheck(); } - @Override - public void whitelistedEnv(String key, String value) throws IOException { - lineWithLenCheck("@set ", key, "=", value); - errorCheck(); - } - //Dummy implementation @Override protected void setStdOut(final Path stdout) throws IOException { @@ -1389,17 +1375,6 @@ public class ContainerLaunch implements Callable { environment.put("JVM_PID", "$$"); } - /** - * Modifiable environment variables - */ - - // allow containers to override these variables - String[] whitelist = conf.get(YarnConfiguration.NM_ENV_WHITELIST, YarnConfiguration.DEFAULT_NM_ENV_WHITELIST).split(","); - - for(String whitelistEnvVariable : whitelist) { - putEnvIfAbsent(environment, whitelistEnvVariable.trim()); - } - // variables here will be forced in, even if the container has specified them. Apps.setEnvFromInputString(environment, conf.get( YarnConfiguration.NM_ADMIN_USER_ENV, 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 6c3ae853aec..e9c58b83470 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,6 +36,7 @@ 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.*; @@ -71,6 +72,11 @@ 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 9fe4927b6e3..517a4e2bad4 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 @@ -93,6 +93,17 @@ 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 29c6fe99894..20133062c4e 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 @@ -281,6 +281,13 @@ public class DockerLinuxContainerRuntime implements LinuxContainerRuntime { YarnConfiguration.DEFAULT_NM_DOCKER_USER_REMAPPING_GID_THRESHOLD); } + @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 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.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 @@ -83,4 +85,13 @@ 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 588fdcda242..1ee6eb6544e 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 @@ -91,6 +91,7 @@ import org.apache.hadoop.yarn.server.api.protocolrecords.NMContainerStatus; import org.apache.hadoop.yarn.server.nodemanager.ContainerExecutor; import org.apache.hadoop.yarn.server.nodemanager.ContainerExecutor.ExitCode; import org.apache.hadoop.yarn.server.nodemanager.DefaultContainerExecutor; +import org.apache.hadoop.yarn.server.nodemanager.LinuxContainerExecutor; import org.apache.hadoop.yarn.server.nodemanager.NodeManager.NMContext; import org.apache.hadoop.yarn.server.nodemanager.NodeStatusUpdater; import org.apache.hadoop.yarn.server.nodemanager.containermanager.BaseContainerManagerTest; @@ -99,6 +100,8 @@ import org.apache.hadoop.yarn.server.nodemanager.containermanager.container.Cont import org.apache.hadoop.yarn.server.nodemanager.containermanager.container.ContainerEventType; import org.apache.hadoop.yarn.server.nodemanager.containermanager.container.ContainerExitEvent; import org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher.ContainerLaunch.ShellScriptBuilder; +import org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.privileged.PrivilegedOperationExecutor; +import org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.runtime.DockerLinuxContainerRuntime; import org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.ContainerLocalizer; import org.apache.hadoop.yarn.server.nodemanager.recovery.NMNullStateStoreService; import org.apache.hadoop.yarn.server.nodemanager.security.NMContainerTokenSecretManager; @@ -304,8 +307,18 @@ public class TestContainerLaunch extends BaseContainerManagerTest { Map> resources = new HashMap>(); FileOutputStream fos = new FileOutputStream(shellFile); List commands = new ArrayList(); + final Map nmEnv = new HashMap<>(); + nmEnv.put("HADOOP_COMMON_HOME", "nodemanager_common_home"); + nmEnv.put("HADOOP_HDFS_HOME", "nodemanager_hdfs_home"); + nmEnv.put("HADOOP_YARN_HOME", "nodemanager_yarn_home"); + nmEnv.put("HADOOP_MAPRED_HOME", "nodemanager_mapred_home"); DefaultContainerExecutor defaultContainerExecutor = - new DefaultContainerExecutor(); + new DefaultContainerExecutor() { + @Override + protected String getNMEnvVar(String varname) { + return nmEnv.get(varname); + } + }; YarnConfiguration conf = new YarnConfiguration(); conf.set(YarnConfiguration.NM_ENV_WHITELIST, "HADOOP_MAPRED_HOME,HADOOP_YARN_HOME"); @@ -317,10 +330,60 @@ public class TestContainerLaunch extends BaseContainerManagerTest { StandardCharsets.UTF_8); Assert.assertTrue(shellContent .contains("export HADOOP_COMMON_HOME=\"/opt/hadoopcommon\"")); - // Not available in env and whitelist - Assert.assertTrue(shellContent.contains("export HADOOP_MAPRED_HOME=" - + "${HADOOP_MAPRED_HOME:-\"/opt/hadoopbuild\"}")); - // Not available in env but in whitelist + // Whitelisted variable overridden by container + Assert.assertTrue(shellContent.contains( + "export HADOOP_MAPRED_HOME=\"/opt/hadoopbuild\"")); + // Available in env but not in whitelist + Assert.assertFalse(shellContent.contains("HADOOP_HDFS_HOME")); + // Available in env and in whitelist + Assert.assertTrue(shellContent.contains( + "export HADOOP_YARN_HOME=\"nodemanager_yarn_home\"")); + fos.flush(); + fos.close(); + } + + @Test(timeout = 20000) + public void testWriteEnvExportDocker() throws Exception { + // Valid only for unix + assumeNotWindows(); + File shellFile = Shell.appendScriptExtension(tmpDir, "hello"); + Map env = new HashMap(); + env.put("HADOOP_COMMON_HOME", "/opt/hadoopcommon"); + env.put("HADOOP_MAPRED_HOME", "/opt/hadoopbuild"); + Map> resources = new HashMap>(); + FileOutputStream fos = new FileOutputStream(shellFile); + List commands = new ArrayList(); + final Map nmEnv = new HashMap<>(); + nmEnv.put("HADOOP_COMMON_HOME", "nodemanager_common_home"); + nmEnv.put("HADOOP_HDFS_HOME", "nodemanager_hdfs_home"); + nmEnv.put("HADOOP_YARN_HOME", "nodemanager_yarn_home"); + nmEnv.put("HADOOP_MAPRED_HOME", "nodemanager_mapred_home"); + DockerLinuxContainerRuntime dockerRuntime = + new DockerLinuxContainerRuntime( + mock(PrivilegedOperationExecutor.class)); + LinuxContainerExecutor lce = + new LinuxContainerExecutor(dockerRuntime) { + @Override + protected String getNMEnvVar(String varname) { + return nmEnv.get(varname); + } + }; + YarnConfiguration conf = new YarnConfiguration(); + conf.set(YarnConfiguration.NM_ENV_WHITELIST, + "HADOOP_MAPRED_HOME,HADOOP_YARN_HOME"); + lce.setConf(conf); + lce.writeLaunchEnv(fos, env, resources, commands, + new Path(localLogDir.getAbsolutePath()), "user"); + String shellContent = + new String(Files.readAllBytes(Paths.get(shellFile.getAbsolutePath())), + StandardCharsets.UTF_8); + Assert.assertTrue(shellContent + .contains("export HADOOP_COMMON_HOME=\"/opt/hadoopcommon\"")); + // Whitelisted variable overridden by container + Assert.assertTrue(shellContent.contains( + "export HADOOP_MAPRED_HOME=\"/opt/hadoopbuild\"")); + // Verify no whitelisted variables inherited from NM env + Assert.assertFalse(shellContent.contains("HADOOP_HDFS_HOME")); Assert.assertFalse(shellContent.contains("HADOOP_YARN_HOME")); fos.flush(); fos.close();