mirror of
https://github.com/basnijholt/compose-farm.git
synced 2026-08-01 19:00:33 +01:00
preflight: Report SSH check failures clearly (#182)
* compose-farm: Restrict web route to local IPs * preflight: Report SSH check failures clearly * preflight: Report SSH check failures clearly
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -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:
|
||||
|
||||
37
tests/test_cli_management.py
Normal file
37
tests/test_cli_management.py
Normal file
@@ -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")]
|
||||
11
tests/test_console.py
Normal file
11
tests/test_console.py
Normal file
@@ -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
|
||||
@@ -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:
|
||||
|
||||
@@ -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."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user