From ebc048cc055d0f7d1b85bc0b6f56cd15673e837d Mon Sep 17 00:00:00 2001 From: Jason Lowe Date: Thu, 13 Jul 2017 17:44:47 -0500 Subject: [PATCH] YARN-6805. NPE in LinuxContainerExecutor due to null PrivilegedOperationException exit code. Contributed by Jason Lowe --- .../nodemanager/LinuxContainerExecutor.java | 19 ++-- .../PrivilegedOperationException.java | 10 +-- .../runtime/ContainerExecutionException.java | 10 +-- .../TestLinuxContainerExecutorWithMocks.java | 89 +++++++++++++++++++ 4 files changed, 111 insertions(+), 17 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/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 9a3b2d25cc6..2aaa8359e7b 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 @@ -275,6 +275,10 @@ protected void addSchedPriorityCommand(List command) { } } + protected PrivilegedOperationExecutor getPrivilegedOperationExecutor() { + return PrivilegedOperationExecutor.getInstance(getConf()); + } + @Override public void init() throws IOException { Configuration conf = super.getConf(); @@ -285,7 +289,7 @@ public void init() throws IOException { PrivilegedOperation checkSetupOp = new PrivilegedOperation( PrivilegedOperation.OperationType.CHECK_SETUP); PrivilegedOperationExecutor privilegedOperationExecutor = - PrivilegedOperationExecutor.getInstance(conf); + getPrivilegedOperationExecutor(); privilegedOperationExecutor.executePrivilegedOperation(checkSetupOp, false); @@ -382,7 +386,7 @@ public void startLocalizer(LocalizerStartContext ctx) try { Configuration conf = super.getConf(); PrivilegedOperationExecutor privilegedOperationExecutor = - PrivilegedOperationExecutor.getInstance(conf); + getPrivilegedOperationExecutor(); privilegedOperationExecutor.executePrivilegedOperation(prefixCommands, initializeContainerOp, null, null, false, true); @@ -530,8 +534,9 @@ public int launchContainer(ContainerStartContext ctx) } builder.append("Stack trace: " + StringUtils.stringifyException(e) + "\n"); - if (!e.getOutput().isEmpty()) { - builder.append("Shell output: " + e.getOutput() + "\n"); + String output = e.getOutput(); + if (output != null && !e.getOutput().isEmpty()) { + builder.append("Shell output: " + output + "\n"); } String diagnostics = builder.toString(); logOutput(diagnostics); @@ -729,7 +734,7 @@ public void deleteAsUser(DeletionAsUserContext ctx) { try { Configuration conf = super.getConf(); PrivilegedOperationExecutor privilegedOperationExecutor = - PrivilegedOperationExecutor.getInstance(conf); + getPrivilegedOperationExecutor(); privilegedOperationExecutor.executePrivilegedOperation(deleteAsUserOp, false); @@ -759,7 +764,7 @@ protected File[] readDirAsUser(String user, Path dir) { try { PrivilegedOperationExecutor privOpExecutor = - PrivilegedOperationExecutor.getInstance(super.getConf()); + getPrivilegedOperationExecutor(); String results = privOpExecutor.executePrivilegedOperation(listAsUserOp, true); @@ -818,7 +823,7 @@ public void mountCgroups(List cgroupKVs, String hierarchy) mountCGroupsOp.appendArgs(cgroupKVs); PrivilegedOperationExecutor privilegedOperationExecutor = - PrivilegedOperationExecutor.getInstance(conf); + getPrivilegedOperationExecutor(); privilegedOperationExecutor.executePrivilegedOperation(mountCGroupsOp, false); 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/privileged/PrivilegedOperationException.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/privileged/PrivilegedOperationException.java index 3622489a499..9a11194f143 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/privileged/PrivilegedOperationException.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/privileged/PrivilegedOperationException.java @@ -24,7 +24,7 @@ public class PrivilegedOperationException extends YarnException { private static final long serialVersionUID = 1L; - private Integer exitCode; + private int exitCode = -1; private String output; private String errorOutput; @@ -36,7 +36,7 @@ public PrivilegedOperationException(String message) { super(message); } - public PrivilegedOperationException(String message, Integer exitCode, + public PrivilegedOperationException(String message, int exitCode, String output, String errorOutput) { super(message); this.exitCode = exitCode; @@ -48,8 +48,8 @@ public PrivilegedOperationException(Throwable cause) { super(cause); } - public PrivilegedOperationException(Throwable cause, Integer exitCode, String - output, String errorOutput) { + public PrivilegedOperationException(Throwable cause, int exitCode, + String output, String errorOutput) { super(cause); this.exitCode = exitCode; this.output = output; @@ -59,7 +59,7 @@ public PrivilegedOperationException(String message, Throwable cause) { super(message, cause); } - public Integer getExitCode() { + public int getExitCode() { return exitCode; } 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/ContainerExecutionException.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/ContainerExecutionException.java index 1fbece2205e..31472777042 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/ContainerExecutionException.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/ContainerExecutionException.java @@ -32,10 +32,10 @@ @InterfaceStability.Unstable public class ContainerExecutionException extends YarnException { private static final long serialVersionUID = 1L; - private static final Integer EXIT_CODE_UNSET = -1; + private static final int EXIT_CODE_UNSET = -1; private static final String OUTPUT_UNSET = ""; - private Integer exitCode; + private int exitCode; private String output; private String errorOutput; @@ -54,7 +54,7 @@ public ContainerExecutionException(Throwable throwable) { } - public ContainerExecutionException(String message, Integer exitCode, String + public ContainerExecutionException(String message, int exitCode, String output, String errorOutput) { super(message); this.exitCode = exitCode; @@ -62,7 +62,7 @@ public ContainerExecutionException(String message, Integer exitCode, String this.errorOutput = errorOutput; } - public ContainerExecutionException(Throwable cause, Integer exitCode, String + public ContainerExecutionException(Throwable cause, int exitCode, String output, String errorOutput) { super(cause); this.exitCode = exitCode; @@ -70,7 +70,7 @@ public ContainerExecutionException(Throwable cause, Integer exitCode, String this.errorOutput = errorOutput; } - public Integer getExitCode() { + public int getExitCode() { return exitCode; } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java index 07134e8e36d..cfd0e364a2d 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/TestLinuxContainerExecutorWithMocks.java @@ -23,7 +23,9 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; @@ -40,6 +42,7 @@ import java.util.HashMap; import java.util.LinkedList; import java.util.List; +import java.util.Map; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -47,6 +50,8 @@ import org.apache.hadoop.fs.FileUtil; import org.apache.hadoop.fs.Path; import org.apache.hadoop.util.StringUtils; +import org.apache.hadoop.yarn.api.records.ApplicationAttemptId; +import org.apache.hadoop.yarn.api.records.ApplicationId; import org.apache.hadoop.yarn.api.records.ContainerId; import org.apache.hadoop.yarn.api.records.ContainerLaunchContext; import org.apache.hadoop.yarn.conf.YarnConfiguration; @@ -54,6 +59,7 @@ import org.apache.hadoop.yarn.server.nodemanager.containermanager.container.Container; import org.apache.hadoop.yarn.server.nodemanager.containermanager.container.ContainerDiagnosticsUpdateEvent; import org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.privileged.PrivilegedOperation; +import org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.privileged.PrivilegedOperationException; import org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.privileged.PrivilegedOperationExecutor; import org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.runtime.DefaultLinuxContainerRuntime; import org.apache.hadoop.yarn.server.nodemanager.containermanager.linux.runtime.LinuxContainerRuntime; @@ -516,4 +522,87 @@ public void testDeleteAsUser() throws IOException { appSubmitter, cmd, "", baseDir0.toString(), baseDir1.toString()), readMockParams()); } + + @Test + public void testNoExitCodeFromPrivilegedOperation() throws Exception { + Configuration conf = new Configuration(); + final PrivilegedOperationExecutor spyPrivilegedExecutor = + spy(PrivilegedOperationExecutor.getInstance(conf)); + doThrow(new PrivilegedOperationException("interrupted")) + .when(spyPrivilegedExecutor).executePrivilegedOperation( + any(List.class), any(PrivilegedOperation.class), + any(File.class), any(Map.class), anyBoolean(), anyBoolean()); + LinuxContainerRuntime runtime = new DefaultLinuxContainerRuntime( + spyPrivilegedExecutor); + runtime.initialize(conf); + mockExec = new LinuxContainerExecutor(runtime); + mockExec.setConf(conf); + LinuxContainerExecutor lce = new LinuxContainerExecutor(runtime) { + @Override + protected PrivilegedOperationExecutor getPrivilegedOperationExecutor() { + return spyPrivilegedExecutor; + } + }; + lce.setConf(conf); + InetSocketAddress address = InetSocketAddress.createUnresolved( + "localhost", 8040); + Path nmPrivateCTokensPath= new Path("file:///bin/nmPrivateCTokensPath"); + LocalDirsHandlerService dirService = new LocalDirsHandlerService(); + dirService.init(conf); + + String appSubmitter = "nobody"; + ApplicationId appId = ApplicationId.newInstance(1, 1); + ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId, 1); + ContainerId cid = ContainerId.newContainerId(attemptId, 1); + HashMap env = new HashMap<>(); + Container container = mock(Container.class); + ContainerLaunchContext context = mock(ContainerLaunchContext.class); + when(container.getContainerId()).thenReturn(cid); + when(container.getLaunchContext()).thenReturn(context); + when(context.getEnvironment()).thenReturn(env); + Path workDir = new Path("/tmp"); + + try { + lce.startLocalizer(new LocalizerStartContext.Builder() + .setNmPrivateContainerTokens(nmPrivateCTokensPath) + .setNmAddr(address) + .setUser(appSubmitter) + .setAppId(appId.toString()) + .setLocId("12345") + .setDirsHandler(dirService) + .build()); + Assert.fail("startLocalizer should have thrown an exception"); + } catch (IOException e) { + assertTrue("Unexpected exception " + e, + e.getMessage().contains("exitCode")); + } + + lce.activateContainer(cid, new Path(workDir, "pid.txt")); + lce.launchContainer(new ContainerStartContext.Builder() + .setContainer(container) + .setNmPrivateContainerScriptPath(new Path("file:///bin/echo")) + .setNmPrivateTokensPath(new Path("file:///dev/null")) + .setUser(appSubmitter) + .setAppId(appId.toString()) + .setContainerWorkDir(workDir) + .setLocalDirs(dirsHandler.getLocalDirs()) + .setLogDirs(dirsHandler.getLogDirs()) + .setFilecacheDirs(new ArrayList<>()) + .setUserLocalDirs(new ArrayList<>()) + .setContainerLocalDirs(new ArrayList<>()) + .setContainerLogDirs(new ArrayList<>()) + .build()); + lce.deleteAsUser(new DeletionAsUserContext.Builder() + .setUser(appSubmitter) + .setSubDir(new Path("/tmp/testdir")) + .build()); + + try { + lce.mountCgroups(new ArrayList(), "hierarchy"); + Assert.fail("mountCgroups should have thrown an exception"); + } catch (IOException e) { + assertTrue("Unexpected exception " + e, + e.getMessage().contains("exit code")); + } + } }