YARN-6805. NPE in LinuxContainerExecutor due to null PrivilegedOperationException exit code. Contributed by Jason Lowe

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<String> 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()) {
+        String output = e.getOutput();
-          builder.append("Shell output: " + e.getOutput() + "\n");
+        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<String> cgroupKVs, String hierarchy)
 
       mountCGroupsOp.appendArgs(cgroupKVs);
       PrivilegedOperationExecutor privilegedOperationExecutor =
-          PrivilegedOperationExecutor.getInstance(conf);
+          getPrivilegedOperationExecutor();
 
       privilegedOperationExecutor.executePrivilegedOperation(mountCGroupsOp,
           false);

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
+  public PrivilegedOperationException(Throwable cause, int exitCode,
-      output, String errorOutput) {
+      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;
   }
 

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 = "<unknown>";
 
-  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;
   }
 

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<String, String> 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<String>(), "hierarchy");
+      Assert.fail("mountCgroups should have thrown an exception");
+    } catch (IOException e) {
+      assertTrue("Unexpected exception " + e,
+          e.getMessage().contains("exit code"));
+    }
+  }
 }