nixos/switch-to-configuration: Ignore some unit keys

Some unit keys don't need to restart the service to make them effective.
Reduce the amount of service restarts by ignoring these keys

4 files changed, 71 insertions, 18 deletions

diff --git a/nixos/doc/manual/development/unit-handling.section.md b/nixos/doc/manual/development/unit-handling.section.md
index bd4fe9e670f..c51704ad0da 100644
--- a/nixos/doc/manual/development/unit-handling.section.md
+++ b/nixos/doc/manual/development/unit-handling.section.md
@@ -17,7 +17,8 @@ checks:
   them and comparing their contents. If they are different but only
   `X-Reload-Triggers` in the `[Unit]` section is changed, **reload** the unit.
   The NixOS module system allows setting these triggers with the option
-  [systemd.services.\<name\>.reloadTriggers](#opt-systemd.services). If the
+  [systemd.services.\<name\>.reloadTriggers](#opt-systemd.services). There are
+  some additional keys in the `[Unit]` section that are ignored as well. If the
   unit files differ in any way, the following actions are performed:
 
   - `.path` and `.slice` units are ignored. There is no need to restart them
diff --git a/nixos/doc/manual/from_md/development/unit-handling.section.xml b/nixos/doc/manual/from_md/development/unit-handling.section.xml
index 57c4754c001..642cc5cccc7 100644
--- a/nixos/doc/manual/from_md/development/unit-handling.section.xml
+++ b/nixos/doc/manual/from_md/development/unit-handling.section.xml
@@ -38,8 +38,9 @@
         <emphasis role="strong">reload</emphasis> the unit. The NixOS
         module system allows setting these triggers with the option
         <link linkend="opt-systemd.services">systemd.services.&lt;name&gt;.reloadTriggers</link>.
-        If the unit files differ in any way, the following actions are
-        performed:
+        There are some additional keys in the <literal>[Unit]</literal>
+        section that are ignored as well. If the unit files differ in
+        any way, the following actions are performed:
       </para>
       <itemizedlist>
         <listitem>
diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl
index a1653d451fe..ca45fc9c286 100644
--- a/nixos/modules/system/activation/switch-to-configuration.pl
+++ b/nixos/modules/system/activation/switch-to-configuration.pl
@@ -226,10 +226,20 @@ sub unrecord_unit {
 sub compare_units {
     my ($old_unit, $new_unit) = @_;
     my $ret = 0;
+    # Keys to ignore in the [Unit] section
+    my %unit_section_ignores = map { $_ => 1 } qw(
+        X-Reload-Triggers
+        Description Documentation
+        OnFailure OnSuccess OnFailureJobMode
+        IgnoreOnIsolate StopWhenUnneeded
+        RefuseManualStart RefuseManualStop
+        AllowIsolate CollectMode
+        SourcePath
+    );
 
     my $comp_array = sub {
       my ($a, $b) = @_;
-      return join("\0", @{$a}) eq join("\0", @{$b});
+      return join("\0", @{$a}) eq join "\0", @{$b};
     };
 
     # Comparison hash for the sections
@@ -238,6 +248,18 @@ sub compare_units {
     foreach my $section_name (keys %{$old_unit}) {
         # Missing section in the new unit?
         if (not exists $section_cmp{$section_name}) {
+            # If the [Unit] section was removed, make sure that only keys
+            # were in it that are ignored
+            if ($section_name eq 'Unit') {
+                foreach my $ini_key (keys %{$old_unit->{'Unit'}}) {
+                    if (not defined $unit_section_ignores{$ini_key}) {
+                        return 1;
+                    }
+                }
+                next; # check the next section
+            } else {
+                return 1;
+            }
             if ($section_name eq 'Unit' and %{$old_unit->{'Unit'}} == 1 and defined(%{$old_unit->{'Unit'}}{'X-Reload-Triggers'})) {
                 # If a new [Unit] section was removed that only contained X-Reload-Triggers,
                 # do nothing.
@@ -255,8 +277,8 @@ sub compare_units {
             my @old_value = @{$old_unit->{$section_name}{$ini_key}};
             # If the key is missing in the new unit, they are different...
             if (not $new_unit->{$section_name}{$ini_key}) {
-                # ... unless the key that is now missing was the reload trigger
-                if ($section_name eq 'Unit' and $ini_key eq 'X-Reload-Triggers') {
+                # ... unless the key that is now missing is one of the ignored keys
+                if ($section_name eq 'Unit' and defined $unit_section_ignores{$ini_key}) {
                     next;
                 }
                 return 1;
@@ -264,19 +286,30 @@ sub compare_units {
             my @new_value = @{$new_unit->{$section_name}{$ini_key}};
             # If the contents are different, the units are different
             if (not $comp_array->(\@old_value, \@new_value)) {
-                # Check if only the reload triggers changed
-                if ($section_name eq 'Unit' and $ini_key eq 'X-Reload-Triggers') {
-                    $ret = 2;
-                } else {
-                    return 1;
+                # Check if only the reload triggers changed or one of the ignored keys
+                if ($section_name eq 'Unit') {
+                    if ($ini_key eq 'X-Reload-Triggers') {
+                        $ret = 2;
+                        next;
+                    } elsif (defined $unit_section_ignores{$ini_key}) {
+                        next;
+                    }
                 }
+                return 1;
             }
         }
         # A key was introduced that was missing in the old unit
         if (%ini_cmp) {
-            if ($section_name eq 'Unit' and %ini_cmp == 1 and defined($ini_cmp{'X-Reload-Triggers'})) {
-                # If the newly introduced key was the reload triggers, reload the unit
-                $ret = 2;
+            if ($section_name eq 'Unit') {
+                foreach my $ini_key (keys %ini_cmp) {
+                    if ($ini_key eq 'X-Reload-Triggers') {
+                        $ret = 2;
+                    } elsif (defined $unit_section_ignores{$ini_key}) {
+                        next;
+                    } else {
+                        return 1;
+                    }
+                }
             } else {
                 return 1;
             }
@@ -284,10 +317,14 @@ sub compare_units {
     }
     # A section was introduced that was missing in the old unit
     if (%section_cmp) {
-        if (%section_cmp == 1 and defined($section_cmp{'Unit'}) and %{$new_unit->{'Unit'}} == 1 and defined(%{$new_unit->{'Unit'}}{'X-Reload-Triggers'})) {
-            # If a new [Unit] section was introduced that only contains X-Reload-Triggers,
-            # reload instead of restarting
-            $ret = 2;
+        if (%section_cmp == 1 and defined $section_cmp{'Unit'}) {
+            foreach my $ini_key (keys %{$new_unit->{'Unit'}}) {
+                if (not defined $unit_section_ignores{$ini_key}) {
+                    return 1;
+                } elsif ($ini_key eq 'X-Reload-Triggers') {
+                    $ret = 2;
+                }
+            }
         } else {
             return 1;
         }
diff --git a/nixos/tests/switch-test.nix b/nixos/tests/switch-test.nix
index 4160e481853..a994fb78160 100644
--- a/nixos/tests/switch-test.nix
+++ b/nixos/tests/switch-test.nix
@@ -64,6 +64,11 @@ in {
           };
         };
 
+        simpleServiceDifferentDescription.configuration = {
+          imports = [ simpleService.configuration ];
+          systemd.services.test.description = "Test unit";
+        };
+
         simpleServiceModified.configuration = {
           imports = [ simpleService.configuration ];
           systemd.services.test.serviceConfig.X-Test = true;
@@ -497,6 +502,15 @@ in {
         assert_lacks(out, "\nstarting the following units:")
         assert_lacks(out, "the following new units were started:")
 
+        # Only changing the description does nothing
+        out = switch_to_specialisation("${machine}", "simpleServiceDifferentDescription")
+        assert_lacks(out, "stopping the following units:")
+        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_lacks(out, "the following new units were started:")
+
         # Restart the simple service
         out = switch_to_specialisation("${machine}", "simpleServiceModified")
         assert_contains(out, "stopping the following units: test.service\n")