diff --git a/src/compose_farm/cli/management.py b/src/compose_farm/cli/management.py index 417b37f..4f779a5 100644 --- a/src/compose_farm/cli/management.py +++ b/src/compose_farm/cli/management.py @@ -238,11 +238,16 @@ def _check_ssh_connectivity(cfg: Config) -> list[str]: def _check_stack_requirements( cfg: Config, stacks: list[str], -) -> tuple[list[tuple[str, str, str]], list[tuple[str, str, str]], list[tuple[str, str, str]]]: +) -> tuple[ + list[tuple[str, str, str]], + list[tuple[str, str, str]], + list[tuple[str, str, str]], + list[tuple[str, str, str]], +]: """Check mounts, networks, and devices for all stacks with a progress bar. - Returns (mount_errors, network_errors, device_errors) where each is a list of - (stack, host, missing_item) tuples. + Returns (mount_errors, network_errors, device_errors, preflight_errors) where + each is a list of (stack, host, item_or_error) tuples. """ async def check_stack( @@ -252,22 +257,23 @@ def _check_stack_requirements( list[tuple[str, str, str]], list[tuple[str, str, str]], list[tuple[str, str, str]], + list[tuple[str, str, str]], ]: """Check requirements for a single stack on all its hosts.""" host_names = cfg.get_hosts(stack) mount_errors: list[tuple[str, str, str]] = [] network_errors: list[tuple[str, str, str]] = [] device_errors: list[tuple[str, str, str]] = [] + preflight_errors: list[tuple[str, str, str]] = [] for host_name in host_names: - missing_paths, missing_nets, missing_devs = await check_stack_requirements( - cfg, stack, host_name - ) - mount_errors.extend((stack, host_name, p) for p in missing_paths) - network_errors.extend((stack, host_name, n) for n in missing_nets) - device_errors.extend((stack, host_name, d) for d in missing_devs) + preflight = await check_stack_requirements(cfg, stack, host_name) + mount_errors.extend((stack, host_name, p) for p in preflight.missing_paths) + network_errors.extend((stack, host_name, n) for n in preflight.missing_networks) + device_errors.extend((stack, host_name, d) for d in preflight.missing_devices) + preflight_errors.extend((stack, host_name, e) for e in preflight.check_errors) - return stack, mount_errors, network_errors, device_errors + return stack, mount_errors, network_errors, device_errors, preflight_errors results = run_parallel_with_progress( "Checking requirements", @@ -278,12 +284,14 @@ def _check_stack_requirements( all_mount_errors: list[tuple[str, str, str]] = [] all_network_errors: list[tuple[str, str, str]] = [] all_device_errors: list[tuple[str, str, str]] = [] - for _, mount_errs, net_errs, dev_errs in results: + all_preflight_errors: list[tuple[str, str, str]] = [] + for _, mount_errs, net_errs, dev_errs, preflight_errs in results: all_mount_errors.extend(mount_errs) all_network_errors.extend(net_errs) all_device_errors.extend(dev_errs) + all_preflight_errors.extend(preflight_errs) - return all_mount_errors, all_network_errors, all_device_errors + return all_mount_errors, all_network_errors, all_device_errors, all_preflight_errors def _report_config_status(cfg: Config) -> bool: @@ -357,6 +365,21 @@ def _report_requirement_errors(errors: list[tuple[str, str, str]], category: str console.print(f" [red]✗[/] {item}") +def _report_preflight_check_errors(errors: list[tuple[str, str, str]]) -> None: + """Report remote preflight failures grouped by stack.""" + by_stack: dict[str, list[tuple[str, str]]] = {} + for stack, host, error in errors: + by_stack.setdefault(stack, []).append((host, error)) + + console.print(f"[red]Remote check failures[/] ({len(errors)}):") + for stack, items in sorted(by_stack.items()): + host = items[0][0] + failures = [failure for _, failure in items] + console.print(f" [cyan]{stack}[/] on [magenta]{host}[/]:") + for failure in failures: + console.print(f" [red]✗[/] {failure}") + + def _report_ssh_status(unreachable_hosts: list[str]) -> bool: """Report SSH connectivity status. Returns True if there are errors.""" if unreachable_hosts: @@ -403,8 +426,13 @@ def _run_remote_checks(cfg: Config, svc_list: list[str], *, show_host_compat: bo console.print() # Spacing before mounts/networks check # Check mounts, networks, and devices - mount_errors, network_errors, device_errors = _check_stack_requirements(cfg, svc_list) + mount_errors, network_errors, device_errors, preflight_errors = _check_stack_requirements( + cfg, svc_list + ) + if preflight_errors: + _report_preflight_check_errors(preflight_errors) + has_errors = True if mount_errors: _report_requirement_errors(mount_errors, "mounts") has_errors = True @@ -414,7 +442,7 @@ def _run_remote_checks(cfg: Config, svc_list: list[str], *, show_host_compat: bo if device_errors: _report_requirement_errors(device_errors, "devices") has_errors = True - if not mount_errors and not network_errors and not device_errors: + if not preflight_errors and not mount_errors and not network_errors and not device_errors: print_success("All mounts, networks, and devices exist") if show_host_compat: diff --git a/src/compose_farm/console.py b/src/compose_farm/console.py index d56dc31..a3f8cf6 100644 --- a/src/compose_farm/console.py +++ b/src/compose_farm/console.py @@ -41,6 +41,14 @@ class _LazyConsole: """Proxy print calls to the underlying Rich console.""" self._get().print(*args, **kwargs) + def __enter__(self) -> Console: + """Delegate context-manager use to the underlying Rich console.""" + return self._get().__enter__() + + def __exit__(self, *args: object) -> Any: + """Delegate context-manager use to the underlying Rich console.""" + return self._get().__exit__(*args) + def __getattr__(self, name: str) -> Any: return getattr(self._get(), name) diff --git a/src/compose_farm/executor.py b/src/compose_farm/executor.py index e4f235a..d2f3b35 100644 --- a/src/compose_farm/executor.py +++ b/src/compose_farm/executor.py @@ -116,6 +116,7 @@ def build_ssh_command(host: Host, command: str, *, tty: bool = False) -> list[st key_path = get_key_path() if key_path: ssh_args.extend(["-i", str(key_path)]) + ssh_args.extend(["-o", "IdentitiesOnly=yes"]) if host.port != _DEFAULT_SSH_PORT: ssh_args.extend(["-p", str(host.port)]) @@ -166,6 +167,17 @@ class CommandResult: return self.exit_code < 0 or self.exit_code == self._SSH_CONNECTION_CLOSED +class RemoteCheckError(RuntimeError): + """Raised when a remote preflight check could not be executed.""" + + def __init__(self, host: str, context: str, detail: str) -> None: + """Initialize the error with host, check context, and failure detail.""" + self.host = host + self.context = context + self.detail = detail.strip() or "unknown error" + super().__init__(f"{context} failed on {host}: {self.detail}") + + def is_local(host: Host) -> bool: """Check if host should run locally (no SSH).""" addr = host.address.lower() @@ -309,7 +321,7 @@ async def _run_ssh_command( ) except (OSError, asyncssh.Error) as e: err_console.print(f"{format_stack_prefix(stack)} [red]SSH error:[/] {e}") - return CommandResult(stack=stack, exit_code=1, success=False) + return CommandResult(stack=stack, exit_code=1, success=False, stderr=str(e)) async def run_command( @@ -657,6 +669,9 @@ async def _batch_check_existence( command = "; ".join(checks) result = await run_command(host, command, context, stream=False) + if not result.success: + detail = result.stderr or result.stdout or f"exit code {result.exit_code}" + raise RemoteCheckError(host_name, context, detail) exists: dict[str, bool] = dict.fromkeys(items, False) for raw_line in result.stdout.splitlines(): diff --git a/src/compose_farm/operations.py b/src/compose_farm/operations.py index 46d9ac2..d2d71a8 100644 --- a/src/compose_farm/operations.py +++ b/src/compose_farm/operations.py @@ -20,6 +20,7 @@ from .console import ( ) from .executor import ( CommandResult, + RemoteCheckError, check_networks_exist, check_paths_exist, check_stack_running, @@ -49,11 +50,14 @@ class PreflightResult(NamedTuple): missing_paths: list[str] missing_networks: list[str] missing_devices: list[str] + check_errors: list[str] @property def ok(self) -> bool: """Return True if all checks passed.""" - return not (self.missing_paths or self.missing_networks or self.missing_devices) + return not ( + self.missing_paths or self.missing_networks or self.missing_devices or self.check_errors + ) async def _run_compose_step( @@ -125,26 +129,37 @@ async def check_stack_requirements( Verifies that all required paths (volumes), networks, and devices exist. """ + check_errors: list[str] = [] + # Check mount paths paths = get_stack_paths(cfg, stack) - path_exists = await check_paths_exist(cfg, host_name, paths) - missing_paths = [p for p, found in path_exists.items() if not found] + try: + path_exists = await check_paths_exist(cfg, host_name, paths) + missing_paths = [p for p, found in path_exists.items() if not found] + except RemoteCheckError as e: + return PreflightResult([], [], [], [str(e)]) # Check external networks networks = parse_external_networks(cfg, stack) missing_networks: list[str] = [] if networks: - net_exists = await check_networks_exist(cfg, host_name, networks) - missing_networks = [n for n, found in net_exists.items() if not found] + try: + net_exists = await check_networks_exist(cfg, host_name, networks) + missing_networks = [n for n, found in net_exists.items() if not found] + except RemoteCheckError as e: + check_errors.append(str(e)) # Check devices devices = parse_devices(cfg, stack) missing_devices: list[str] = [] if devices: - dev_exists = await check_paths_exist(cfg, host_name, devices) - missing_devices = [d for d, found in dev_exists.items() if not found] + try: + dev_exists = await check_paths_exist(cfg, host_name, devices) + missing_devices = [d for d, found in dev_exists.items() if not found] + except RemoteCheckError as e: + check_errors.append(str(e)) - return PreflightResult(missing_paths, missing_networks, missing_devices) + return PreflightResult(missing_paths, missing_networks, missing_devices, check_errors) async def _cleanup_and_rollback( @@ -182,6 +197,8 @@ def _report_preflight_failures( ) -> None: """Report pre-flight check failures.""" print_error(f"{format_stack_prefix(stack)} Cannot start on [magenta]{target_host}[/]:") + for err in preflight.check_errors: + print_error(f" remote check failed: {err}") for path in preflight.missing_paths: print_error(f" missing path: {path}") for net in preflight.missing_networks: diff --git a/tests/test_cli_management.py b/tests/test_cli_management.py new file mode 100644 index 0000000..6ad36cc --- /dev/null +++ b/tests/test_cli_management.py @@ -0,0 +1,37 @@ +"""Tests for CLI management helpers.""" + +from pathlib import Path +from unittest.mock import AsyncMock, patch + +from compose_farm.cli import management +from compose_farm.config import Config, Host +from compose_farm.operations import PreflightResult + + +def test_check_stack_requirements_aggregates_remote_check_errors(tmp_path: Path) -> None: + """Cf check should keep remote check failures separate from missing resources.""" + config = Config( + compose_dir=tmp_path, + hosts={"host1": Host(address="localhost")}, + stacks={"svc": "host1"}, + ) + preflight = PreflightResult( + missing_paths=[], + missing_networks=[], + missing_devices=[], + check_errors=["mount-check failed on host1: Permission denied"], + ) + + with patch( + "compose_farm.cli.management.check_stack_requirements", + new_callable=AsyncMock, + return_value=preflight, + ): + mount_errors, network_errors, device_errors, preflight_errors = ( + management._check_stack_requirements(config, ["svc"]) + ) + + assert mount_errors == [] + assert network_errors == [] + assert device_errors == [] + assert preflight_errors == [("svc", "host1", "mount-check failed on host1: Permission denied")] diff --git a/tests/test_console.py b/tests/test_console.py new file mode 100644 index 0000000..b3b5f3a --- /dev/null +++ b/tests/test_console.py @@ -0,0 +1,11 @@ +"""Tests for console helpers.""" + +from compose_farm.console import _LazyConsole + + +def test_lazy_console_supports_context_manager() -> None: + """Rich Progress enters the provided console as a context manager.""" + console = _LazyConsole() + + with console as rich_console: + assert rich_console is not None diff --git a/tests/test_executor.py b/tests/test_executor.py index a987423..d192a22 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -11,8 +11,10 @@ import pytest from compose_farm.config import Config, Host from compose_farm.executor import ( CommandResult, + RemoteCheckError, _run_local_command, _stream_output_lines, + build_ssh_command, check_networks_exist, check_paths_exist, check_stack_running, @@ -129,6 +131,24 @@ class TestRunCommand: assert result.success is True +class TestBuildSshCommand: + """Tests for native SSH command construction.""" + + def test_uses_only_compose_farm_key_when_present(self, tmp_path: Path) -> None: + """Native ssh should match asyncssh by not falling back to agent keys.""" + key_path = tmp_path / "id_ed25519" + host = Host(address="192.168.1.10", user="me") + + with patch("compose_farm.executor.get_key_path", return_value=key_path): + args = build_ssh_command(host, "true") + + assert args[0] == "ssh" + assert "-i" in args + assert str(key_path) in args + assert "-o" in args + assert "IdentitiesOnly=yes" in args + + class TestRunCompose: """Tests for compose command execution.""" @@ -383,6 +403,28 @@ class TestCheckPathsExist: result = await check_paths_exist(config, "local", []) assert result == {} + async def test_remote_command_failure_raises(self, tmp_path: Path) -> None: + """SSH/preflight failures should not look like every path is missing.""" + config = Config( + compose_dir=tmp_path, + hosts={"remote": Host(address="192.168.1.10")}, + stacks={}, + ) + failed = CommandResult( + stack="mount-check", + exit_code=1, + success=False, + stderr="Permission denied for user basnijholt", + ) + + with patch("compose_farm.executor.run_command", new_callable=AsyncMock) as mock_run: + mock_run.return_value = failed + with pytest.raises(RemoteCheckError) as exc_info: + await check_paths_exist(config, "remote", ["/opt/stacks"]) + + assert "mount-check failed on remote" in str(exc_info.value) + assert "Permission denied" in str(exc_info.value) + @linux_only class TestCheckNetworksExist: diff --git a/tests/test_operations.py b/tests/test_operations.py index 712000e..702ec14 100644 --- a/tests/test_operations.py +++ b/tests/test_operations.py @@ -10,11 +10,14 @@ import pytest from compose_farm.cli import lifecycle from compose_farm.config import Config, Host -from compose_farm.executor import CommandResult +from compose_farm.executor import CommandResult, RemoteCheckError from compose_farm.operations import ( + PreflightResult, _migrate_stack, + _report_preflight_failures, build_discovery_results, build_up_cmd, + check_stack_requirements, ) @@ -126,6 +129,52 @@ class TestBuildUpCmd: ) +class TestPreflightRequirements: + """Tests for stack preflight checks.""" + + async def test_remote_check_error_is_not_reported_as_missing_paths( + self, basic_config: Config + ) -> None: + """A failed SSH check should be distinct from real missing paths.""" + with patch( + "compose_farm.operations.check_paths_exist", + side_effect=RemoteCheckError( + "host2", + "mount-check", + "Permission denied for user basnijholt", + ), + ): + result = await check_stack_requirements(basic_config, "test-service", "host2") + + assert result.ok is False + assert result.missing_paths == [] + assert result.missing_networks == [] + assert result.missing_devices == [] + assert result.check_errors == [ + "mount-check failed on host2: Permission denied for user basnijholt" + ] + + def test_report_preflight_failures_includes_remote_check_error( + self, capsys: pytest.CaptureFixture[str] + ) -> None: + """User-facing preflight output should show the real remote-check cause.""" + _report_preflight_failures( + "test-service", + "host2", + PreflightResult( + missing_paths=[], + missing_networks=[], + missing_devices=[], + check_errors=["mount-check failed on host2: Permission denied"], + ), + ) + + captured = capsys.readouterr() + assert "remote check failed" in captured.err + assert "Permission denied" in captured.err + assert "missing path" not in captured.err + + class TestUpdateCommandSequence: """Tests for update command sequence."""