diff --git hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java index 05818788daa..94d654fe339 100644 --- hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java +++ hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/ContainerExecutor.java @@ -44,6 +44,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.permission.FsPermission; +import org.apache.hadoop.yarn.api.ApplicationConstants; import org.apache.hadoop.yarn.api.records.ContainerId; import org.apache.hadoop.yarn.api.records.Resource; import org.apache.hadoop.yarn.conf.YarnConfiguration; @@ -63,6 +64,9 @@ import org.apache.hadoop.util.Shell; import org.apache.hadoop.util.StringUtils; +import static org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher.ContainerLaunch.CONTAINER_PRE_LAUNCH_STDERR; +import static org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher.ContainerLaunch.CONTAINER_PRE_LAUNCH_STDOUT; + /** * This class is abstraction of the mechanism used to launch a container on the * underlying OS. All executor implementations must extend ContainerExecutor. @@ -328,6 +332,11 @@ public void writeLaunchEnv(OutputStream out, Map environment, String user, String outFilename) throws IOException { ContainerLaunch.ShellScriptBuilder sb = ContainerLaunch.ShellScriptBuilder.create(); + + //Redirect stdout and stderr for launch_container script + 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, @@ -337,6 +346,7 @@ public void writeLaunchEnv(OutputStream out, Map environment, } 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()); @@ -347,6 +357,7 @@ public void writeLaunchEnv(OutputStream out, Map environment, } if (resources != null) { + sb.echo("Setting up job resources"); for (Map.Entry> resourceEntry : resources.entrySet()) { for (String linkName : resourceEntry.getValue()) { @@ -368,11 +379,12 @@ public void writeLaunchEnv(OutputStream out, Map environment, if (getConf() != null && getConf().getBoolean(YarnConfiguration.NM_LOG_CONTAINER_DEBUG_INFO, YarnConfiguration.DEFAULT_NM_LOG_CONTAINER_DEBUG_INFO)) { + sb.echo("Copying debugging information"); sb.copyDebugInformation(new Path(outFilename), new Path(logDir, outFilename)); sb.listDebugInformation(new Path(logDir, DIRECTORY_CONTENTS)); } - + sb.echo("Launching container"); sb.command(command); PrintStream pout = null; diff --git 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 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 0b599a873f2..1ba2b61d38a 100644 --- 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 +++ 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 @@ -36,8 +36,6 @@ import java.util.concurrent.Callable; import java.util.concurrent.atomic.AtomicBoolean; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FSDataInputStream; import org.apache.hadoop.fs.FileContext; @@ -86,13 +84,20 @@ import org.apache.hadoop.yarn.util.AuxiliaryServiceHelper; import com.google.common.annotations.VisibleForTesting; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class ContainerLaunch implements Callable { - private static final Log LOG = LogFactory.getLog(ContainerLaunch.class); + private static final Logger LOG = LoggerFactory.getLogger(ContainerLaunch.class); + + private static final String CONTAINER_PRE_LAUNCH_PREFIX = "prelaunch"; + public static final String CONTAINER_PRE_LAUNCH_STDOUT = CONTAINER_PRE_LAUNCH_PREFIX + ".out"; + public static final String CONTAINER_PRE_LAUNCH_STDERR = CONTAINER_PRE_LAUNCH_PREFIX + ".err"; public static final String CONTAINER_SCRIPT = Shell.appendScriptExtension("launch_container"); + public static final String FINAL_CONTAINER_TOKENS_FILE = "container_tokens"; private static final String PID_FILE_NAME_FMT = "%s.pid"; @@ -269,7 +274,7 @@ public Integer call() { creds.writeTokenStorageToStream(tokensOutStream); // /////////// End of writing out container-tokens } finally { - IOUtils.cleanup(LOG, containerScriptOutStream, tokensOutStream); + IOUtils.cleanupWithLogger(LOG, containerScriptOutStream, tokensOutStream); } ret = launchContainer(new ContainerStartContext.Builder() @@ -518,7 +523,7 @@ protected void handleContainerExitCode(int exitCode, Path containerLogDir) { @SuppressWarnings("unchecked") protected void handleContainerExitWithFailure(ContainerId containerID, int ret, Path containerLogDir, StringBuilder diagnosticInfo) { - LOG.warn(diagnosticInfo); + LOG.warn("Container launch failed : " + diagnosticInfo); String errorFileNamePattern = conf.get(YarnConfiguration.NM_CONTAINER_STDERR_PATTERN, @@ -569,7 +574,7 @@ protected void handleContainerExitWithFailure(ContainerId containerID, } catch (IOException e) { LOG.error("Failed to get tail of the container's error log file", e); } finally { - IOUtils.cleanup(LOG, errorFileIS); + IOUtils.cleanupWithLogger(LOG, errorFileIS); } this.dispatcher.getEventHandler() @@ -851,8 +856,43 @@ public static ShellScriptBuilder create() { public abstract void whitelistedEnv(String key, String value) throws IOException; + protected static final String ENV_STDOUT = "STDOUT"; + protected static final String ENV_STDERR = "STDERR"; + + /** + * Set stdout for the shell script + * @param stdoutDir stdout must be an absolute path + * @param stdOutFile stdout file name + * @throws IOException thrown when stdout path is not absolute + */ + public final void stdout(Path stdoutDir, String stdOutFile) throws IOException { + if (!stdoutDir.isAbsolute()) { + throw new IOException("Stdout path must be absolute"); + } + setStdOut(new Path(stdoutDir, stdOutFile)); + } + + /** + * Set stderr for the shell script + * @param stderrDir stderr must be an absolute path + * @param stdErrFile stderr file name + * @throws IOException thrown when stderr path is not absolute + */ + public final void stderr(Path stderrDir, String stdErrFile) throws IOException { + if (!stderrDir.isAbsolute()) { + throw new IOException("Stdout path must be absolute"); + } + setStdErr(new Path(stderrDir, stdErrFile)); + } + + protected abstract void setStdOut(Path stdout) throws IOException; + + protected abstract void setStdErr(Path stdout) throws IOException; + public abstract void env(String key, String value) throws IOException; + public abstract void echo(String echoStr) throws IOException; + public final void symlink(Path src, Path dst) throws IOException { if (!src.isAbsolute()) { throw new IOException("Source must be absolute"); @@ -897,20 +937,28 @@ public final void write(PrintStream out) throws IOException { out.append(sb); } - protected final void line(String... command) { + protected final void buildCommand(String... command) { for (String s : command) { sb.append(s); } + } + + protected final void linebreak(String... command) { sb.append(LINE_SEPARATOR); } + protected final void line(String... command) { + buildCommand(command); + linebreak(); + } + protected abstract void link(Path src, Path dst) throws IOException; protected abstract void mkdir(Path path) throws IOException; + } private static final class UnixShellScriptBuilder extends ShellScriptBuilder { - private void errorCheck() { line("hadoop_shell_errorcode=$?"); line("if [ $hadoop_shell_errorcode -ne 0 ]"); @@ -919,7 +967,7 @@ private void errorCheck() { line("fi"); } - public UnixShellScriptBuilder(){ + public UnixShellScriptBuilder() { line("#!/bin/bash"); line(); } @@ -931,24 +979,43 @@ public void command(List command) { } @Override - public void whitelistedEnv(String key, String value) { + public void whitelistedEnv(String key, String value) throws IOException { line("export ", key, "=${", key, ":-", "\"", value, "\"}"); } @Override - public void env(String key, String value) { + public void setStdOut(final Path stdout) throws IOException { + line("export ", ENV_STDOUT, "=\"", stdout.toString(), "\""); + // Close stdout of subprocess to prevent it from writing to the stdout file + // tee is needed for DefaultContainerExecutor error propagation to stdout and stderr + line("exec > >(tee -ia \"${STDOUT}\" >&1; exec 1>&-;)"); + } + + @Override + public void setStdErr(final Path stderr) throws IOException { + line("export ", ENV_STDERR, "=\"", stderr.toString(), "\""); + line("exec 2> >(tee -ia \"${STDERR}\" >&2; exec 1>&-;)"); + } + + @Override + public void env(String key, String value) throws IOException { line("export ", key, "=\"", value, "\""); } @Override + public void echo(final String echoStr) throws IOException { + line("echo \"" + echoStr + "\""); + } + + @Override protected void link(Path src, Path dst) throws IOException { line("ln -sf \"", src.toUri().getPath(), "\" \"", dst.toString(), "\""); errorCheck(); } @Override - protected void mkdir(Path path) { - line("mkdir -p ", path.toString()); + protected void mkdir(Path path) throws IOException { + line("mkdir -p", path.toString()); errorCheck(); } @@ -1013,12 +1080,25 @@ public void whitelistedEnv(String key, String value) throws IOException { } @Override + protected void setStdOut(final Path stdout) throws IOException { + } + + @Override + protected void setStdErr(final Path stderr) throws IOException { + } + + @Override public void env(String key, String value) throws IOException { lineWithLenCheck("@set ", key, "=", value); errorCheck(); } @Override + public void echo(final String echoStr) throws IOException { + lineWithLenCheck("@echo \"", echoStr, "\""); + } + + @Override protected void link(Path src, Path dst) throws IOException { File srcFile = new File(src.toUri().getPath()); String srcFileStr = srcFile.getPath(); diff --git 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 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 e71ce75c3dd..161f2ef2847 100644 --- 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 +++ 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 @@ -1529,4 +1529,110 @@ public void testContainerLaunchOnConfigurationError() throws Exception { verify(updaterNoCall, never()).reportException(any()); } + + /** + * Test that script exists with non-zero exit code when command fails. + * @throws IOException + */ + @Test + public void testShellScriptBuilderStdOutandErrRedirection() throws IOException { + ShellScriptBuilder builder = ShellScriptBuilder.create(); + + Path logDir = new Path(localLogDir.getAbsolutePath()); + File stdout = new File(logDir.toString(), ContainerLaunch.CONTAINER_PRE_LAUNCH_STDOUT); + File stderr = new File(logDir.toString(), ContainerLaunch.CONTAINER_PRE_LAUNCH_STDERR); + + builder.stdout(logDir, ContainerLaunch.CONTAINER_PRE_LAUNCH_STDOUT); + builder.stderr(logDir, ContainerLaunch.CONTAINER_PRE_LAUNCH_STDERR); + + //should redirect to specified stdout path + String TEST_STDOUT_ECHO = "Test stdout redirection"; + builder.echo(TEST_STDOUT_ECHO); + //should fail and redirect to stderr + builder.mkdir(new Path("/invalidSrcDir")); + + builder.command(Arrays.asList(new String[] {"unknownCommand"})); + + File shellFile = Shell.appendScriptExtension(tmpDir, "testShellScriptBuilderStdOutandErrRedirection"); + PrintStream writer = new PrintStream(new FileOutputStream(shellFile)); + builder.write(writer); + writer.close(); + try { + FileUtil.setExecutable(shellFile, true); + + Shell.ShellCommandExecutor shexc = new Shell.ShellCommandExecutor( + new String[]{shellFile.getAbsolutePath()}, tmpDir); + try { + shexc.execute(); + fail("builder shell command was expected to throw"); + } + catch(IOException e) { + // expected + System.out.println("Received an expected exception: " + e.getMessage()); + + Assert.assertEquals(true, stdout.exists()); + BufferedReader stdoutReader = new BufferedReader(new FileReader(stdout)); + // Get the pid of the process + String line = stdoutReader.readLine().trim(); + Assert.assertEquals(TEST_STDOUT_ECHO, line); + // No more lines + Assert.assertEquals(null, stdoutReader.readLine()); + stdoutReader.close(); + + Assert.assertEquals(true, stderr.exists()); + Assert.assertTrue(stderr.length() > 0); + } + } + finally { + FileUtil.fullyDelete(shellFile); + FileUtil.fullyDelete(stdout); + FileUtil.fullyDelete(stderr); + } + } + + /** + * Test that script exists with non-zero exit code when command fails. + * @throws IOException + */ + @Test + public void testShellScriptBuilderWithNoRedirection() throws IOException { + ShellScriptBuilder builder = ShellScriptBuilder.create(); + + Path logDir = new Path(localLogDir.getAbsolutePath()); + File stdout = new File(logDir.toString(), ContainerLaunch.CONTAINER_PRE_LAUNCH_STDOUT); + File stderr = new File(logDir.toString(), ContainerLaunch.CONTAINER_PRE_LAUNCH_STDERR); + + //should redirect to specified stdout path + String TEST_STDOUT_ECHO = "Test stdout redirection"; + builder.echo(TEST_STDOUT_ECHO); + //should fail and redirect to stderr + builder.mkdir(new Path("/invalidSrcDir")); + + builder.command(Arrays.asList(new String[] {"unknownCommand"})); + + File shellFile = Shell.appendScriptExtension(tmpDir, "testShellScriptBuilderStdOutandErrRedirection"); + PrintStream writer = new PrintStream(new FileOutputStream(shellFile)); + builder.write(writer); + writer.close(); + try { + FileUtil.setExecutable(shellFile, true); + + Shell.ShellCommandExecutor shexc = new Shell.ShellCommandExecutor( + new String[]{shellFile.getAbsolutePath()}, tmpDir); + try { + shexc.execute(); + fail("builder shell command was expected to throw"); + } + catch(IOException e) { + // expected + System.out.println("Received an expected exception: " + e.getMessage()); + + Assert.assertEquals(false, stdout.exists()); + Assert.assertEquals(false, stderr.exists()); + } + } + finally { + FileUtil.fullyDelete(shellFile); + } + } }