feat(admin-config-tool): task 4.1 - xceadmin.cli
Wire config build/config push --dry-run onto the click group. Thin layer only: one XceAdminError dispatch surface plus an OSError catch (ConnectionRefusedError and friends propagate unwrapped from xcecommon.wire by design and need formatting here), and a KeyboardInterrupt handler for a clean "cancelled" message (socket cleanup already happens via push.py's own try/finally).
This commit is contained in:
parent
c33ef7d479
commit
eeda32c696
3 changed files with 356 additions and 16 deletions
|
|
@ -83,7 +83,7 @@
|
|||
- _Depends: 2.4, 3.2_
|
||||
|
||||
- [ ] 4. Integration: wire the CLI entry point and hook xceadmin into cluster deployment
|
||||
- [ ] 4.1 Expose config build and config push as a single command-line tool
|
||||
- [x] 4.1 Expose config build and config push as a single command-line tool
|
||||
- `xceadmin config build` and `xceadmin config push` (with `--dry-run`) are both reachable from one installed command
|
||||
- Any failure surfaces as a specific, human-readable message with a non-zero exit code, not a raw traceback
|
||||
- Interrupting a push mid-flight leaves no dangling connection and reports a clean cancellation
|
||||
|
|
@ -154,3 +154,4 @@
|
|||
- **3.2**: design.md's Dependencies list for `xceadmin.discovery` names only `xcecommon.wire`, but decoding a reply to inspect `RequestReply.redirectHost`/`.redirectPort` genuinely requires `xcecommon.schema` too (verified real field names against `controller.capnp:594-595`, which matched design.md exactly this time — unlike task 2.2's `Reply`/`RequestReply` struct-name slip). Treated as a design-doc omission, not license to hand-roll capnp decoding. `follow_redirects`'s `cert_dir` parameter is accepted for signature parity with design.md but unused internally — all I/O, including TLS setup, is delegated to the caller's `send_fn` closure, which is expected to already carry its own `cert_dir`. A connection timeout to any hop (including a non-initial one reached via redirect) raises `NoPrimaryError` naming that specific hop, not the original target.
|
||||
- **bug-1461 (out-of-band, between 3.2 and 3.3)**: `controller.capnp` had a real, confirmed schema bug — a locally-declared `struct Data` (an `OPCode_get` query-result tagged union) shadowed Cap'n Proto's builtin `Data` blob type for every later `: Data` field in the file, miscompiling `UpgradeConfigRequest.sha256`/`.bytes` (and 5 other fields across `ReplConfigReply`/`ReplConfigPushRequest`/`ReplEnvelope`/`SnapshotTransfer`/`WalCatchupEntry`/`CtrlHelloRequest`, several HA-critical) as a struct-pointer type instead of a byte blob — blocking `push_config()` from constructing a valid request via pycapnp. Fixed at the root (renamed the local struct to `XceData`, `$C.mapname("xce_data_t")` unchanged so the C typedef is stable) across 20 files spanning controller/lib/scheduler/Go — see commit `e53c086` and `.wolf/buglog.json` bug-1461 for the full investigation and fix. This was NOT an admin-config-tool-boundary change; it's a pre-existing, cross-component schema defect that happened to block this task. `push_config()`'s real controller counterpart remains untested live — a *separate*, already-documented dispatch-gate defect (`ha_dispatch_gate()`/`cluster.c`, spec.json `blocked_on`) still blocks that independently of bug-1461.
|
||||
- **3.3**: `probe_config()` does NOT go through `xceadmin.discovery.follow_redirects` — `ProbeConfigTagReply` is a direct struct with only `configTag`, no `redirectHost`/`.Port` fields exist on it at all, so there is no redirect signal for `follow_redirects` (which unconditionally decodes replies as `RequestReply`) to read; only `push_config()`'s `Request`/`RequestReply`-wrapped exchange supports redirects. `ProbeConfigTagReply` also carries no accept/reject signal — `DryRunResult.accepted` is a client-side inference (true on any successful round-trip), matching the real C reference client (`config_repl.c`'s `xce_probe_config_tag`, which also always sends an empty `name` and returns only a bare tag). pycapnp union field assignment for Void/Text-typed variants (e.g. `reply.data.nodata = None`, `reply.data.debugmsg = "..."`) must use direct assignment, not `.init(name)` — `.init()` without a size is only valid for struct/list/data-typed variants, confirmed via `KjException: init() without a size is only valid for struct and object fields`.
|
||||
- **4.1**: `xcecommon.wire.connect()` deliberately lets `ConnectionRefusedError`/other bare `OSError`s propagate unwrapped (documented in `wire.py`'s own module docstring) — `xceadmin.cli` is the layer that catches `OSError` and formats it per Requirement 7.1, alongside its `except XceAdminError` dispatch surface; a task 4.1 implementer must not assume `XceAdminError` alone covers every network failure mode. click 8.3.1's `CliRunner` no longer accepts `mix_stderr=`; `result.stdout`/`.stderr` are separated by default, but `CliRunner`'s output capture decodes as text and is **lossy for arbitrary binary** (confirmed: `bytes(range(256))` does not round-trip through the captured `.output` string) — do not assert exact byte content of a `-o -` binary write through `CliRunner`; that's already covered at the `write_output()` unit level (task 3.1's `test_config_build.py`). A CLI test that omits `-o` falls through to the real default (`cluster.bin`, relative to CWD) — always pass an explicit `-o <tmp_path>` in tests that reach the write step, or the test will write a stray artifact into the actual repo working directory (caught via `git status` before commit, not by the test itself).
|
||||
|
|
|
|||
|
|
@ -1,19 +1,96 @@
|
|||
"""xceadmin command-line entry point.
|
||||
"""xceadmin command-line entry point (design.md, ``xceadmin.cli``;
|
||||
Requirements 1.2, 1.3, 2.2, 2.4, 7.1, 7.2, 7.3, 7.4, 7.5, 7.6, 8.1, 8.2).
|
||||
|
||||
This module wires the ``xceadmin`` console-script entry point to a click
|
||||
command group. It intentionally has zero subcommands for now — ``config
|
||||
build`` and ``config push`` are added by a later task (see
|
||||
.kiro/specs/admin-config-tool/tasks.md, task 4.1).
|
||||
Thin ``click`` layer only -- no business logic. Every raising exception
|
||||
across ``xcecommon``/``xceadmin`` subclasses :class:`XceAdminError`
|
||||
(``xcecommon.errors``), so this module has exactly one dispatch surface:
|
||||
``except XceAdminError as e: raise click.ClickException(e.message)``.
|
||||
Per-failure-mode message formatting happens at each exception's raise
|
||||
site (7.1, 7.2, 7.3, 7.4), not here.
|
||||
|
||||
SIGINT handling (7.5): Python's default SIGINT handler already raises
|
||||
``KeyboardInterrupt`` inside whatever blocking call is running (socket
|
||||
recv included); ``xcecommon.wire.connect``'s callers (``xceadmin.push``)
|
||||
already wrap every socket use in ``try/finally: sock.close()``, so the
|
||||
connection closes automatically as ``KeyboardInterrupt`` unwinds -- no
|
||||
custom ``signal.signal()`` registration is needed to get that guarantee.
|
||||
This module's job is only to catch it and report a clean "cancelled"
|
||||
message instead of a raw traceback.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import click
|
||||
|
||||
from xcecommon.errors import XceAdminError
|
||||
from xceadmin.build import build_cluster_config, write_output
|
||||
from xceadmin.push import probe_config, push_config
|
||||
|
||||
_DEFAULT_OUTPUT = "cluster.bin"
|
||||
_DEFAULT_CONTROLLER = "localhost:9991"
|
||||
_DEFAULT_CERT_DIR = "ha/data"
|
||||
|
||||
|
||||
@click.group()
|
||||
def cli() -> None:
|
||||
"""xceadmin: build and deploy XCE cluster configuration."""
|
||||
|
||||
|
||||
@cli.group()
|
||||
def config() -> None:
|
||||
"""Build and push cluster configuration."""
|
||||
|
||||
|
||||
@config.command("build")
|
||||
@click.argument("configs_dir")
|
||||
@click.option("-o", "output", default=_DEFAULT_OUTPUT, help="Output path, or - for stdout.")
|
||||
def config_build(configs_dir: str, output: str) -> None:
|
||||
"""Compile CONFIGS_DIR's YAML files into a ClusterConfig binary (1.1-1.7, 1.9)."""
|
||||
try:
|
||||
data = build_cluster_config(configs_dir)
|
||||
digest = write_output(data, output)
|
||||
except XceAdminError as e:
|
||||
raise click.ClickException(e.message) from e
|
||||
click.echo(digest, err=True) # SHA-256 to stderr, always (1.4)
|
||||
|
||||
|
||||
@config.command("push")
|
||||
@click.argument("cluster_bin_path")
|
||||
@click.option(
|
||||
"--controller", default=_DEFAULT_CONTROLLER, help="host:port of any controller node (2.2)."
|
||||
)
|
||||
@click.option(
|
||||
"--cert-dir", default=_DEFAULT_CERT_DIR, help="Directory containing xce.crt/xce.key (2.4)."
|
||||
)
|
||||
@click.option("--dry-run", is_flag=True, help="Probe acceptance without committing (Requirement 6).")
|
||||
def config_push(cluster_bin_path: str, controller: str, cert_dir: str, dry_run: bool) -> None:
|
||||
"""Push CLUSTER_BIN_PATH to the controller cluster."""
|
||||
try:
|
||||
if dry_run:
|
||||
result = probe_config(cluster_bin_path, controller, cert_dir)
|
||||
if not result.accepted:
|
||||
raise click.ClickException(result.reason) # 6.3
|
||||
click.echo(
|
||||
"OK: controller will accept this config "
|
||||
f"(generation {result.current_generation}→{result.would_be_generation})"
|
||||
) # 6.2
|
||||
else:
|
||||
result = push_config(cluster_bin_path, controller, cert_dir)
|
||||
click.echo(f"versionTag={result.version_tag} ackedCount={result.acked_count}") # 2.5
|
||||
if result.error or result.acked_count <= 0:
|
||||
raise click.ClickException(result.error or "push rejected (ackedCount == 0)") # 2.7
|
||||
# 2.6: positive ackedCount and no error -- fall through to exit 0.
|
||||
except XceAdminError as e:
|
||||
raise click.ClickException(e.message) from e
|
||||
except OSError as e:
|
||||
# ConnectionRefusedError and other bare OS-level failures propagate
|
||||
# unwrapped out of xcecommon.wire by design (see wire.py's module
|
||||
# docstring) -- this is the layer that formats them (7.1).
|
||||
raise click.ClickException(f"cannot connect to {controller}: {e}") from e
|
||||
except KeyboardInterrupt:
|
||||
raise click.ClickException("cancelled") from None
|
||||
|
||||
|
||||
def main() -> None:
|
||||
"""Console-script entry point (see pyproject.toml [project.scripts])."""
|
||||
cli()
|
||||
|
|
|
|||
|
|
@ -1,14 +1,29 @@
|
|||
"""Tests for the xceadmin package skeleton (task 1.1).
|
||||
|
||||
Covers Requirement 5.4: xcecommon is importable as a standalone package
|
||||
independent of xceadmin's CLI, and the xceadmin console-script entry point
|
||||
is backed by a genuinely working (if currently empty) click group.
|
||||
"""Tests for the xceadmin CLI (design.md, ``xceadmin.cli``; task 1.1's
|
||||
package-skeleton coverage plus task 4.1's ``config build``/``config
|
||||
push`` wiring; Requirements 1.2, 1.3, 2.2, 2.4, 5.4, 7.1, 7.2, 7.3, 7.4,
|
||||
7.5, 7.6, 8.1, 8.2).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import ssl
|
||||
import socket
|
||||
import threading
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
from click.testing import CliRunner
|
||||
|
||||
from xcecommon.schema import load_schema
|
||||
from xceadmin.cli import cli
|
||||
|
||||
REPO_HA_DATA_DIR = Path(__file__).resolve().parents[3] / "ha" / "data"
|
||||
CERT_FILE = REPO_HA_DATA_DIR / "xce.crt"
|
||||
KEY_FILE = REPO_HA_DATA_DIR / "xce.key"
|
||||
CERT_DIR = str(REPO_HA_DATA_DIR)
|
||||
|
||||
|
||||
def test_cli_help_exits_zero() -> None:
|
||||
"""`xceadmin --help` must run cleanly and print usage text."""
|
||||
|
|
@ -19,19 +34,30 @@ def test_cli_help_exits_zero() -> None:
|
|||
assert "Usage:" in result.output
|
||||
|
||||
|
||||
def test_cli_help_shows_zero_commands() -> None:
|
||||
"""The click group has no subcommands yet (config build/push land later)."""
|
||||
def test_cli_help_shows_config_command() -> None:
|
||||
"""`config build`/`config push` are reachable from the top-level group
|
||||
(4.1's core acceptance bullet)."""
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(cli, ["--help"])
|
||||
|
||||
assert result.exit_code == 0
|
||||
assert "Commands:" not in result.output
|
||||
assert "Commands:" in result.output
|
||||
assert "config" in result.output
|
||||
|
||||
|
||||
def test_cli_config_help_shows_build_and_push() -> None:
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(cli, ["config", "--help"])
|
||||
|
||||
assert result.exit_code == 0
|
||||
assert "build" in result.output
|
||||
assert "push" in result.output
|
||||
|
||||
|
||||
def test_cli_group_invoked_with_no_args_does_not_traceback() -> None:
|
||||
"""Invoking the bare group with no subcommand exits cleanly via click's
|
||||
own SystemExit (no subcommand registered yet), not an unhandled
|
||||
traceback from application code."""
|
||||
own SystemExit (no subcommand given), not an unhandled traceback from
|
||||
application code."""
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(cli, [])
|
||||
|
||||
|
|
@ -42,3 +68,239 @@ def test_cli_group_invoked_with_no_args_does_not_traceback() -> None:
|
|||
def test_xcecommon_is_importable_independent_of_xceadmin() -> None:
|
||||
"""xcecommon must be importable on its own, with no xceadmin import."""
|
||||
import xcecommon # noqa: F401 - import success is the assertion
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# config build
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_config_build_writes_file_and_prints_sha256_to_stderr(tmp_path):
|
||||
(tmp_path / "xce.yaml").write_text(
|
||||
yaml.safe_dump(
|
||||
{"cluster_name": "eda1", "heartbeat_interval": 30, "inactive_heartbeat_count": 3}
|
||||
)
|
||||
)
|
||||
out_path = tmp_path / "out.bin"
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(cli, ["config", "build", str(tmp_path), "-o", str(out_path)])
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert out_path.is_file()
|
||||
expected_digest = hashlib.sha256(out_path.read_bytes()).hexdigest()
|
||||
assert expected_digest in result.stderr
|
||||
|
||||
|
||||
def test_config_build_dash_writes_to_stdout(tmp_path):
|
||||
"""Byte-exact stdout content is already covered at the unit level
|
||||
(test_config_build.py::test_write_output_dash_writes_to_stdout_buffer)
|
||||
-- click's CliRunner captures output as decoded text and is lossy for
|
||||
arbitrary binary (confirmed: UTF-8 encode/decode does not round-trip
|
||||
an arbitrary byte sequence), so this test only confirms the CLI wires
|
||||
`-o -` through to write_output without erroring, not exact bytes.
|
||||
"""
|
||||
(tmp_path / "xce.yaml").write_text(
|
||||
yaml.safe_dump(
|
||||
{"cluster_name": "eda1", "heartbeat_interval": 30, "inactive_heartbeat_count": 3}
|
||||
)
|
||||
)
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(cli, ["config", "build", str(tmp_path), "-o", "-"], color=False)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
|
||||
def test_config_build_schema_violation_exits_nonzero_with_message(tmp_path):
|
||||
(tmp_path / "queues.yaml").write_text(yaml.safe_dump({"queues": [{"priority": 1}]})) # missing name
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(cli, ["config", "build", str(tmp_path)])
|
||||
|
||||
assert result.exit_code != 0
|
||||
assert not isinstance(result.exception, KeyError | AttributeError | TypeError)
|
||||
assert result.output.strip() != ""
|
||||
|
||||
|
||||
def test_config_build_missing_configs_dir_does_not_raw_traceback(tmp_path):
|
||||
missing = tmp_path / "does-not-exist"
|
||||
out_path = tmp_path / "out.bin"
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(cli, ["config", "build", str(missing), "-o", str(out_path)])
|
||||
|
||||
assert result.exit_code == 0, "a nonexistent dir behaves like one with zero known files present (1.5)"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# config push / config push --dry-run (mock controller)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _server_context() -> ssl.SSLContext:
|
||||
context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
|
||||
context.load_cert_chain(certfile=str(CERT_FILE), keyfile=str(KEY_FILE))
|
||||
return context
|
||||
|
||||
|
||||
class _MockController:
|
||||
def __init__(self, handler):
|
||||
self._handler = handler
|
||||
self._listener = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
|
||||
self._listener.bind(("127.0.0.1", 0))
|
||||
self._listener.listen(1)
|
||||
self.port = self._listener.getsockname()[1]
|
||||
self._thread = threading.Thread(target=self._run, daemon=True)
|
||||
self._thread.start()
|
||||
|
||||
def _run(self) -> None:
|
||||
try:
|
||||
raw_conn, _ = self._listener.accept()
|
||||
except OSError:
|
||||
return
|
||||
try:
|
||||
tls_conn = _server_context().wrap_socket(raw_conn, server_side=True)
|
||||
except (ssl.SSLError, OSError):
|
||||
raw_conn.close()
|
||||
return
|
||||
try:
|
||||
self._handler(tls_conn)
|
||||
except OSError:
|
||||
pass
|
||||
finally:
|
||||
tls_conn.close()
|
||||
|
||||
def close(self) -> None:
|
||||
self._listener.close()
|
||||
self._thread.join(timeout=5)
|
||||
|
||||
|
||||
pytestmark = pytest.mark.skipif(
|
||||
not (CERT_FILE.is_file() and KEY_FILE.is_file()),
|
||||
reason="ha/data/xce.crt or ha/data/xce.key not present",
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_controller():
|
||||
servers: list[_MockController] = []
|
||||
|
||||
def _make(handler) -> _MockController:
|
||||
srv = _MockController(handler)
|
||||
servers.append(srv)
|
||||
return srv
|
||||
|
||||
yield _make
|
||||
for srv in servers:
|
||||
srv.close()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def cluster_bin(tmp_path):
|
||||
path = tmp_path / "cluster.bin"
|
||||
path.write_bytes(b"fake cluster config binary")
|
||||
return str(path)
|
||||
|
||||
|
||||
def test_config_push_success_exits_zero(mock_controller, cluster_bin):
|
||||
from xcecommon import wire
|
||||
|
||||
mod = load_schema()
|
||||
|
||||
def _handler(conn):
|
||||
wire.recv_frame(conn)
|
||||
reply = mod.RequestReply.new_message()
|
||||
reply.status = 0
|
||||
upgrade_reply = reply.data.init("upgradeconfigreplymsg")
|
||||
upgrade_reply.versionTag = 5
|
||||
upgrade_reply.ackedCount = 3
|
||||
upgrade_reply.error = ""
|
||||
wire.send_frame(conn, reply.to_bytes())
|
||||
|
||||
srv = mock_controller(_handler)
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(
|
||||
cli,
|
||||
["config", "push", cluster_bin, "--controller", f"127.0.0.1:{srv.port}", "--cert-dir", CERT_DIR],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert "versionTag=5" in result.output
|
||||
assert "ackedCount=3" in result.output
|
||||
|
||||
|
||||
def test_config_push_zero_acked_count_exits_nonzero(mock_controller, cluster_bin):
|
||||
from xcecommon import wire
|
||||
|
||||
mod = load_schema()
|
||||
|
||||
def _handler(conn):
|
||||
wire.recv_frame(conn)
|
||||
reply = mod.RequestReply.new_message()
|
||||
reply.status = 0
|
||||
upgrade_reply = reply.data.init("upgradeconfigreplymsg")
|
||||
upgrade_reply.versionTag = 0
|
||||
upgrade_reply.ackedCount = 0
|
||||
upgrade_reply.error = "no quorum"
|
||||
wire.send_frame(conn, reply.to_bytes())
|
||||
|
||||
srv = mock_controller(_handler)
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(
|
||||
cli,
|
||||
["config", "push", cluster_bin, "--controller", f"127.0.0.1:{srv.port}", "--cert-dir", CERT_DIR],
|
||||
)
|
||||
|
||||
assert result.exit_code != 0
|
||||
assert "no quorum" in result.output
|
||||
|
||||
|
||||
def test_config_push_connection_refused_exits_nonzero_not_raw_traceback(cluster_bin):
|
||||
listener = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
|
||||
listener.bind(("127.0.0.1", 0))
|
||||
port = listener.getsockname()[1]
|
||||
listener.close()
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(
|
||||
cli,
|
||||
["config", "push", cluster_bin, "--controller", f"127.0.0.1:{port}", "--cert-dir", CERT_DIR],
|
||||
)
|
||||
|
||||
assert result.exit_code != 0
|
||||
assert result.output.strip() != ""
|
||||
|
||||
|
||||
def test_config_push_dry_run_reports_generations(mock_controller, cluster_bin):
|
||||
from xcecommon import wire
|
||||
|
||||
mod = load_schema()
|
||||
|
||||
def _handler(conn):
|
||||
wire.recv_frame(conn)
|
||||
reply = mod.ProbeConfigTagReply.new_message()
|
||||
reply.configTag = 4
|
||||
wire.send_frame(conn, reply.to_bytes())
|
||||
|
||||
srv = mock_controller(_handler)
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(
|
||||
cli,
|
||||
[
|
||||
"config",
|
||||
"push",
|
||||
"--dry-run",
|
||||
cluster_bin,
|
||||
"--controller",
|
||||
f"127.0.0.1:{srv.port}",
|
||||
"--cert-dir",
|
||||
CERT_DIR,
|
||||
],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert "generation 4→5" in result.output
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue