nixos/switch-to-configuration: Fix reloading of stopped services

nixos/doc/manual/development/unit-handling.section.md

@@ -34,6 +34,9 @@ checks:
   - The rest of the units (mostly `.service` units) are then **reload**ed if
     `X-ReloadIfChanged` in the `[Service]` section is set to `true` (exposed
     via [systemd.services.\<name\>.reloadIfChanged](#opt-systemd.services)).
+    A little exception is done for units that were deactivated in the meantime,
+    for example because they require a unit that got stopped before. These
+    are **start**ed instead of reloaded.
 
   - If the reload flag is not set, some more flags decide if the unit is
     skipped. These flags are `X-RestartIfChanged` in the `[Service]` section

nixos/doc/manual/from_md/development/unit-handling.section.xml

@@ -72,6 +72,11 @@
             <literal>[Service]</literal> section is set to
             <literal>true</literal> (exposed via
             <link linkend="opt-systemd.services">systemd.services.&lt;name&gt;.reloadIfChanged</link>).
+            A little exception is done for units that were deactivated
+            in the meantime, for example because they require a unit
+            that got stopped before. These are
+            <emphasis role="strong">start</emphasis>ed instead of
+            reloaded.
           </para>
         </listitem>
         <listitem>

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

@@ -104,6 +104,19 @@ sub getActiveUnits {
     return $res;
 }
 
+# Returns whether a systemd unit is active
+sub unit_is_active {
+    my ($unit_name) = @_;
+
+    my $mgr = Net::DBus->system->get_service('org.freedesktop.systemd1')->get_object('/org/freedesktop/systemd1');
+    my $units = $mgr->ListUnitsByNames([$unit_name]);
+    if (@{$units} == 0) {
+        return 0;
+    }
+    my $active_state = $units->[0]->[3]; ## no critic (ValuesAndExpressions::ProhibitMagicNumbers)
+    return $active_state eq 'active' || $active_state eq 'activating';
+}
+
 sub parseFstab {
     my ($filename) = @_;
     my ($fss, $swaps);
@@ -744,6 +757,24 @@ close $listActiveUsers;
 print STDERR "setting up tmpfiles\n";
 system("@systemd@/bin/systemd-tmpfiles", "--create", "--remove", "--exclude-prefix=/dev") == 0 or $res = 3;
 
+# Before reloading we need to ensure that the units are still active. They may have been
+# deactivated because one of their requirements got stopped. If they are inactive
+# but should have been reloaded, the user probably expects them to be started.
+if (scalar(keys %unitsToReload) > 0) {
+    for my $unit (keys %unitsToReload) {
+        if (!unit_is_active($unit)) {
+            # Figure out if we need to start the unit
+            my %unit_info = parse_unit("$out/etc/systemd/system/$unit");
+            if (!(parseSystemdBool(\%unit_info, 'Unit', 'RefuseManualStart', 0) || parseSystemdBool(\%unit_info, 'Unit', 'X-OnlyManualStart', 0))) {
+                $unitsToStart{$unit} = 1;
+                recordUnit($startListFile, $unit);
+            }
+            # Don't reload the unit, reloading would fail
+            delete %unitsToReload{$unit};
+            unrecord_unit($reloadListFile, $unit);
+        }
+    }
+}
 # Reload units that need it. This includes remounting changed mount
 # units.
 if (scalar(keys %unitsToReload) > 0) {

nixos/tests/switch-test.nix

@@ -208,6 +208,39 @@ in {
           systemd.services."escaped\\x2ddash".serviceConfig.X-Test = "test";
         };
 
+        unitWithRequirement.configuration = {
+          systemd.services.required-service = {
+            wantedBy = [ "multi-user.target" ];
+            serviceConfig = {
+              Type = "oneshot";
+              RemainAfterExit = true;
+              ExecStart = "${pkgs.coreutils}/bin/true";
+              ExecReload = "${pkgs.coreutils}/bin/true";
+            };
+          };
+          systemd.services.test-service = {
+            wantedBy = [ "multi-user.target" ];
+            requires = [ "required-service.service" ];
+            serviceConfig = {
+              Type = "oneshot";
+              RemainAfterExit = true;
+              ExecStart = "${pkgs.coreutils}/bin/true";
+              ExecReload = "${pkgs.coreutils}/bin/true";
+            };
+          };
+        };
+
+        unitWithRequirementModified.configuration = {
+          imports = [ unitWithRequirement.configuration ];
+          systemd.services.required-service.serviceConfig.X-Test = "test";
+          systemd.services.test-service.reloadTriggers = [ "test" ];
+        };
+
+        unitWithRequirementModifiedNostart.configuration = {
+          imports = [ unitWithRequirement.configuration ];
+          systemd.services.test-service.unitConfig.RefuseManualStart = true;
+        };
+
         restart-and-reload-by-activation-script.configuration = {
           systemd.services = rec {
             simple-service = {
@@ -574,6 +607,32 @@ in {
         assert_contains(out, "\nstarting the following units: escaped\\x2ddash.service\n")
         assert_lacks(out, "the following new units were started:")
 
+        # Ensure units that require changed units are properly reloaded
+        out = switch_to_specialisation("${machine}", "unitWithRequirement")
+        assert_contains(out, "stopping the following units: escaped\\x2ddash.service\n")
+        assert_lacks(out, "NOT restarting the following changed units:")
+        assert_lacks(out, "reloading the following units:")
+        assert_lacks(out, "\nrestarting the following units:")
+        assert_lacks(out, "\nstarting the following units:")
+        assert_contains(out, "the following new units were started: required-service.service, test-service.service\n")
+
+        out = switch_to_specialisation("${machine}", "unitWithRequirementModified")
+        assert_contains(out, "stopping the following units: required-service.service\n")
+        assert_lacks(out, "NOT restarting the following changed units:")
+        assert_lacks(out, "reloading the following units:")
+        assert_lacks(out, "\nrestarting the following units:")
+        assert_contains(out, "\nstarting the following units: required-service.service, test-service.service\n")
+        assert_lacks(out, "the following new units were started:")
+
+        # Unless the unit asks to be not restarted
+        out = switch_to_specialisation("${machine}", "unitWithRequirementModifiedNostart")
+        assert_contains(out, "stopping the following units: required-service.service\n")
+        assert_lacks(out, "NOT restarting the following changed units:")
+        assert_lacks(out, "reloading the following units:")
+        assert_lacks(out, "\nrestarting the following units:")
+        assert_contains(out, "\nstarting the following units: required-service.service\n")
+        assert_lacks(out, "the following new units were started:")
+
     with subtest("failing units"):
         # Let the simple service fail
         switch_to_specialisation("${machine}", "simpleServiceModified")