From ec619ca36951d6641a9c22201f8ae1fea28808fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 10 Oct 2021 12:54:28 +0200 Subject: [PATCH 01/14] nixos/switch-to-configuration: Remove unused variable --- nixos/modules/system/activation/switch-to-configuration.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index 053496441d81c..ce7aef8826b3a 100644 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -485,7 +485,7 @@ sub filterUnits { # Print failed and new units. -my (@failed, @new, @restarting); +my (@failed, @new); my $activeNew = getActiveUnits; while (my ($unit, $state) = each %{$activeNew}) { if ($state->{state} eq "failed") { From f0a31f9b9f7a96c22907e9b508cda891861ecee7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 10 Oct 2021 12:55:00 +0200 Subject: [PATCH 02/14] nixos/switch-to-configuration: Ignore started scopes They are not managed by us and it might be weird to users to see units they didn't expect to be started. --- nixos/modules/system/activation/switch-to-configuration.pl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index ce7aef8826b3a..4adcf1a167909 100644 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -501,7 +501,9 @@ sub filterUnits { push @failed, $unit; } } - elsif ($state->{state} ne "failed" && !defined $activePrev->{$unit}) { + # Ignore scopes since they are not managed by this script but rather + # created and managed by third-party services via the systemd dbus API. + elsif ($state->{state} ne "failed" && !defined $activePrev->{$unit} && $unit !~ /\.scope$/) { push @new, $unit; } } From c4d34cd1844d3784c69c5fdb06cad716330707e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 10 Oct 2021 13:02:09 +0200 Subject: [PATCH 03/14] nixos/top-level: Check Syntax of switch-to-configuration --- nixos/modules/system/activation/top-level.nix | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/nixos/modules/system/activation/top-level.nix b/nixos/modules/system/activation/top-level.nix index 026fd1791d33f..68da910d29cc8 100644 --- a/nixos/modules/system/activation/top-level.nix +++ b/nixos/modules/system/activation/top-level.nix @@ -84,6 +84,13 @@ let export localeArchive="${config.i18n.glibcLocales}/lib/locale/locale-archive" substituteAll ${./switch-to-configuration.pl} $out/bin/switch-to-configuration chmod +x $out/bin/switch-to-configuration + ${optionalString (pkgs.stdenv.hostPlatform == pkgs.stdenv.buildPlatform) '' + if ! output=$($perl/bin/perl -c $out/bin/switch-to-configuration 2>&1); then + echo "switch-to-configuration syntax is not valid:" + echo "$output" + exit 1 + fi + ''} echo -n "${toString config.system.extraDependencies}" > $out/extra-dependencies From 744162ffb68f03c12fa60460baaa2a6dcafacf6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 10 Oct 2021 15:24:49 +0200 Subject: [PATCH 04/14] nixos/switch-to-configuration: Fix perlcritic warning --- nixos/modules/system/activation/switch-to-configuration.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index 4adcf1a167909..027e050511fb3 100644 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -25,7 +25,7 @@ my $dryRestartByActivationFile = "/run/nixos/dry-activation-restart-list"; my $dryReloadByActivationFile = "/run/nixos/dry-activation-reload-list"; -make_path("/run/nixos", { mode => 0755 }); +make_path("/run/nixos", { mode => oct(755) }); my $action = shift @ARGV; From cfad5e3403de1037ee5b8fd27b814f29fbd64790 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 10 Oct 2021 17:42:23 +0200 Subject: [PATCH 05/14] nixos/switch-to-configuration: Improve socket support This commit changes a lot more that you'd expect but it also adds a lot of new testing code so nothing breaks in the future. The main change is that sockets are now restarted when they change. The main reason for the large amount of changes is the ability of activation scripts to restart/reload units. This also works for socket-activated units now, and honors reloadIfChanged and restartIfChanged. The two changes don't really work without each other so they are done in the one large commit. The test should show what works now and ensure it will continue to do so in the future. --- .../from_md/release-notes/rl-2111.section.xml | 9 + .../manual/release-notes/rl-2111.section.md | 2 + .../activation/switch-to-configuration.pl | 241 ++++++++++++------ nixos/tests/switch-test.nix | 235 ++++++++++++++++- 4 files changed, 408 insertions(+), 79 deletions(-) diff --git a/nixos/doc/manual/from_md/release-notes/rl-2111.section.xml b/nixos/doc/manual/from_md/release-notes/rl-2111.section.xml index 023f0f57a9b22..d3046b88bfec5 100644 --- a/nixos/doc/manual/from_md/release-notes/rl-2111.section.xml +++ b/nixos/doc/manual/from_md/release-notes/rl-2111.section.xml @@ -1554,6 +1554,15 @@ Superuser created successfully. encapsulation. + + + Changing systemd .socket units now restarts + them and stops the service that is activated by them. + Additionally, services with + stopOnChange = false don’t break anymore + when they are socket-activated. + + diff --git a/nixos/doc/manual/release-notes/rl-2111.section.md b/nixos/doc/manual/release-notes/rl-2111.section.md index 658f3b59da67c..e84ca7ed8cc9c 100644 --- a/nixos/doc/manual/release-notes/rl-2111.section.md +++ b/nixos/doc/manual/release-notes/rl-2111.section.md @@ -449,3 +449,5 @@ In addition to numerous new and upgraded packages, this release has the followin - The `networking` module has a new `networking.fooOverUDP` option to configure Foo-over-UDP encapsulations. - `networking.sits` now supports Foo-over-UDP encapsulation. + +- Changing systemd `.socket` units now restarts them and stops the service that is activated by them. Additionally, services with `stopOnChange = false` don't break anymore when they are socket-activated. diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index 027e050511fb3..ced49773d1edd 100644 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -19,11 +19,14 @@ 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`. +# This also works for socket-activated units. 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) }); @@ -147,6 +150,87 @@ sub fingerprintUnit { return abs_path($s) . (-f "${s}.d/overrides.conf" ? " " . abs_path "${s}.d/overrides.conf" : ""); } +sub handleModifiedUnit { + my ($unit, $baseName, $newUnitFile, $activePrev, $unitsToStop, $unitsToStart, $unitsToReload, $unitsToRestart, $unitsToSkip) = @_; + + if ($unit eq "sysinit.target" || $unit eq "basic.target" || $unit eq "multi-user.target" || $unit eq "graphical.target") { + # Do nothing. These cannot be restarted directly. + # Slices and Paths don't have to be restarted since + # properties (resource limits and inotify watches) + # seem to get applied on daemon-reload. + } elsif ($unit =~ /\.mount$/) { + # Reload the changed mount unit to force a remount. + $unitsToReload->{$unit} = 1; + recordUnit($reloadListFile, $unit); + } elsif ($unit =~ /\.slice$/ || $unit =~ /\.path$/) { + # FIXME: do something? + } else { + my $unitInfo = parseUnit($newUnitFile); + if (boolIsTrue($unitInfo->{'X-ReloadIfChanged'} // "no")) { + $unitsToReload->{$unit} = 1; + recordUnit($reloadListFile, $unit); + } + elsif (!boolIsTrue($unitInfo->{'X-RestartIfChanged'} // "yes") || boolIsTrue($unitInfo->{'RefuseManualStop'} // "no") || boolIsTrue($unitInfo->{'X-OnlyManualStart'} // "no")) { + $unitsToSkip->{$unit} = 1; + } else { + # If this unit is socket-activated, then stop it instead + # of restarting it to make sure the new version of it is + # socket-activated. + my $socketActivated = 0; + if ($unit =~ /\.service$/) { + my @sockets = split / /, ($unitInfo->{Sockets} // ""); + if (scalar @sockets == 0) { + @sockets = ("$baseName.socket"); + } + foreach my $socket (@sockets) { + if (defined $activePrev->{$socket}) { + # Only restart sockets that actually + # exist in new configuration + if (-e "$out/etc/systemd/system/$socket") { + $socketActivated = 1; + $unitsToStop->{$unit} = 1; + } + } + } + } + # Don't do the rest of this for socket-activated units + # because we handled these above where we stop the unit. + # Since only services can be socket-activated, the + # following condition always evaluates to `true` for + # non-service units. + if (!$socketActivated) { + # If we are restarting a socket, also stop the corresponding + # service. This is required because restarting a socket + # when the service is already activated fails. + if ($unit =~ /\.socket$/) { + my $service = $unitInfo->{Service} // ""; + if ($service eq "") { + $service = "$baseName.service"; + } + if (defined $activePrev->{$service}) { + $unitsToStop->{$service} = 1; + } + $unitsToRestart->{$unit} = 1; + recordUnit($restartListFile, $unit); + } else { + if (!boolIsTrue($unitInfo->{'X-StopIfChanged'} // "yes")) { + # This unit should be restarted instead of + # stopped and started. + $unitsToRestart->{$unit} = 1; + recordUnit($restartListFile, $unit); + } else { + # We write to a file to ensure that the + # service gets restarted if we're interrupted. + $unitsToStart->{$unit} = 1; + recordUnit($startListFile, $unit); + $unitsToStop->{$unit} = 1; + } + } + } + } + } +} + # Figure out what units need to be stopped, started, restarted or reloaded. my (%unitsToStop, %unitsToSkip, %unitsToStart, %unitsToRestart, %unitsToReload); @@ -219,65 +303,7 @@ sub fingerprintUnit { } elsif (fingerprintUnit($prevUnitFile) ne fingerprintUnit($newUnitFile)) { - if ($unit eq "sysinit.target" || $unit eq "basic.target" || $unit eq "multi-user.target" || $unit eq "graphical.target") { - # Do nothing. These cannot be restarted directly. - } elsif ($unit =~ /\.mount$/) { - # Reload the changed mount unit to force a remount. - $unitsToReload{$unit} = 1; - recordUnit($reloadListFile, $unit); - } elsif ($unit =~ /\.socket$/ || $unit =~ /\.path$/ || $unit =~ /\.slice$/) { - # FIXME: do something? - } else { - my $unitInfo = parseUnit($newUnitFile); - if (boolIsTrue($unitInfo->{'X-ReloadIfChanged'} // "no")) { - $unitsToReload{$unit} = 1; - recordUnit($reloadListFile, $unit); - } - elsif (!boolIsTrue($unitInfo->{'X-RestartIfChanged'} // "yes") || boolIsTrue($unitInfo->{'RefuseManualStop'} // "no") || boolIsTrue($unitInfo->{'X-OnlyManualStart'} // "no")) { - $unitsToSkip{$unit} = 1; - } else { - if (!boolIsTrue($unitInfo->{'X-StopIfChanged'} // "yes")) { - # This unit should be restarted instead of - # stopped and started. - $unitsToRestart{$unit} = 1; - recordUnit($restartListFile, $unit); - } else { - # If this unit is socket-activated, then stop the - # socket unit(s) as well, and restart the - # socket(s) instead of the service. - my $socketActivated = 0; - if ($unit =~ /\.service$/) { - my @sockets = split / /, ($unitInfo->{Sockets} // ""); - if (scalar @sockets == 0) { - @sockets = ("$baseName.socket"); - } - foreach my $socket (@sockets) { - if (defined $activePrev->{$socket}) { - $unitsToStop{$socket} = 1; - # Only restart sockets that actually - # exist in new configuration: - if (-e "$out/etc/systemd/system/$socket") { - $unitsToStart{$socket} = 1; - recordUnit($startListFile, $socket); - $socketActivated = 1; - } - } - } - } - - # If the unit is not socket-activated, record - # that this unit needs to be started below. - # We write this to a file to ensure that the - # service gets restarted if we're interrupted. - if (!$socketActivated) { - $unitsToStart{$unit} = 1; - recordUnit($startListFile, $unit); - } - - $unitsToStop{$unit} = 1; - } - } - } + handleModifiedUnit($unit, $baseName, $newUnitFile, $activePrev, \%unitsToStop, \%unitsToStart, \%unitsToReload, \%unitsToRestart, %unitsToSkip); } } } @@ -362,8 +388,6 @@ sub filterUnits { } my @unitsToStopFiltered = filterUnits(\%unitsToStop); -my @unitsToStartFiltered = filterUnits(\%unitsToStart); - # Show dry-run actions. if ($action eq "dry-activate") { @@ -375,21 +399,45 @@ sub filterUnits { 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. + my %unitsToAlsoStop; + my %unitsToAlsoSkip; + 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"; + } - $unitsToReload{$_} = 1 foreach - split('\n', read_file($dryReloadByActivationFile, err_mode => 'quiet') // ""); + my $baseName = $baseUnit; + $baseName =~ s/\.[a-z]*$//; + + handleModifiedUnit($unit, $baseName, $newUnitFile, $activePrev, \%unitsToAlsoStop, \%unitsToStart, \%unitsToReload, \%unitsToRestart, %unitsToAlsoSkip); + } + unlink($dryRestartByActivationFile); + + my @unitsToAlsoStopFiltered = filterUnits(\%unitsToAlsoStop); + if (scalar(keys %unitsToAlsoStop) > 0) { + print STDERR "would stop the following units as well: ", join(", ", @unitsToAlsoStopFiltered), "\n" + if scalar @unitsToAlsoStopFiltered; + } + +print STDERR "NOT restarting the following changed units as well: ", join(", ", sort(keys %unitsToAlsoSkip)), "\n" + if scalar(keys %unitsToAlsoSkip) > 0; print STDERR "would restart systemd\n" if $restartSystemd; 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; print STDERR "would reload the following units: ", join(", ", sort(keys %unitsToReload)), "\n" if scalar(keys %unitsToReload) > 0; unlink($dryRestartByActivationFile); - unlink($dryReloadByActivationFile); exit 0; } @@ -414,12 +462,38 @@ sub filterUnits { # 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') // ""); +# stopped are already stopped before the activation script is run. We do however +# make an exception for services that are socket-activated and that have to be stopped +# instead of being restarted. +my %unitsToAlsoStop; +my %unitsToAlsoSkip; +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]*$//; + + handleModifiedUnit($unit, $baseName, $newUnitFile, $activePrev, \%unitsToAlsoStop, \%unitsToStart, \%unitsToReload, \%unitsToRestart, %unitsToAlsoSkip); +} +unlink($restartByActivationFile); + +my @unitsToAlsoStopFiltered = filterUnits(\%unitsToAlsoStop); +if (scalar(keys %unitsToAlsoStop) > 0) { + print STDERR "stopping the following units as well: ", join(", ", @unitsToAlsoStopFiltered), "\n" + if scalar @unitsToAlsoStopFiltered; + system("$curSystemd/systemctl", "stop", "--", sort(keys %unitsToAlsoStop)); +} + +print STDERR "NOT restarting the following changed units as well: ", join(", ", sort(keys %unitsToAlsoSkip)), "\n" + if scalar(keys %unitsToAlsoSkip) > 0; # Restart systemd if necessary. Note that this is done using the # current version of systemd, just in case the new one has trouble @@ -460,14 +534,28 @@ sub filterUnits { 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 # than stopped and started). 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; + + # We split the units to be restarted into sockets and non-sockets. + # This is because restarting sockets may fail which is not bad by + # itself but which will prevent changes on the sockets. We usually + # restart the socket and stop the service before that. Restarting + # the socket will fail however when the service was re-activated + # in the meantime. There is no proper way to prevent that from happening. + my @unitsWithErrorHandling = grep { $_ !~ /\.socket$/ } sort(keys %unitsToRestart); + my @unitsWithoutErrorHandling = grep { $_ =~ /\.socket$/ } sort(keys %unitsToRestart); + + if (scalar(@unitsWithErrorHandling) > 0) { + system("@systemd@/bin/systemctl", "restart", "--", @unitsWithErrorHandling) == 0 or $res = 4; + } + if (scalar(@unitsWithoutErrorHandling) > 0) { + system("@systemd@/bin/systemctl", "restart", "--", @unitsWithoutErrorHandling); + } unlink($restartListFile); unlink($restartByActivationFile); } @@ -478,6 +566,7 @@ sub filterUnits { # 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; diff --git a/nixos/tests/switch-test.nix b/nixos/tests/switch-test.nix index 78adf7ffa7da5..76a9ef6f624ed 100644 --- a/nixos/tests/switch-test.nix +++ b/nixos/tests/switch-test.nix @@ -7,15 +7,135 @@ import ./make-test-python.nix ({ pkgs, ...} : { }; nodes = { - machine = { ... }: { + machine = { config, pkgs, lib, ... }: { + environment.systemPackages = [ pkgs.socat ]; # for the socket activation stuff users.mutableUsers = false; + + specialisation = { + # A system with a simple socket-activated unit + simple-socket.configuration = { + systemd.services.socket-activated.serviceConfig = { + ExecStart = pkgs.writeScript "socket-test.py" /* python */ '' + #!${pkgs.python3}/bin/python3 + + from socketserver import TCPServer, StreamRequestHandler + import socket + + class Handler(StreamRequestHandler): + def handle(self): + self.wfile.write("hello".encode("utf-8")) + + class Server(TCPServer): + def __init__(self, server_address, handler_cls): + # Invoke base but omit bind/listen steps (performed by systemd activation!) + TCPServer.__init__( + self, server_address, handler_cls, bind_and_activate=False) + # Override socket + self.socket = socket.fromfd(3, self.address_family, self.socket_type) + + if __name__ == "__main__": + server = Server(("localhost", 1234), Handler) + server.serve_forever() + ''; + }; + systemd.sockets.socket-activated = { + wantedBy = [ "sockets.target" ]; + listenStreams = [ "/run/test.sock" ]; + socketConfig.SocketMode = lib.mkDefault "0777"; + }; + }; + + # The same system but the socket is modified + modified-socket.configuration = { + imports = [ config.specialisation.simple-socket.configuration ]; + systemd.sockets.socket-activated.socketConfig.SocketMode = "0666"; + }; + + # The same system but the service is modified + modified-service.configuration = { + imports = [ config.specialisation.simple-socket.configuration ]; + systemd.services.socket-activated.serviceConfig.X-Test = "test"; + }; + + # The same system but both service and socket are modified + modified-service-and-socket.configuration = { + imports = [ config.specialisation.simple-socket.configuration ]; + systemd.services.socket-activated.serviceConfig.X-Test = "some_value"; + systemd.sockets.socket-activated.socketConfig.SocketMode = "0444"; + }; + + # A system with a socket-activated service and some simple services + service-and-socket.configuration = { + imports = [ config.specialisation.simple-socket.configuration ]; + systemd.services.simple-service = { + wantedBy = [ "multi-user.target" ]; + serviceConfig = { + Type = "oneshot"; + RemainAfterExit = true; + ExecStart = "${pkgs.coreutils}/bin/true"; + }; + }; + + systemd.services.simple-restart-service = { + stopIfChanged = false; + wantedBy = [ "multi-user.target" ]; + serviceConfig = { + Type = "oneshot"; + RemainAfterExit = true; + ExecStart = "${pkgs.coreutils}/bin/true"; + }; + }; + + systemd.services.simple-reload-service = { + reloadIfChanged = true; + wantedBy = [ "multi-user.target" ]; + serviceConfig = { + Type = "oneshot"; + RemainAfterExit = true; + ExecStart = "${pkgs.coreutils}/bin/true"; + ExecReload = "${pkgs.coreutils}/bin/true"; + }; + }; + + systemd.services.no-restart-service = { + restartIfChanged = false; + wantedBy = [ "multi-user.target" ]; + serviceConfig = { + Type = "oneshot"; + RemainAfterExit = true; + ExecStart = "${pkgs.coreutils}/bin/true"; + }; + }; + }; + restart-and-reload-by-activation-script.configuration = { + imports = [ config.specialisation.service-and-socket.configuration ]; + 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 <> "$f" + simple-service.service + simple-restart-service.service + simple-reload-service.service + no-restart-service.service + socket-activated.service + EOF + ''; + }; + }; + }; }; other = { ... }: { users.mutableUsers = true; }; }; - testScript = {nodes, ...}: let + testScript = { nodes, ... }: let originalSystem = nodes.machine.config.system.build.toplevel; otherSystem = nodes.other.config.system.build.toplevel; @@ -27,12 +147,121 @@ import ./make-test-python.nix ({ pkgs, ...} : { set -o pipefail exec env -i "$@" | tee /dev/stderr ''; - in '' + in /* python */ '' + def switch_to_specialisation(name, action="test"): + out = machine.succeed(f"${originalSystem}/specialisation/{name}/bin/switch-to-configuration {action} 2>&1") + assert_lacks(out, "switch-to-configuration line") # Perl warnings + return out + + def assert_contains(haystack, needle): + if needle not in haystack: + print("The haystack that will cause the following exception is:") + print("---") + print(haystack) + print("---") + raise Exception(f"Expected string '{needle}' was not found") + + def assert_lacks(haystack, needle): + if needle in haystack: + print("The haystack that will cause the following exception is:") + print("---") + print(haystack, end="") + print("---") + raise Exception(f"Unexpected string '{needle}' was found") + + machine.succeed( "${stderrRunner} ${originalSystem}/bin/switch-to-configuration test" ) machine.succeed( "${stderrRunner} ${otherSystem}/bin/switch-to-configuration test" ) + + with subtest("systemd sockets"): + machine.succeed("${originalSystem}/bin/switch-to-configuration test") + + # Simple socket is created + out = switch_to_specialisation("simple-socket") + assert_lacks(out, "stopping the following units:") + # not checking for reload because dbus gets reloaded + assert_lacks(out, "restarting the following units:") + assert_lacks(out, "\nstarting the following units:") + assert_contains(out, "the following new units were started: socket-activated.socket\n") + assert_lacks(out, "as well:") + machine.succeed("[ $(stat -c%a /run/test.sock) = 777 ]") + + # Changing the socket restarts it + out = switch_to_specialisation("modified-socket") + assert_lacks(out, "stopping the following units:") + #assert_lacks(out, "reloading the following units:") + assert_contains(out, "restarting the following units: socket-activated.socket\n") + assert_lacks(out, "\nstarting the following units:") + assert_lacks(out, "the following new units were started:") + assert_lacks(out, "as well:") + machine.succeed("[ $(stat -c%a /run/test.sock) = 666 ]") # change was applied + + # The unit is properly activated when the socket is accessed + if machine.succeed("socat - UNIX-CONNECT:/run/test.sock") != "hello": + raise Exception("Socket was not properly activated") + + # Changing the socket restarts it and ignores the active service + out = switch_to_specialisation("simple-socket") + assert_contains(out, "stopping the following units: socket-activated.service\n") + assert_lacks(out, "reloading the following units:") + assert_contains(out, "restarting the following units: socket-activated.socket\n") + assert_lacks(out, "\nstarting the following units:") + assert_lacks(out, "the following new units were started:") + assert_lacks(out, "as well:") + machine.succeed("[ $(stat -c%a /run/test.sock) = 777 ]") # change was applied + + # Changing the service does nothing when the service is not active + out = switch_to_specialisation("modified-service") + assert_lacks(out, "stopping the following units:") + assert_lacks(out, "reloading the following units:") + assert_lacks(out, "restarting the following units:") + assert_lacks(out, "\nstarting the following units:") + assert_lacks(out, "the following new units were started:") + assert_lacks(out, "as well:") + + # Activating the service and modifying it stops it but leaves the socket untouched + machine.succeed("socat - UNIX-CONNECT:/run/test.sock") + out = switch_to_specialisation("simple-socket") + assert_contains(out, "stopping the following units: socket-activated.service\n") + assert_lacks(out, "reloading the following units:") + assert_lacks(out, "restarting the following units:") + assert_lacks(out, "\nstarting the following units:") + assert_lacks(out, "the following new units were started:") + assert_lacks(out, "as well:") + + # Activating the service and both the service and the socket stops the service and restarts the socket + machine.succeed("socat - UNIX-CONNECT:/run/test.sock") + out = switch_to_specialisation("modified-service-and-socket") + assert_contains(out, "stopping the following units: socket-activated.service\n") + assert_lacks(out, "reloading the following units:") + assert_contains(out, "restarting the following units: socket-activated.socket\n") + assert_lacks(out, "\nstarting the following units:") + assert_lacks(out, "the following new units were started:") + assert_lacks(out, "as well:") + + with subtest("restart and reload by activation file"): + out = switch_to_specialisation("service-and-socket") + # Switch to a system where the example services get restarted + # by the activation script + out = switch_to_specialisation("restart-and-reload-by-activation-script") + assert_lacks(out, "stopping the following units:") + assert_contains(out, "stopping the following units as well: simple-service.service, socket-activated.service\n") + assert_contains(out, "reloading the following units: simple-reload-service.service\n") + assert_contains(out, "restarting the following units: simple-restart-service.service\n") + assert_contains(out, "\nstarting the following units: simple-service.service") + + # The same, but in dry mode + switch_to_specialisation("service-and-socket") + out = switch_to_specialisation("restart-and-reload-by-activation-script", action="dry-activate") + assert_lacks(out, "would stop the following units:") + assert_contains(out, "would stop the following units as well: simple-service.service, socket-activated.service\n") + 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\n") + assert_contains(out, "\nwould start the following units: simple-service.service") + ''; }) From b515bae5cfa67e1407df1942252c98e401890c09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 10 Oct 2021 17:46:01 +0200 Subject: [PATCH 06/14] nixos/switch-to-configuration: Remove some FIXMEs The first FIXME is removed because it doesn't make sense to use /proc/1/exe since that points to a directory that doesn't have all tools the activation script needs (like systemd-escape). The second one is removed because there is already no error handling (compare with the restart logic where the return code is checked). --- nixos/modules/system/activation/switch-to-configuration.pl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index ced49773d1edd..58453dddc602b 100644 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -11,7 +11,6 @@ my $out = "@out@"; -# FIXME: maybe we should use /proc/1/exe to get the current systemd. my $curSystemd = abs_path("/run/current-system/sw/bin"); # To be robust against interruption, record what units need to be started etc. @@ -448,7 +447,7 @@ sub filterUnits { print STDERR "stopping the following units: ", join(", ", @unitsToStopFiltered), "\n" if scalar @unitsToStopFiltered; # Use current version of systemctl binary before daemon is reexeced. - system("$curSystemd/systemctl", "stop", "--", sort(keys %unitsToStop)); # FIXME: ignore errors? + system("$curSystemd/systemctl", "stop", "--", sort(keys %unitsToStop)); } print STDERR "NOT restarting the following changed units: ", join(", ", sort(keys %unitsToSkip)), "\n" From de128feaccaf9751d7092d83f7dcc488f51a4dda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 10 Oct 2021 17:58:06 +0200 Subject: [PATCH 07/14] nixos/switch-to-configuration: Ignore slice units --- .../activation/switch-to-configuration.pl | 4 +-- nixos/tests/switch-test.nix | 34 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index 58453dddc602b..d5b7f0c07300d 100644 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -152,7 +152,7 @@ sub fingerprintUnit { sub handleModifiedUnit { my ($unit, $baseName, $newUnitFile, $activePrev, $unitsToStop, $unitsToStart, $unitsToReload, $unitsToRestart, $unitsToSkip) = @_; - if ($unit eq "sysinit.target" || $unit eq "basic.target" || $unit eq "multi-user.target" || $unit eq "graphical.target") { + if ($unit eq "sysinit.target" || $unit eq "basic.target" || $unit eq "multi-user.target" || $unit eq "graphical.target" || $unit =~ /\.slice$/) { # Do nothing. These cannot be restarted directly. # Slices and Paths don't have to be restarted since # properties (resource limits and inotify watches) @@ -161,7 +161,7 @@ sub handleModifiedUnit { # Reload the changed mount unit to force a remount. $unitsToReload->{$unit} = 1; recordUnit($reloadListFile, $unit); - } elsif ($unit =~ /\.slice$/ || $unit =~ /\.path$/) { + } elsif ($unit =~ /\.path$/) { # FIXME: do something? } else { my $unitInfo = parseUnit($newUnitFile); diff --git a/nixos/tests/switch-test.nix b/nixos/tests/switch-test.nix index 76a9ef6f624ed..1acccbd175406 100644 --- a/nixos/tests/switch-test.nix +++ b/nixos/tests/switch-test.nix @@ -107,6 +107,8 @@ import ./make-test-python.nix ({ pkgs, ...} : { }; }; }; + + # The same system but with an activation script that restarts all services restart-and-reload-by-activation-script.configuration = { imports = [ config.specialisation.service-and-socket.configuration ]; system.activationScripts.restart-and-reload-test = { @@ -128,6 +130,25 @@ import ./make-test-python.nix ({ pkgs, ...} : { ''; }; }; + + # A system with a slice + with-slice.configuration = { + systemd.slices.testslice.sliceConfig.MemoryMax = "1"; # don't allow memory allocation + systemd.services.testservice = { + serviceConfig = { + Type = "oneshot"; + RemainAfterExit = true; + ExecStart = "${pkgs.coreutils}/bin/true"; + Slice = "testslice.slice"; + }; + }; + }; + + # The same system but the slice allows to allocate memory + with-slice-non-crashing.configuration = { + imports = [ config.specialisation.with-slice.configuration ]; + systemd.slices.testslice.sliceConfig.MemoryMax = lib.mkForce null; + }; }; }; other = { ... }: { @@ -263,5 +284,18 @@ import ./make-test-python.nix ({ pkgs, ...} : { assert_contains(out, "would restart the following units: simple-restart-service.service\n") assert_contains(out, "\nwould start the following units: simple-service.service") + # This test ensures that changes to slice configuration get applied. + # We test this by having a slice that allows no memory allocation at + # all and starting a service within it. If the service crashes, the slice + # is applied and if we modify the slice to allow memory allocation, the + # service should successfully start. + with subtest("slices"): + machine.succeed("echo 0 > /proc/sys/vm/panic_on_oom") # allow OOMing + out = switch_to_specialisation("with-slice") + machine.fail("systemctl start testservice.service") + out = switch_to_specialisation("with-slice-non-crashing") + machine.succeed("systemctl start testservice.service") + machine.succeed("echo 1 > /proc/sys/vm/panic_on_oom") # disallow OOMing + ''; }) From adc033cd5935feb85dc3477a37d2ce1c1384837b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 10 Oct 2021 18:08:36 +0200 Subject: [PATCH 08/14] nixos/switch-to-configuration: Ignore path units --- .../activation/switch-to-configuration.pl | 4 +-- nixos/tests/switch-test.nix | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index d5b7f0c07300d..776e75ce470ea 100644 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -152,7 +152,7 @@ sub fingerprintUnit { sub handleModifiedUnit { my ($unit, $baseName, $newUnitFile, $activePrev, $unitsToStop, $unitsToStart, $unitsToReload, $unitsToRestart, $unitsToSkip) = @_; - if ($unit eq "sysinit.target" || $unit eq "basic.target" || $unit eq "multi-user.target" || $unit eq "graphical.target" || $unit =~ /\.slice$/) { + if ($unit eq "sysinit.target" || $unit eq "basic.target" || $unit eq "multi-user.target" || $unit eq "graphical.target" || $unit =~ /\.slice$/ || $unit =~ /\.path$/) { # Do nothing. These cannot be restarted directly. # Slices and Paths don't have to be restarted since # properties (resource limits and inotify watches) @@ -161,8 +161,6 @@ sub handleModifiedUnit { # Reload the changed mount unit to force a remount. $unitsToReload->{$unit} = 1; recordUnit($reloadListFile, $unit); - } elsif ($unit =~ /\.path$/) { - # FIXME: do something? } else { my $unitInfo = parseUnit($newUnitFile); if (boolIsTrue($unitInfo->{'X-ReloadIfChanged'} // "no")) { diff --git a/nixos/tests/switch-test.nix b/nixos/tests/switch-test.nix index 1acccbd175406..be8f22f5082a0 100644 --- a/nixos/tests/switch-test.nix +++ b/nixos/tests/switch-test.nix @@ -131,6 +131,26 @@ import ./make-test-python.nix ({ pkgs, ...} : { }; }; + # A system with a path unit + with-path.configuration = { + systemd.paths.test-watch = { + wantedBy = [ "paths.target" ]; + pathConfig.PathExists = "/testpath"; + }; + systemd.services.test-watch = { + serviceConfig = { + Type = "oneshot"; + ExecStart = "${pkgs.coreutils}/bin/touch /testpath-modified"; + }; + }; + }; + + # The same system but watching another file + with-path-modified.configuration = { + imports = [ config.specialisation.with-path.configuration ]; + systemd.paths.test-watch.pathConfig.PathExists = lib.mkForce "/testpath2"; + }; + # A system with a slice with-slice.configuration = { systemd.slices.testslice.sliceConfig.MemoryMax = "1"; # don't allow memory allocation @@ -284,6 +304,22 @@ import ./make-test-python.nix ({ pkgs, ...} : { assert_contains(out, "would restart the following units: simple-restart-service.service\n") assert_contains(out, "\nwould start the following units: simple-service.service") + with subtest("paths"): + switch_to_specialisation("with-path") + machine.fail("test -f /testpath-modified") + + # touch the file, unit should be triggered + machine.succeed("touch /testpath") + machine.wait_until_succeeds("test -f /testpath-modified") + + machine.succeed("rm /testpath /testpath-modified") + switch_to_specialisation("with-path-modified") + + machine.succeed("touch /testpath") + machine.fail("test -f /testpath-modified") + machine.succeed("touch /testpath2") + machine.wait_until_succeeds("test -f /testpath-modified") + # This test ensures that changes to slice configuration get applied. # We test this by having a slice that allows no memory allocation at # all and starting a service within it. If the service crashes, the slice From 4f870c7d709888f935d7caf6f685c1a4930b1f6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Wed, 13 Oct 2021 22:19:18 +0200 Subject: [PATCH 09/14] nixos/switch-to-configuration: Restart timers --- .../activation/switch-to-configuration.pl | 5 ++- nixos/tests/switch-test.nix | 36 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index 776e75ce470ea..2d38af41985c3 100644 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -210,7 +210,10 @@ sub handleModifiedUnit { $unitsToRestart->{$unit} = 1; recordUnit($restartListFile, $unit); } else { - if (!boolIsTrue($unitInfo->{'X-StopIfChanged'} // "yes")) { + # Always restart non-services instead of stopping and starting them + # because it doesn't make sense to stop them with a config from + # the old evaluation. + if (!boolIsTrue($unitInfo->{'X-StopIfChanged'} // "yes") || $unit !~ /\.service$/) { # This unit should be restarted instead of # stopped and started. $unitsToRestart->{$unit} = 1; diff --git a/nixos/tests/switch-test.nix b/nixos/tests/switch-test.nix index be8f22f5082a0..e7891c18ec473 100644 --- a/nixos/tests/switch-test.nix +++ b/nixos/tests/switch-test.nix @@ -131,6 +131,26 @@ import ./make-test-python.nix ({ pkgs, ...} : { }; }; + # A system with a timer + with-timer.configuration = { + systemd.timers.test-timer = { + wantedBy = [ "timers.target" ]; + timerConfig.OnCalendar = "@1395716396"; # chosen by fair dice roll + }; + systemd.services.test-timer = { + serviceConfig = { + Type = "oneshot"; + ExecStart = "${pkgs.coreutils}/bin/true"; + }; + }; + }; + + # The same system but with another time + with-timer-modified.configuration = { + imports = [ config.specialisation.with-timer.configuration ]; + systemd.timers.test-timer.timerConfig.OnCalendar = lib.mkForce "Fri 2012-11-23 16:00:00"; + }; + # A system with a path unit with-path.configuration = { systemd.paths.test-watch = { @@ -304,6 +324,22 @@ import ./make-test-python.nix ({ pkgs, ...} : { assert_contains(out, "would restart the following units: simple-restart-service.service\n") assert_contains(out, "\nwould start the following units: simple-service.service") + with subtest("timers"): + switch_to_specialisation("with-timer") + out = machine.succeed("systemctl show test-timer.timer") + assert_contains(out, "OnCalendar=2014-03-25 02:59:56 UTC") + + out = switch_to_specialisation("with-timer-modified") + assert_lacks(out, "stopping the following units:") + assert_lacks(out, "reloading the following units:") + assert_contains(out, "restarting the following units: test-timer.timer\n") + assert_lacks(out, "\nstarting the following units:") + assert_lacks(out, "the following new units were started:") + assert_lacks(out, "as well:") + # It changed + out = machine.succeed("systemctl show test-timer.timer") + assert_contains(out, "OnCalendar=Fri 2012-11-23 16:00:00") + with subtest("paths"): switch_to_specialisation("with-path") machine.fail("test -f /testpath-modified") From 720571eefab65c0c1c3efc7a5801c0eee1fe0414 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Wed, 13 Oct 2021 22:50:21 +0200 Subject: [PATCH 10/14] nixos/switchTest: Also test mounts --- nixos/tests/switch-test.nix | 44 +++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/nixos/tests/switch-test.nix b/nixos/tests/switch-test.nix index e7891c18ec473..4caa7d98f47fe 100644 --- a/nixos/tests/switch-test.nix +++ b/nixos/tests/switch-test.nix @@ -151,6 +151,34 @@ import ./make-test-python.nix ({ pkgs, ...} : { systemd.timers.test-timer.timerConfig.OnCalendar = lib.mkForce "Fri 2012-11-23 16:00:00"; }; + # A system with a systemd mount + with-mount.configuration = { + systemd.mounts = [ + { + description = "Testmount"; + what = "tmpfs"; + type = "tmpfs"; + where = "/testmount"; + options = "size=1M"; + wantedBy = [ "local-fs.target" ]; + } + ]; + }; + + # The same system but with another time + with-mount-modified.configuration = { + systemd.mounts = [ + { + description = "Testmount"; + what = "tmpfs"; + type = "tmpfs"; + where = "/testmount"; + options = "size=10M"; + wantedBy = [ "local-fs.target" ]; + } + ]; + }; + # A system with a path unit with-path.configuration = { systemd.paths.test-watch = { @@ -324,6 +352,22 @@ import ./make-test-python.nix ({ pkgs, ...} : { assert_contains(out, "would restart the following units: simple-restart-service.service\n") assert_contains(out, "\nwould start the following units: simple-service.service") + with subtest("mounts"): + switch_to_specialisation("with-mount") + out = machine.succeed("mount | grep 'on /testmount'") + assert_contains(out, "size=1024k") + + out = switch_to_specialisation("with-mount-modified") + assert_lacks(out, "stopping the following units:") + assert_contains(out, "reloading the following units: testmount.mount\n") + assert_lacks(out, "restarting the following units:") + assert_lacks(out, "\nstarting the following units:") + assert_lacks(out, "the following new units were started:") + assert_lacks(out, "as well:") + # It changed + out = machine.succeed("mount | grep 'on /testmount'") + assert_contains(out, "size=10240k") + with subtest("timers"): switch_to_specialisation("with-timer") out = machine.succeed("systemctl show test-timer.timer") From 047aa1a0e9ec382ab21dea73820edd1642350215 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Thu, 14 Oct 2021 21:18:31 +0200 Subject: [PATCH 11/14] nixos/switch-to-configuration: Use early return --- .../activation/switch-to-configuration.pl | 57 ++++++++++--------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index 2d38af41985c3..ad69447f94030 100644 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -190,41 +190,44 @@ sub handleModifiedUnit { } } } + # Don't do the rest of this for socket-activated units # because we handled these above where we stop the unit. # Since only services can be socket-activated, the # following condition always evaluates to `true` for # non-service units. - if (!$socketActivated) { - # If we are restarting a socket, also stop the corresponding - # service. This is required because restarting a socket - # when the service is already activated fails. - if ($unit =~ /\.socket$/) { - my $service = $unitInfo->{Service} // ""; - if ($service eq "") { - $service = "$baseName.service"; - } - if (defined $activePrev->{$service}) { - $unitsToStop->{$service} = 1; - } + if ($socketActivated) { + return; + } + + # If we are restarting a socket, also stop the corresponding + # service. This is required because restarting a socket + # when the service is already activated fails. + if ($unit =~ /\.socket$/) { + my $service = $unitInfo->{Service} // ""; + if ($service eq "") { + $service = "$baseName.service"; + } + if (defined $activePrev->{$service}) { + $unitsToStop->{$service} = 1; + } + $unitsToRestart->{$unit} = 1; + recordUnit($restartListFile, $unit); + } else { + # Always restart non-services instead of stopping and starting them + # because it doesn't make sense to stop them with a config from + # the old evaluation. + if (!boolIsTrue($unitInfo->{'X-StopIfChanged'} // "yes") || $unit !~ /\.service$/) { + # This unit should be restarted instead of + # stopped and started. $unitsToRestart->{$unit} = 1; recordUnit($restartListFile, $unit); } else { - # Always restart non-services instead of stopping and starting them - # because it doesn't make sense to stop them with a config from - # the old evaluation. - if (!boolIsTrue($unitInfo->{'X-StopIfChanged'} // "yes") || $unit !~ /\.service$/) { - # This unit should be restarted instead of - # stopped and started. - $unitsToRestart->{$unit} = 1; - recordUnit($restartListFile, $unit); - } else { - # We write to a file to ensure that the - # service gets restarted if we're interrupted. - $unitsToStart->{$unit} = 1; - recordUnit($startListFile, $unit); - $unitsToStop->{$unit} = 1; - } + # We write to a file to ensure that the + # service gets restarted if we're interrupted. + $unitsToStart->{$unit} = 1; + recordUnit($startListFile, $unit); + $unitsToStop->{$unit} = 1; } } } From 558158b4f541628a9e28ebca9f3c2568cf2d468f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Thu, 14 Oct 2021 22:20:54 +0200 Subject: [PATCH 12/14] nixos/switch-to-configuration: Hide socket warnings --- .../system/activation/switch-to-configuration.pl | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index ad69447f94030..745a150eb70c4 100644 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -557,7 +557,19 @@ sub filterUnits { system("@systemd@/bin/systemctl", "restart", "--", @unitsWithErrorHandling) == 0 or $res = 4; } if (scalar(@unitsWithoutErrorHandling) > 0) { - system("@systemd@/bin/systemctl", "restart", "--", @unitsWithoutErrorHandling); + # Don't print warnings from systemctl + no warnings 'once'; + open(OLDERR, ">&", \*STDERR); + close(STDERR); + + my $ret = system("@systemd@/bin/systemctl", "restart", "--", @unitsWithoutErrorHandling); + + # Print stderr again + open(STDERR, ">&OLDERR"); + + if ($ret ne 0) { + print STDERR "warning: some sockets failed to restart. Please check your journal (journalctl -eb) and act accordingly.\n"; + } } unlink($restartListFile); unlink($restartByActivationFile); From ad09f7be1436dfbe043c58f2c85c2da8f8445622 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 17 Oct 2021 14:23:01 +0200 Subject: [PATCH 13/14] nixos/switch-to-configuration: Handle stopped sockets The previous logic failed to detect that units were socket-activated when the socket was stopped before switch-to-configuration was run. This commit fixes that and also starts the socket in question. --- .../system/activation/switch-to-configuration.pl | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index 745a150eb70c4..638998ac18407 100644 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -180,12 +180,13 @@ sub handleModifiedUnit { @sockets = ("$baseName.socket"); } foreach my $socket (@sockets) { - if (defined $activePrev->{$socket}) { - # Only restart sockets that actually - # exist in new configuration - if (-e "$out/etc/systemd/system/$socket") { - $socketActivated = 1; - $unitsToStop->{$unit} = 1; + if (-e "$out/etc/systemd/system/$socket") { + $socketActivated = 1; + $unitsToStop->{$unit} = 1; + # If the socket was not running previously, + # start it now. + if (not defined $activePrev->{$socket}) { + $unitsToStart->{$socket} = 1; } } } From 4cdbb2d891694870d5005f370458eeaf885f2c4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janne=20He=C3=9F?= Date: Sun, 17 Oct 2021 14:29:11 +0200 Subject: [PATCH 14/14] nixos/switch-to-configuration: Fix ordering and indentation This makes the order of operations the same in dry-activate and a "true" activate. Also fixes the indentation I messed up and drop a useless unlink() call (we are already unlinking that file earlier). --- .../modules/system/activation/switch-to-configuration.pl | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index 638998ac18407..e105502cf3a48 100644 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -430,18 +430,17 @@ sub filterUnits { if scalar @unitsToAlsoStopFiltered; } -print STDERR "NOT restarting the following changed units as well: ", join(", ", sort(keys %unitsToAlsoSkip)), "\n" - if scalar(keys %unitsToAlsoSkip) > 0; + print STDERR "would NOT restart the following changed units as well: ", join(", ", sort(keys %unitsToAlsoSkip)), "\n" + if scalar(keys %unitsToAlsoSkip) > 0; 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; - print STDERR "would reload the following units: ", join(", ", sort(keys %unitsToReload)), "\n" - if scalar(keys %unitsToReload) > 0; - unlink($dryRestartByActivationFile); exit 0; }