populate: stop the ssh master also when a signal ends populate

The shell doesn't run the EXIT trap when a signal ends it, so an
interrupted deploy leaves the ssh master, for an hour, and populate's
temporary directory.  Populate traps HUP, INT, and TERM and exits, which
runs the EXIT trap.

The shell runs a trap once the command it waits for ends.  A signal to
the process group, like the one of an interrupt key, of a closed terminal,
or of timeout, ends that command as well.  A signal to populate alone
takes effect after the step that is running.

Phases of scenarios can end populate after some seconds.  An edge case
does so while a pipe source's command sleeps, and checks that neither the
ssh master nor the temporary directory remain.

4 files changed, 50 insertions, 6 deletions

diff --git a/pkgs/populate/default.nix b/pkgs/populate/default.nix
index c8229fb..97c2c4b 100644
--- a/pkgs/populate/default.nix
+++ b/pkgs/populate/default.nix
@@ -1338,7 +1338,15 @@ writers.writeDash "populate.${target.host}" ''
       ${ssh' target} -q -O stop ${quote target.host}
     fi
   }
-  trap 'stop_ssh_master; rm -R "$TMPDIR"' EXIT
+  trap 'trap "" HUP INT TERM; stop_ssh_master; rm -R "$TMPDIR"' EXIT
+  # A signal ends the shell without running the trap above, unless it's
+  # trapped itself.  The shell runs these traps once the command it waits
+  # for ends, which a signal to the process group, like the one of an
+  # interrupt key, ends as well.  Further signals get ignored, so they
+  # don't end the trap above halfway.
+  trap 'trap "" HUP INT TERM; exit 129' HUP
+  trap 'trap "" HUP INT TERM; exit 130' INT
+  trap 'trap "" HUP INT TERM; exit 143' TERM
 
   ${check { inherit force target; }}
   set -x
diff --git a/tests/fixtures.sh b/tests/fixtures.sh
index 4e794db..ccd55a0 100755
--- a/tests/fixtures.sh
+++ b/tests/fixtures.sh
@@ -337,6 +337,14 @@ echo piped
 SH
 chmod +x /fixtures/pipe
 
+# pipe-slow: a command that takes longer than the tests wait for it.
+cat > /fixtures/pipe-slow <<'SH'
+#! /bin/sh
+sleep 60
+echo piped
+SH
+chmod +x /fixtures/pipe-slow
+
 # patches: hello and hello2 change dep's greeting, hello2 only after hello;
 # sub changes the flake in mono's subdirectory; bad applies to nothing.
 mkdir -p /fixtures/patches
diff --git a/tests/scenarios.nix b/tests/scenarios.nix
index 2fb6609..3113586 100644
--- a/tests/scenarios.nix
+++ b/tests/scenarios.nix
@@ -97,10 +97,11 @@ let
     # targetPath deploys to another path than the scenario's target
     # directory, and controlTargetPath is the control for a path that makes
     # evaluation fail.  env sets environment variables for populate, as
-    # arguments of env.
+    # arguments of env.  interrupt ends populate after that many seconds,
+    # which it's expected not to finish before.
     phase = {
       sources, backup ? false, prune ? false, deploy ? null,
-      targetPath ? path, env ? "",
+      targetPath ? path, env ? "", interrupt ? null,
       expect ? "success", control ? null, controlTargetPath ? null,
       check ? "", mutate ? null, checkDeployer ? null, mutateDeployer ? null,
     }: let
@@ -135,7 +136,7 @@ let
           null
       else {
         populate = toString (command sources targetPath);
-        inherit env;
+        inherit env interrupt;
         expect = if expect == "success" then null else expect.fail;
         # No phase gets around checking the canaries.
         check = script (check + "\ncanary_check\n");
@@ -1320,6 +1321,29 @@ assert !(krops.isLocalTarget (targets.remote "/"));
         }
       ];
     }))
+    # A signal ending populate leaves neither the ssh master nor the
+    # temporary directory.
+    (s "interrupt" {
+      prepareDeployer = ''
+        find "''${TMPDIR:-/tmp}" -maxdepth 1 -name 'populate.*' -exec rm -fR {} +
+      '';
+      phases = [{
+        sources.x.pipe = "/fixtures/pipe-slow";
+        interrupt = 3;
+        checkDeployer = ''
+          for i in 1 2 3 4 5 6 7 8 9 10; do
+            if ! pgrep -f 'ssh: .*/populate\..*\[mux\]' >/dev/null; then
+              break
+            fi
+            sleep 0.5
+          done
+          ! pgrep -f 'ssh: .*/populate\..*\[mux\]' >/dev/null ||
+            fail "ssh master remains"
+          expect_eq "temporary directories" \
+              "$(find "''${TMPDIR:-/tmp}" -maxdepth 1 -name 'populate.*')" ""
+        '';
+      }];
+    })
     # An input of an input drawn from a source that draws inputs itself
     # gets drawn from another source.
     (modes "override" ({ draw, flake, ... }: {
diff --git a/tests/vm.nix b/tests/vm.nix
index 014897a..a203962 100644
--- a/tests/vm.nix
+++ b/tests/vm.nix
@@ -151,11 +151,15 @@ in pkgs.testers.runNixOSTest {
                 run(node, f"sh {scenario['prepare']}")
                 for i, phase in enumerate(scenario["phases"]):
                     # A hang fails the scenario, not the whole test.
+                    limit = phase["interrupt"] or 600
                     status, out = deployer.execute(
-                        f"env HOME=/root {phase['env']} timeout 600 {phase['populate']} 2>&1",
+                        f"env HOME=/root {phase['env']} timeout {limit} {phase['populate']} 2>&1",
                         timeout=900,
                     )
-                    if phase["expect"] is None:
+                    if phase["interrupt"] is not None:
+                        if status != 124:
+                            raise Exception(f"populate {i} ends on its own:\n{out}")
+                    elif phase["expect"] is None:
                         if status != 0:
                             raise Exception(f"populate {i} fails:\n{out}")
                     elif status == 0 or phase["expect"] not in out: