From 6725c129d4c2b58a90d56433c7f5192ec2dd2c63 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Mon, 10 Feb 2025 15:06:19 +0000 Subject: [PATCH] nixos-rebuild-ng: add systemd-run to switch-to-configuration See the original reason for this here: https://github.com/NixOS/nixpkgs/pull/258571. While it seems the original reason may not be valid anymore since `nixos-rebuild` do not allocate TTY in SSH, it seems that some people depends on the `nixos-rebuild-switch-to-configuration` to exist. --- pkgs/by-name/ni/nixos-rebuild-ng/README.md | 2 - .../nixos-rebuild-ng/src/nixos_rebuild/nix.py | 31 +++++- .../nixos-rebuild-ng/src/tests/test_main.py | 99 ++++++++++++++++--- .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 61 +++++++++++- 4 files changed, 178 insertions(+), 15 deletions(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/README.md b/pkgs/by-name/ni/nixos-rebuild-ng/README.md index 206cfdbb4912..10bd1a90e9d4 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/README.md +++ b/pkgs/by-name/ni/nixos-rebuild-ng/README.md @@ -148,8 +148,6 @@ not possible to fix, please open an issue and we can discuss a solution. ## TODON'T -- Reimplement `systemd-run` logic: will be moved to the new - [`apply`](https://github.com/NixOS/nixpkgs/pull/344407) script - Nix bootstrap: it is only used for non-Flake paths and it is basically useless nowadays. It was created at a time when Nix was changing frequently and there was a need to bootstrap a new version of Nix before evaluating the 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 a8fed7d8b9de..8ea339e7b064 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 @@ -29,6 +29,22 @@ 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", + "-E", + # Will be set to new value early in switch-to-configuration script, + # but interpreter starts out with old value + "LOCALE_ARCHIVE", + "-E", + "NIXOS_INSTALL_BOOTLOADER", + "--collect", + "--no-ask-password", + "--pipe", + "--quiet", + "--same-dir", + "--service-type=exec", + "--unit=nixos-rebuild-switch-to-configuration", +] logger = logging.getLogger(__name__) @@ -628,8 +644,21 @@ def switch_to_configuration( if not path_to_config.exists(): raise NRError(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: + logger.debug( + "skipping systemd-run to switch configuration since systemd is " + + "not working in target host" + ) + cmd = [] + run_wrapper( - [path_to_config / "bin/switch-to-configuration", str(action)], + [*cmd, path_to_config / "bin/switch-to-configuration", str(action)], extra_env={"NIXOS_INSTALL_BOOTLOADER": "1" if install_bootloader else "0"}, remote=target_host, sudo=sudo, 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 35306fc7e30c..db21ddd9d64a 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 @@ -234,7 +234,7 @@ def test_execute_nix_boot(mock_run: Mock, tmp_path: Path) -> None: nr.execute(["nixos-rebuild", "boot", "--no-flake", "-vvv", "--no-reexec"]) - assert mock_run.call_count == 6 + assert mock_run.call_count == 7 mock_run.assert_has_calls( [ call( @@ -279,7 +279,16 @@ def test_execute_nix_boot(mock_run: Mock, tmp_path: Path) -> None: **DEFAULT_RUN_KWARGS, ), call( - [config_path / "bin/switch-to-configuration", "boot"], + ["test", "-d", "/run/systemd/system"], + check=False, + **DEFAULT_RUN_KWARGS, + ), + call( + [ + *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, + config_path / "bin/switch-to-configuration", + "boot", + ], check=True, **(DEFAULT_RUN_KWARGS | {"env": {"NIXOS_INSTALL_BOOTLOADER": "0"}}), ), @@ -442,7 +451,7 @@ def test_execute_nix_switch_flake(mock_run: Mock, tmp_path: Path) -> None: ] ) - assert mock_run.call_count == 3 + assert mock_run.call_count == 4 mock_run.assert_has_calls( [ call( @@ -476,7 +485,17 @@ def test_execute_nix_switch_flake(mock_run: Mock, tmp_path: Path) -> None: **DEFAULT_RUN_KWARGS, ), call( - ["sudo", config_path / "bin/switch-to-configuration", "switch"], + ["test", "-d", "/run/systemd/system"], + check=False, + **DEFAULT_RUN_KWARGS, + ), + call( + [ + "sudo", + *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, + config_path / "bin/switch-to-configuration", + "switch", + ], check=True, **(DEFAULT_RUN_KWARGS | {"env": {"NIXOS_INSTALL_BOOTLOADER": "1"}}), ), @@ -535,7 +554,7 @@ def test_execute_nix_switch_build_target_host( ] ) - assert mock_run.call_count == 10 + assert mock_run.call_count == 11 mock_run.assert_has_calls( [ call( @@ -661,6 +680,19 @@ def test_execute_nix_switch_build_target_host( check=True, **DEFAULT_RUN_KWARGS, ), + call( + [ + "ssh", + *nr.process.SSH_DEFAULT_OPTS, + "user@target-host", + "--", + "test", + "-d", + "/run/systemd/system", + ], + check=False, + **DEFAULT_RUN_KWARGS, + ), call( [ "ssh", @@ -670,6 +702,7 @@ def test_execute_nix_switch_build_target_host( "sudo", "env", "NIXOS_INSTALL_BOOTLOADER=0", + *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, str(config_path / "bin/switch-to-configuration"), "switch", ], @@ -712,7 +745,7 @@ def test_execute_nix_switch_flake_target_host( ] ) - assert mock_run.call_count == 4 + assert mock_run.call_count == 5 mock_run.assert_has_calls( [ call( @@ -750,6 +783,19 @@ def test_execute_nix_switch_flake_target_host( check=True, **DEFAULT_RUN_KWARGS, ), + call( + [ + "ssh", + *nr.process.SSH_DEFAULT_OPTS, + "user@localhost", + "--", + "test", + "-d", + "/run/systemd/system", + ], + check=False, + **DEFAULT_RUN_KWARGS, + ), call( [ "ssh", @@ -759,6 +805,7 @@ def test_execute_nix_switch_flake_target_host( "sudo", "env", "NIXOS_INSTALL_BOOTLOADER=0", + *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, str(config_path / "bin/switch-to-configuration"), "switch", ], @@ -802,7 +849,7 @@ def test_execute_nix_switch_flake_build_host( ] ) - assert mock_run.call_count == 6 + assert mock_run.call_count == 7 mock_run.assert_has_calls( [ call( @@ -863,7 +910,16 @@ def test_execute_nix_switch_flake_build_host( **DEFAULT_RUN_KWARGS, ), call( - [config_path / "bin/switch-to-configuration", "switch"], + ["test", "-d", "/run/systemd/system"], + check=False, + **DEFAULT_RUN_KWARGS, + ), + call( + [ + *nr.nix.SWITCH_TO_CONFIGURATION_CMD_PREFIX, + config_path / "bin/switch-to-configuration", + "switch", + ], check=True, **DEFAULT_RUN_KWARGS, ), @@ -881,6 +937,8 @@ def test_execute_switch_rollback(mock_run: Mock, tmp_path: Path) -> None: return CompletedProcess([], 0, str(nixpkgs_path)) elif args[0] == "git": return CompletedProcess([], 0, "") + elif args[0] == "test": + return CompletedProcess([], 1) else: return CompletedProcess([], 0) @@ -897,7 +955,7 @@ def test_execute_switch_rollback(mock_run: Mock, tmp_path: Path) -> None: ] ) - assert mock_run.call_count == 4 + assert mock_run.call_count == 5 mock_run.assert_has_calls( [ call( @@ -929,6 +987,11 @@ def test_execute_switch_rollback(mock_run: Mock, tmp_path: Path) -> None: check=True, **DEFAULT_RUN_KWARGS, ), + call( + ["test", "-d", "/run/systemd/system"], + check=False, + **DEFAULT_RUN_KWARGS, + ), call( [ Path("/nix/var/nix/profiles/system/bin/switch-to-configuration"), @@ -978,6 +1041,8 @@ def test_execute_test_flake(mock_run: Mock, tmp_path: Path) -> None: def run_side_effect(args: list[str], **kwargs: Any) -> CompletedProcess[str]: if args[0] == "nix": return CompletedProcess([], 0, str(config_path)) + elif args[0] == "test": + return CompletedProcess([], 1) else: return CompletedProcess([], 0) @@ -987,7 +1052,7 @@ def test_execute_test_flake(mock_run: Mock, tmp_path: Path) -> None: ["nixos-rebuild", "test", "--flake", "github:user/repo#hostname", "--no-reexec"] ) - assert mock_run.call_count == 2 + assert mock_run.call_count == 3 mock_run.assert_has_calls( [ call( @@ -1003,6 +1068,11 @@ def test_execute_test_flake(mock_run: Mock, tmp_path: Path) -> None: stdout=PIPE, **DEFAULT_RUN_KWARGS, ), + call( + ["test", "-d", "/run/systemd/system"], + check=False, + **DEFAULT_RUN_KWARGS, + ), call( [config_path / "bin/switch-to-configuration", "test"], check=True, @@ -1031,6 +1101,8 @@ def test_execute_test_rollback( 2084 2024-11-07 23:54:17 (current) """), ) + elif args[0] == "test": + return CompletedProcess([], 1) else: return CompletedProcess([], 0) @@ -1040,7 +1112,7 @@ def test_execute_test_rollback( ["nixos-rebuild", "test", "--rollback", "--profile-name", "foo", "--no-reexec"] ) - assert mock_run.call_count == 2 + assert mock_run.call_count == 3 mock_run.assert_has_calls( [ call( @@ -1054,6 +1126,11 @@ def test_execute_test_rollback( stdout=PIPE, **DEFAULT_RUN_KWARGS, ), + call( + ["test", "-d", "/run/systemd/system"], + check=False, + **DEFAULT_RUN_KWARGS, + ), call( [ Path( 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 6f615872ac43..a6fac1c6de1e 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 @@ -689,9 +689,12 @@ def test_set_profile(mock_run: Mock) -> None: @patch(get_qualified_name(n.run_wrapper, n), autospec=True) -def test_switch_to_configuration(mock_run: Mock, monkeypatch: MonkeyPatch) -> None: +def test_switch_to_configuration_without_systemd_run( + mock_run: Any, 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", "") @@ -749,6 +752,62 @@ def test_switch_to_configuration(mock_run: Mock, monkeypatch: MonkeyPatch) -> No ) +@patch(get_qualified_name(n.run_wrapper, n), autospec=True) +def test_switch_to_configuration_with_systemd_run( + mock_run: Mock, monkeypatch: MonkeyPatch +) -> None: + 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", "") + + 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.SWITCH_TO_CONFIGURATION_CMD_PREFIX, + profile_path / "bin/switch-to-configuration", + "switch", + ], + extra_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) + + 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.SWITCH_TO_CONFIGURATION_CMD_PREFIX, + config_path / "specialisation/special/bin/switch-to-configuration", + "test", + ], + extra_env={"NIXOS_INSTALL_BOOTLOADER": "1"}, + sudo=True, + remote=target_host, + ) + + @patch( get_qualified_name(n.Path.glob, n), autospec=True,