populate: stop the ssh master also after failures

Populate stopped the ssh master at the end of its script, which a failure
skips.  The master then stays for an hour, with its control socket gone
along with populate's temporary directory.  Populate stops it on exit
now, before removing that directory.

An edge case checks that no master remains after a failing and after a
succeeding deploy.

2 files changed, 32 insertions, 5 deletions

diff --git a/pkgs/populate/default.nix b/pkgs/populate/default.nix
index 75fbf61..7a52ca9 100644
--- a/pkgs/populate/default.nix
+++ b/pkgs/populate/default.nix
@@ -867,7 +867,14 @@ writers.writeDash "populate.${target.host}" ''
   TMPDIR=$(${coreutils}/bin/mktemp -dt populate.XXXXXXXX)
   readonly TMPDIR # Ensure the trap will delete the correct thing.
   export TMPDIR # Share it with the jobs and the ssh wrapper.
-  trap 'rm -R "$TMPDIR"' EXIT
+  # Stops the ssh master, if any source started one, also after failures,
+  # before its control socket goes away with $TMPDIR.
+  stop_ssh_master() {
+    if ${ssh' target} -q -O check ${quote target.host} 2>/dev/null; then
+      ${ssh' target} -q -O stop ${quote target.host}
+    fi
+  }
+  trap 'stop_ssh_master; rm -R "$TMPDIR"' EXIT
 
   ${check { inherit force target; }}
   set -x
@@ -876,8 +883,4 @@ writers.writeDash "populate.${target.host}" ''
   ${flakes target source}
   ${optionalString prune (prune' target source)}
 
-  # Stop the ssh master, if any source started one.
-  if ${ssh' target} -q -O check ${quote target.host} 2>/dev/null; then
-    ${ssh' target} -q -O stop ${quote target.host}
-  fi
 ''
diff --git a/tests/scenarios.nix b/tests/scenarios.nix
index 9760e54..307e521 100644
--- a/tests/scenarios.nix
+++ b/tests/scenarios.nix
@@ -843,6 +843,30 @@ assert !(krops.isLocalTarget (targets.remote "/"));
         }
       ];
     })
+    # The ssh master stops, after failing as well as after succeeding.
+    (s "ssh-master" (let
+      checkDeployer = ''
+        for i in 1 2 3 4 5 6 7 8 9 10; do
+          if ! pgrep -f 'ssh: .*/populate\..*\[mux\]' >/dev/null; then
+            exit 0
+          fi
+          sleep 0.5
+        done
+        fail "ssh master remains"
+      '';
+    in {
+      phases = [
+        {
+          sources.x.flake-input = { flake = "/fixtures/top"; name = "nope"; inherit fetchOnTarget; };
+          expect.fail = "error: flake /fixtures/top has no input nope";
+          inherit checkDeployer;
+        }
+        {
+          sources = alt;
+          inherit checkDeployer;
+        }
+      ];
+    }))
   ] ++ lib.optional (targetKind == "local")
     # The target has the path the input is locked to.
     (s "uselocal" {