modify ProcessUtils & ProcessUtilsTest

dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/ProcessUtils.java

@@ -67,14 +67,7 @@ public class ProcessUtils {
         String cmdstr;
         String[] cmd = commandList.toArray(new String[0]);
         SecurityManager security = System.getSecurityManager();
-        boolean allowAmbiguousCommands = false;
-        if (security == null) {
-            allowAmbiguousCommands = true;
-            String value = System.getProperty("jdk.lang.Process.allowAmbiguousCommands");
-            if (value != null) {
-                allowAmbiguousCommands = !"false".equalsIgnoreCase(value);
-            }
-        }
+        boolean allowAmbiguousCommands = isAllowAmbiguousCommands(security);
         if (allowAmbiguousCommands) {
 
             String executablePath = new File(cmd[0]).getPath();
@@ -112,6 +105,24 @@ public class ProcessUtils {
         return cmdstr;
     }
 
+    /**
+     * check is allow ambiguous commands
+     *
+     * @param security security manager
+     * @return allow ambiguous command flag
+     */
+    private static boolean isAllowAmbiguousCommands(SecurityManager security) {
+        boolean allowAmbiguousCommands = false;
+        if (security == null) {
+            allowAmbiguousCommands = true;
+            String value = System.getProperty("jdk.lang.Process.allowAmbiguousCommands");
+            if (value != null) {
+                allowAmbiguousCommands = !"false".equalsIgnoreCase(value);
+            }
+        }
+        return allowAmbiguousCommands;
+    }
+
     /**
      * get executable path.
      *
@@ -165,8 +176,10 @@ public class ProcessUtils {
      * Lazy Pattern.
      */
     private static class LazyPattern {
-        // Escape-support version:
-        // "(\")((?:\\\\\\1|.)+?)\\1|([^\\s\"]+)";
+        /**
+         * Escape-support version:
+         * "(\")((?:\\\\\\1|.)+?)\\1|([^\\s\"]+)";
+         */
         private static final Pattern PATTERN = Pattern.compile("[^\\s\"]+|\"[^\"]*\"");
     }
 
@@ -234,20 +247,16 @@ public class ProcessUtils {
         int lastPos = arg.length() - 1;
         if (lastPos >= 1 && arg.charAt(0) == '"' && arg.charAt(lastPos) == '"') {
             // The argument has already been quoted.
-            if (noQuotesInside) {
-                if (arg.indexOf('"', 1) != lastPos) {
+            if (noQuotesInside && arg.indexOf('"', 1) != lastPos) {
                 // There is ["] inside.
                 throw new IllegalArgumentException(errorMessage);
             }
-            }
             return true;
         }
-        if (noQuotesInside) {
-            if (arg.indexOf('"') >= 0) {
+        if (noQuotesInside && arg.indexOf('"') >= 0) {
             // There is ["] inside.
             throw new IllegalArgumentException(errorMessage);
         }
-        }
         return false;
     }
 
@@ -292,6 +301,25 @@ public class ProcessUtils {
                         String commandFile = String
                             .format("%s/%s.kill", executePath, appId);
                         String cmd = "yarn application -kill " + appId;
+                        execYarnKillCommand(logger, tenantCode, appId, commandFile, cmd);
+                    }
+                } catch (Exception e) {
+                    logger.error(String.format("Get yarn application app id [%s] status failed: [%s]", appId, e.getMessage()));
+                }
+            }
+        }
+    }
+
+    /**
+     * build kill command for yarn application
+     *
+     * @param logger      logger
+     * @param tenantCode  tenant code
+     * @param appId       app id
+     * @param commandFile command file
+     * @param cmd         cmd
+     */
+    private static void execYarnKillCommand(Logger logger, String tenantCode, String appId, String commandFile, String cmd) {
         try {
             StringBuilder sb = new StringBuilder();
             sb.append("#!/bin/sh\n");
@@ -315,18 +343,11 @@ public class ProcessUtils {
             }
 
             logger.info("kill cmd:{}", runCmd);
-
             Runtime.getRuntime().exec(runCmd);
         } catch (Exception e) {
             logger.error(String.format("Kill yarn application app id [%s] failed: [%s]", appId, e.getMessage()));
         }
     }
-                } catch (Exception e) {
-                    logger.error(String.format("Get yarn application app id [%s] status failed: [%s]", appId, e.getMessage()));
-                }
-            }
-        }
-    }
 
     /**
      * kill tasks according to different task types.
@@ -360,7 +381,7 @@ public class ProcessUtils {
      * get pids str.
      *
      * @param processId process id
-     * @return pids
+     * @return pids pid String
      * @throws Exception exception
      */
     public static String getPidsStr(int processId) throws Exception {
@@ -413,7 +434,7 @@ public class ProcessUtils {
                     logger.error("task instance work dir is empty");
                     throw new RuntimeException("task instance work dir is empty");
                 }
-                if (appIds.size() > 0) {
+                if (CollectionUtils.isNotEmpty(appIds)) {
                     cancelApplication(appIds, logger, taskExecutionContext.getTenantCode(), taskExecutionContext.getExecutePath());
                 }
             }

dolphinscheduler-server/src/test/java/org/apache/dolphinscheduler/server/utils/ProcessUtilsTest.java

@@ -59,7 +59,7 @@ public class ProcessUtilsTest {
         when(OSUtils.isMacOS()).thenReturn(true);
         when(OSUtils.exeCmd("pstree -sp " + processId)).thenReturn(null);
         String pidListMac = ProcessUtils.getPidsStr(processId);
-        Assert.assertEquals(pidListMac, "");
+        Assert.assertEquals("", pidListMac);
     }
 
     @Test
@@ -69,18 +69,18 @@ public class ProcessUtilsTest {
         commands.add("-u");
         commands.add("tenantCode");
         //allowAmbiguousCommands false
-        Assert.assertEquals(ProcessUtils.buildCommandStr(commands), "sudo -u tenantCode");
+        Assert.assertEquals("sudo -u tenantCode", ProcessUtils.buildCommandStr(commands));
 
         //quota
         commands.clear();
         commands.add("\"sudo\"");
-        Assert.assertEquals(ProcessUtils.buildCommandStr(commands), "\"sudo\"");
+        Assert.assertEquals("\"sudo\"", ProcessUtils.buildCommandStr(commands));
 
         //allowAmbiguousCommands true
         commands.clear();
         commands.add("sudo");
         System.setProperty("jdk.lang.Process.allowAmbiguousCommands", "false");
-        Assert.assertEquals(ProcessUtils.buildCommandStr(commands), "\"sudo\"");
+        Assert.assertEquals("\"sudo\"", ProcessUtils.buildCommandStr(commands));
     }
 
     @Test
@@ -105,6 +105,7 @@ public class ProcessUtilsTest {
         taskExecutionContext.setHost("127.0.0.1:8888");
         taskExecutionContext.setLogPath("/log/1.log");
         ProcessUtils.kill(taskExecutionContext);
+        Assert.assertEquals(1, taskExecutionContext.getProcessId());
     }
 
     @Test
@@ -129,8 +130,9 @@ public class ProcessUtilsTest {
             when(hadoop.getApplicationStatus("application_1598885606600_3677")).thenReturn(running);
         } catch (Exception e) {
             e.printStackTrace();
-
             ProcessUtils.cancelApplication(appIds, logger, tenantCode, executePath);
         }
+
+        Assert.assertNotNull(appIds);
     }
 }