HADOOP-12441. Fixed shell-kill command behaviour to work correctly on some Linux distributions after HADOOP-12317. Contributed by Wangda Tan.

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -1185,6 +1185,9 @@ Release 2.8.0 - UNRELEASED
     HADOOP-12442. Display help if the command option to 'hdfs dfs' is not valid
     (nijel via vinayakumarb)
 
+    HADOOP-12441. Fixed shell-kill command behaviour to work correctly on some
+    Linux distributions after HADOOP-12317. (Wangda Tan via vinodkv)
+
 Release 2.7.2 - UNRELEASED
 
   INCOMPATIBLE CHANGES

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java

@@ -210,20 +210,26 @@ abstract public class Shell {
 
   /** Return a command for determining if process with specified pid is alive. */
   public static String[] getCheckProcessIsAliveCommand(String pid) {
-    return Shell.WINDOWS ?
+    return getSignalKillCommand(0, pid);
-      new String[] { Shell.WINUTILS, "task", "isAlive", pid } :
-      isSetsidAvailable ?
-        new String[] { "kill", "-0", "--", "-" + pid } :
-        new String[] { "kill", "-0", pid };
   }
 
   /** Return a command to send a signal to a given pid */
   public static String[] getSignalKillCommand(int code, String pid) {
-    return Shell.WINDOWS ?
+    // Code == 0 means check alive
-      new String[] { Shell.WINUTILS, "task", "kill", pid } :
+    if (Shell.WINDOWS) {
-      isSetsidAvailable ?
+      if (0 == code) {
-        new String[] { "kill", "-" + code, "--", "-" + pid } :
+        return new String[] { Shell.WINUTILS, "task", "isAlive", pid };
-        new String[] { "kill", "-" + code, pid };
+      } else {
+        return new String[] { Shell.WINUTILS, "task", "kill", pid };
+      }
+    }
+
+    if (isSetsidAvailable) {
+      // Use the shell-builtin as it support "--" in all Hadoop supported OSes
+      return new String[] { "bash", "-c", "kill -" + code + " -- -" + pid };
+    } else {
+      return new String[] { "bash", "-c", "kill -" + code + " " + pid };
+    }
   }
 
   public static final String ENV_NAME_REGEX = "[A-Za-z_][A-Za-z0-9_]*";
@@ -386,6 +392,26 @@ abstract public class Shell {
     return winUtilsPath;
   }
 
+  public static final boolean isBashSupported = checkIsBashSupported();
+  private static boolean checkIsBashSupported() {
+    if (Shell.WINDOWS) {
+      return false;
+    }
+
+    ShellCommandExecutor shexec;
+    boolean supported = true;
+    try {
+      String[] args = {"bash", "-c", "echo 1000"};
+      shexec = new ShellCommandExecutor(args);
+      shexec.execute();
+    } catch (IOException ioe) {
+      LOG.warn("Bash is not supported by the OS", ioe);
+      supported = false;
+    }
+
+    return supported;
+  }
+
   public static final boolean isSetsidAvailable = isSetsidSupported();
   private static boolean isSetsidSupported() {
     if (Shell.WINDOWS) {

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java

@@ -163,9 +163,9 @@ public class TestShell extends TestCase {
       expectedCommand =
           new String[]{ Shell.WINUTILS, "task", "isAlive", anyPid };
     } else if (Shell.isSetsidAvailable) {
-      expectedCommand = new String[]{ "kill", "-0", "--", "-" + anyPid };
+      expectedCommand = new String[] { "bash", "-c", "kill -0 -- -" + anyPid };
     } else {
-      expectedCommand = new String[]{"kill", "-0", anyPid};
+      expectedCommand = new String[]{ "bash", "-c", "kill -0 " + anyPid };
     }
     Assert.assertArrayEquals(expectedCommand, checkProcessAliveCommand);
   }
@@ -177,15 +177,14 @@ public class TestShell extends TestCase {
         anyPid);
 
     String[] expectedCommand;
+
     if (Shell.WINDOWS) {
       expectedCommand =
-          new String[]{ Shell.WINUTILS, "task", "kill", anyPid };
+          new String[]{ Shell.WINUTILS, "task", "isAlive", anyPid };
     } else if (Shell.isSetsidAvailable) {
-      expectedCommand =
+      expectedCommand = new String[] { "bash", "-c", "kill -9 -- -" + anyPid };
-          new String[]{ "kill", "-" + anySignal, "--", "-" + anyPid };
     } else {
-      expectedCommand =
+      expectedCommand = new String[]{ "bash", "-c", "kill -9 " + anyPid };
-          new String[]{ "kill", "-" + anySignal, anyPid };
     }
     Assert.assertArrayEquals(expectedCommand, checkProcessAliveCommand);
   }

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

@@ -43,6 +43,7 @@ import org.apache.hadoop.util.GenericOptionsParser;
 import org.apache.hadoop.util.JvmPauseMonitor;
 import org.apache.hadoop.util.NodeHealthScriptRunner;
 import org.apache.hadoop.util.ReflectionUtils;
+import org.apache.hadoop.util.Shell;
 import org.apache.hadoop.util.ShutdownHookManager;
 import org.apache.hadoop.util.StringUtils;
 import org.apache.hadoop.yarn.YarnUncaughtExceptionHandler;
@@ -595,6 +596,17 @@ public class NodeManager extends CompositeService
 
   private void initAndStartNodeManager(Configuration conf, boolean hasToReboot) {
     try {
+      // Failed to start if we're a Unix based system but we don't have bash.
+      // Bash is necessary to launch containers under Unix-based systems.
+      if (!Shell.WINDOWS) {
+        if (!Shell.isBashSupported) {
+          String message =
+              "Failing NodeManager start since we're on a "
+                  + "Unix-based system but bash doesn't seem to be available.";
+          LOG.fatal(message);
+          throw new YarnRuntimeException(message);
+        }
+      }
 
       // Remove the old hook if we are rebooting.
       if (hasToReboot && null != nodeManagerShutdownHook) {