From c96180c53fcd4f36a7163c3e59a2e6bcd9233f06 Mon Sep 17 00:00:00 2001 From: Janne Heß Date: Sun, 6 Mar 2022 19:22:04 +0100 Subject: 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 --- .../manual/development/unit-handling.section.md | 3 +- .../from_md/development/unit-handling.section.xml | 5 +- .../system/activation/switch-to-configuration.pl | 67 +++++++++++++++++----- nixos/tests/switch-test.nix | 14 +++++ 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.\.reloadTriggers](#opt-systemd.services). If the + [systemd.services.\.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 @@ reload the unit. The NixOS module system allows setting these triggers with the option systemd.services.<name>.reloadTriggers. - If the unit files differ in any way, the following actions are - performed: + 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: 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") -- cgit 1.4.1