From 5df5ff8fd234921c56c9a418ddf2cb285a061590 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Wed, 12 Feb 2025 18:04:25 +0000 Subject: [PATCH] nixos-rebuild-ng: fix non-flake remote build evaluation Fix: #381457. --- .../src/nixos_rebuild/__init__.py | 15 +- .../nixos-rebuild-ng/src/nixos_rebuild/nix.py | 4 +- .../nixos-rebuild-ng/src/tests/test_main.py | 201 +++++++++++++++++- .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 4 +- 4 files changed, 211 insertions(+), 13 deletions(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py index 1c725b60f617..7709f1af8742 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py @@ -389,8 +389,6 @@ def execute(argv: list[str]) -> None: dry_run = action == Action.DRY_BUILD no_link = action in (Action.SWITCH, Action.BOOT) - build_flags |= {"no_out_link": no_link, "dry_run": dry_run} - flake_build_flags |= {"no_link": no_link, "dry_run": dry_run} rollback = bool(args.rollback) def validate_image_variant(variants: ImageVariants) -> None: @@ -444,29 +442,32 @@ def execute(argv: list[str]) -> None: flake, build_host, eval_flags=flake_common_flags, - flake_build_flags=flake_build_flags, + flake_build_flags=flake_build_flags + | {"no_link": no_link, "dry_run": dry_run}, copy_flags=copy_flags, ) case (_, False, None, Flake(_)): path_to_config = nix.build_flake( attr, flake, - flake_build_flags=flake_build_flags, + flake_build_flags=flake_build_flags + | {"no_link": no_link, "dry_run": dry_run}, ) case (_, False, Remote(_), None): path_to_config = nix.build_remote( attr, build_attr, build_host, - instantiate_flags=common_flags, + realise_flags=common_flags, + instantiate_flags=build_flags, copy_flags=copy_flags, - build_flags=build_flags, ) case (_, False, None, None): path_to_config = nix.build( attr, build_attr, - build_flags=build_flags, + build_flags=build_flags + | {"no_out_link": no_link, "dry_run": dry_run}, ) case never: # should never happen, but mypy is not smart enough to 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 04ddaa6e762f..c7b2010229ab 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 @@ -77,7 +77,7 @@ def build_remote( attr: str, build_attr: BuildAttr, build_host: Remote | None, - build_flags: Args | None = None, + realise_flags: Args | None = None, instantiate_flags: Args | None = None, copy_flags: Args | None = None, ) -> Path: @@ -112,7 +112,7 @@ def build_remote( drv, "--add-root", remote_tmpdir / uuid4().hex, - *dict_to_flags(build_flags), + *dict_to_flags(realise_flags), ], remote=build_host, stdout=PIPE, 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 afb5929f00ce..9ae6bf50a080 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 @@ -1,5 +1,6 @@ import logging import textwrap +import uuid from pathlib import Path from subprocess import PIPE, CompletedProcess from typing import Any @@ -390,6 +391,202 @@ def test_execute_nix_switch_flake(mock_run: Any, tmp_path: Path) -> None: ) +@patch.dict(nr.process.os.environ, {}, clear=True) +@patch(get_qualified_name(nr.process.subprocess.run), autospec=True) +@patch(get_qualified_name(nr.cleanup_ssh, nr), autospec=True) +@patch(get_qualified_name(nr.nix.uuid4, nr.nix), autospec=True) +def test_execute_nix_switch_build_target_host( + mock_uuid4: Any, + mock_cleanup_ssh: Any, + mock_run: Any, + tmp_path: Path, +) -> None: + config_path = tmp_path / "test" + config_path.touch() + + def run_side_effect(args: list[str], **kwargs: Any) -> CompletedProcess[str]: + if args[0] == "nix": + return CompletedProcess([], 0, str(config_path)) + elif args[0] == "nix-instantiate" and "--find-file" in args: + return CompletedProcess([], 1) + elif args[0] == "nix-instantiate": + return CompletedProcess([], 0, str(config_path)) + elif args[0] == "ssh" and "nix-store" in args: + return CompletedProcess([], 0, "/tmp/tmpdir/config") + elif args[0] == "ssh" and "mktemp" in args: + return CompletedProcess([], 0, "/tmp/tmpdir") + elif args[0] == "ssh" and "readlink" in args: + return CompletedProcess([], 0, str(config_path)) + else: + return CompletedProcess([], 0) + + mock_run.side_effect = run_side_effect + mock_uuid4.return_value = uuid.UUID(int=0) + + nr.execute( + [ + "nixos-rebuild", + "switch", + "--no-flake", + "--sudo", + "--build-host", + "user@build-host", + "--target-host", + "user@target-host", + "--no-reexec", + # https://github.com/NixOS/nixpkgs/issues/381457 + "-I", + "nixos-config=./configuration.nix", + "-I", + "nixpkgs=$HOME/.nix-defexpr/channels/pinned_nixpkgs", + ] + ) + + assert mock_run.call_count == 10 + mock_run.assert_has_calls( + [ + call( + [ + "nix-instantiate", + "--find-file", + "nixpkgs", + "--include", + "nixos-config=./configuration.nix", + "--include", + "nixpkgs=$HOME/.nix-defexpr/channels/pinned_nixpkgs", + ], + check=False, + stdout=PIPE, + **DEFAULT_RUN_KWARGS, + ), + call( + [ + "nix-instantiate", + "", + "--attr", + "config.system.build.toplevel", + "--add-root", + nr.tmpdir.TMPDIR_PATH / "00000000000000000000000000000000", + "--include", + "nixos-config=./configuration.nix", + "--include", + "nixpkgs=$HOME/.nix-defexpr/channels/pinned_nixpkgs", + ], + check=True, + stdout=PIPE, + **DEFAULT_RUN_KWARGS, + ), + call( + ["nix-copy-closure", "--to", "user@build-host", config_path], + check=True, + **DEFAULT_RUN_KWARGS, + ), + call( + [ + "ssh", + *nr.process.SSH_DEFAULT_OPTS, + "user@build-host", + "--", + "mktemp", + "-d", + "-t", + "nixos-rebuild.XXXXX", + ], + check=True, + stdout=PIPE, + **DEFAULT_RUN_KWARGS, + ), + call( + [ + "ssh", + *nr.process.SSH_DEFAULT_OPTS, + "user@build-host", + "--", + "nix-store", + "--realise", + str(config_path), + "--add-root", + "/tmp/tmpdir/00000000000000000000000000000000", + ], + check=True, + stdout=PIPE, + **DEFAULT_RUN_KWARGS, + ), + call( + [ + "ssh", + *nr.process.SSH_DEFAULT_OPTS, + "user@build-host", + "--", + "readlink", + "-f", + "/tmp/tmpdir/config", + ], + check=True, + stdout=PIPE, + **DEFAULT_RUN_KWARGS, + ), + call( + [ + "ssh", + *nr.process.SSH_DEFAULT_OPTS, + "user@build-host", + "--", + "rm", + "-rf", + "/tmp/tmpdir", + ], + check=False, + **DEFAULT_RUN_KWARGS, + ), + call( + [ + "nix", + "copy", + "--from", + "ssh://user@build-host", + "--to", + "ssh://user@target-host", + config_path, + ], + check=True, + **DEFAULT_RUN_KWARGS, + ), + call( + [ + "ssh", + *nr.process.SSH_DEFAULT_OPTS, + "user@target-host", + "--", + "sudo", + "nix-env", + "-p", + "/nix/var/nix/profiles/system", + "--set", + str(config_path), + ], + check=True, + **DEFAULT_RUN_KWARGS, + ), + call( + [ + "ssh", + *nr.process.SSH_DEFAULT_OPTS, + "user@target-host", + "--", + "sudo", + "env", + "NIXOS_INSTALL_BOOTLOADER=0", + str(config_path / "bin/switch-to-configuration"), + "switch", + ], + check=True, + **DEFAULT_RUN_KWARGS, + ), + ] + ) + + @patch.dict(nr.process.os.environ, {}, clear=True) @patch(get_qualified_name(nr.process.subprocess.run), autospec=True) @patch(get_qualified_name(nr.cleanup_ssh, nr), autospec=True) @@ -469,7 +666,7 @@ def test_execute_nix_switch_flake_target_host( "sudo", "env", "NIXOS_INSTALL_BOOTLOADER=0", - f"{config_path / 'bin/switch-to-configuration'}", + str(config_path / "bin/switch-to-configuration"), "switch", ], check=True, @@ -493,7 +690,7 @@ def test_execute_nix_switch_flake_build_host( def run_side_effect(args: list[str], **kwargs: Any) -> CompletedProcess[str]: if args[0] == "nix" and "eval" in args: return CompletedProcess([], 0, str(config_path)) - if args[0] == "ssh" and "nix" in args: + elif args[0] == "ssh" and "nix" in args: return CompletedProcess([], 0, str(config_path)) else: return CompletedProcess([], 0) 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 a2bf189ad716..35fea00cbeef 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 @@ -104,7 +104,7 @@ def test_build_remote(mock_uuid4: Any, mock_run: Any, monkeypatch: MonkeyPatch) "config.system.build.toplevel", m.BuildAttr("", "preAttr"), build_host, - build_flags={"build": True}, + realise_flags={"realise": True}, instantiate_flags={"inst": True}, copy_flags={"copy": True}, ) == Path("/path/to/config") @@ -147,7 +147,7 @@ def test_build_remote(mock_uuid4: Any, mock_run: Any, monkeypatch: MonkeyPatch) Path("/path/to/file"), "--add-root", Path("/tmp/tmpdir/00000000000000000000000000000002"), - "--build", + "--realise", ], remote=build_host, stdout=PIPE,