nixos/switch-to-configuration: Rework activation script restarts

This removes `/run/nixos/activation-reload-list` (which we will need in
the future when reworking the reload logic) and makes
`/run/nixos/activation-restart-list` honor `restartIfChanged` and
`reloadIfChanged`. This way activation scripts don't have to bother with
choosing between reloading and restarting.

nixos/doc/manual/from_md/release-notes/rl-2205.section.xml

@@ -377,6 +377,16 @@
           include serif fonts.
         </para>
       </listitem>
+      <listitem>
+        <para>
+          The interface that allows activation scripts to restart units
+          has been reworked. Restarting and reloading is now done by a
+          single file
+          <literal>/run/nixos/activation-restart-list</literal> that
+          honors <literal>restartIfChanged</literal> and
+          <literal>reloadIfChanged</literal> of the units.
+        </para>
+      </listitem>
     </itemizedlist>
   </section>
   <section xml:id="sec-release-22.05-notable-changes">

nixos/doc/manual/release-notes/rl-2205.section.md

@@ -119,6 +119,8 @@ In addition to numerous new and upgraded packages, this release has the followin
   `pkgs.noto-fonts-cjk` is currently an alias of `pkgs.noto-fonts-cjk-sans` and
   doesn't include serif fonts.
 
+- The interface that allows activation scripts to restart units has been reworked. Restarting and reloading is now done by a single file `/run/nixos/activation-restart-list` that honors `restartIfChanged` and `reloadIfChanged` of the units.
+
 ## Other Notable Changes {#sec-release-22.05-notable-changes}
 
 - The option [services.redis.servers](#opt-services.redis.servers) was added

nixos/modules/system/activation/switch-to-configuration.pl

@@ -18,11 +18,13 @@ my $startListFile = "/run/nixos/start-list";
 my $restartListFile = "/run/nixos/restart-list";
 my $reloadListFile = "/run/nixos/reload-list";
 
-# Parse restart/reload requests by the activation script
+# Parse restart/reload requests by the activation script.
+# Activation scripts may write newline-separated units to this
+# file and switch-to-configuration will handle them. While
+# `stopIfChanged = true` is ignored, switch-to-configuration will
+# handle `restartIfChanged = false` and `reloadIfChanged = true`.
 my $restartByActivationFile = "/run/nixos/activation-restart-list";
-my $reloadByActivationFile = "/run/nixos/activation-reload-list";
 my $dryRestartByActivationFile = "/run/nixos/dry-activation-restart-list";
-my $dryReloadByActivationFile = "/run/nixos/dry-activation-reload-list";
 
 make_path("/run/nixos", { mode => oct(755) });
 
@@ -382,7 +384,6 @@ sub filterUnits {
 }
 
 my @unitsToStopFiltered = filterUnits(\%unitsToStop);
-my @unitsToStartFiltered = filterUnits(\%unitsToStart);
 
 
 # Show dry-run actions.
@@ -395,21 +396,39 @@ if ($action eq "dry-activate") {
     print STDERR "would activate the configuration...\n";
     system("$out/dry-activate", "$out");
 
-    $unitsToRestart{$_} = 1 foreach
-        split('\n', read_file($dryRestartByActivationFile, err_mode => 'quiet') // "");
+    # Handle the activation script requesting the restart or reload of a unit.
+    foreach (split('\n', read_file($dryRestartByActivationFile, err_mode => 'quiet') // "")) {
+        my $unit = $_;
+        my $baseUnit = $unit;
+        my $newUnitFile = "$out/etc/systemd/system/$baseUnit";
+
+        # Detect template instances.
+        if (!-e $newUnitFile && $unit =~ /^(.*)@[^\.]*\.(.*)$/) {
+          $baseUnit = "$1\@.$2";
+          $newUnitFile = "$out/etc/systemd/system/$baseUnit";
+        }
+
+        my $baseName = $baseUnit;
+        $baseName =~ s/\.[a-z]*$//;
 
-    $unitsToReload{$_} = 1 foreach
-        split('\n', read_file($dryReloadByActivationFile, err_mode => 'quiet') // "");
+        # Start units if they were not active previously
+        if (not defined $activePrev->{$unit}) {
+            $unitsToStart{$unit} = 1;
+            next;
+        }
+
+        handleModifiedUnit($unit, $baseName, $newUnitFile, $activePrev, \%unitsToRestart, \%unitsToRestart, \%unitsToReload, \%unitsToRestart, \%unitsToSkip);
+    }
+    unlink($dryRestartByActivationFile);
 
     print STDERR "would restart systemd\n" if $restartSystemd;
     print STDERR "would reload the following units: ", join(", ", sort(keys %unitsToReload)), "\n"
         if scalar(keys %unitsToReload) > 0;
     print STDERR "would restart the following units: ", join(", ", sort(keys %unitsToRestart)), "\n"
         if scalar(keys %unitsToRestart) > 0;
+    my @unitsToStartFiltered = filterUnits(\%unitsToStart);
     print STDERR "would start the following units: ", join(", ", @unitsToStartFiltered), "\n"
         if scalar @unitsToStartFiltered;
-    unlink($dryRestartByActivationFile);
-    unlink($dryReloadByActivationFile);
     exit 0;
 }
 
@@ -433,13 +452,31 @@ print STDERR "activating the configuration...\n";
 system("$out/activate", "$out") == 0 or $res = 2;
 
 # Handle the activation script requesting the restart or reload of a unit.
-# We can only restart and reload (not stop/start) because the units to be
-# stopped are already stopped before the activation script is run.
-$unitsToRestart{$_} = 1 foreach
-    split('\n', read_file($restartByActivationFile, err_mode => 'quiet') // "");
+foreach (split('\n', read_file($restartByActivationFile, err_mode => 'quiet') // "")) {
+    my $unit = $_;
+    my $baseUnit = $unit;
+    my $newUnitFile = "$out/etc/systemd/system/$baseUnit";
 
-$unitsToReload{$_} = 1 foreach
-    split('\n', read_file($reloadByActivationFile, err_mode => 'quiet') // "");
+    # Detect template instances.
+    if (!-e $newUnitFile && $unit =~ /^(.*)@[^\.]*\.(.*)$/) {
+      $baseUnit = "$1\@.$2";
+      $newUnitFile = "$out/etc/systemd/system/$baseUnit";
+    }
+
+    my $baseName = $baseUnit;
+    $baseName =~ s/\.[a-z]*$//;
+
+    # Start units if they were not active previously
+    if (not defined $activePrev->{$unit}) {
+        $unitsToStart{$unit} = 1;
+        recordUnit($startListFile, $unit);
+        next;
+    }
+
+    handleModifiedUnit($unit, $baseName, $newUnitFile, $activePrev, \%unitsToRestart, \%unitsToRestart, \%unitsToReload, \%unitsToRestart, \%unitsToSkip);
+}
+# We can remove the file now because it has been propagated to the other restart/reload files
+unlink($restartByActivationFile);
 
 # Restart systemd if necessary. Note that this is done using the
 # current version of systemd, just in case the new one has trouble
@@ -480,7 +517,6 @@ if (scalar(keys %unitsToReload) > 0) {
     print STDERR "reloading the following units: ", join(", ", sort(keys %unitsToReload)), "\n";
     system("@systemd@/bin/systemctl", "reload", "--", sort(keys %unitsToReload)) == 0 or $res = 4;
     unlink($reloadListFile);
-    unlink($reloadByActivationFile);
 }
 
 # Restart changed services (those that have to be restarted rather
@@ -489,7 +525,6 @@ if (scalar(keys %unitsToRestart) > 0) {
     print STDERR "restarting the following units: ", join(", ", sort(keys %unitsToRestart)), "\n";
     system("@systemd@/bin/systemctl", "restart", "--", sort(keys %unitsToRestart)) == 0 or $res = 4;
     unlink($restartListFile);
-    unlink($restartByActivationFile);
 }
 
 # Start all active targets, as well as changed units we stopped above.
@@ -498,6 +533,7 @@ if (scalar(keys %unitsToRestart) > 0) {
 # that are symlinks to other units.  We shouldn't start both at the
 # same time because we'll get a "Failed to add path to set" error from
 # systemd.
+my @unitsToStartFiltered = filterUnits(\%unitsToStart);
 print STDERR "starting the following units: ", join(", ", @unitsToStartFiltered), "\n"
     if scalar @unitsToStartFiltered;
 system("@systemd@/bin/systemctl", "start", "--", sort(keys %unitsToStart)) == 0 or $res = 4;

nixos/tests/switch-test.nix

@@ -45,6 +45,50 @@ import ./make-test-python.nix ({ pkgs, ...} : {
           systemd.services.test.restartIfChanged = false;
         };
 
+        restart-and-reload-by-activation-script.configuration = {
+          systemd.services = rec {
+            simple-service = {
+              # No wantedBy so we can check if the activation script restart triggers them
+              serviceConfig = {
+                Type = "oneshot";
+                RemainAfterExit = true;
+                ExecStart = "${pkgs.coreutils}/bin/true";
+                ExecReload = "${pkgs.coreutils}/bin/true";
+              };
+            };
+
+            simple-restart-service = simple-service // {
+              stopIfChanged = false;
+            };
+
+            simple-reload-service = simple-service // {
+              reloadIfChanged = true;
+            };
+
+            no-restart-service = simple-service // {
+              restartIfChanged = false;
+            };
+          };
+
+          system.activationScripts.restart-and-reload-test = {
+            supportsDryActivation = true;
+            deps = [];
+            text = ''
+              if [ "$NIXOS_ACTION" = dry-activate ]; then
+                f=/run/nixos/dry-activation-restart-list
+              else
+                f=/run/nixos/activation-restart-list
+              fi
+              cat <<EOF >> "$f"
+              simple-service.service
+              simple-restart-service.service
+              simple-reload-service.service
+              no-restart-service.service
+              EOF
+            '';
+          };
+        };
+
         mount.configuration = {
           systemd.mounts = [
             {
@@ -261,6 +305,32 @@ import ./make-test-python.nix ({ pkgs, ...} : {
         assert_lacks(out, "as well:")
         assert_contains(out, "would start the following units: test.service\n")
 
+    with subtest("restart and reload by activation script"):
+        out = switch_to_specialisation("${machine}", "restart-and-reload-by-activation-script")
+        assert_contains(out, "stopping the following units: test.service\n")
+        assert_lacks(out, "NOT restarting the following changed units:")
+        assert_lacks(out, "reloading the following units:")
+        assert_lacks(out, "restarting the following units:")
+        assert_contains(out, "\nstarting the following units: no-restart-service.service, simple-reload-service.service, simple-restart-service.service, simple-service.service\n")
+        assert_lacks(out, "as well:")
+        # Switch to the same system where the example services get restarted
+        # by the activation script
+        out = switch_to_specialisation("${machine}", "restart-and-reload-by-activation-script")
+        assert_lacks(out, "stopping the following units:")
+        assert_lacks(out, "NOT restarting the following changed units:")
+        assert_contains(out, "reloading the following units: simple-reload-service.service\n")
+        assert_contains(out, "restarting the following units: simple-restart-service.service, simple-service.service\n")
+        assert_lacks(out, "\nstarting the following units:")
+        assert_lacks(out, "as well:")
+        # The same, but in dry mode
+        out = switch_to_specialisation("${machine}", "restart-and-reload-by-activation-script", action="dry-activate")
+        assert_lacks(out, "would stop the following units:")
+        assert_lacks(out, "would NOT stop the following changed units:")
+        assert_contains(out, "would reload the following units: simple-reload-service.service\n")
+        assert_contains(out, "would restart the following units: simple-restart-service.service, simple-service.service\n")
+        assert_lacks(out, "\nwould start the following units:")
+        assert_lacks(out, "as well:")
+
     with subtest("mounts"):
         switch_to_specialisation("${machine}", "mount")
         out = machine.succeed("mount | grep 'on /testmount'")