YARN-1865 Merging change r1588693 from trunk.
git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1588696 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
parent
a7a7a16673
commit
2fb649a668
|
@ -53,6 +53,34 @@ abstract public class Shell {
|
|||
return IS_JAVA7_OR_ABOVE;
|
||||
}
|
||||
|
||||
/**
|
||||
* Maximum command line length in Windows
|
||||
* KB830473 documents this as 8191
|
||||
*/
|
||||
public static final int WINDOWS_MAX_SHELL_LENGHT = 8191;
|
||||
|
||||
/**
|
||||
* Checks if a given command (String[]) fits in the Windows maximum command line length
|
||||
* Note that the input is expected to already include space delimiters, no extra count
|
||||
* will be added for delimiters.
|
||||
*
|
||||
* @param commands command parts, including any space delimiters
|
||||
*/
|
||||
public static void checkWindowsCommandLineLength(String...commands)
|
||||
throws IOException {
|
||||
int len = 0;
|
||||
for (String s: commands) {
|
||||
len += s.length();
|
||||
}
|
||||
if (len > WINDOWS_MAX_SHELL_LENGHT) {
|
||||
throw new IOException(String.format(
|
||||
"The command line has a length of %d exceeds maximum allowed length of %d. " +
|
||||
"Command starts with: %s",
|
||||
len, WINDOWS_MAX_SHELL_LENGHT,
|
||||
StringUtils.join("", commands).substring(0, 100)));
|
||||
}
|
||||
}
|
||||
|
||||
/** a Unix command to get the current user's name */
|
||||
public final static String USER_NAME_COMMAND = "whoami";
|
||||
|
||||
|
|
|
@ -56,6 +56,9 @@ Release 2.5.0 - UNRELEASED
|
|||
YARN-1940. deleteAsUser() terminates early without deleting more files on
|
||||
error (Rushabh S Shah via jlowe)
|
||||
|
||||
YARN-1865. ShellScriptBuilder does not check for some error conditions.
|
||||
(Remus Rusanu via ivanmi)
|
||||
|
||||
Release 2.4.1 - UNRELEASED
|
||||
|
||||
INCOMPATIBLE CHANGES
|
||||
|
|
|
@ -479,15 +479,20 @@ public class ContainerLaunch implements Callable<Integer> {
|
|||
+ appIdStr;
|
||||
}
|
||||
|
||||
private static abstract class ShellScriptBuilder {
|
||||
@VisibleForTesting
|
||||
static abstract class ShellScriptBuilder {
|
||||
public static ShellScriptBuilder create() {
|
||||
return Shell.WINDOWS ? new WindowsShellScriptBuilder() :
|
||||
new UnixShellScriptBuilder();
|
||||
}
|
||||
|
||||
private static final String LINE_SEPARATOR =
|
||||
System.getProperty("line.separator");
|
||||
private final StringBuilder sb = new StringBuilder();
|
||||
|
||||
public abstract void command(List<String> command);
|
||||
public abstract void command(List<String> command) throws IOException;
|
||||
|
||||
public abstract void env(String key, String value);
|
||||
public abstract void env(String key, String value) throws IOException;
|
||||
|
||||
public final void symlink(Path src, Path dst) throws IOException {
|
||||
if (!src.isAbsolute()) {
|
||||
|
@ -520,11 +525,19 @@ public class ContainerLaunch implements Callable<Integer> {
|
|||
|
||||
protected abstract void link(Path src, Path dst) throws IOException;
|
||||
|
||||
protected abstract void mkdir(Path path);
|
||||
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 ]");
|
||||
line("then");
|
||||
line(" exit $hadoop_shell_errorcode");
|
||||
line("fi");
|
||||
}
|
||||
|
||||
public UnixShellScriptBuilder(){
|
||||
line("#!/bin/bash");
|
||||
line();
|
||||
|
@ -533,6 +546,7 @@ public class ContainerLaunch implements Callable<Integer> {
|
|||
@Override
|
||||
public void command(List<String> command) {
|
||||
line("exec /bin/bash -c \"", StringUtils.join(" ", command), "\"");
|
||||
errorCheck();
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -543,31 +557,43 @@ public class ContainerLaunch implements Callable<Integer> {
|
|||
@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());
|
||||
errorCheck();
|
||||
}
|
||||
}
|
||||
|
||||
private static final class WindowsShellScriptBuilder
|
||||
extends ShellScriptBuilder {
|
||||
|
||||
private void errorCheck() {
|
||||
line("@if %errorlevel% neq 0 exit /b %errorlevel%");
|
||||
}
|
||||
|
||||
private void lineWithLenCheck(String... commands) throws IOException {
|
||||
Shell.checkWindowsCommandLineLength(commands);
|
||||
line(commands);
|
||||
}
|
||||
|
||||
public WindowsShellScriptBuilder() {
|
||||
line("@setlocal");
|
||||
line();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void command(List<String> command) {
|
||||
line("@call ", StringUtils.join(" ", command));
|
||||
public void command(List<String> command) throws IOException {
|
||||
lineWithLenCheck("@call ", StringUtils.join(" ", command));
|
||||
errorCheck();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void env(String key, String value) {
|
||||
line("@set ", key, "=", value,
|
||||
"\nif %errorlevel% neq 0 exit /b %errorlevel%");
|
||||
public void env(String key, String value) throws IOException {
|
||||
lineWithLenCheck("@set ", key, "=", value);
|
||||
errorCheck();
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -578,16 +604,20 @@ public class ContainerLaunch implements Callable<Integer> {
|
|||
// If not on Java7+ on Windows, then copy file instead of symlinking.
|
||||
// See also FileUtil#symLink for full explanation.
|
||||
if (!Shell.isJava7OrAbove() && srcFile.isFile()) {
|
||||
line(String.format("@copy \"%s\" \"%s\"", srcFileStr, dstFileStr));
|
||||
lineWithLenCheck(String.format("@copy \"%s\" \"%s\"", srcFileStr, dstFileStr));
|
||||
errorCheck();
|
||||
} else {
|
||||
line(String.format("@%s symlink \"%s\" \"%s\"", Shell.WINUTILS,
|
||||
lineWithLenCheck(String.format("@%s symlink \"%s\" \"%s\"", Shell.WINUTILS,
|
||||
dstFileStr, srcFileStr));
|
||||
errorCheck();
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void mkdir(Path path) {
|
||||
line("@if not exist ", path.toString(), " mkdir ", path.toString());
|
||||
protected void mkdir(Path path) throws IOException {
|
||||
lineWithLenCheck(String.format("@if not exist \"%s\" mkdir \"%s\"",
|
||||
path.toString(), path.toString()));
|
||||
errorCheck();
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -730,8 +760,7 @@ public class ContainerLaunch implements Callable<Integer> {
|
|||
Map<String,String> environment, Map<Path,List<String>> resources,
|
||||
List<String> command)
|
||||
throws IOException {
|
||||
ShellScriptBuilder sb = Shell.WINDOWS ? new WindowsShellScriptBuilder() :
|
||||
new UnixShellScriptBuilder();
|
||||
ShellScriptBuilder sb = ShellScriptBuilder.create();
|
||||
if (environment != null) {
|
||||
for (Map.Entry<String,String> env : environment.entrySet()) {
|
||||
sb.env(env.getKey().toString(), env.getValue().toString());
|
||||
|
|
|
@ -19,6 +19,9 @@
|
|||
package org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertThat;
|
||||
import static org.junit.Assert.fail;
|
||||
import static org.junit.matchers.JUnitMatchers.*;
|
||||
import static org.mockito.Mockito.mock;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
|
@ -27,6 +30,7 @@ import java.io.File;
|
|||
import java.io.FileOutputStream;
|
||||
import java.io.FileReader;
|
||||
import java.io.IOException;
|
||||
import java.io.PrintStream;
|
||||
import java.io.PrintWriter;
|
||||
import java.nio.ByteBuffer;
|
||||
import java.util.ArrayList;
|
||||
|
@ -37,7 +41,6 @@ import java.util.List;
|
|||
import java.util.Map;
|
||||
|
||||
import org.junit.Assert;
|
||||
|
||||
import org.apache.commons.codec.binary.Base64;
|
||||
import org.apache.hadoop.conf.Configuration;
|
||||
import org.apache.hadoop.fs.FileUtil;
|
||||
|
@ -76,6 +79,7 @@ import org.apache.hadoop.yarn.server.nodemanager.containermanager.BaseContainerM
|
|||
import org.apache.hadoop.yarn.server.nodemanager.containermanager.container.Container;
|
||||
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.localizer.ContainerLocalizer;
|
||||
import org.apache.hadoop.yarn.server.utils.BuilderUtils;
|
||||
import org.apache.hadoop.yarn.util.Apps;
|
||||
|
@ -83,6 +87,7 @@ import org.apache.hadoop.yarn.util.AuxiliaryServiceHelper;
|
|||
import org.apache.hadoop.yarn.util.ConverterUtils;
|
||||
import org.apache.hadoop.yarn.util.LinuxResourceCalculatorPlugin;
|
||||
import org.apache.hadoop.yarn.util.ResourceCalculatorPlugin;
|
||||
import org.junit.Assume;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
|
||||
|
@ -743,18 +748,18 @@ public class TestContainerLaunch extends BaseContainerManagerTest {
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
@Test (timeout = 30000)
|
||||
public void testDelayedKill() throws Exception {
|
||||
internalKillTest(true);
|
||||
}
|
||||
|
||||
@Test
|
||||
@Test (timeout = 30000)
|
||||
public void testImmediateKill() throws Exception {
|
||||
internalKillTest(false);
|
||||
}
|
||||
|
||||
@SuppressWarnings("rawtypes")
|
||||
@Test
|
||||
@Test (timeout = 10000)
|
||||
public void testCallFailureWithNullLocalizedResources() {
|
||||
Container container = mock(Container.class);
|
||||
when(container.getContainerId()).thenReturn(ContainerId.newInstance(
|
||||
|
@ -792,4 +797,166 @@ public class TestContainerLaunch extends BaseContainerManagerTest {
|
|||
return containerToken;
|
||||
}
|
||||
|
||||
/**
|
||||
* Test that script exists with non-zero exit code when command fails.
|
||||
* @throws IOException
|
||||
*/
|
||||
@Test (timeout = 10000)
|
||||
public void testShellScriptBuilderNonZeroExitCode() throws IOException {
|
||||
ShellScriptBuilder builder = ShellScriptBuilder.create();
|
||||
builder.command(Arrays.asList(new String[] {"unknownCommand"}));
|
||||
File shellFile = Shell.appendScriptExtension(tmpDir, "testShellScriptBuilderError");
|
||||
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());
|
||||
}
|
||||
}
|
||||
finally {
|
||||
FileUtil.fullyDelete(shellFile);
|
||||
}
|
||||
}
|
||||
|
||||
private static final String expectedMessage = "The command line has a length of";
|
||||
|
||||
@Test (timeout = 10000)
|
||||
public void testWindowsShellScriptBuilderCommand() throws IOException {
|
||||
String callCmd = "@call ";
|
||||
|
||||
// Test is only relevant on Windows
|
||||
Assume.assumeTrue(Shell.WINDOWS);
|
||||
|
||||
// The tests are built on assuming 8191 max command line length
|
||||
assertEquals(8191, Shell.WINDOWS_MAX_SHELL_LENGHT);
|
||||
|
||||
ShellScriptBuilder builder = ShellScriptBuilder.create();
|
||||
|
||||
// Basic tests: less length, exact length, max+1 length
|
||||
builder.command(Arrays.asList(
|
||||
org.apache.commons.lang.StringUtils.repeat("A", 1024)));
|
||||
builder.command(Arrays.asList(
|
||||
org.apache.commons.lang.StringUtils.repeat(
|
||||
"E", Shell.WINDOWS_MAX_SHELL_LENGHT - callCmd.length())));
|
||||
try {
|
||||
builder.command(Arrays.asList(
|
||||
org.apache.commons.lang.StringUtils.repeat(
|
||||
"X", Shell.WINDOWS_MAX_SHELL_LENGHT -callCmd.length() + 1)));
|
||||
fail("longCommand was expected to throw");
|
||||
} catch(IOException e) {
|
||||
assertThat(e.getMessage(), containsString(expectedMessage));
|
||||
}
|
||||
|
||||
// Composite tests, from parts: less, exact and +
|
||||
builder.command(Arrays.asList(
|
||||
org.apache.commons.lang.StringUtils.repeat("A", 1024),
|
||||
org.apache.commons.lang.StringUtils.repeat("A", 1024),
|
||||
org.apache.commons.lang.StringUtils.repeat("A", 1024)));
|
||||
|
||||
// buildr.command joins the command parts with an extra space
|
||||
builder.command(Arrays.asList(
|
||||
org.apache.commons.lang.StringUtils.repeat("E", 4095),
|
||||
org.apache.commons.lang.StringUtils.repeat("E", 2047),
|
||||
org.apache.commons.lang.StringUtils.repeat("E", 2047 - callCmd.length())));
|
||||
|
||||
try {
|
||||
builder.command(Arrays.asList(
|
||||
org.apache.commons.lang.StringUtils.repeat("X", 4095),
|
||||
org.apache.commons.lang.StringUtils.repeat("X", 2047),
|
||||
org.apache.commons.lang.StringUtils.repeat("X", 2048 - callCmd.length())));
|
||||
fail("long commands was expected to throw");
|
||||
} catch(IOException e) {
|
||||
assertThat(e.getMessage(), containsString(expectedMessage));
|
||||
}
|
||||
}
|
||||
|
||||
@Test (timeout = 10000)
|
||||
public void testWindowsShellScriptBuilderEnv() throws IOException {
|
||||
// Test is only relevant on Windows
|
||||
Assume.assumeTrue(Shell.WINDOWS);
|
||||
|
||||
// The tests are built on assuming 8191 max command line length
|
||||
assertEquals(8191, Shell.WINDOWS_MAX_SHELL_LENGHT);
|
||||
|
||||
ShellScriptBuilder builder = ShellScriptBuilder.create();
|
||||
|
||||
// test env
|
||||
builder.env("somekey", org.apache.commons.lang.StringUtils.repeat("A", 1024));
|
||||
builder.env("somekey", org.apache.commons.lang.StringUtils.repeat(
|
||||
"A", Shell.WINDOWS_MAX_SHELL_LENGHT - ("@set somekey=").length()));
|
||||
try {
|
||||
builder.env("somekey", org.apache.commons.lang.StringUtils.repeat(
|
||||
"A", Shell.WINDOWS_MAX_SHELL_LENGHT - ("@set somekey=").length()) + 1);
|
||||
fail("long env was expected to throw");
|
||||
} catch(IOException e) {
|
||||
assertThat(e.getMessage(), containsString(expectedMessage));
|
||||
}
|
||||
}
|
||||
|
||||
@Test (timeout = 10000)
|
||||
public void testWindowsShellScriptBuilderMkdir() throws IOException {
|
||||
String mkDirCmd = "@if not exist \"\" mkdir \"\"";
|
||||
|
||||
// Test is only relevant on Windows
|
||||
Assume.assumeTrue(Shell.WINDOWS);
|
||||
|
||||
// The tests are built on assuming 8191 max command line length
|
||||
assertEquals(8191, Shell.WINDOWS_MAX_SHELL_LENGHT);
|
||||
|
||||
ShellScriptBuilder builder = ShellScriptBuilder.create();
|
||||
|
||||
// test mkdir
|
||||
builder.mkdir(new Path(org.apache.commons.lang.StringUtils.repeat("A", 1024)));
|
||||
builder.mkdir(new Path(org.apache.commons.lang.StringUtils.repeat(
|
||||
"E", (Shell.WINDOWS_MAX_SHELL_LENGHT - mkDirCmd.length())/2)));
|
||||
try {
|
||||
builder.mkdir(new Path(org.apache.commons.lang.StringUtils.repeat(
|
||||
"X", (Shell.WINDOWS_MAX_SHELL_LENGHT - mkDirCmd.length())/2 +1)));
|
||||
fail("long mkdir was expected to throw");
|
||||
} catch(IOException e) {
|
||||
assertThat(e.getMessage(), containsString(expectedMessage));
|
||||
}
|
||||
}
|
||||
|
||||
@Test (timeout = 10000)
|
||||
public void testWindowsShellScriptBuilderLink() throws IOException {
|
||||
// Test is only relevant on Windows
|
||||
Assume.assumeTrue(Shell.WINDOWS);
|
||||
|
||||
String linkCmd = "@" +Shell.WINUTILS + " symlink \"\" \"\"";
|
||||
|
||||
// The tests are built on assuming 8191 max command line length
|
||||
assertEquals(8191, Shell.WINDOWS_MAX_SHELL_LENGHT);
|
||||
|
||||
ShellScriptBuilder builder = ShellScriptBuilder.create();
|
||||
|
||||
// test link
|
||||
builder.link(new Path(org.apache.commons.lang.StringUtils.repeat("A", 1024)),
|
||||
new Path(org.apache.commons.lang.StringUtils.repeat("B", 1024)));
|
||||
builder.link(
|
||||
new Path(org.apache.commons.lang.StringUtils.repeat(
|
||||
"E", (Shell.WINDOWS_MAX_SHELL_LENGHT - linkCmd.length())/2)),
|
||||
new Path(org.apache.commons.lang.StringUtils.repeat(
|
||||
"F", (Shell.WINDOWS_MAX_SHELL_LENGHT - linkCmd.length())/2)));
|
||||
try {
|
||||
builder.link(
|
||||
new Path(org.apache.commons.lang.StringUtils.repeat(
|
||||
"X", (Shell.WINDOWS_MAX_SHELL_LENGHT - linkCmd.length())/2 + 1)),
|
||||
new Path(org.apache.commons.lang.StringUtils.repeat(
|
||||
"Y", (Shell.WINDOWS_MAX_SHELL_LENGHT - linkCmd.length())/2) + 1));
|
||||
fail("long link was expected to throw");
|
||||
} catch(IOException e) {
|
||||
assertThat(e.getMessage(), containsString(expectedMessage));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue