diff --git a/nixos/tests/all-tests.nix b/nixos/tests/all-tests.nix index c1a46aba39e2..502f186408ee 100644 --- a/nixos/tests/all-tests.nix +++ b/nixos/tests/all-tests.nix @@ -1129,6 +1129,9 @@ in nixos-rebuild-target-host = runTest { imports = [ ./nixos-rebuild-target-host.nix ]; }; + nixos-rebuild-target-host-interrupted = runTest { + imports = [ ./nixos-rebuild-target-host-interrupted.nix ]; + }; nixpkgs = pkgs.callPackage ../modules/misc/nixpkgs/test.nix { inherit evalMinimalConfig; }; nixpkgs-config-allow-unfree = pkgs.callPackage ../modules/misc/nixpkgs/test-nixpkgs-config-allow-unfree.nix diff --git a/nixos/tests/nixos-rebuild-target-host-interrupted.nix b/nixos/tests/nixos-rebuild-target-host-interrupted.nix new file mode 100644 index 000000000000..e780019cc68d --- /dev/null +++ b/nixos/tests/nixos-rebuild-target-host-interrupted.nix @@ -0,0 +1,237 @@ +{ hostPkgs, ... }: + +# This test recreates a remote deployment scenario where the connection +# between deployer and target is closed during the deployment - in this +# case because the connection goes over a 'reverse ssh' tunnel service +# that has changes that are being deployed. + +# This is not seamless (the deployer doesn't get to see the logs after +# the disconnect), but is a lot better than the old behaviour, where +# the switch was aborted and the connection never restored. + +{ + name = "nixos-rebuild-target-host-interrupted"; + + # TODO: remove overlay from nixos/modules/profiles/installation-device.nix + # make it a _small package instead, then remove pkgsReadOnly = false;. + node.pkgsReadOnly = false; + + nodes = { + deployer = + { + nodes, + lib, + pkgs, + ... + }: + let + inherit (import ./ssh-keys.nix pkgs) snakeOilPrivateKey snakeOilPublicKey; + in + { + imports = [ + ../modules/profiles/installation-device.nix + ]; + + nix.settings = { + substituters = lib.mkForce [ ]; + hashed-mirrors = null; + connect-timeout = 1; + }; + + system.includeBuildDependencies = true; + + virtualisation = { + cores = 2; + memorySize = 3072; + }; + + services.openssh.enable = true; + users.users.root.openssh.authorizedKeys.keys = [ nodes.target.system.build.publicKey ]; + system.extraDependencies = [ + # so that it doesn't need to be built inside the test + pkgs.nixVersions.latest + ]; + + system.build.privateKey = snakeOilPrivateKey; + system.build.publicKey = snakeOilPublicKey; + system.switch.enable = true; + + services.getty.autologinUser = lib.mkForce "root"; + }; + + target = + { + nodes, + lib, + pkgs, + ... + }: + let + inherit (import ./ssh-keys.nix pkgs) snakeOilPrivateKey snakeOilPublicKey; + targetConfig = { + documentation.enable = false; + services.openssh.enable = true; + system.build.privateKey = snakeOilPrivateKey; + system.build.publicKey = snakeOilPublicKey; + + users.users.root.openssh.authorizedKeys.keys = [ nodes.deployer.system.build.publicKey ]; + users.users.alice.openssh.authorizedKeys.keys = [ nodes.deployer.system.build.publicKey ]; + users.users.bob.openssh.authorizedKeys.keys = [ nodes.deployer.system.build.publicKey ]; + + users.users.alice.extraGroups = [ "wheel" ]; + users.users.bob.extraGroups = [ "wheel" ]; + + # Disable sudo for root to ensure sudo isn't called without `--sudo` + security.sudo.extraRules = lib.mkForce [ + { + groups = [ "wheel" ]; + commands = [ { command = "ALL"; } ]; + } + { + users = [ "alice" ]; + commands = [ + { + command = "ALL"; + options = [ "NOPASSWD" ]; + } + ]; + } + ]; + + nix.settings.trusted-users = [ "@wheel" ]; + + systemd.services."autossh-ng" = { + after = [ "network.target" ]; + wantedBy = [ "multi-user.target" ]; + serviceConfig = { + User = "root"; + Restart = "always"; + RestartSec = "10s"; + ExecStart = "${pkgs.openssh}/bin/ssh -o \"ServerAliveInterval 30\" -o \"ServerAliveCountMax 3\" -o ExitOnForwardFailure=yes -N -R2222:localhost:22 deployer"; + }; + }; + + }; + in + { + imports = [ ./common/user-account.nix ]; + + config = lib.mkMerge [ + targetConfig + { + system.build = { + inherit targetConfig; + }; + system.switch.enable = true; + + networking.hostName = "target"; + } + ]; + }; + }; + + testScript = + { nodes, ... }: + let + sshConfig = builtins.toFile "ssh.conf" '' + UserKnownHostsFile=/dev/null + StrictHostKeyChecking=no + ''; + + targetConfigJSON = hostPkgs.writeText "target-configuration.json" ( + builtins.toJSON nodes.target.system.build.targetConfig + ); + + targetNetworkJSON = hostPkgs.writeText "target-network.json" ( + builtins.toJSON nodes.target.system.build.networkConfig + ); + + configFile = + hostname: + hostPkgs.writeText "configuration.nix" # nix + '' + { lib, pkgs, modulesPath, ... }: { + imports = [ + (modulesPath + "/virtualisation/qemu-vm.nix") + (modulesPath + "/testing/test-instrumentation.nix") + (modulesPath + "/../tests/common/user-account.nix") + (lib.modules.importJSON ./target-configuration.json) + (lib.modules.importJSON ./target-network.json) + ./hardware-configuration.nix + ]; + + boot.loader.grub = { + enable = true; + device = "/dev/vda"; + forceInstall = true; + }; + + # needed to make NIX_SSHOPTS work for nix-copy-closure + nix.package = pkgs.nixVersions.latest; + + # We're changing the '-E' parameter to the new hostname here, + # not because we care about the logs, but because we want to + # force the scenario where the connection is broken during the + # deployment (because the autossh-ng service is stopped and + # started): + systemd.services."autossh-ng".serviceConfig.ExecStart = + lib.mkForce "''${pkgs.openssh}/bin/ssh -o \"ServerAliveInterval 30\" -o \"ServerAliveCountMax 3\" -o ExitOnForwardFailure=yes -N -R2222:localhost:22 -E ${hostname} deployer"; + + # this will be asserted to validate the switch happened: + networking.hostName = "${hostname}"; + } + ''; + in + # python + '' + start_all() + target.wait_for_open_port(22) + + deployer.wait_until_succeeds("ping -c1 target") + deployer.succeed("install -Dm 600 ${nodes.deployer.system.build.privateKey} ~root/.ssh/id_ecdsa") + deployer.succeed("install ${sshConfig} ~root/.ssh/config") + + target.succeed("nixos-generate-config") + target.succeed("install -Dm 600 ${nodes.target.system.build.privateKey} ~root/.ssh/id_ecdsa") + target.succeed("install ${sshConfig} ~root/.ssh/config") + deployer.succeed("scp alice@target:/etc/nixos/hardware-configuration.nix /root/hardware-configuration.nix") + target.wait_for_unit("autossh-ng.service") + + deployer.copy_from_host("${configFile "config-1-deployed"}", "/root/configuration-1.nix") + deployer.copy_from_host("${configFile "config-2-deployed"}", "/root/configuration-2.nix") + deployer.copy_from_host("${targetNetworkJSON}", "/root/target-network.json") + deployer.copy_from_host("${targetConfigJSON}", "/root/target-configuration.json") + + with subtest("Deploy to alice@target via reverse ssh"): + deployer.wait_for_unit("multi-user.target") + # Uses TTY/send_chars instead of deployer.succeed to set NIX_SSHOPTS + deployer.send_chars("NIX_SSHOPTS=\"-p 2222\" nixos-rebuild switch -I nixos-config=/root/configuration-1.nix --target-host alice@localhost --sudo\n") + + # the connection breaks, but the 'switch' should now continue in the background: + deployer.wait_until_tty_matches("1", "error: while running command with remote sudo") + + def deployed(last_try: bool) -> bool: + target_hostname = deployer.succeed("ssh alice@target cat /etc/hostname", timeout=20).rstrip() + if last_try: + print(f"Still seeing hostname {target_hostname}") + return target_hostname == "config-1-deployed" + retry(deployed) + + with subtest("Deploy to bob@target via reverse ssh with password-based sudo"): + deployer.wait_for_unit("multi-user.target") + # Uses TTY/send_chars instead of deployer.succeed to set NIX_SSHOPTS and for ask-sudo-password + deployer.send_chars("NIX_SSHOPTS=\"-p 2222\" nixos-rebuild switch -I nixos-config=/root/configuration-2.nix --target-host bob@localhost --ask-sudo-password\n") + deployer.wait_until_tty_matches("1", "password for bob") + deployer.send_chars("${nodes.target.users.users.bob.password}\n") + + # the connection breaks, but the 'switch' should now continue in the background: + deployer.wait_until_tty_matches("1", "error: while running command with remote sudo") + + def deployed(last_try: bool) -> bool: + target_hostname = deployer.succeed("ssh alice@target cat /etc/hostname", timeout=20).rstrip() + if last_try: + print(f"Still seeing hostname {target_hostname}") + return target_hostname == "config-2-deployed" + retry(deployed) + ''; +} diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/nix.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/nix.py index ee1179b9f440..03c9e9fe5b7b 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/nix.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/nix.py @@ -25,12 +25,14 @@ from .models import ( Profile, Remote, ) -from .process import PRESERVE_ENV, SSH_DEFAULT_OPTS, run_wrapper +from .process import PRESERVE_ENV, SSH_DEFAULT_OPTS, run_wrapper, run_wrapper_bg +from .process import Args as ProcessArgs from .utils import Args, dict_to_flags FLAKE_FLAGS: Final = ["--extra-experimental-features", "nix-command flakes"] FLAKE_REPL_TEMPLATE: Final = "repl.nix.template" -SWITCH_TO_CONFIGURATION_CMD_PREFIX: Final = [ +SYSTEMD_RUN_UNIT_PREFIX: Final = "nixos-rebuild-switch-to-configuration" +SYSTEMD_RUN_CMD_PREFIX: Final = [ "systemd-run", "-E", # Will be set to new value early in switch-to-configuration script, @@ -41,11 +43,10 @@ SWITCH_TO_CONFIGURATION_CMD_PREFIX: Final = [ "-E", "NIXOS_NO_CHECK", "--collect", + "--wait", "--no-ask-password", - "--pipe", "--quiet", "--service-type=exec", - "--unit=nixos-rebuild-switch-to-configuration", ] logger: Final = logging.getLogger(__name__) @@ -669,6 +670,78 @@ def set_profile( ) +def _has_systemd(target_host: Remote | None) -> bool: + r = run_wrapper( + ["test", "-d", "/run/systemd/system"], + remote=target_host, + check=False, + ) + return r.returncode == 0 + + +def _run_action( + action: Literal[Action.SWITCH, Action.BOOT, Action.TEST, Action.DRY_ACTIVATE], + path_to_config: Path, + install_bootloader: bool, + target_host: Remote | None, + sudo: bool, + prefix: ProcessArgs | None = None, +) -> None: + cmd: ProcessArgs = [path_to_config / "bin/switch-to-configuration", str(action)] + if prefix: + cmd = [*prefix, *cmd] + + run_wrapper( + cmd, + env={ + "LOCALE_ARCHIVE": PRESERVE_ENV, + "NIXOS_NO_CHECK": PRESERVE_ENV, + "NIXOS_INSTALL_BOOTLOADER": "1" if install_bootloader else "0", + }, + remote=target_host, + sudo=sudo, + ) + + +def _run_action_with_systemd( + action: Literal[Action.SWITCH, Action.BOOT, Action.TEST, Action.DRY_ACTIVATE], + path_to_config: Path, + install_bootloader: bool, + target_host: Remote | None, + sudo: bool, +) -> None: + unique_unit_name = SYSTEMD_RUN_UNIT_PREFIX + "-" + uuid.uuid4().hex[:8] + journalctl = run_wrapper_bg( + [ + "journalctl", + "-f", + f"--unit={unique_unit_name}", + "--output=cat", + ], + remote=target_host, + sudo=sudo, + ) + + try: + _run_action( + action, + path_to_config, + install_bootloader, + target_host, + sudo, + prefix=[*SYSTEMD_RUN_CMD_PREFIX, f"--unit={unique_unit_name}"], + ) + except KeyboardInterrupt: + run_wrapper( + ["systemctl", "stop", unique_unit_name], + remote=target_host, + sudo=sudo, + ) + raise + finally: + journalctl.terminate() + + def switch_to_configuration( path_to_config: Path, action: Literal[Action.SWITCH, Action.BOOT, Action.TEST, Action.DRY_ACTIVATE], @@ -692,29 +765,26 @@ def switch_to_configuration( if not path_to_config.exists(): raise NixOSRebuildError(f"specialisation not found: {specialisation}") - r = run_wrapper( - ["test", "-d", "/run/systemd/system"], - remote=target_host, - check=False, - ) - cmd = SWITCH_TO_CONFIGURATION_CMD_PREFIX - if r.returncode: + if _has_systemd(target_host): + _run_action_with_systemd( + action, + path_to_config, + install_bootloader, + target_host, + sudo, + ) + else: logger.debug( "skipping systemd-run to switch configuration since systemd is " "not working in target host" ) - cmd = [] - - run_wrapper( - [*cmd, path_to_config / "bin/switch-to-configuration", str(action)], - env={ - "LOCALE_ARCHIVE": PRESERVE_ENV, - "NIXOS_NO_CHECK": PRESERVE_ENV, - "NIXOS_INSTALL_BOOTLOADER": "1" if install_bootloader else "0", - }, - remote=target_host, - sudo=sudo, - ) + _run_action( + action, + path_to_config, + install_bootloader, + target_host, + sudo, + ) def upgrade_channels(all_channels: bool = False, sudo: bool = False) -> None: diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py index ac83d92039f7..aa32a20a7fac 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py @@ -9,7 +9,7 @@ from collections.abc import Mapping, Sequence from dataclasses import dataclass from enum import Enum from ipaddress import AddressValueError, IPv6Address -from typing import Final, Literal, Self, TextIO, TypedDict, Unpack, override +from typing import Final, Literal, NamedTuple, Self, TextIO, TypedDict, Unpack, override from . import tmpdir @@ -117,16 +117,18 @@ def cleanup_ssh() -> None: atexit.register(cleanup_ssh) -def run_wrapper( +class _RunParams(NamedTuple): + args: Args + popen_env: dict[str, str] | None + process_input: str | None + + +def _build_run_params( args: Args, - *, - check: bool = True, - env: Mapping[str, EnvValue] | None = None, - remote: Remote | None = None, - sudo: bool = False, - **kwargs: Unpack[RunKwargs], -) -> subprocess.CompletedProcess[str]: - "Wrapper around `subprocess.run` that supports extra functionality." + env: Mapping[str, EnvValue] | None, + remote: Remote | None, + sudo: bool, +) -> _RunParams: process_input = None run_args: list[Arg] = list(args) final_args: list[Arg] @@ -188,9 +190,63 @@ def run_wrapper( final_args = run_args popen_env = None if env is None else resolved_env + return _RunParams(final_args, popen_env, process_input) + + +def run_wrapper_bg( + args: Args, + env: Mapping[str, EnvValue] | None = None, + remote: Remote | None = None, + sudo: bool = False, +) -> subprocess.Popen[str]: + "Wrapper around `subprocess.Popen` that supports extra functionality." + (final_args, popen_env, process_input) = _build_run_params(args, env, remote, sudo) + + logger.debug( + "calling Popen with args=%r", + _sanitize_env_run_args(list(final_args)), + ) + + if process_input: + stdin = subprocess.PIPE + else: + stdin = None + + r = subprocess.Popen( + final_args, + env=popen_env, + stdin=stdin, + # Hope nobody is using NixOS with non-UTF8 encodings, but + # "surrogateescape" should still work in those systems. + text=True, + errors="surrogateescape", + ) + + if r.stdin and process_input: + r.stdin.write(process_input) + r.stdin.write("\n") + r.stdin.close() + + return r + + +def run_wrapper( + args: Args, + *, + check: bool = True, + env: Mapping[str, EnvValue] | None = None, + remote: Remote | None = None, + sudo: bool = False, + **kwargs: Unpack[RunKwargs], +) -> subprocess.CompletedProcess[str]: + "Wrapper around `subprocess.run` that supports extra functionality." + (final_args, popen_env, process_input) = _build_run_params( + list(args), env, remote, sudo + ) + logger.debug( "calling run with args=%r, kwargs=%r, env=%r", - _sanitize_env_run_args(remote_run_args if remote else run_args), + _sanitize_env_run_args(list(final_args)), kwargs, env, ) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py index 7f51c09d7497..986f6883a42d 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py @@ -166,8 +166,15 @@ def test_parse_args() -> None: {"NIXOS_REBUILD_I_UNDERSTAND_THE_CONSEQUENCES_PLEASE_BREAK_MY_SYSTEM": "1"}, clear=True, ) +@patch("uuid.uuid4") @patch("subprocess.run", autospec=True) -def test_execute_nix_boot(mock_run: Mock, tmp_path: Path) -> None: +@patch("subprocess.Popen") +def test_execute_nix_boot( + mock_popen: Mock, mock_run: Mock, mock_uuid4: Mock, tmp_path: Path +) -> None: + test_uuid = uuid.UUID("58fe9784-f60a-42bc-aa94-eb8f1a7e5c17") + mock_uuid4.return_value = test_uuid + nixpkgs_path = tmp_path / "nixpkgs" (nixpkgs_path / ".git").mkdir(parents=True) config_path = tmp_path / "test" @@ -238,7 +245,8 @@ def test_execute_nix_boot(mock_run: Mock, tmp_path: Path) -> None: ), call( [ - *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, + *nr.nix.SYSTEMD_RUN_CMD_PREFIX, + f"--unit={nr.nix.SYSTEMD_RUN_UNIT_PREFIX}-{test_uuid.hex[:8]}", config_path / "bin/switch-to-configuration", "boot", ], @@ -259,7 +267,8 @@ def test_execute_nix_boot(mock_run: Mock, tmp_path: Path) -> None: # https://github.com/NixOS/nixpkgs/issues/437872 @patch.dict(os.environ, {}, clear=True) @patch("subprocess.run", autospec=True) -def test_execute_nix_build(mock_run: Mock, tmp_path: Path) -> None: +@patch("subprocess.Popen") +def test_execute_nix_build(mock_popen: Mock, mock_run: Mock, tmp_path: Path) -> None: config_path = tmp_path / "test" config_path.touch() @@ -301,7 +310,8 @@ def test_execute_nix_build(mock_run: Mock, tmp_path: Path) -> None: @patch.dict(os.environ, {}, clear=True) @patch("subprocess.run", autospec=True) -def test_execute_nix_build_vm(mock_run: Mock, tmp_path: Path) -> None: +@patch("subprocess.Popen") +def test_execute_nix_build_vm(mock_popen: Mock, mock_run: Mock, tmp_path: Path) -> None: config_path = tmp_path / "test" config_path.touch() @@ -350,7 +360,10 @@ def test_execute_nix_build_vm(mock_run: Mock, tmp_path: Path) -> None: @patch.dict(os.environ, {}, clear=True) @patch("subprocess.run", autospec=True) -def test_execute_nix_build_image_flake(mock_run: Mock, tmp_path: Path) -> None: +@patch("subprocess.Popen") +def test_execute_nix_build_image_flake( + mock_popen: Mock, mock_run: Mock, tmp_path: Path +) -> None: config_path = tmp_path / "test" config_path.touch() @@ -432,11 +445,18 @@ def test_execute_nix_build_image_flake(mock_run: Mock, tmp_path: Path) -> None: {"NIXOS_REBUILD_I_UNDERSTAND_THE_CONSEQUENCES_PLEASE_BREAK_MY_SYSTEM": "1"}, clear=True, ) +@patch("uuid.uuid4") @patch("subprocess.run", autospec=True) -def test_execute_nix_switch_flake(mock_run: Mock, tmp_path: Path) -> None: +@patch("subprocess.Popen") +def test_execute_nix_switch_flake( + mock_popen: Mock, mock_run: Mock, mock_uuid4: Mock, tmp_path: Path +) -> None: config_path = tmp_path / "test" config_path.touch() + test_uuid = uuid.UUID("58fe9784-f60a-42bc-aa94-eb8f1a7e5c17") + mock_uuid4.return_value = test_uuid + def run_side_effect(args: list[str], **kwargs: Any) -> CompletedProcess[str]: if args[0] == "nix": return CompletedProcess([], 0, str(config_path)) @@ -506,7 +526,8 @@ def test_execute_nix_switch_flake(mock_run: Mock, tmp_path: Path) -> None: "env", "-i", "NIXOS_INSTALL_BOOTLOADER=1", - *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, + *nr.nix.SYSTEMD_RUN_CMD_PREFIX, + f"--unit={nr.nix.SYSTEMD_RUN_UNIT_PREFIX}-{test_uuid.hex[:8]}", config_path / "bin/switch-to-configuration", "switch", ], @@ -523,11 +544,13 @@ def test_execute_nix_switch_flake(mock_run: Mock, tmp_path: Path) -> None: clear=True, ) @patch("subprocess.run", autospec=True) +@patch("subprocess.Popen") @patch("uuid.uuid4", autospec=True) @patch(get_qualified_name(nr.services.cleanup_ssh), autospec=True) def test_execute_nix_switch_build_target_host( mock_cleanup_ssh: Mock, mock_uuid4: Mock, + mock_popen: Mock, mock_run: Mock, tmp_path: Path, ) -> None: @@ -551,7 +574,8 @@ def test_execute_nix_switch_build_target_host( return CompletedProcess([], 0) mock_run.side_effect = run_side_effect - mock_uuid4.return_value = uuid.UUID(int=0) + test_uuid = uuid.UUID("58fe9784-f60a-42bc-aa94-eb8f1a7e5c17") + mock_uuid4.side_effect = [uuid.UUID(int=0), uuid.UUID(int=1), test_uuid] nr.execute( [ @@ -644,7 +668,7 @@ def test_execute_nix_switch_build_target_host( "--realise", str(config_path), "--add-root", - "/tmp/tmpdir/00000000000000000000000000000000", + "/tmp/tmpdir/00000000000000000000000000000001", ], check=True, stdout=PIPE, @@ -748,7 +772,8 @@ def test_execute_nix_switch_build_target_host( "-c", """'exec env -i PATH="${PATH-}" LOCALE_ARCHIVE="${LOCALE_ARCHIVE-}" NIXOS_NO_CHECK="${NIXOS_NO_CHECK-}" NIXOS_INSTALL_BOOTLOADER=0 "$@"'""", "sh", - *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, + *nr.nix.SYSTEMD_RUN_CMD_PREFIX, + f"--unit={nr.nix.SYSTEMD_RUN_UNIT_PREFIX}-{test_uuid.hex[:8]}", str(config_path / "bin/switch-to-configuration"), "switch", ], @@ -764,13 +789,20 @@ def test_execute_nix_switch_build_target_host( {"NIXOS_REBUILD_I_UNDERSTAND_THE_CONSEQUENCES_PLEASE_BREAK_MY_SYSTEM": "1"}, clear=True, ) +@patch("uuid.uuid4") @patch("subprocess.run", autospec=True) +@patch("subprocess.Popen") @patch(get_qualified_name(nr.services.cleanup_ssh), autospec=True) def test_execute_nix_switch_flake_target_host( mock_cleanup_ssh: Mock, + mock_popen: Mock, mock_run: Mock, + mock_uuid4: Mock, tmp_path: Path, ) -> None: + test_uuid = uuid.UUID("58fe9784-f60a-42bc-aa94-eb8f1a7e5c17") + mock_uuid4.return_value = test_uuid + config_path = tmp_path / "test" config_path.touch() @@ -865,7 +897,8 @@ def test_execute_nix_switch_flake_target_host( "-c", """'exec env -i PATH="${PATH-}" LOCALE_ARCHIVE="${LOCALE_ARCHIVE-}" NIXOS_NO_CHECK="${NIXOS_NO_CHECK-}" NIXOS_INSTALL_BOOTLOADER=0 "$@"'""", "sh", - *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, + *nr.nix.SYSTEMD_RUN_CMD_PREFIX, + f"--unit={nr.nix.SYSTEMD_RUN_UNIT_PREFIX}-{test_uuid.hex[:8]}", str(config_path / "bin/switch-to-configuration"), "switch", ], @@ -881,13 +914,20 @@ def test_execute_nix_switch_flake_target_host( {"NIXOS_REBUILD_I_UNDERSTAND_THE_CONSEQUENCES_PLEASE_BREAK_MY_SYSTEM": "1"}, clear=True, ) +@patch("uuid.uuid4") @patch("subprocess.run", autospec=True) +@patch("subprocess.Popen") @patch(get_qualified_name(nr.services.cleanup_ssh), autospec=True) def test_execute_nix_switch_flake_build_host( mock_cleanup_ssh: Mock, + mock_popen: Mock, mock_run: Mock, + mock_uuid4: Mock, tmp_path: Path, ) -> None: + test_uuid = uuid.UUID("58fe9784-f60a-42bc-aa94-eb8f1a7e5c17") + mock_uuid4.return_value = test_uuid + config_path = tmp_path / "test" config_path.touch() @@ -984,7 +1024,8 @@ def test_execute_nix_switch_flake_build_host( ), call( [ - *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, + *nr.nix.SYSTEMD_RUN_CMD_PREFIX, + f"--unit={nr.nix.SYSTEMD_RUN_UNIT_PREFIX}-{test_uuid.hex[:8]}", config_path / "bin/switch-to-configuration", "switch", ], @@ -996,7 +1037,10 @@ def test_execute_nix_switch_flake_build_host( @patch("subprocess.run", autospec=True) -def test_execute_switch_rollback(mock_run: Mock, tmp_path: Path) -> None: +@patch("subprocess.Popen") +def test_execute_switch_rollback( + mock_popen: Mock, mock_run: Mock, tmp_path: Path +) -> None: nixpkgs_path = tmp_path / "nixpkgs" (nixpkgs_path / ".git").mkdir(parents=True) @@ -1073,7 +1117,8 @@ def test_execute_switch_rollback(mock_run: Mock, tmp_path: Path) -> None: @patch("subprocess.run", autospec=True) -def test_execute_build(mock_run: Mock, tmp_path: Path) -> None: +@patch("subprocess.Popen") +def test_execute_build(mock_popen: Mock, mock_run: Mock, tmp_path: Path) -> None: config_path = tmp_path / "test" config_path.touch() mock_run.side_effect = [ @@ -1102,8 +1147,9 @@ def test_execute_build(mock_run: Mock, tmp_path: Path) -> None: @patch("subprocess.run", autospec=True) +@patch("subprocess.Popen") def test_execute_build_dry_run_build_and_target_remote( - mock_run: Mock, tmp_path: Path + mock_popen: Mock, mock_run: Mock, tmp_path: Path ) -> None: config_path = tmp_path / "test" config_path.touch() @@ -1174,7 +1220,8 @@ def test_execute_build_dry_run_build_and_target_remote( @patch("subprocess.run", autospec=True) -def test_execute_test_flake(mock_run: Mock, tmp_path: Path) -> None: +@patch("subprocess.Popen") +def test_execute_test_flake(mock_popen: Mock, mock_run: Mock, tmp_path: Path) -> None: config_path = tmp_path / "test" config_path.touch() @@ -1224,11 +1271,13 @@ def test_execute_test_flake(mock_run: Mock, tmp_path: Path) -> None: @patch("subprocess.run", autospec=True) +@patch("subprocess.Popen") @patch("pathlib.Path.exists", autospec=True, return_value=True) @patch("pathlib.Path.mkdir", autospec=True) def test_execute_test_rollback( mock_path_mkdir: Mock, mock_path_exists: Mock, + mock_popen: Mock, mock_run: Mock, ) -> None: def run_side_effect(args: list[str], **kwargs: Any) -> CompletedProcess[str]: @@ -1291,8 +1340,15 @@ def test_execute_test_rollback( {"NIXOS_REBUILD_I_UNDERSTAND_THE_CONSEQUENCES_PLEASE_BREAK_MY_SYSTEM": "1"}, clear=True, ) +@patch("uuid.uuid4", autospec=True) @patch("subprocess.run", autospec=True) -def test_execute_switch_store_path(mock_run: Mock, tmp_path: Path) -> None: +@patch("subprocess.Popen") +def test_execute_switch_store_path( + mock_popen: Mock, mock_run: Mock, mock_uuid4: Mock, tmp_path: Path +) -> None: + test_uuid = uuid.UUID("58fe9784-f60a-42bc-aa94-eb8f1a7e5c17") + mock_uuid4.return_value = test_uuid + config_path = tmp_path / "test-system" config_path.mkdir() @@ -1330,7 +1386,8 @@ def test_execute_switch_store_path(mock_run: Mock, tmp_path: Path) -> None: ), call( [ - *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, + *nr.nix.SYSTEMD_RUN_CMD_PREFIX, + f"--unit={nr.nix.SYSTEMD_RUN_UNIT_PREFIX}-{test_uuid.hex[:8]}", config_path / "bin/switch-to-configuration", "switch", ], @@ -1349,13 +1406,20 @@ def test_execute_switch_store_path(mock_run: Mock, tmp_path: Path) -> None: @patch.dict(os.environ, {}, clear=True) +@patch("uuid.uuid4") @patch("subprocess.run", autospec=True) @patch(get_qualified_name(nr.services.cleanup_ssh), autospec=True) +@patch("subprocess.Popen") def test_execute_switch_store_path_target_host( + mock_popen: Mock, mock_cleanup_ssh: Mock, mock_run: Mock, + mock_uuid4: Mock, tmp_path: Path, ) -> None: + test_uuid = uuid.UUID("58fe9784-f60a-42bc-aa94-eb8f1a7e5c17") + mock_uuid4.return_value = test_uuid + config_path = tmp_path / "test-system" config_path.mkdir() @@ -1448,7 +1512,8 @@ def test_execute_switch_store_path_target_host( "-c", """'exec env -i PATH="${PATH-}" LOCALE_ARCHIVE="${LOCALE_ARCHIVE-}" NIXOS_NO_CHECK="${NIXOS_NO_CHECK-}" NIXOS_INSTALL_BOOTLOADER=0 "$@"'""", "sh", - *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, + *nr.nix.SYSTEMD_RUN_CMD_PREFIX, + f"--unit={nr.nix.SYSTEMD_RUN_UNIT_PREFIX}-{test_uuid.hex[:8]}", str(config_path / "bin/switch-to-configuration"), "switch", ], diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_nix.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_nix.py index fdc9b3377ad3..7a369ab9023f 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_nix.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_nix.py @@ -3,7 +3,7 @@ import sys import textwrap import uuid from pathlib import Path -from subprocess import PIPE, CompletedProcess +from subprocess import PIPE, CompletedProcess, Popen from typing import Any from unittest.mock import ANY, Mock, call, patch @@ -714,139 +714,159 @@ def test_set_profile(mock_run: Mock) -> None: @patch(get_qualified_name(n.run_wrapper, n), autospec=True) +@patch(get_qualified_name(n.run_wrapper_bg, n), autospec=True) def test_switch_to_configuration_without_systemd_run( - mock_run: Any, monkeypatch: MonkeyPatch + mock_run_bg: Mock, mock_run: Mock, monkeypatch: MonkeyPatch ) -> None: profile_path = Path("/path/to/profile") config_path = Path("/path/to/config") - mock_run.return_value = CompletedProcess([], 1) - with monkeypatch.context() as mp: - mp.setenv("LOCALE_ARCHIVE", "") + proc = Popen(["echo"]) + try: + mock_run_bg.return_value = p + mock_run.return_value = CompletedProcess([], 1) - n.switch_to_configuration( - profile_path, - m.Action.SWITCH, + with monkeypatch.context() as mp: + mp.setenv("LOCALE_ARCHIVE", "") + + n.switch_to_configuration( + profile_path, + m.Action.SWITCH, + sudo=False, + target_host=None, + specialisation=None, + install_bootloader=False, + ) + mock_run.assert_called_with( + [profile_path / "bin/switch-to-configuration", "switch"], + env={ + "LOCALE_ARCHIVE": p.PRESERVE_ENV, + "NIXOS_NO_CHECK": p.PRESERVE_ENV, + "NIXOS_INSTALL_BOOTLOADER": "0", + }, sudo=False, - target_host=None, - specialisation=None, - install_bootloader=False, + remote=None, ) - mock_run.assert_called_with( - [profile_path / "bin/switch-to-configuration", "switch"], - env={ - "LOCALE_ARCHIVE": p.PRESERVE_ENV, - "NIXOS_NO_CHECK": p.PRESERVE_ENV, - "NIXOS_INSTALL_BOOTLOADER": "0", - }, - sudo=False, - remote=None, - ) - with pytest.raises(m.NixOSRebuildError) as e: - n.switch_to_configuration( - config_path, - m.Action.BOOT, - sudo=False, - target_host=None, - specialisation="special", + with pytest.raises(m.NixOSRebuildError) as e: + n.switch_to_configuration( + config_path, + m.Action.BOOT, + sudo=False, + target_host=None, + specialisation="special", + ) + assert ( + str(e.value) + == "error: '--specialisation' can only be used with 'switch' and 'test'" ) - assert ( - str(e.value) - == "error: '--specialisation' can only be used with 'switch' and 'test'" - ) - target_host = m.Remote("user@localhost", [], None) - with monkeypatch.context() as mp: - mp.setenv("LOCALE_ARCHIVE", "/path/to/locale") - mp.setenv("PATH", "/path/to/bin") - mp.setattr(Path, Path.exists.__name__, lambda self: True) + target_host = m.Remote("user@localhost", [], None) + with monkeypatch.context() as mp: + mp.setenv("LOCALE_ARCHIVE", "/path/to/locale") + mp.setenv("PATH", "/path/to/bin") + mp.setattr(Path, Path.exists.__name__, lambda self: True) - n.switch_to_configuration( - Path("/path/to/config"), - m.Action.TEST, + n.switch_to_configuration( + Path("/path/to/config"), + m.Action.TEST, + sudo=True, + target_host=target_host, + install_bootloader=True, + specialisation="special", + ) + mock_run.assert_called_with( + [ + config_path / "specialisation/special/bin/switch-to-configuration", + "test", + ], + env={ + "LOCALE_ARCHIVE": p.PRESERVE_ENV, + "NIXOS_NO_CHECK": p.PRESERVE_ENV, + "NIXOS_INSTALL_BOOTLOADER": "1", + }, sudo=True, - target_host=target_host, - install_bootloader=True, - specialisation="special", + remote=target_host, ) - mock_run.assert_called_with( - [ - config_path / "specialisation/special/bin/switch-to-configuration", - "test", - ], - env={ - "LOCALE_ARCHIVE": p.PRESERVE_ENV, - "NIXOS_NO_CHECK": p.PRESERVE_ENV, - "NIXOS_INSTALL_BOOTLOADER": "1", - }, - sudo=True, - remote=target_host, - ) + finally: + proc.communicate(timeout=1) +@patch("uuid.uuid4") @patch(get_qualified_name(n.run_wrapper, n), autospec=True) +@patch(get_qualified_name(n.run_wrapper_bg, n), autospec=True) def test_switch_to_configuration_with_systemd_run( - mock_run: Mock, monkeypatch: MonkeyPatch + mock_run_bg: Mock, mock_run: Mock, mock_uuid4: Mock, monkeypatch: MonkeyPatch ) -> None: + test_uuid = uuid.UUID("58fe9784-f60a-42bc-aa94-eb8f1a7e5c17") + mock_uuid4.return_value = test_uuid + profile_path = Path("/path/to/profile") config_path = Path("/path/to/config") - mock_run.return_value = CompletedProcess([], 0) - with monkeypatch.context() as mp: - mp.setenv("LOCALE_ARCHIVE", "") + proc = Popen(["echo"]) + try: + mock_run_bg.return_value = proc + mock_run.return_value = CompletedProcess([], 0) - n.switch_to_configuration( - profile_path, - m.Action.SWITCH, + with monkeypatch.context() as mp: + mp.setenv("LOCALE_ARCHIVE", "") + + n.switch_to_configuration( + profile_path, + m.Action.SWITCH, + sudo=False, + target_host=None, + specialisation=None, + install_bootloader=False, + ) + mock_run.assert_called_with( + [ + *n.SYSTEMD_RUN_CMD_PREFIX, + f"--unit={n.SYSTEMD_RUN_UNIT_PREFIX}-{test_uuid.hex[:8]}", + profile_path / "bin/switch-to-configuration", + "switch", + ], + env={ + "LOCALE_ARCHIVE": p.PRESERVE_ENV, + "NIXOS_NO_CHECK": p.PRESERVE_ENV, + "NIXOS_INSTALL_BOOTLOADER": "0", + }, sudo=False, - target_host=None, - specialisation=None, - install_bootloader=False, + remote=None, ) - mock_run.assert_called_with( - [ - *n.SWITCH_TO_CONFIGURATION_CMD_PREFIX, - profile_path / "bin/switch-to-configuration", - "switch", - ], - env={ - "LOCALE_ARCHIVE": p.PRESERVE_ENV, - "NIXOS_NO_CHECK": p.PRESERVE_ENV, - "NIXOS_INSTALL_BOOTLOADER": "0", - }, - sudo=False, - remote=None, - ) - target_host = m.Remote("user@localhost", [], None) - with monkeypatch.context() as mp: - mp.setenv("LOCALE_ARCHIVE", "/path/to/locale") - mp.setenv("PATH", "/path/to/bin") - mp.setattr(Path, Path.exists.__name__, lambda self: True) + target_host = m.Remote("user@localhost", [], None) + with monkeypatch.context() as mp: + mp.setenv("LOCALE_ARCHIVE", "/path/to/locale") + mp.setenv("PATH", "/path/to/bin") + mp.setattr(Path, Path.exists.__name__, lambda self: True) - n.switch_to_configuration( - Path("/path/to/config"), - m.Action.TEST, + n.switch_to_configuration( + Path("/path/to/config"), + m.Action.TEST, + sudo=True, + target_host=target_host, + install_bootloader=True, + specialisation="special", + ) + mock_run.assert_called_with( + [ + *n.SYSTEMD_RUN_CMD_PREFIX, + f"--unit={n.SYSTEMD_RUN_UNIT_PREFIX}-{test_uuid.hex[:8]}", + config_path / "specialisation/special/bin/switch-to-configuration", + "test", + ], + env={ + "LOCALE_ARCHIVE": p.PRESERVE_ENV, + "NIXOS_NO_CHECK": p.PRESERVE_ENV, + "NIXOS_INSTALL_BOOTLOADER": "1", + }, sudo=True, - target_host=target_host, - install_bootloader=True, - specialisation="special", + remote=target_host, ) - mock_run.assert_called_with( - [ - *n.SWITCH_TO_CONFIGURATION_CMD_PREFIX, - config_path / "specialisation/special/bin/switch-to-configuration", - "test", - ], - env={ - "LOCALE_ARCHIVE": p.PRESERVE_ENV, - "NIXOS_NO_CHECK": p.PRESERVE_ENV, - "NIXOS_INSTALL_BOOTLOADER": "1", - }, - sudo=True, - remote=target_host, - ) + finally: + proc.communicate(timeout=1) @patch(