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

This reverts commit f76f5c0919.

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/LinuxContainerExecutor.java

@@ -275,10 +275,6 @@ public class LinuxContainerExecutor extends ContainerExecutor {
     }
   }
 
-  protected PrivilegedOperationExecutor getPrivilegedOperationExecutor() {
-    return PrivilegedOperationExecutor.getInstance(getConf());
-  }
-
   @Override
   public void init() throws IOException {
     Configuration conf = super.getConf();
@@ -289,7 +285,7 @@ public class LinuxContainerExecutor extends ContainerExecutor {
       PrivilegedOperation checkSetupOp = new PrivilegedOperation(
           PrivilegedOperation.OperationType.CHECK_SETUP);
       PrivilegedOperationExecutor privilegedOperationExecutor =
-          getPrivilegedOperationExecutor();
+          PrivilegedOperationExecutor.getInstance(conf);
 
       privilegedOperationExecutor.executePrivilegedOperation(checkSetupOp,
           false);
@@ -386,7 +382,7 @@ public class LinuxContainerExecutor extends ContainerExecutor {
     try {
       Configuration conf = super.getConf();
       PrivilegedOperationExecutor privilegedOperationExecutor =
-          getPrivilegedOperationExecutor();
+          PrivilegedOperationExecutor.getInstance(conf);
 
       privilegedOperationExecutor.executePrivilegedOperation(prefixCommands,
           initializeContainerOp, null, null, false, true);
@@ -534,9 +530,8 @@ public class LinuxContainerExecutor extends ContainerExecutor {
         }
         builder.append("Stack trace: "
             + StringUtils.stringifyException(e) + "\n");
-        String output = e.getOutput();
+        if (!e.getOutput().isEmpty()) {
-        if (output!= null && !e.getOutput().isEmpty()) {
+          builder.append("Shell output: " + e.getOutput() + "\n");
-          builder.append("Shell output: " + output + "\n");
         }
         String diagnostics = builder.toString();
         logOutput(diagnostics);
@@ -734,7 +729,7 @@ public class LinuxContainerExecutor extends ContainerExecutor {
     try {
       Configuration conf = super.getConf();
       PrivilegedOperationExecutor privilegedOperationExecutor =
-          getPrivilegedOperationExecutor();
+          PrivilegedOperationExecutor.getInstance(conf);
 
       privilegedOperationExecutor.executePrivilegedOperation(deleteAsUserOp,
           false);
@@ -764,7 +759,7 @@ public class LinuxContainerExecutor extends ContainerExecutor {
 
     try {
       PrivilegedOperationExecutor privOpExecutor =
-          getPrivilegedOperationExecutor();
+          PrivilegedOperationExecutor.getInstance(super.getConf());
 
       String results =
           privOpExecutor.executePrivilegedOperation(listAsUserOp, true);
@@ -823,7 +818,7 @@ public class LinuxContainerExecutor extends ContainerExecutor {
 
       mountCGroupsOp.appendArgs(cgroupKVs);
       PrivilegedOperationExecutor privilegedOperationExecutor =
-          getPrivilegedOperationExecutor();
+          PrivilegedOperationExecutor.getInstance(conf);
 
       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 @@ import org.apache.hadoop.yarn.exceptions.YarnException;
 
 public class PrivilegedOperationException extends YarnException {
   private static final long serialVersionUID = 1L;
-  private int exitCode = -1;
+  private Integer exitCode;
   private String output;
   private String errorOutput;
 
@@ -36,7 +36,7 @@ public class PrivilegedOperationException extends YarnException {
     super(message);
   }
 
-  public PrivilegedOperationException(String message, int exitCode,
+  public PrivilegedOperationException(String message, Integer exitCode,
       String output, String errorOutput) {
     super(message);
     this.exitCode = exitCode;
@@ -48,8 +48,8 @@ public class PrivilegedOperationException extends YarnException {
     super(cause);
   }
 
-  public PrivilegedOperationException(Throwable cause, int exitCode,
+  public PrivilegedOperationException(Throwable cause, Integer exitCode, String
-      String output, String errorOutput) {
+      output, String errorOutput) {
     super(cause);
     this.exitCode = exitCode;
     this.output = output;
@@ -59,7 +59,7 @@ public class PrivilegedOperationException extends YarnException {
     super(message, cause);
   }
 
-  public int getExitCode() {
+  public Integer 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 @@ import org.apache.hadoop.yarn.exceptions.YarnException;
 @InterfaceStability.Unstable
 public class ContainerExecutionException extends YarnException {
   private static final long serialVersionUID = 1L;
-  private static final int EXIT_CODE_UNSET = -1;
+  private static final Integer EXIT_CODE_UNSET = -1;
   private static final String OUTPUT_UNSET = "<unknown>";
 
-  private int exitCode;
+  private Integer exitCode;
   private String output;
   private String errorOutput;
 
@@ -54,7 +54,7 @@ public class ContainerExecutionException extends YarnException {
   }
 
 
-  public ContainerExecutionException(String message, int exitCode, String
+  public ContainerExecutionException(String message, Integer exitCode, String
       output, String errorOutput) {
     super(message);
     this.exitCode = exitCode;
@@ -62,7 +62,7 @@ public class ContainerExecutionException extends YarnException {
     this.errorOutput = errorOutput;
   }
 
-  public ContainerExecutionException(Throwable cause, int exitCode, String
+  public ContainerExecutionException(Throwable cause, Integer exitCode, String
       output, String errorOutput) {
     super(cause);
     this.exitCode = exitCode;
@@ -70,7 +70,7 @@ public class ContainerExecutionException extends YarnException {
     this.errorOutput = errorOutput;
   }
 
-  public int getExitCode() {
+  public Integer 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,9 +23,7 @@ import static org.junit.Assert.assertEquals;
 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;
@@ -42,7 +40,6 @@ import java.util.Arrays;
 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;
@@ -50,8 +47,6 @@ import org.apache.hadoop.conf.Configuration;
 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;
@@ -59,7 +54,6 @@ import org.apache.hadoop.yarn.exceptions.ConfigurationException;
 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;
@@ -522,87 +516,4 @@ public class TestLinuxContainerExecutorWithMocks {
         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"));
-    }
-  }
 }