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..7e6f2db87ba 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,13 @@ 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..a1607cacb1d 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 @@ -91,8 +91,13 @@ private static final Log LOG = LogFactory.getLog(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"; @@ -851,8 +856,54 @@ public static ShellScriptBuilder create() { public abstract void whitelistedEnv(String key, String value) throws IOException; + private boolean redirectStdOut = false; + private boolean redirectStdErr = false; + + protected final String ENV_STDOUT = "STDOUT"; + protected final String ENV_STDERR = "STDERR"; + + protected String redirectStdOutStream() throws IOException { + return ""; + } + + protected String redirectStdErrStream() throws IOException { + return ""; + } + + /** + * Set stdout for the shell script + * @param stdoutDir stdout must be an absolute path + * @throws IOException + */ + public final void stdout(Path stdoutDir, String stdOutFile) throws IOException { + redirectStdOut = true; + 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 + * @throws IOException + */ + public final void stderr(Path stderrDir, String stdErrFile) throws IOException { + redirectStdErr = true; + 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 +948,35 @@ 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; + + boolean doRedirectStdOut() { + return redirectStdOut; + } + + boolean doRedirectStdErr() { + return redirectStdErr; + } } private static final class UnixShellScriptBuilder extends ShellScriptBuilder { - private void errorCheck() { line("hadoop_shell_errorcode=$?"); line("if [ $hadoop_shell_errorcode -ne 0 ]"); @@ -919,7 +985,7 @@ private void errorCheck() { line("fi"); } - public UnixShellScriptBuilder(){ + public UnixShellScriptBuilder() { line("#!/bin/bash"); line(); } @@ -931,24 +997,55 @@ public void command(List command) { } @Override - public void whitelistedEnv(String key, String value) { - line("export ", key, "=${", key, ":-", "\"", value, "\"}"); + public void whitelistedEnv(String key, String value) throws IOException { + line("export ", key, "=${", key, ":-", "\"", value, "\"}", redirectStdErrStream()); + } + + @Override + protected String redirectStdOutStream() throws IOException { + if (doRedirectStdOut()) { + return " 1> >(tee -a $STDOUT >&1)"; + } + return ""; + } + + @Override + protected String redirectStdErrStream() throws IOException { + if (doRedirectStdErr()) { + return " 2> >(tee -a $STDERR >&2)"; + } + return ""; + } + + @Override + public void setStdOut(final Path stdout) throws IOException { + line("export ", ENV_STDOUT, "=\"", stdout.toString(), "\""); } @Override - public void env(String key, String value) { - line("export ", key, "=\"", value, "\""); + public void setStdErr(final Path stderr) throws IOException { + line("export ", ENV_STDERR, "=\"", stderr.toString(), "\""); + } + + @Override + public void env(String key, String value) throws IOException { + line("export ", key, "=\"", value, "\"", redirectStdErrStream()); + } + + @Override + public void echo(final String echoStr) throws IOException { + line("echo \"" + echoStr + "\"", redirectStdOutStream()); } @Override protected void link(Path src, Path dst) throws IOException { - line("ln -sf \"", src.toUri().getPath(), "\" \"", dst.toString(), "\""); + line("ln -sf \"", src.toUri().getPath(), "\" \"", dst.toString(), "\"", redirectStdErrStream()); errorCheck(); } @Override - protected void mkdir(Path path) { - line("mkdir -p ", path.toString()); + protected void mkdir(Path path) throws IOException { + line("mkdir -p ", path.toString(), redirectStdErrStream()); errorCheck(); } @@ -1008,30 +1105,45 @@ public void command(List command) throws IOException { @Override public void whitelistedEnv(String key, String value) throws IOException { - lineWithLenCheck("@set ", key, "=", value); + lineWithLenCheck("@set ", key, "=", value, redirectStdErrStream()); errorCheck(); } @Override + protected void setStdOut(final Path stdout) throws IOException { + lineWithLenCheck("@set ", ENV_STDOUT, "=", stdout.toString()); + } + + @Override + protected void setStdErr(final Path stderr) throws IOException { + lineWithLenCheck("@set ", ENV_STDERR, "=", stderr.toString()); + } + + @Override public void env(String key, String value) throws IOException { - lineWithLenCheck("@set ", key, "=", value); + lineWithLenCheck("@set ", key, "=", value, redirectStdErrStream()); errorCheck(); } @Override + public void echo(final String echoStr) throws IOException { + lineWithLenCheck("@echo \"", echoStr, "\"", redirectStdOutStream()); + } + + @Override protected void link(Path src, Path dst) throws IOException { File srcFile = new File(src.toUri().getPath()); String srcFileStr = srcFile.getPath(); String dstFileStr = new File(dst.toString()).getPath(); lineWithLenCheck(String.format("@%s symlink \"%s\" \"%s\"", - Shell.getWinUtilsPath(), dstFileStr, srcFileStr)); + Shell.getWinUtilsPath(), dstFileStr, srcFileStr, redirectStdErrStream())); errorCheck(); } @Override protected void mkdir(Path path) throws IOException { lineWithLenCheck(String.format("@if not exist \"%s\" mkdir \"%s\"", - path.toString(), path.toString())); + path.toString(), path.toString(), redirectStdErrStream())); errorCheck(); } 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..e95817315bb 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 @@ -42,6 +42,8 @@ import java.util.List; import java.util.Map; import java.util.StringTokenizer; +import java.util.function.BiFunction; +import java.util.function.Function; import java.util.jar.JarFile; import java.util.jar.Manifest; @@ -1529,4 +1531,116 @@ 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); + + Assert.assertEquals(true, builder.doRedirectStdErr()); + Assert.assertEquals(true, builder.doRedirectStdOut()); + + //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); + + Assert.assertEquals(false, builder.doRedirectStdErr()); + Assert.assertEquals(false, builder.doRedirectStdOut()); + + //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); + } + } }