nixos-rebuild-ng: change copy closure logic when copying from_host -> to_host

- If user is using Nix 2.18+ (the default), we will switch to use `nix
  copy` since it supports using `--from` and `--to` at the same time
- If user is using older versions of Nix, we will call
  `nix-copy-closure` twice, once with `--from` and another with `--to`

The reason for this change is because the previous logic, that SSH'd to
from_host to copy to to_host using `nix-copy-closure --to` means that
`from_host` needs to have credentials to communicate with `to_host`,
that is not always possible nor expected. It breaks even in simple cases
like `--from` and `--to` is the same host because it is not common for a
host to SSH'd itself. This also makes the behavior more consistent,
since `--from` and `--to` is interpreted from the eval host point of
view EXCEPT when they're used together.

This may of course break in the cases where this old behavior was
assumed, e.g.: deploying with a bastion host. I am not sure how common
this was, and I think this change in behavior should enable more cases
than break, but let's see. For that particular case, running
`nixos-rebuild-ng` inside SSH command (e.g.: `ssh build_host --
nixos-rebuild-ng switch --target-host target_host`) should do the
trick.
This commit is contained in:
Thiago Kenji Okada
2024-12-13 13:12:50 +00:00
parent f98a28c1f0
commit 210faf5166
5 changed files with 120 additions and 56 deletions
+7 -1
View File
@@ -19,6 +19,11 @@
}:
let
executable = if withNgSuffix then "nixos-rebuild-ng" else "nixos-rebuild";
# This version is kind of arbitrary, we use some features that were
# implemented in newer versions of Nix, but not necessary 2.18.
# However, Lix is a fork of Nix 2.18, so this looks like a good version
# to cut specific functionality.
withNix218 = lib.versionAtLeast nix.version "2.18";
in
python3Packages.buildPythonApplication rec {
pname = "nixos-rebuild-ng";
@@ -53,8 +58,9 @@ python3Packages.buildPythonApplication rec {
];
postPatch = ''
substituteInPlace nixos_rebuild/__init__.py \
substituteInPlace nixos_rebuild/constants.py \
--subst-var-by executable ${executable} \
--subst-var-by withNix218 ${lib.boolToString withNix218} \
--subst-var-by withReexec ${lib.boolToString withReexec} \
--subst-var-by withShellFiles ${lib.boolToString withShellFiles}
@@ -9,6 +9,7 @@ from subprocess import CalledProcessError, run
from typing import assert_never
from . import nix
from .constants import EXECUTABLE, WITH_NIX_2_18, WITH_REEXEC, WITH_SHELL_FILES
from .models import Action, BuildAttr, Flake, NRError, Profile
from .process import Remote, cleanup_ssh
from .utils import Args, LogFormatter
@@ -16,12 +17,6 @@ from .utils import Args, LogFormatter
logger = logging.getLogger()
logger.setLevel(logging.INFO)
# Build-time flags
# Strings to avoid breaking standalone (e.g.: `python -m nixos_rebuild`) usage
EXECUTABLE = "@executable@"
WITH_REEXEC = "@withReexec@"
WITH_SHELL_FILES = "@withShellFiles@"
def get_parser() -> tuple[argparse.ArgumentParser, dict[str, argparse.ArgumentParser]]:
common_flags = argparse.ArgumentParser(add_help=False)
@@ -187,7 +182,7 @@ def parse_args(
}
if args.help or args.action is None:
if WITH_SHELL_FILES == "true":
if WITH_SHELL_FILES:
r = run(["man", "8", EXECUTABLE], check=False)
parser.exit(r.returncode)
else:
@@ -281,6 +276,9 @@ def reexec(
def execute(argv: list[str]) -> None:
args, args_groups = parse_args(argv)
if not WITH_NIX_2_18:
logger.warning("you're using Nix <2.18, some features will not work correctly")
atexit.register(cleanup_ssh)
common_flags = vars(args_groups["common_flags"])
@@ -303,7 +301,7 @@ def execute(argv: list[str]) -> None:
# Re-exec to a newer version of the script before building to ensure we get
# the latest fixes
if (
WITH_REEXEC == "true"
WITH_REEXEC
and can_run
and not args.fast
and not os.environ.get("_NIXOS_REBUILD_REEXEC")
@@ -0,0 +1,6 @@
# Build-time flags
# Strings to avoid breaking standalone (e.g.: `python -m nixos_rebuild`) usage
EXECUTABLE = "@executable@"
WITH_NIX_2_18 = "@withNix218@" == "true" # type: ignore
WITH_REEXEC = "@withReexec@" == "true" # type: ignore
WITH_SHELL_FILES = "@withShellFiles@" == "true" # type: ignore
@@ -7,6 +7,7 @@ from string import Template
from subprocess import PIPE, CalledProcessError
from typing import Final
from .constants import WITH_NIX_2_18
from .models import (
Action,
BuildAttr,
@@ -139,30 +140,56 @@ def copy_closure(
"""Copy a nix closure to or from host to localhost.
Also supports copying a closure from a remote to another remote."""
host = to_host or from_host
if not host:
return
sshopts = os.getenv("NIX_SSHOPTS", "")
run_wrapper(
[
"nix-copy-closure",
*dict_to_flags(copy_flags),
"--to" if to_host else "--from",
host.host,
closure,
],
extra_env={
# Using raw NIX_SSHOPTS here to avoid messing up with the passed
# parameters, and we do not add the SSH_DEFAULT_OPTS in the remote
# to remote case, otherwise it will fail because of ControlPath
# will not exist in remote
"NIX_SSHOPTS": sshopts
if from_host and to_host
else " ".join(filter(lambda x: x, [*SSH_DEFAULT_OPTS, sshopts]))
},
remote=from_host if to_host else None,
)
extra_env = {
"NIX_SSHOPTS": " ".join(filter(lambda x: x, [*SSH_DEFAULT_OPTS, sshopts]))
}
def nix_copy_closure(host: Remote, to: bool) -> None:
run_wrapper(
[
"nix-copy-closure",
*dict_to_flags(copy_flags),
"--to" if to else "--from",
host.host,
closure,
],
extra_env=extra_env,
)
def nix_copy(to_host: Remote, from_host: Remote) -> None:
run_wrapper(
[
"nix",
"copy",
"--from",
f"ssh://{from_host.host}",
"--to",
f"ssh://{to_host.host}",
closure,
],
extra_env=extra_env,
)
match (to_host, from_host):
case (None, None):
return
case (Remote(_) as host, None) | (None, Remote(_) as host):
nix_copy_closure(host, to=bool(to_host))
case (Remote(_), Remote(_)):
if WITH_NIX_2_18:
# With newer Nix, use `nix copy` instead of `nix-copy-closure`
# since it supports `--to` and `--from` at the same time
# TODO: once we drop Nix 2.3 from nixpkgs, remove support for
# `nix-copy-closure`
nix_copy(to_host, from_host)
else:
# With older Nix, we need to copy from to local and local to
# host. This means it is slower and need additional disk space
# in local
nix_copy_closure(from_host, to=False)
nix_copy_closure(to_host, to=True)
def edit(flake: Flake | None, **flake_flags: Args) -> None:
@@ -112,7 +112,6 @@ def test_remote_build(mock_run: Any, monkeypatch: Any) -> None:
extra_env={
"NIX_SSHOPTS": " ".join(p.SSH_DEFAULT_OPTS + ["--ssh opts"])
},
remote=None,
),
call(
["nix-store", "--realise", Path("/path/to/file"), "--build"],
@@ -166,7 +165,6 @@ def test_remote_build_flake(mock_run: Any, monkeypatch: Any) -> None:
extra_env={
"NIX_SSHOPTS": " ".join(p.SSH_DEFAULT_OPTS + ["--ssh opts"])
},
remote=None,
),
call(
[
@@ -185,37 +183,66 @@ def test_remote_build_flake(mock_run: Any, monkeypatch: Any) -> None:
)
@patch(get_qualified_name(n.run_wrapper, n), autospec=True)
def test_copy_closure(mock_run: Any, monkeypatch: Any) -> None:
def test_copy_closure(monkeypatch: Any) -> None:
closure = Path("/path/to/closure")
n.copy_closure(closure, None)
mock_run.assert_not_called()
with patch(get_qualified_name(n.run_wrapper, n), autospec=True) as mock_run:
n.copy_closure(closure, None)
mock_run.assert_not_called()
target_host = m.Remote("user@target.host", [], None)
build_host = m.Remote("user@build.host", [], None)
n.copy_closure(closure, target_host)
mock_run.assert_called_with(
["nix-copy-closure", "--to", "user@target.host", closure],
extra_env={"NIX_SSHOPTS": " ".join(p.SSH_DEFAULT_OPTS)},
remote=None,
)
with patch(get_qualified_name(n.run_wrapper, n), autospec=True) as mock_run:
n.copy_closure(closure, target_host)
mock_run.assert_called_with(
["nix-copy-closure", "--to", "user@target.host", closure],
extra_env={"NIX_SSHOPTS": " ".join(p.SSH_DEFAULT_OPTS)},
)
monkeypatch.setenv("NIX_SSHOPTS", "--ssh build-opt")
n.copy_closure(closure, None, build_host)
mock_run.assert_called_with(
["nix-copy-closure", "--from", "user@build.host", closure],
extra_env={"NIX_SSHOPTS": " ".join(p.SSH_DEFAULT_OPTS + ["--ssh build-opt"])},
remote=None,
)
with patch(get_qualified_name(n.run_wrapper, n), autospec=True) as mock_run:
n.copy_closure(closure, None, build_host)
mock_run.assert_called_with(
["nix-copy-closure", "--from", "user@build.host", closure],
extra_env={
"NIX_SSHOPTS": " ".join(p.SSH_DEFAULT_OPTS + ["--ssh build-opt"])
},
)
monkeypatch.setenv("NIX_SSHOPTS", "--ssh build-target-opt")
n.copy_closure(closure, target_host, build_host)
mock_run.assert_called_with(
["nix-copy-closure", "--to", "user@target.host", closure],
remote=build_host,
extra_env={"NIX_SSHOPTS": "--ssh build-target-opt"},
)
monkeypatch.setattr(n, "WITH_NIX_2_18", True)
extra_env = {
"NIX_SSHOPTS": " ".join(p.SSH_DEFAULT_OPTS + ["--ssh build-target-opt"])
}
with patch(get_qualified_name(n.run_wrapper, n), autospec=True) as mock_run:
n.copy_closure(closure, target_host, build_host)
mock_run.assert_called_with(
[
"nix",
"copy",
"--from",
"ssh://user@build.host",
"--to",
"ssh://user@target.host",
closure,
],
extra_env=extra_env,
)
monkeypatch.setattr(n, "WITH_NIX_2_18", False)
with patch(get_qualified_name(n.run_wrapper, n), autospec=True) as mock_run:
n.copy_closure(closure, target_host, build_host)
mock_run.assert_has_calls(
[
call(
["nix-copy-closure", "--from", "user@build.host", closure],
extra_env=extra_env,
),
call(
["nix-copy-closure", "--to", "user@target.host", closure],
extra_env=extra_env,
),
]
)
@patch(get_qualified_name(n.run_wrapper, n), autospec=True)