Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jan 17, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions nixos/doc/manual/from_md/release-notes/rl-2205.section.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
2 changes: 2 additions & 0 deletions nixos/doc/manual/release-notes/rl-2205.section.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
72 changes: 54 additions & 18 deletions nixos/modules/system/activation/switch-to-configuration.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@
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) });

Expand Down Expand Up @@ -382,7 +384,6 @@ sub filterUnits {
}

my @unitsToStopFiltered = filterUnits(\%unitsToStop);
my @unitsToStartFiltered = filterUnits(\%unitsToStart);


# Show dry-run actions.
Expand All @@ -395,21 +396,39 @@ 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.
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]*$//;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe File::Basename::basename would be safer. (It's in core)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying this but this isn't really the basename we both were thinking ;) It's doing /a/b/c.service → /a/b/c and not the classic basename


$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;
}

Expand All @@ -433,13 +452,31 @@ sub filterUnits {
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
Expand Down Expand Up @@ -480,7 +517,6 @@ 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
Expand All @@ -489,7 +525,6 @@ sub filterUnits {
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.
Expand All @@ -498,6 +533,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;
Expand Down
70 changes: 70 additions & 0 deletions nixos/tests/switch-test.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
{
Expand Down Expand Up @@ -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'")
Expand Down