diff --git a/nixos/tests/activation/template-dropin.nix b/nixos/tests/activation/template-dropin.nix new file mode 100644 index 000000000000..b4c3de4a29c6 --- /dev/null +++ b/nixos/tests/activation/template-dropin.nix @@ -0,0 +1,91 @@ +{ lib, ... }: +{ + name = "stc-template-dropin"; + + nodes.machine = + { pkgs, lib, ... }: + { + # Define the base template. This file exists in both generations. + systemd.services."test-template@" = { + description = "A base template for testing"; + serviceConfig.ExecStart = "${pkgs.coreutils}/bin/sleep infinity"; + }; + + # Define the managed instance using drop-ins. + systemd.services."test-template@managed" = { + overrideStrategy = "asDropin"; + wantedBy = [ "multi-user.target" ]; + serviceConfig.Environment = "TEST_VAR=1"; + }; + + # Also define a service which will be changed + systemd.services."test-template@changed" = { + overrideStrategy = "asDropin"; + wantedBy = [ "multi-user.target" ]; + serviceConfig.Environment = "TEST_VAR=1"; + }; + + # Create a new generation that explicitly removes the managed instance + specialisation.new-generation.configuration = { + systemd.services."test-template@managed" = { + enable = lib.mkForce false; + wantedBy = lib.mkForce [ ]; + }; + systemd.services."test-template@changed" = { + serviceConfig.Environment = lib.mkForce "TEST_VAR=2"; + }; + }; + }; + + testScript = # python + '' + managed_unit = "test-template@managed.service" + changed_unit = "test-template@changed.service" + manual_unit = "test-template@manual.service" + + with subtest("Start the machine and ensure the managed instance is running"): + machine.wait_for_unit("multi-user.target") + machine.wait_for_unit(managed_unit) + machine.wait_for_unit(changed_unit) + + with subtest("Imperatively start an unmanaged instance"): + machine.succeed(f"systemctl start {manual_unit}") + machine.wait_for_unit(manual_unit) + + with subtest("Run dry-activate on the new generation"): + new_gen = "/run/booted-system/specialisation/new-generation" + + # switch-to-configuration prints to stderr, so we redirect it to stdout for parsing + output = machine.succeed(f"{new_gen}/bin/switch-to-configuration dry-activate 2>&1") + machine.log("dry-activate output:\n" + output) + + found_stop = False + found_start = False + found_changed = False + found_manual_stop = False + for line in output.splitlines(): + if line.startswith("would stop"): + found_stop = found_stop or managed_unit in line + found_manual_stop = found_manual_stop or manual_unit in line + elif line.startswith("would start"): + found_start = found_start or managed_unit in line + found_changed = found_changed or changed_unit in line + + assert found_stop, "The managed instance was not marked for stopping." + assert found_changed, "The changed unit was not marked for stopping + starting (restarting)." + assert not found_start, "switch-to-configuration wants to start the removed managed instance!" + assert not found_manual_stop, "switch-to-configuration wants to stop the manual instance!" + + with subtest("Perform the actual switch and verify system state"): + machine.succeed(f"{new_gen}/bin/switch-to-configuration switch") + + # The managed instance should be dead + machine.fail(f"systemctl is-active {managed_unit}") + + # The changed instance should be running + machine.succeed(f"systemctl is-active {changed_unit}") + + # The manual instance should survive the configuration switch untouched + machine.succeed(f"systemctl is-active {manual_unit}") + ''; +} diff --git a/nixos/tests/all-tests.nix b/nixos/tests/all-tests.nix index 28ca9b54009d..1283bb8bf3cd 100644 --- a/nixos/tests/all-tests.nix +++ b/nixos/tests/all-tests.nix @@ -189,6 +189,7 @@ in activation-nix-channel = runTest ./activation/nix-channel.nix; activation-nixos-init = runTest ./activation/nixos-init.nix; activation-perlless = runTest ./activation/perlless.nix; + activation-template-dropin = runTest ./activation/template-dropin.nix; activation-var = runTest ./activation/var.nix; actual = runTest ./actual.nix; adguardhome = runTest ./adguardhome.nix; diff --git a/pkgs/by-name/sw/switch-to-configuration-ng/package.nix b/pkgs/by-name/sw/switch-to-configuration-ng/package.nix index cffc771405f4..ccb3409d7eea 100644 --- a/pkgs/by-name/sw/switch-to-configuration-ng/package.nix +++ b/pkgs/by-name/sw/switch-to-configuration-ng/package.nix @@ -30,7 +30,7 @@ rustPlatform.buildRustPackage { cargo clippy -- -Dwarnings ''; - passthru.tests = { inherit (nixosTests) switchTest; }; + passthru.tests = { inherit (nixosTests) switchTest activation-template-dropin; }; meta = { description = "NixOS switch-to-configuration program"; diff --git a/pkgs/by-name/sw/switch-to-configuration-ng/src/main.rs b/pkgs/by-name/sw/switch-to-configuration-ng/src/main.rs index f084f27b9ae2..a6879cece260 100644 --- a/pkgs/by-name/sw/switch-to-configuration-ng/src/main.rs +++ b/pkgs/by-name/sw/switch-to-configuration-ng/src/main.rs @@ -410,6 +410,14 @@ fn parse_systemd_ini(data: &mut UnitInfo, mut unit_file: impl Read) -> Result<() Ok(()) } +/// Glob for `.d/*.conf`, escaping any glob metacharacters in the +/// path prefix so unit names containing e.g. `\` (from systemd-escape) are +/// matched literally. +fn unit_dropin_glob(unit_path: &Path) -> Result { + let prefix = glob::Pattern::escape(&format!("{}.d", unit_path.display())); + glob(&format!("{prefix}/*.conf")).context("Invalid glob pattern") +} + // This function takes the path to a systemd configuration file (like a unit configuration) and // parses it into a UnitInfo structure. // @@ -428,9 +436,7 @@ fn parse_unit(unit_file: &Path, base_unit_path: &Path) -> Result { ) })?; - for entry in - glob(&format!("{}.d/*.conf", base_unit_path.display())).context("Invalid glob pattern")? - { + for entry in unit_dropin_glob(base_unit_path)? { let Ok(entry) = entry else { continue; }; @@ -442,9 +448,7 @@ fn parse_unit(unit_file: &Path, base_unit_path: &Path) -> Result { // Handle drop-in template-unit instance overrides if unit_file != base_unit_path { - for entry in - glob(&format!("{}.d/*.conf", unit_file.display())).context("Invalid glob pattern")? - { + for entry in unit_dropin_glob(unit_file)? { let Ok(entry) = entry else { continue; }; @@ -1036,6 +1040,37 @@ fn remove_file_if_exists(p: impl AsRef) -> std::io::Result<()> { } } +#[derive(Debug, PartialEq)] +enum UnitFileState { + /// The file exists and resolves to a real unit file. + Present, + /// The file is a (chain of) symlink(s) to /dev/null, i.e. masked. + Masked, + /// The file does not exist, or is a dangling symlink. + Missing, +} + +impl UnitFileState { + /// Whether the unit file is absent from the configuration, either because + /// it does not exist or because it has been masked to /dev/null. + fn is_gone(&self) -> bool { + matches!(self, UnitFileState::Masked | UnitFileState::Missing) + } +} + +/// Classify a unit-file path. Unexpected I/O errors are propagated. +fn unit_file_state(unit_file: impl AsRef) -> Result { + let path = unit_file.as_ref(); + match path.canonicalize() { + Ok(target) if target == Path::new("/dev/null") => Ok(UnitFileState::Masked), + Ok(_) => Ok(UnitFileState::Present), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(UnitFileState::Missing), + Err(err) => { + Err(err).with_context(|| format!("Failed to canonicalize unit file {}", path.display())) + } + } +} + /// Iterate over currently active units in the given scope, compare the unit /// file in `old_unit_dir` against the one in `new_unit_dir`, and populate the /// action maps accordingly. @@ -1088,6 +1123,7 @@ fn collect_unit_changes( let mut base_unit = unit.clone(); let mut current_base_unit_file = current_unit_file.clone(); let mut new_base_unit_file = new_unit_file.clone(); + let mut dropins_removed = false; // Detect template instances if let Some((Some(template_name), Some(template_instance))) = @@ -1102,6 +1138,33 @@ fn collect_unit_changes( base_unit = format!("{template_name}@.{template_instance}"); current_base_unit_file = old_unit_dir.join(&base_unit); new_base_unit_file = new_unit_dir.join(&base_unit); + + // Handle instances defined as drop-ins. When the unit is + // disabled, the override files will be a symlink to + // /dev/null instead. + + // The instance was NixOS-managed in the old generation iff + // at least one current drop-in is a real file (not masked). + let mut currently_managed = false; + for entry in unit_dropin_glob(¤t_unit_file)?.flatten() { + if unit_file_state(&entry)? == UnitFileState::Present { + currently_managed = true; + break; + } + } + + // If the instance was managed before, check whether it + // still is: gone iff no new drop-ins remain (or all are + // masked to /dev/null). + if currently_managed { + dropins_removed = true; + for entry in unit_dropin_glob(&new_unit_file)?.flatten() { + if unit_file_state(&entry)? == UnitFileState::Present { + dropins_removed = false; + break; + } + } + } } } @@ -1116,11 +1179,9 @@ fn collect_unit_changes( if current_base_unit_file.exists() && (unit_state.state == "active" || unit_state.state == "activating") { - if new_base_unit_file - .canonicalize() - .map(|full_path| full_path == Path::new("/dev/null")) - .unwrap_or(true) - { + // Account for template unit instances where overrideStrategy == "asDropin" + // whilst also allowing manual instances to keep running. + if dropins_removed || unit_file_state(&new_base_unit_file)?.is_gone() { let current_unit_info = parse_unit(¤t_unit_file, ¤t_base_unit_file)?; if parse_systemd_bool(Some(¤t_unit_info), "Unit", "X-StopOnRemoval", true) { _ = units_to_stop.insert(unit.to_string(), ());