fix(exec): cleaning up persistent processes didn't work in some cases

Shelling out to the `kill` command was not portable, and the parent process
also did not get the termination signal.

core/src/plugins/exec/deploy.ts

@@ -287,8 +287,8 @@ async function killProcess(log: Log, pidFilePath: string, deployName: string) {
       const oldPid = parseInt(pidString, 10)
       if (isRunning(oldPid)) {
         try {
-          await killRecursive("INT", oldPid)
-          log.debug(`Sent SIGINT to existing ${deployName} process (PID ${oldPid})`)
+          await killRecursive("SIGTERM", oldPid)
+          log.debug(`Sent SIGTERM to existing ${deployName} process (PID ${oldPid})`)
         } catch (err) {
           // This most likely means that the process had already been terminated, which is fine for our purposes here.
           log.debug(`An error occurred while deleting existing ${deployName} process (PID ${oldPid}): ${err.message}`)
@@ -311,6 +311,7 @@ async function resetLogFile(logFilePath: string) {
 
 function runPersistent({
   action,
+  log,
   env,
   deployName,
   logFilePath,
@@ -345,6 +346,8 @@ function runPersistent({
   const shell = !!action.getSpec().shell
   const { cmd, args } = convertCommandSpec(action.getSpec("deployCommand"), shell)
 
+  log.debug(`Starting command '${cmd} ${args.join(" ")}'`)
+
   const proc = execa(cmd, args, {
     cwd: action.getBuildPath(),
     env: {

core/src/process.ts

@@ -11,7 +11,7 @@ import Bluebird from "bluebird"
 import { Garden } from "./garden"
 import { Log } from "./logger/log-entry"
 import { GardenProcess, GlobalConfigStore } from "./config-store/global"
-import { SpawnOutput, sleep, spawn } from "./util/util"
+import { sleep } from "./util/util"
 import psTree from "ps-tree"
 
 export async function waitForExitEvent(garden: Garden, log: Log) {
@@ -77,21 +77,24 @@ export async function registerProcess(
 /**
  * Kills the process with the provided pid, and any of its child processes.
  *
- * `signalName` should be a POSIX kill signal, e.g. + `INT` or `KILL`
+ * `signalName` should be a POSIX kill signal, e.g. `SIGINT` or `SIGKILL`
  *
  * See: https://github.com/sindresorhus/execa/issues/96#issuecomment-776280798
  */
-export async function killRecursive(signalName: string, pid: number) {
-  return new Promise<SpawnOutput>((resolve, reject) => {
+export async function killRecursive(signalName: "SIGINT" | "SIGTERM" | "SIGKILL", pid: number) {
+  return new Promise<void>((resolve) => {
     psTree(pid, function (_err, children) {
-      const killArgs = ["-s", signalName, "" + pid].concat(
-          children.map(function (p) {
-            return p.PID
-          })
-        )
-      spawn("kill", killArgs)
-        .then(resolve)
-        .catch(reject)
+      for (const p of children) {
+        try {
+          process.kill(parseInt(p.PID, 10), signalName)
+        } catch {
+          continue
+        }
+      }
+      try {
+        process.kill(pid, signalName)
+      } catch {}
+      resolve()
     })
   })
 }

core/test/data/test-project-exec/module-local-persistent/garden.yml

@@ -5,7 +5,7 @@ local: true
 services:
   - name: sync-mode
     syncMode:
-      command: [/bin/sh -c "while true; do sleep 10000; done"]
+      command: [/bin/sh -c "while true; do sleep 1000; done"]
     deployCommand: []
   - name: sync-mode-with-logs
     syncMode:
@@ -18,7 +18,7 @@ services:
   - name: sync-mode-timeout
     persistent: true
     devMode:
-      command: [/bin/sh -c "while true; do sleep 10000; done"]
+      command: [/bin/sh -c "while true; do sleep 1000; done"]
       statusCommand: [/bin/sh -c "echo Status command output; exit 1"]
       timeout: 3
     deployCommand: []

core/test/unit/src/plugins/exec/exec.ts

@@ -757,7 +757,7 @@ describe("exec plugin", () => {
         afterEach(async () => {
           if (pid > 1) {
             try {
-              await killRecursive("KILL", pid)
+              await killRecursive("SIGKILL", pid)
             } catch (_err) {}
           }
         })