From bf9c6c9861b86e566b1bf22f251194a489b8f97c Mon Sep 17 00:00:00 2001 From: Andreas Fuchs Date: Wed, 27 Nov 2024 21:06:35 -0500 Subject: [PATCH 1/5] switch-to-configuration-ng: Better handling of socket-activated units Previously, if any unit had a socket associated with it, stc-ng counted it as "socket-activated", meaning that the unit would get stopped and the socket get restarted. That can wreak havoc on units like systemd-udevd and systemd-networkd. Instead, let units set the new flag notSocketActivated, which sets a boolean on the unit indicating to stc-ng that the unit wants to be treated like any other non-socket-activated unit instead. That will stop/start or restart these units on upgrades, without unnecessarily tearing down any machinery that the system needs to run. --- nixos/lib/systemd-lib.nix | 2 ++ nixos/lib/systemd-unit-options.nix | 12 ++++++++++++ nixos/modules/services/hardware/udev.nix | 5 +++-- nixos/modules/system/boot/networkd.nix | 1 + .../sw/switch-to-configuration-ng/src/src/main.rs | 10 ++++++++++ 5 files changed, 28 insertions(+), 2 deletions(-) diff --git a/nixos/lib/systemd-lib.nix b/nixos/lib/systemd-lib.nix index 39961f0f25d53..fdf5525c53852 100644 --- a/nixos/lib/systemd-lib.nix +++ b/nixos/lib/systemd-lib.nix @@ -579,6 +579,8 @@ in rec { '' else "") + optionalString (def ? stopIfChanged && !def.stopIfChanged) '' X-StopIfChanged=false + '' + optionalString (def ? notSocketActivated && def.notSocketActivated) '' + X-NotSocketActivated=true '' + attrsToSection def.serviceConfig); }; diff --git a/nixos/lib/systemd-unit-options.nix b/nixos/lib/systemd-unit-options.nix index b02a11ef33a6a..44fef28ec11e5 100644 --- a/nixos/lib/systemd-unit-options.nix +++ b/nixos/lib/systemd-unit-options.nix @@ -535,6 +535,18 @@ in rec { ''; }; + notSocketActivated = mkOption { + type = types.bool; + default = false; + description = '' + If set, a changed unit is never assumed to be + socket-activated on configuration activation, even if + it might have associated socket units. Instead, the unit + will be restarted (or stopped/started) as if it had no + associated sockets. + ''; + }; + startAt = mkOption { type = with types; either str (listOf str); default = []; diff --git a/nixos/modules/services/hardware/udev.nix b/nixos/modules/services/hardware/udev.nix index a532f629efd06..1ea001fcc26a9 100644 --- a/nixos/modules/services/hardware/udev.nix +++ b/nixos/modules/services/hardware/udev.nix @@ -433,8 +433,9 @@ in fi ''; - systemd.services.systemd-udevd = - { restartTriggers = [ config.environment.etc."udev/rules.d".source ]; + systemd.services.systemd-udevd = { + restartTriggers = [ config.environment.etc."udev/rules.d".source ]; + notSocketActivated = true; }; }; diff --git a/nixos/modules/system/boot/networkd.nix b/nixos/modules/system/boot/networkd.nix index 130d6098b1e23..9fd5f8fcfa698 100644 --- a/nixos/modules/system/boot/networkd.nix +++ b/nixos/modules/system/boot/networkd.nix @@ -2883,6 +2883,7 @@ let config.environment.etc."systemd/networkd.conf".source ]; aliases = [ "dbus-org.freedesktop.network1.service" ]; + notSocketActivated = true; }; networking.iproute2 = mkIf (cfg.config.addRouteTablesToIPRoute2 && cfg.config.routeTables != { }) { diff --git a/pkgs/by-name/sw/switch-to-configuration-ng/src/src/main.rs b/pkgs/by-name/sw/switch-to-configuration-ng/src/src/main.rs index 61932cb55591c..811945bdad4a3 100644 --- a/pkgs/by-name/sw/switch-to-configuration-ng/src/src/main.rs +++ b/pkgs/by-name/sw/switch-to-configuration-ng/src/src/main.rs @@ -68,6 +68,8 @@ const RELOAD_LIST_FILE: &str = "/run/nixos/reload-list"; // `stopIfChanged = true` is ignored, switch-to-configuration will handle `restartIfChanged = // false` and `reloadIfChanged = true`. This is the same as specifying a restart trigger in the // NixOS module. +// In addition, switch-to-configuration will handle notSocketActivated=true to disable treatment +// of units as "socket-activated" even though they might have any associated sockets. // // The reload file asks this program to reload a unit. This is the same as specifying a reload // trigger in the NixOS module and can be ignored if the unit is restarted in this activation. @@ -583,6 +585,8 @@ fn handle_modified_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. + // We count as "socket-activated" any unit that doesn't declare itself not so + // via X-NotSocketActivated, that has any associated .socket units. let mut socket_activated = false; if unit.ends_with(".service") { let mut sockets = if let Some(Some(Some(sockets))) = new_unit_info.map(|info| { @@ -634,6 +638,12 @@ fn handle_modified_unit( } } } + if parse_systemd_bool(new_unit_info, "Service", "X-NotSocketActivated", false) { + // If the unit explicitly opts out of socket + // activation, restart it as if it weren't (but do + // restart its sockets, that's fine): + socket_activated = false; + } // 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 From 1fed2312e7a3735c87e8a6570ae734bbcecb4180 Mon Sep 17 00:00:00 2001 From: Andreas Fuchs Date: Fri, 29 Nov 2024 09:53:10 -0500 Subject: [PATCH 2/5] Add X-NotSocketActivated logic to switch-to-configuration.pl, as well --- nixos/modules/system/activation/switch-to-configuration.pl | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/nixos/modules/system/activation/switch-to-configuration.pl b/nixos/modules/system/activation/switch-to-configuration.pl index 4beca4f0a42a9..db5cef36344d2 100755 --- a/nixos/modules/system/activation/switch-to-configuration.pl +++ b/nixos/modules/system/activation/switch-to-configuration.pl @@ -530,6 +530,13 @@ sub handle_modified_unit { ## no critic(Subroutines::ProhibitManyArgs, Subroutin } } + if (parse_systemd_bool(\%new_unit_info, "Service", "X-NotSocketActivated", 0)) { + # If the unit explicitly opts out of socket + # activation, restart it as if it weren't (but do + # restart its sockets, that's fine): + $socket_activated = 0; + } + # 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 From d95a8f5a13dda838574cd1cb6196b767cf596a3f Mon Sep 17 00:00:00 2001 From: Andreas Fuchs Date: Mon, 9 Dec 2024 08:57:47 -0500 Subject: [PATCH 3/5] Document the logic around X-NotSocketActivated in the manual --- .../manual/development/unit-handling.section.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/nixos/doc/manual/development/unit-handling.section.md b/nixos/doc/manual/development/unit-handling.section.md index deb120b9fadda..709a55f934d69 100644 --- a/nixos/doc/manual/development/unit-handling.section.md +++ b/nixos/doc/manual/development/unit-handling.section.md @@ -58,11 +58,16 @@ checks: before the activation script is run. This behavior is different when the service is socket-activated, as outlined in the following steps. - - The last thing that is taken into account is whether the unit is a service - and socket-activated. If `X-StopIfChanged` is **not** set, the service - is **restart**ed with the others. If it is set, both the service and the - socket are **stop**ped and the socket is **start**ed, leaving socket - activation to start the service when it's needed. + - The last thing that is taken into account is whether the unit is a + service and socket-activated. A correspondence between a + `.service` and its `.socket` unit is detected automatically, but + services can **opt out** of that detection by setting + `X-NotSocketActivated` to `yes` in their `[Service]` + section. Otherwise, if `X-StopIfChanged` is **not** set, the + service is **restart**ed with the others. If it is set, both the + service and the socket are **stop**ped and the socket is + **start**ed, leaving socket activation to start the service when + it's needed. ## Sysinit reactivation {#sec-sysinit-reactivation} From 878be9c20b2876cc364534dead058c09a46225a7 Mon Sep 17 00:00:00 2001 From: Andreas Fuchs Date: Wed, 18 Dec 2024 08:17:04 -0500 Subject: [PATCH 4/5] Address review feedback: Attempt to fix wonky indentation --- nixos/lib/systemd-lib.nix | 3 ++- nixos/modules/services/hardware/udev.nix | 7 +++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/nixos/lib/systemd-lib.nix b/nixos/lib/systemd-lib.nix index fdf5525c53852..b8848f56a6c0b 100644 --- a/nixos/lib/systemd-lib.nix +++ b/nixos/lib/systemd-lib.nix @@ -579,7 +579,8 @@ in rec { '' else "") + optionalString (def ? stopIfChanged && !def.stopIfChanged) '' X-StopIfChanged=false - '' + optionalString (def ? notSocketActivated && def.notSocketActivated) '' + '' + + optionalString (def ? notSocketActivated && def.notSocketActivated) '' X-NotSocketActivated=true '' + attrsToSection def.serviceConfig); }; diff --git a/nixos/modules/services/hardware/udev.nix b/nixos/modules/services/hardware/udev.nix index 1ea001fcc26a9..f46c845c08483 100644 --- a/nixos/modules/services/hardware/udev.nix +++ b/nixos/modules/services/hardware/udev.nix @@ -434,10 +434,9 @@ in ''; systemd.services.systemd-udevd = { - restartTriggers = [ config.environment.etc."udev/rules.d".source ]; - notSocketActivated = true; - }; - + restartTriggers = [ config.environment.etc."udev/rules.d".source ]; + notSocketActivated = true; + }; }; imports = [ From bc1cfec92030da8d95deb0e46fcf095b7a3e3918 Mon Sep 17 00:00:00 2001 From: Andreas Fuchs Date: Wed, 18 Dec 2024 08:18:27 -0500 Subject: [PATCH 5/5] Address review feedback: It's "configuration switch" --- nixos/lib/systemd-unit-options.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixos/lib/systemd-unit-options.nix b/nixos/lib/systemd-unit-options.nix index 44fef28ec11e5..b158ab7fca96c 100644 --- a/nixos/lib/systemd-unit-options.nix +++ b/nixos/lib/systemd-unit-options.nix @@ -540,7 +540,7 @@ in rec { default = false; description = '' If set, a changed unit is never assumed to be - socket-activated on configuration activation, even if + socket-activated on configuration switch, even if it might have associated socket units. Instead, the unit will be restarted (or stopped/started) as if it had no associated sockets.