From e0ccea33c9e12f6930b2867e14b1b37569fed659 Mon Sep 17 00:00:00 2001 From: Ravi Prakash Date: Sat, 28 Mar 2015 08:00:41 -0700 Subject: [PATCH] YARN-3288. Document and fix indentation in the DockerContainerExecutor code --- hadoop-yarn-project/CHANGES.txt | 2 + .../server/nodemanager/ContainerExecutor.java | 18 +- .../nodemanager/DockerContainerExecutor.java | 227 +++++++++++------- .../launcher/ContainerLaunch.java | 8 +- .../TestDockerContainerExecutor.java | 98 ++++---- .../TestDockerContainerExecutorWithMocks.java | 110 +++++---- 6 files changed, 276 insertions(+), 187 deletions(-) diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index d6ded778af..fb233e34a2 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -81,6 +81,8 @@ Release 2.8.0 - UNRELEASED YARN-3397. yarn rmadmin should skip -failover. (J.Andreina via kasha) + YARN-3288. Document and fix indentation in the DockerContainerExecutor code + OPTIMIZATIONS YARN-3339. TestDockerContainerExecutor should pull a single image and not 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 377fd1d2d1..1c670a16ae 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 @@ -210,8 +210,22 @@ public int reacquireContainer(String user, ContainerId containerId) } } - public void writeLaunchEnv(OutputStream out, Map environment, Map> resources, List command) throws IOException{ - ContainerLaunch.ShellScriptBuilder sb = ContainerLaunch.ShellScriptBuilder.create(); + /** + * This method writes out the launch environment of a container. This can be + * overridden by extending ContainerExecutors to provide different behaviors + * @param out the output stream to which the environment is written (usually + * a script file which will be executed by the Launcher) + * @param environment The environment variables and their values + * @param resources The resources which have been localized for this container + * Symlinks will be created to these localized resources + * @param command The command that will be run. + * @throws IOException if any errors happened writing to the OutputStream, + * while creating symlinks + */ + public void writeLaunchEnv(OutputStream out, Map environment, + Map> resources, List command) throws IOException{ + ContainerLaunch.ShellScriptBuilder sb = + ContainerLaunch.ShellScriptBuilder.create(); if (environment != null) { for (Map.Entry env : environment.entrySet()) { sb.env(env.getKey().toString(), env.getValue().toString()); 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 c854173fad..71eaa04b3c 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 @@ -18,10 +18,24 @@ package org.apache.hadoop.yarn.server.nodemanager; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Joiner; -import com.google.common.base.Preconditions; -import com.google.common.base.Strings; +import static org.apache.hadoop.fs.CreateFlag.CREATE; +import static org.apache.hadoop.fs.CreateFlag.OVERWRITE; + +import java.io.ByteArrayOutputStream; +import java.io.DataOutputStream; +import java.io.File; +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; import org.apache.commons.logging.Log; @@ -45,38 +59,35 @@ import org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.ContainerLocalizer; import org.apache.hadoop.yarn.util.ConverterUtils; -import java.io.ByteArrayOutputStream; -import java.io.DataOutputStream; -import java.io.File; -import java.io.IOException; -import java.io.OutputStream; -import java.io.PrintStream; -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.Random; -import java.util.Set; -import java.util.regex.Pattern; -import java.net.InetSocketAddress; -import static org.apache.hadoop.fs.CreateFlag.CREATE; -import static org.apache.hadoop.fs.CreateFlag.OVERWRITE; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Joiner; +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; /** - * This executor will launch a docker container and run the task inside the container. + * This executor will launch and run tasks inside Docker containers. It + * currently only supports simple authentication mode. It shares a lot of code + * with the DefaultContainerExecutor (and it may make sense to pull out those + * common pieces later). */ public class DockerContainerExecutor extends ContainerExecutor { - private static final Log LOG = LogFactory - .getLog(DockerContainerExecutor.class); - public static final String DOCKER_CONTAINER_EXECUTOR_SCRIPT = "docker_container_executor"; - public static final String DOCKER_CONTAINER_EXECUTOR_SESSION_SCRIPT = "docker_container_executor_session"; - - // This validates that the image is a proper docker image and would not crash docker. - public static final String DOCKER_IMAGE_PATTERN = "^(([\\w\\.-]+)(:\\d+)*\\/)?[\\w\\.:-]+$"; + .getLog(DockerContainerExecutor.class); + //The name of the script file that will launch the Docker containers + public static final String DOCKER_CONTAINER_EXECUTOR_SCRIPT = + "docker_container_executor"; + //The name of the session script that the DOCKER_CONTAINER_EXECUTOR_SCRIPT + //launches in turn + public static final String DOCKER_CONTAINER_EXECUTOR_SESSION_SCRIPT = + "docker_container_executor_session"; + //This validates that the image is a proper docker image and would not crash + //docker. The image name is not allowed to contain spaces. e.g. + //registry.somecompany.com:9999/containername:0.1 or + //containername:0.1 or + //containername + public static final String DOCKER_IMAGE_PATTERN = + "^(([\\w\\.-]+)(:\\d+)*\\/)?[\\w\\.:-]+$"; private final FileContext lfs; private final Pattern dockerImagePattern; @@ -96,23 +107,26 @@ protected void copyFile(Path src, Path dst, String owner) throws IOException { @Override public void init() throws IOException { - String auth = getConf().get(CommonConfigurationKeys.HADOOP_SECURITY_AUTHENTICATION); + String auth = + getConf().get(CommonConfigurationKeys.HADOOP_SECURITY_AUTHENTICATION); if (auth != null && !auth.equals("simple")) { - throw new IllegalStateException("DockerContainerExecutor only works with simple authentication mode"); + throw new IllegalStateException( + "DockerContainerExecutor only works with simple authentication mode"); } - String dockerExecutor = getConf().get(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_EXEC_NAME, + String dockerExecutor = getConf().get( + YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_EXEC_NAME, YarnConfiguration.NM_DEFAULT_DOCKER_CONTAINER_EXECUTOR_EXEC_NAME); if (!new File(dockerExecutor).exists()) { - throw new IllegalStateException("Invalid docker exec path: " + dockerExecutor); + throw new IllegalStateException( + "Invalid docker exec path: " + dockerExecutor); } } @Override public synchronized void startLocalizer(Path nmPrivateContainerTokensPath, - InetSocketAddress nmAddr, String user, String appId, String locId, - LocalDirsHandlerService dirsHandler) + InetSocketAddress nmAddr, String user, String appId, String locId, + LocalDirsHandlerService dirsHandler) throws IOException, InterruptedException { - List localDirs = dirsHandler.getLocalDirs(); List logDirs = dirsHandler.getLogDirs(); @@ -128,7 +142,8 @@ public synchronized void startLocalizer(Path nmPrivateContainerTokensPath, // randomly choose the local directory Path appStorageDir = getWorkingDir(localDirs, user, appId); - String tokenFn = String.format(ContainerLocalizer.TOKEN_FILE_NAME_FMT, locId); + String tokenFn = + String.format(ContainerLocalizer.TOKEN_FILE_NAME_FMT, locId); Path tokenDst = new Path(appStorageDir, tokenFn); copyFile(nmPrivateContainerTokensPath, tokenDst, user); LOG.info("Copying from " + nmPrivateContainerTokensPath + " to " + tokenDst); @@ -140,31 +155,34 @@ public synchronized void startLocalizer(Path nmPrivateContainerTokensPath, @Override - public int launchContainer(Container container, - Path nmPrivateContainerScriptPath, Path nmPrivateTokensPath, - String userName, String appId, Path containerWorkDir, - List localDirs, List logDirs) throws IOException { + public int launchContainer(Container container, Path + nmPrivateContainerScriptPath, Path nmPrivateTokensPath, String userName, + String appId, Path containerWorkDir, List localDirs, List + logDirs) throws IOException { + //Variables for the launch environment can be injected from the command-line + //while submitting the application String containerImageName = container.getLaunchContext().getEnvironment() - .get(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME); + .get(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME); if (LOG.isDebugEnabled()) { LOG.debug("containerImageName from launchContext: " + containerImageName); } - Preconditions.checkArgument(!Strings.isNullOrEmpty(containerImageName), "Container image must not be null"); + Preconditions.checkArgument(!Strings.isNullOrEmpty(containerImageName), + "Container image must not be null"); containerImageName = containerImageName.replaceAll("['\"]", ""); - Preconditions.checkArgument(saneDockerImage(containerImageName), "Image: " + containerImageName + " is not a proper docker image"); - String dockerExecutor = getConf().get(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_EXEC_NAME, - YarnConfiguration.NM_DEFAULT_DOCKER_CONTAINER_EXECUTOR_EXEC_NAME); + Preconditions.checkArgument(saneDockerImage(containerImageName), "Image: " + + containerImageName + " is not a proper docker image"); + String dockerExecutor = getConf().get( + YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_EXEC_NAME, + YarnConfiguration.NM_DEFAULT_DOCKER_CONTAINER_EXECUTOR_EXEC_NAME); FsPermission dirPerm = new FsPermission(APPDIR_PERM); ContainerId containerId = container.getContainerId(); // create container dirs on all disks String containerIdStr = ConverterUtils.toString(containerId); - String appIdStr = - ConverterUtils.toString( - containerId.getApplicationAttemptId(). - getApplicationId()); + String appIdStr = ConverterUtils.toString( + containerId.getApplicationAttemptId().getApplicationId()); for (String sLocalDir : localDirs) { Path usersdir = new Path(sLocalDir, ContainerLocalizer.USERCACHE); Path userdir = new Path(usersdir, userName); @@ -178,46 +196,57 @@ public int launchContainer(Container container, createContainerLogDirs(appIdStr, containerIdStr, logDirs, userName); Path tmpDir = new Path(containerWorkDir, - YarnConfiguration.DEFAULT_CONTAINER_TEMP_DIR); + YarnConfiguration.DEFAULT_CONTAINER_TEMP_DIR); createDir(tmpDir, dirPerm, false, userName); // copy launch script to work dir Path launchDst = - new Path(containerWorkDir, ContainerLaunch.CONTAINER_SCRIPT); + new Path(containerWorkDir, ContainerLaunch.CONTAINER_SCRIPT); lfs.util().copy(nmPrivateContainerScriptPath, launchDst); // copy container tokens to work dir Path tokenDst = - new Path(containerWorkDir, ContainerLaunch.FINAL_CONTAINER_TOKENS_FILE); + new Path(containerWorkDir, ContainerLaunch.FINAL_CONTAINER_TOKENS_FILE); lfs.util().copy(nmPrivateTokensPath, tokenDst); - - String localDirMount = toMount(localDirs); String logDirMount = toMount(logDirs); - String containerWorkDirMount = toMount(Collections.singletonList(containerWorkDir.toUri().getPath())); + String containerWorkDirMount = toMount(Collections.singletonList( + containerWorkDir.toUri().getPath())); StringBuilder commands = new StringBuilder(); + //Use docker run to launch the docker container. See man pages for + //docker-run + //--rm removes the container automatically once the container finishes + //--net=host allows the container to take on the host's network stack + //--name sets the Docker Container name to the YARN containerId string + //-v is used to bind mount volumes for local, log and work dirs. String commandStr = commands.append(dockerExecutor) - .append(" ") - .append("run") - .append(" ") - .append("--rm --net=host") - .append(" ") - .append(" --name " + containerIdStr) - .append(localDirMount) - .append(logDirMount) - .append(containerWorkDirMount) - .append(" ") - .append(containerImageName) - .toString(); - String dockerPidScript = "`" + dockerExecutor + " inspect --format {{.State.Pid}} " + containerIdStr + "`"; + .append(" ") + .append("run") + .append(" ") + .append("--rm --net=host") + .append(" ") + .append(" --name " + containerIdStr) + .append(localDirMount) + .append(logDirMount) + .append(containerWorkDirMount) + .append(" ") + .append(containerImageName) + .toString(); + //Get the pid of the process which has been launched as a docker container + //using docker inspect + String dockerPidScript = "`" + dockerExecutor + + " inspect --format {{.State.Pid}} " + containerIdStr + "`"; + // Create new local launch wrapper script - LocalWrapperScriptBuilder sb = - new UnixLocalWrapperScriptBuilder(containerWorkDir, commandStr, dockerPidScript); + LocalWrapperScriptBuilder sb = new UnixLocalWrapperScriptBuilder( + containerWorkDir, commandStr, dockerPidScript); Path pidFile = getPidFilePath(containerId); if (pidFile != null) { sb.writeLocalWrapperScript(launchDst, pidFile); } else { + //Although the container was activated by ContainerLaunch before exec() + //was called, since then deactivateContainer() has been called. LOG.info("Container " + containerIdStr + " was marked as inactive. Returning terminated error"); return ExitCode.TERMINATED.getExitCode(); @@ -234,12 +263,13 @@ public int launchContainer(Container container, String[] command = getRunCommand(sb.getWrapperScriptPath().toString(), containerIdStr, userName, pidFile, this.getConf()); if (LOG.isDebugEnabled()) { - LOG.debug("launchContainer: " + commandStr + " " + Joiner.on(" ").join(command)); + LOG.debug("launchContainer: " + commandStr + " " + + Joiner.on(" ").join(command)); } shExec = new ShellCommandExecutor( - command, - new File(containerWorkDir.toUri().getPath()), - container.getLaunchContext().getEnvironment()); // sanitized env + command, + new File(containerWorkDir.toUri().getPath()), + container.getLaunchContext().getEnvironment()); // sanitized env if (isContainerActive(containerId)) { shExec.execute(); } else { @@ -279,9 +309,17 @@ public int launchContainer(Container container, } @Override - public void writeLaunchEnv(OutputStream out, Map environment, Map> resources, List command) throws IOException { - ContainerLaunch.ShellScriptBuilder sb = ContainerLaunch.ShellScriptBuilder.create(); + /** + * Filter the environment variables that may conflict with the ones set in + * the docker image and write them out to an OutputStream. + */ + public void writeLaunchEnv(OutputStream out, Map environment, + Map> resources, List command) + 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()); @@ -427,6 +465,9 @@ private String toMount(List dirs) { return builder.toString(); } + //This class facilitates (only) the creation of platform-specific scripts that + //will be used to launch the containers + //TODO: This should be re-used from the DefaultContainerExecutor. private abstract class LocalWrapperScriptBuilder { private final Path wrapperScriptPath; @@ -435,7 +476,8 @@ public Path getWrapperScriptPath() { return wrapperScriptPath; } - public void writeLocalWrapperScript(Path launchDst, Path pidFile) throws IOException { + public void writeLocalWrapperScript(Path launchDst, Path pidFile) + throws IOException { DataOutputStream out = null; PrintStream pout = null; @@ -448,8 +490,8 @@ public void writeLocalWrapperScript(Path launchDst, Path pidFile) throws IOExcep } } - protected abstract void writeLocalWrapperScript(Path launchDst, Path pidFile, - PrintStream pout); + protected abstract void writeLocalWrapperScript(Path launchDst, + Path pidFile, PrintStream pout); protected LocalWrapperScriptBuilder(Path containerWorkDir) { this.wrapperScriptPath = new Path(containerWorkDir, @@ -457,13 +499,15 @@ protected LocalWrapperScriptBuilder(Path containerWorkDir) { } } + //TODO: This class too should be used from DefaultContainerExecutor. private final class UnixLocalWrapperScriptBuilder - extends LocalWrapperScriptBuilder { + extends LocalWrapperScriptBuilder { private final Path sessionScriptPath; private final String dockerCommand; private final String dockerPidScript; - public UnixLocalWrapperScriptBuilder(Path containerWorkDir, String dockerCommand, String dockerPidScript) { + public UnixLocalWrapperScriptBuilder(Path containerWorkDir, + String dockerCommand, String dockerPidScript) { super(containerWorkDir); this.dockerCommand = dockerCommand; this.dockerPidScript = dockerPidScript; @@ -480,8 +524,7 @@ public void writeLocalWrapperScript(Path launchDst, Path pidFile) @Override public void writeLocalWrapperScript(Path launchDst, Path pidFile, - PrintStream pout) { - + PrintStream pout) { String exitCodeFile = ContainerLaunch.getExitCodeFile( pidFile.toString()); String tmpFile = exitCodeFile + ".tmp"; @@ -505,7 +548,8 @@ private void writeSessionScript(Path launchDst, Path pidFile) // hence write pid to tmp file first followed by a mv pout.println("#!/usr/bin/env bash"); pout.println(); - pout.println("echo "+ dockerPidScript +" > " + pidFile.toString() + ".tmp"); + pout.println("echo "+ dockerPidScript +" > " + pidFile.toString() + + ".tmp"); pout.println("/bin/mv -f " + pidFile.toString() + ".tmp " + pidFile); pout.println(dockerCommand + " bash \"" + launchDst.toUri().getPath().toString() + "\""); @@ -518,7 +562,7 @@ private void writeSessionScript(Path launchDst, Path pidFile) } protected void createDir(Path dirPath, FsPermission perms, - boolean createParent, String user) throws IOException { + boolean createParent, String user) throws IOException { lfs.mkdir(dirPath, perms, createParent); if (!perms.equals(perms.applyUMask(lfs.getUMask()))) { lfs.setPermission(dirPath, perms); @@ -532,13 +576,14 @@ protected void createDir(Path dirPath, FsPermission perms, * */ void createUserLocalDirs(List localDirs, String user) - throws IOException { + throws IOException { boolean userDirStatus = false; FsPermission userperms = new FsPermission(USER_PERM); for (String localDir : localDirs) { // create $local.dir/usercache/$user and its immediate parent try { - createDir(getUserCacheDir(new Path(localDir), user), userperms, true, user); + createDir(getUserCacheDir(new Path(localDir), user), userperms, true, + user); } catch (IOException e) { LOG.warn("Unable to create the user directory : " + localDir, e); continue; @@ -633,7 +678,7 @@ void createAppDirs(List localDirs, String user, String appId) * Create application log directories on all disks. */ void createContainerLogDirs(String appId, String containerId, - List logDirs, String user) throws IOException { + List logDirs, String user) throws IOException { boolean containerLogDirStatus = false; FsPermission containerLogDirPerms = new FsPermission(LOGDIR_PERM); @@ -707,7 +752,7 @@ private Path getFileCacheDir(Path base, String user) { } protected Path getWorkingDir(List localDirs, String user, - String appId) throws IOException { + String appId) throws IOException { Path appStorageDir = null; long totalAvailable = 0L; long[] availableOnDisk = new long[localDirs.size()]; 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 a87238d793..5a9229b1d6 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 @@ -267,11 +267,11 @@ public Integer call() { // Sanitize the container's environment sanitizeEnv(environment, containerWorkDir, appDirs, containerLogDirs, localResources, nmPrivateClasspathJarDir); - + // Write out the environment - exec.writeLaunchEnv(containerScriptOutStream, environment, localResources, - launchContext.getCommands()); - + exec.writeLaunchEnv(containerScriptOutStream, environment, + localResources, launchContext.getCommands()); + // /////////// End of writing out container-script // /////////// Write out the container-tokens in the nmPrivate space. diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDockerContainerExecutor.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDockerContainerExecutor.java index ac025421e9..65e381c526 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDockerContainerExecutor.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDockerContainerExecutor.java @@ -18,7 +18,18 @@ package org.apache.hadoop.yarn.server.nodemanager; -import com.google.common.base.Strings; +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.RETURNS_DEEP_STUBS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.PrintWriter; +import java.util.HashMap; +import java.util.Map; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; @@ -26,48 +37,29 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.util.Shell; -import org.apache.hadoop.yarn.api.ApplicationConstants; import org.apache.hadoop.yarn.api.records.ContainerId; import org.apache.hadoop.yarn.api.records.ContainerLaunchContext; import org.apache.hadoop.yarn.conf.YarnConfiguration; import org.apache.hadoop.yarn.server.nodemanager.containermanager.container.Container; -import org.apache.hadoop.yarn.util.ConverterUtils; import org.junit.After; import org.junit.Before; import org.junit.Test; -import java.io.File; -import java.io.FileOutputStream; -import java.io.FileReader; -import java.io.IOException; -import java.io.LineNumberReader; -import java.io.PrintWriter; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assume.assumeTrue; -import static org.mockito.Mockito.RETURNS_DEEP_STUBS; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import com.google.common.base.Strings; /** - * This is intended to test the DockerContainerExecutor code, but it requires docker - * to be installed. + * This is intended to test the DockerContainerExecutor code, but it requires + * docker to be installed. *
    - *
  1. Install docker, and Compile the code with docker-service-url set to the host and port - * where docker service is running. + *
  2. Install docker, and Compile the code with docker-service-url set to the + * host and port where docker service is running. * 
    
- * > mvn clean install -Ddocker-service-url=tcp://0.0.0.0:4243
- *                          -DskipTests
+ * > mvn clean install -Ddocker-service-url=tcp://0.0.0.0:4243 -DskipTests
  *
    */ public class TestDockerContainerExecutor { private static final Log LOG = LogFactory - .getLog(TestDockerContainerExecutor.class); + .getLog(TestDockerContainerExecutor.class); private static File workSpace = null; private DockerContainerExecutor exec = null; private LocalDirsHandlerService dirsHandler; @@ -75,14 +67,10 @@ public class TestDockerContainerExecutor { private FileContext lfs; private String yarnImage; - private int id = 0; private String appSubmitter; private String dockerUrl; private String testImage = "centos:latest"; private String dockerExec; - private String containerIdStr; - - private ContainerId getNextContainerId() { ContainerId cId = mock(ContainerId.class, RETURNS_DEEP_STUBS); String id = "CONTAINER_" + System.currentTimeMillis(); @@ -91,6 +79,8 @@ private ContainerId getNextContainerId() { } @Before + //Initialize a new DockerContainerExecutor that will be used to launch mocked + //containers. public void setup() { try { lfs = FileContext.getLocalFSFileContext(); @@ -113,8 +103,10 @@ public void setup() { } dockerUrl = " -H " + dockerUrl; dockerExec = "docker " + dockerUrl; - conf.set(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, yarnImage); - conf.set(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_EXEC_NAME, dockerExec); + conf.set( + YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, yarnImage); + conf.set( + YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_EXEC_NAME, dockerExec); exec = new DockerContainerExecutor(); dirsHandler = new LocalDirsHandlerService(); dirsHandler.init(conf); @@ -129,11 +121,10 @@ public void setup() { private Shell.ShellCommandExecutor shellExec(String command) { try { - Shell.ShellCommandExecutor shExec = new Shell.ShellCommandExecutor( - command.split("\\s+"), - new File(workDir.toUri().getPath()), - System.getenv()); + command.split("\\s+"), + new File(workDir.toUri().getPath()), + System.getenv()); shExec.execute(); return shExec; } catch (IOException e) { @@ -145,14 +136,24 @@ private boolean shouldRun() { return exec != null; } - private int runAndBlock(ContainerId cId, Map launchCtxEnv, String... cmd) throws IOException { + /** + * Test that a docker container can be launched to run a command + * @param cId a fake ContainerID + * @param launchCtxEnv + * @param cmd the command to launch inside the docker container + * @return the exit code of the process used to launch the docker container + * @throws IOException + */ + private int runAndBlock(ContainerId cId, Map launchCtxEnv, + String... cmd) throws IOException { String appId = "APP_" + System.currentTimeMillis(); Container container = mock(Container.class); ContainerLaunchContext context = mock(ContainerLaunchContext.class); when(container.getContainerId()).thenReturn(cId); when(container.getLaunchContext()).thenReturn(context); - when(cId.getApplicationAttemptId().getApplicationId().toString()).thenReturn(appId); + when(cId.getApplicationAttemptId().getApplicationId().toString()) + .thenReturn(appId); when(context.getEnvironment()).thenReturn(launchCtxEnv); String script = writeScriptFile(launchCtxEnv, cmd); @@ -164,11 +165,13 @@ private int runAndBlock(ContainerId cId, Map launchCtxEnv, Strin exec.activateContainer(cId, pidFile); return exec.launchContainer(container, scriptPath, tokensPath, - appSubmitter, appId, workDir, dirsHandler.getLocalDirs(), - dirsHandler.getLogDirs()); + appSubmitter, appId, workDir, dirsHandler.getLocalDirs(), + dirsHandler.getLogDirs()); } - private String writeScriptFile(Map launchCtxEnv, String... cmd) throws IOException { + // Write the script used to launch the docker container in a temp file + private String writeScriptFile(Map launchCtxEnv, + String... cmd) throws IOException { File f = File.createTempFile("TestDockerContainerExecutor", ".sh"); f.deleteOnExit(); PrintWriter p = new PrintWriter(new FileOutputStream(f)); @@ -193,6 +196,10 @@ public void tearDown() { } } + /** + * Test that a touch command can be launched successfully in a docker + * container + */ @Test public void testLaunchContainer() throws IOException { if (!shouldRun()) { @@ -201,12 +208,13 @@ public void testLaunchContainer() throws IOException { } Map env = new HashMap(); - env.put(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, testImage); + env.put(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, + testImage); String touchFileName = "touch-file-" + System.currentTimeMillis(); File touchFile = new File(dirsHandler.getLocalDirs().get(0), touchFileName); ContainerId cId = getNextContainerId(); - int ret = runAndBlock( - cId, env, "touch", touchFile.getAbsolutePath(), "&&", "cp", touchFile.getAbsolutePath(), "/"); + int ret = runAndBlock(cId, env, "touch", touchFile.getAbsolutePath(), "&&", + "cp", touchFile.getAbsolutePath(), "/"); assertEquals(0, ret); } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDockerContainerExecutorWithMocks.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDockerContainerExecutorWithMocks.java index 3584fedde4..8acd9caaa9 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDockerContainerExecutorWithMocks.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestDockerContainerExecutorWithMocks.java @@ -18,7 +18,23 @@ package org.apache.hadoop.yarn.server.nodemanager; -import com.google.common.base.Strings; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; +import static org.mockito.Mockito.RETURNS_DEEP_STUBS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.File; +import java.io.FileReader; +import java.io.IOException; +import java.io.LineNumberReader; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; @@ -36,30 +52,13 @@ import org.junit.Before; import org.junit.Test; -import java.io.File; -import java.io.FileReader; -import java.io.IOException; -import java.io.LineNumberReader; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assume.assumeTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import static org.mockito.Mockito.RETURNS_DEEP_STUBS; - /** * Mock tests for docker container executor */ public class TestDockerContainerExecutorWithMocks { private static final Log LOG = LogFactory - .getLog(TestDockerContainerExecutorWithMocks.class); + .getLog(TestDockerContainerExecutorWithMocks.class); public static final String DOCKER_LAUNCH_COMMAND = "/bin/true"; private DockerContainerExecutor dockerContainerExecutor = null; private LocalDirsHandlerService dirsHandler; @@ -81,8 +80,10 @@ public void setup() { conf.set(YarnConfiguration.NM_LINUX_CONTAINER_EXECUTOR_PATH, executorPath); conf.set(YarnConfiguration.NM_LOCAL_DIRS, "/tmp/nm-local-dir" + time); conf.set(YarnConfiguration.NM_LOG_DIRS, "/tmp/userlogs" + time); - conf.set(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, yarnImage); - conf.set(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_EXEC_NAME , DOCKER_LAUNCH_COMMAND); + conf.set(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, + yarnImage); + conf.set(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_EXEC_NAME, + DOCKER_LAUNCH_COMMAND); dockerContainerExecutor = new DockerContainerExecutor(); dirsHandler = new LocalDirsHandlerService(); dirsHandler.init(conf); @@ -95,7 +96,6 @@ public void setup() { } catch (IOException e) { throw new RuntimeException(e); } - } @After @@ -110,13 +110,17 @@ public void tearDown() { } @Test(expected = IllegalStateException.class) + //Test that DockerContainerExecutor doesn't successfully init on a secure + //cluster public void testContainerInitSecure() throws IOException { dockerContainerExecutor.getConf().set( - CommonConfigurationKeys.HADOOP_SECURITY_AUTHENTICATION, "kerberos"); + CommonConfigurationKeys.HADOOP_SECURITY_AUTHENTICATION, "kerberos"); dockerContainerExecutor.init(); } @Test(expected = IllegalArgumentException.class) + //Test that when the image name is null, the container launch throws an + //IllegalArgumentException public void testContainerLaunchNullImage() throws IOException { String appSubmitter = "nobody"; String appId = "APP_ID"; @@ -126,17 +130,19 @@ public void testContainerLaunchNullImage() throws IOException { Container container = mock(Container.class, RETURNS_DEEP_STUBS); ContainerId cId = mock(ContainerId.class, RETURNS_DEEP_STUBS); ContainerLaunchContext context = mock(ContainerLaunchContext.class); - HashMap env = new HashMap(); + HashMap env = new HashMap(); when(container.getContainerId()).thenReturn(cId); when(container.getLaunchContext()).thenReturn(context); - when(cId.getApplicationAttemptId().getApplicationId().toString()).thenReturn(appId); + when(cId.getApplicationAttemptId().getApplicationId().toString()) + .thenReturn(appId); when(cId.toString()).thenReturn(containerId); when(context.getEnvironment()).thenReturn(env); - env.put(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, testImage); - dockerContainerExecutor.getConf() - .set(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, testImage); + env.put( + YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, testImage); + dockerContainerExecutor.getConf().set( + YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, testImage); Path scriptPath = new Path("file:///bin/echo"); Path tokensPath = new Path("file:///dev/null"); @@ -149,6 +155,8 @@ public void testContainerLaunchNullImage() throws IOException { } @Test(expected = IllegalArgumentException.class) + //Test that when the image name is invalid, the container launch throws an + //IllegalArgumentException public void testContainerLaunchInvalidImage() throws IOException { String appSubmitter = "nobody"; String appId = "APP_ID"; @@ -162,13 +170,15 @@ public void testContainerLaunchInvalidImage() throws IOException { when(container.getContainerId()).thenReturn(cId); when(container.getLaunchContext()).thenReturn(context); - when(cId.getApplicationAttemptId().getApplicationId().toString()).thenReturn(appId); + when(cId.getApplicationAttemptId().getApplicationId().toString()) + .thenReturn(appId); when(cId.toString()).thenReturn(containerId); when(context.getEnvironment()).thenReturn(env); - env.put(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, testImage); - dockerContainerExecutor.getConf() - .set(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, testImage); + env.put( + YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, testImage); + dockerContainerExecutor.getConf().set( + YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, testImage); Path scriptPath = new Path("file:///bin/echo"); Path tokensPath = new Path("file:///dev/null"); @@ -181,6 +191,8 @@ public void testContainerLaunchInvalidImage() throws IOException { } @Test + //Test that a container launch correctly wrote the session script with the + //commands we expected public void testContainerLaunch() throws IOException { String appSubmitter = "nobody"; String appId = "APP_ID"; @@ -194,40 +206,48 @@ public void testContainerLaunch() throws IOException { when(container.getContainerId()).thenReturn(cId); when(container.getLaunchContext()).thenReturn(context); - when(cId.getApplicationAttemptId().getApplicationId().toString()).thenReturn(appId); + when(cId.getApplicationAttemptId().getApplicationId().toString()) + .thenReturn(appId); when(cId.toString()).thenReturn(containerId); when(context.getEnvironment()).thenReturn(env); - env.put(YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, testImage); + env.put( + YarnConfiguration.NM_DOCKER_CONTAINER_EXECUTOR_IMAGE_NAME, testImage); Path scriptPath = new Path("file:///bin/echo"); Path tokensPath = new Path("file:///dev/null"); Path pidFile = new Path(workDir, "pid"); dockerContainerExecutor.activateContainer(cId, pidFile); - int ret = dockerContainerExecutor.launchContainer(container, scriptPath, tokensPath, - appSubmitter, appId, workDir, dirsHandler.getLocalDirs(), - dirsHandler.getLogDirs()); + int ret = dockerContainerExecutor.launchContainer(container, scriptPath, + tokensPath, appSubmitter, appId, workDir, dirsHandler.getLocalDirs(), + dirsHandler.getLogDirs()); assertEquals(0, ret); //get the script Path sessionScriptPath = new Path(workDir, - Shell.appendScriptExtension( - DockerContainerExecutor.DOCKER_CONTAINER_EXECUTOR_SESSION_SCRIPT)); - LineNumberReader lnr = new LineNumberReader(new FileReader(sessionScriptPath.toString())); + Shell.appendScriptExtension( + DockerContainerExecutor.DOCKER_CONTAINER_EXECUTOR_SESSION_SCRIPT)); + LineNumberReader lnr = new LineNumberReader(new FileReader( + sessionScriptPath.toString())); boolean cmdFound = false; List localDirs = dirsToMount(dirsHandler.getLocalDirs()); List logDirs = dirsToMount(dirsHandler.getLogDirs()); - List workDirMount = dirsToMount(Collections.singletonList(workDir.toUri().getPath())); - List expectedCommands = new ArrayList( - Arrays.asList(DOCKER_LAUNCH_COMMAND, "run", "--rm", "--net=host", "--name", containerId)); + List workDirMount = dirsToMount(Collections.singletonList( + workDir.toUri().getPath())); + List expectedCommands = new ArrayList(Arrays.asList( + DOCKER_LAUNCH_COMMAND, "run", "--rm", "--net=host", "--name", + containerId)); expectedCommands.addAll(localDirs); expectedCommands.addAll(logDirs); expectedCommands.addAll(workDirMount); String shellScript = workDir + "/launch_container.sh"; - expectedCommands.addAll(Arrays.asList(testImage.replaceAll("['\"]", ""), "bash","\"" + shellScript + "\"")); + expectedCommands.addAll(Arrays.asList(testImage.replaceAll("['\"]", ""), + "bash","\"" + shellScript + "\"")); - String expectedPidString = "echo `/bin/true inspect --format {{.State.Pid}} " + containerId+"` > "+ pidFile.toString() + ".tmp"; + String expectedPidString = + "echo `/bin/true inspect --format {{.State.Pid}} " + containerId+"` > "+ + pidFile.toString() + ".tmp"; boolean pidSetterFound = false; while(lnr.ready()){ String line = lnr.readLine();