From 0f1df4998ea690795b73ddbd4a453beb966e1eef Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Mon, 17 Aug 2026 10:20:52 +0000 Subject: [PATCH] feat(deploy,xceadmin): standalone xceadmin packaging + deploy-ha usability (bug-1566) Three related enhancements to make /xce-deploy-ha's output a self-contained, directly-usable cluster for manual testing: - xceadmin now builds an installable wheel (meson-build.sh) with correct dependencies (click/pycapnp/PyYAML/jsonschema, verified against actual imports -- pyproject.toml was missing several). - deploy-ha.sh resolves the deploy environment's own bash ($BASH_BIN, baked into every generated script's shebang -- this environment's deploy-ha.sh had drifted to #!/bin/sh while still using bash-only syntax) and generates scripts/start-worker.sh / scripts/start-scheduler.sh sharing the controllers' --peers, plus a standalone xceadmin install (wheel extracted directly, resolved $PYTHONPATH baked into a wrapper script -- nix devShell packages are $PYTHONPATH entries, not merged into python3's own site-packages, so a --system-site-packages venv can't see them). - bug-1566: xceadmin's --controller option had no relationship to a deployment's actual peers (hardcoded localhost:9991 default) -- every command required an explicit --controller or silently tried the wrong port. Fixed via a $XCEADMIN_CONTROLLER envvar fallback (click precedence: explicit flag > envvar > bare default), exported by deploy-ha.sh's generated scripts/xceadmin wrapper. Fixed/validated via TDD + live reproduction against a real deployed cluster in a fully stripped environment. Co-Authored-By: Claude Sonnet 5 --- src/tools/deploy-ha.sh | 152 ++++++++++++++++++++++++++++++- src/xceadmin/meson-build.sh | 17 ++++ src/xceadmin/pyproject.toml | 28 +++++- src/xceadmin/src/xceadmin/cli.py | 32 ++++++- src/xceadmin/tests/test_cli.py | 49 ++++++++++ 5 files changed, 266 insertions(+), 12 deletions(-) diff --git a/src/tools/deploy-ha.sh b/src/tools/deploy-ha.sh index cbc3347e..cea834c5 100755 --- a/src/tools/deploy-ha.sh +++ b/src/tools/deploy-ha.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh set -e SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) @@ -79,6 +79,39 @@ if [ -z "$XCEADMIN" ]; then exit 1 fi +# Worker/scheduler binaries, same-build-generation sibling lookup as +# xceadmin above (/worker/xcecd_virtual, /scheduler/ +# xce_scheduler next to /controller/xce_controller). Soft-check +# only: xce-test-ha's soak driver also calls this script and never builds +# either, so a missing worker/scheduler binary must not fail the deploy -- +# it just skips generating that one start script, with a warning. +WORKER_BIN="$BINARY_BUILD_DIR/worker/xcecd_virtual" +if [ ! -x "$WORKER_BIN" ]; then + echo "Warning: no xcecd_virtual found at $WORKER_BIN -- skipping scripts/start-worker.sh generation (run '/xce-build worker' first if you need it)" + WORKER_BIN="" +fi +SCHED_BIN="$BINARY_BUILD_DIR/scheduler/xce_scheduler" +if [ ! -x "$SCHED_BIN" ]; then + echo "Warning: no xce_scheduler found at $SCHED_BIN -- skipping scripts/start-scheduler.sh generation (run '/xce-build scheduler' first if you need it)" + SCHED_BIN="" +fi + +# xceadmin wheel (same-build-generation sibling again): /xce-build xceadmin +# leaves one at /xceadmin/dist/xceadmin-*.whl (meson-build.sh). +# Installed below into its OWN venv under $TOP, not just pointed at +# $XCEADMIN's build-dir .venv above -- a live deployment can outlive the +# build dir (a later `/xce-build xceadmin` run rm -rf's and rebuilds that +# .venv out from under anything still depending on it), so this deploy gets +# a genuinely self-contained copy. Soft-skip (warn), same reasoning as +# worker/scheduler above: a missing wheel must not fail the deploy. +XCEADMIN_WHEEL="" +if [ -d "$BINARY_BUILD_DIR/xceadmin/dist" ]; then + XCEADMIN_WHEEL=$(ls "$BINARY_BUILD_DIR/xceadmin/dist"/xceadmin-*.whl 2>/dev/null | head -1 || true) +fi +if [ -z "$XCEADMIN_WHEEL" ]; then + echo "Warning: no xceadmin wheel found under $BINARY_BUILD_DIR/xceadmin/dist -- skipping xceadmin-venv install into \$TOP (run '/xce-build xceadmin' first, same build dir as $BINARY, if you need it)" +fi + if ! command -v openssl >/dev/null 2>&1; then echo "Error: openssl not found -- run under 'nix develop' (provides openssl on PATH)" exit 1 @@ -122,9 +155,26 @@ for i in "${!PEER_ARRAY[@]}"; do PEERS_STR+="ctrl-$n=${PEER_ARRAY[$i]}" done +# Resolve the bash actually present in THIS invocation's environment (e.g. +# the nix dev shell's bash-interactive package) instead of hardcoding +# #!/bin/bash into every generated script -- a static shebang can silently +# point at the wrong (or a missing) interpreter once these scripts leave +# the environment they were generated in, and "which bash" run right here, +# in the same shell that just built cluster.bin/certs, is the actual source +# of truth for which bash this deploy is meant to run under. Every script +# below is also always invoked explicitly as `"$BASH_BIN" script.sh ...` +# (never `./script.sh` relying on the shebang) by both this skill and +# xce-test-ha -- the shebang is belt-and-suspenders for a human running one +# directly, not load-bearing for either automated caller. +GEN_BASH=$(command -v bash) +if [ -z "$GEN_BASH" ]; then + echo "Error: bash not found on PATH -- run under 'nix develop' (provides bash-interactive)" + exit 1 +fi + # Generate start script cat > "$TOP/scripts/start.sh" << EOF -#!/bin/bash +#!$GEN_BASH set -e NAME=\$1 if [ -z "\$NAME" ]; then @@ -143,7 +193,7 @@ chmod +x "$TOP/scripts/start.sh" # Generate cleanup script cat > "$TOP/scripts/cleanup.sh" << EOF -#!/bin/bash +#!$GEN_BASH set -e rm -f $TOP/logs/*.log for i in \$(seq 1 $N); do @@ -155,4 +205,100 @@ done EOF chmod +x "$TOP/scripts/cleanup.sh" +# Generate worker start script (component-failover-adoption's --peers path): +# same $PEERS_STR the controllers themselves were just given, so a worker +# started via this script fails over across the identical candidate list. +# xcecd_virtual (not production xcecd) is the default -- it skips the +# root-only sqlite/db-recovery and domain-socket subsystems, so it runs as +# an ordinary user in this same dev/CI environment with no sudo, which is +# what this skill's "local testing" scope calls for. Override with +# XCE_WORKER_BIN=/path/to/xcecd for a real worker if you specifically need +# job execution / cgroups (requires root -- run it yourself, outside this +# script, since this skill/session cannot sudo). +if [ -n "$WORKER_BIN" ]; then + cat > "$TOP/scripts/start-worker.sh" << EOF +#!$GEN_BASH +set -e +NAME=\${1:-worker-1} +[ \$# -gt 0 ] && shift +WORKER_BIN="\${XCE_WORKER_BIN:-$WORKER_BIN}" +mkdir -p $TOP/logs +"\$WORKER_BIN" --peers "$PEERS_STR" -n "\$NAME" -l $TOP/logs \$* +EOF + chmod +x "$TOP/scripts/start-worker.sh" +fi + +# Generate scheduler start script -- same $PEERS_STR again. xce_scheduler is +# a cluster singleton (no --myname / identity flag; its identity is fixed +# internally to "xce_scheduler"), so this script takes no name argument. +if [ -n "$SCHED_BIN" ]; then + cat > "$TOP/scripts/start-scheduler.sh" << EOF +#!$GEN_BASH +set -e +mkdir -p $TOP/logs +$SCHED_BIN --peers "$PEERS_STR" --logfile $TOP/logs/scheduler.log -v \$* +EOF + chmod +x "$TOP/scripts/start-scheduler.sh" +fi + +# Install xceadmin into THIS deployment ($TOP/xceadmin-site), not just +# pointed at $XCEADMIN's build-dir .venv resolved above -- a real, standalone +# copy, so the deployment keeps a working xceadmin long after the build dir +# it came from is rebuilt/cleaned. +# +# NOT a venv + `pip install`: a `python3 -m venv --system-site-packages` +# venv only inherits the BASE python's OWN site-packages directory -- and in +# this nix devShell, python3 itself is the bare interpreter derivation with +# an empty site-packages; click/pycapnp/PyYAML/jsonschema are each a +# SEPARATE nix store path made importable via $PYTHONPATH (verified: `nix +# develop … --command sh -c 'echo $PYTHONPATH'` lists them individually), +# not merged into python3's own site-packages tree the way a traditional +# system python + apt/dnf-installed packages would be. A --system-site- +# packages venv silently CANNOT see any of that (proven: an earlier version +# of this script's xceadmin-venv approach failed `ModuleNotFoundError: +# No module named 'click'` from the generated wrapper) -- $PYTHONPATH has to +# be captured explicitly. This mirrors meson-install.sh's own nix-packaging +# wrapper exactly (same "every path baked in below is a real /nix/store +# path" reasoning), just scoped to $TOP instead of a nix $out. +# +# `python3 -m zipfile -e` unzips the wheel (a wheel IS a zip file) straight +# into $TOP/xceadmin-site -- import-ready, no pip/venv machinery needed, +# since dependency resolution here is "capture $PYTHONPATH", not "install +# packages". +if [ -n "$XCEADMIN_WHEEL" ]; then + echo "=== Installing xceadmin ($XCEADMIN_WHEEL) into $TOP/xceadmin-site ===" + rm -rf "$TOP/xceadmin-site" + mkdir -p "$TOP/xceadmin-site" + python3 -m zipfile -e "$XCEADMIN_WHEEL" "$TOP/xceadmin-site" + + # Thin wrapper at scripts/xceadmin, same invocation convention as every + # other generated script (`"$BASH_BIN" "$TOP/scripts/xceadmin" ...`). + # $PYTHONPATH is expanded NOW (this script's own nix devShell + # environment), baking the resolved nix store paths in as a literal + # string -- not left as a `${PYTHONPATH:-}` runtime reference, which + # would silently go back to empty/wrong if scripts/xceadmin is ever + # invoked outside `nix develop`. + # + # XCEADMIN_CONTROLLER defaults every --controller-accepting xceadmin + # subcommand (config push, scheduler record start/stop/status) to this + # deploy's own first candidate -- xceadmin's baked-in fallback + # (_DEFAULT_CONTROLLER = localhost:9991) has no relationship to THIS + # deployment's actual ports (a custom --peers list, e.g. localhost:9901 + # here, silently left `./scripts/xceadmin scheduler record status` + # connecting to the wrong port with no override given). An explicit + # `--controller ...` on the command line still wins (click's own + # envvar-fallback precedence, xceadmin/cli.py). + cat > "$TOP/scripts/xceadmin" << EOF +#!$GEN_BASH +export PYTHONPATH="$TOP/xceadmin-site:$PYTHONPATH" +export XCEADMIN_CONTROLLER="${PEER_ARRAY[0]}" +exec $(command -v python3) -m xceadmin.cli "\$@" +EOF + chmod +x "$TOP/scripts/xceadmin" +fi + echo "Deployed $N-node cluster to $TOP (tag: $TAG)" +if [ -n "$XCEADMIN_WHEEL" ]; then + echo "xceadmin installed at $TOP/xceadmin-site -- use it via $TOP/scripts/xceadmin, e.g.:" + echo " $TOP/scripts/xceadmin config push $TOP/controller/ctrl-1/conf/cluster.bin.$TAG --controller ${PEER_ARRAY[0]}" +fi diff --git a/src/xceadmin/meson-build.sh b/src/xceadmin/meson-build.sh index 62f485cb..4e83d045 100755 --- a/src/xceadmin/meson-build.sh +++ b/src/xceadmin/meson-build.sh @@ -60,5 +60,22 @@ rm -rf "$VENV_DIR" echo "=== xceadmin (meson): running tests ===" "$VENV_DIR/bin/python" -m pytest "$WORK_DIR/tests" -v +# Build an installable wheel from the SAME schema-synced $WORK_DIR the pinned +# install above just tested -- not a second, separately-assembled tree -- +# so the wheel is guaranteed to carry the identical schema_files/ package +# data (Requirement 5.5) the just-passed test run actually exercised. Only +# reached after `set -e` has let the test run above succeed, so a wheel is +# never produced (or left stale) from a red test run. --no-deps: this is +# xceadmin's own wheel only, not a vendored bundle of click/pycapnp/etc -- +# installing it still needs those from PyPI or (preferably) the nix shell, +# same as the pinned install just above. +DIST_DIR="$BUILD_DIR/dist" +echo "=== xceadmin (meson): building installable wheel ===" +rm -rf "$DIST_DIR" +mkdir -p "$DIST_DIR" +"$VENV_DIR/bin/pip" wheel "$WORK_DIR" --no-deps -w "$DIST_DIR" --quiet +WHEEL_FILE=$(ls "$DIST_DIR"/xceadmin-*.whl) +echo "=== xceadmin (meson): wheel built at $WHEEL_FILE ===" + touch "$STAMP_FILE" echo "=== xceadmin (meson): done ===" diff --git a/src/xceadmin/pyproject.toml b/src/xceadmin/pyproject.toml index d6005095..ba4f194e 100644 --- a/src/xceadmin/pyproject.toml +++ b/src/xceadmin/pyproject.toml @@ -9,7 +9,22 @@ description = "XCE cluster configuration admin CLI (config build / config push)" requires-python = ">=3.10" dependencies = [ "click>=8.0", + "pycapnp>=1.3", + "PyYAML>=6.0", + "jsonschema>=4.0", ] +# All four are real runtime imports (capnp/click/jsonschema/yaml — grep +# src/xceadmin src/xcecommon for `^import |^from `), not just dev/test +# tooling: an installed-but-undeclared dependency here would let `pip +# install` "succeed" while leaving the CLI broken at first real use +# (`ModuleNotFoundError` the moment `config build` touches capnp/jsonschema/ +# yaml) — exactly the class of gap a wheel built for install-outside-this- +# repo needs to not have. pycapnp specifically: the nix dev shell's +# python3Packages.pycapnp (src/nixSupport/xce.nix) is preferred when +# available because it links against the same capnproto C++ runtime this +# repo's C code uses; PyPI's prebuilt pycapnp wheels are usually fine too +# for a plain `pip install` outside the nix shell, but that's a "usually," +# not the guaranteed-consistent link this repo's own build gets. # Wired into meson (src/xceadmin/meson.build custom_target, pulled in by # src/meson.build's subdir('xceadmin')) so `nix build .#xcecore[_debug]` @@ -31,10 +46,13 @@ include = ["xceadmin*", "xcecommon*"] "" = "src" # Bundled copies of controller.capnp/c.capnp and the 7 JSON validation -# schemas (synced from src/schema/ and src/configs/schema/ by -# src/tools/xce-build.sh's `xceadmin)` case, see Requirement 5.5). Without -# this, a non-editable `pip install .` builds a wheel containing only *.py -# files and silently drops schema_files/ — an editable install would never -# catch this since it resolves straight into the checked-out source tree. +# schemas (synced from src/schema/ and src/configs/schema/ into +# xcecommon/schema_files/ by src/xceadmin/meson-build.sh, the meson +# custom_target xce-build.sh's `xceadmin)` case now delegates to, before +# either the pinned test-install or the wheel build run against that same +# tree — see Requirement 5.5). Without this, a non-editable `pip install .` +# (or a wheel build) produces a package containing only *.py files and +# silently drops schema_files/ — an editable install would never catch this +# since it resolves straight into the checked-out source tree. [tool.setuptools.package-data] xcecommon = ["schema_files/*.capnp", "schema_files/json/*.schema.json"] diff --git a/src/xceadmin/src/xceadmin/cli.py b/src/xceadmin/src/xceadmin/cli.py index 960fe5ff..260e8804 100644 --- a/src/xceadmin/src/xceadmin/cli.py +++ b/src/xceadmin/src/xceadmin/cli.py @@ -39,6 +39,14 @@ from xceadmin.scheduler import RecordResult, dump, record_start, record_status, _DEFAULT_OUTPUT = "cluster.bin" _DEFAULT_CONTROLLER = "localhost:9991" +# Every --controller option below also reads this env var (click precedence: +# explicit --controller flag > $XCEADMIN_CONTROLLER > _DEFAULT_CONTROLLER). +# _DEFAULT_CONTROLLER has no relationship to any REAL deployment's actual +# ports -- it's a bare fallback for a totally bare invocation. A deployment +# that knows its own real controller ports (e.g. /xce-deploy-ha's generated +# scripts/xceadmin wrapper) should export this instead of requiring every +# command to be typed with an explicit --controller. +_CONTROLLER_ENV_VAR = "XCEADMIN_CONTROLLER" _STATUS_LABEL_WIDTH = 13 # len("started at:") + 2 -- requirements.md's "Status output" example @@ -68,7 +76,11 @@ def config_build(configs_dir: str, output: str) -> None: @config.command("push") @click.argument("cluster_bin_path") @click.option( - "--controller", default=_DEFAULT_CONTROLLER, help="host:port of any controller node (2.2)." + "--controller", + default=_DEFAULT_CONTROLLER, + envvar=_CONTROLLER_ENV_VAR, + help=f"host:port of any controller node (2.2). Falls back to ${_CONTROLLER_ENV_VAR} " + "if set, then the bare default.", ) @click.option("--dry-run", is_flag=True, help="Probe acceptance without committing (Requirement 6).") def config_push(cluster_bin_path: str, controller: str, dry_run: bool) -> None: @@ -195,7 +207,11 @@ def _run_scheduler_admin_command(controller: str, call: Callable[[], RecordResul @scheduler_record.command("start") @click.argument("file", required=False) @click.option( - "--controller", default=_DEFAULT_CONTROLLER, help="host:port of any controller node (2.2)." + "--controller", + default=_DEFAULT_CONTROLLER, + envvar=_CONTROLLER_ENV_VAR, + help=f"host:port of any controller node (2.2). Falls back to ${_CONTROLLER_ENV_VAR} " + "if set, then the bare default.", ) def scheduler_record_start(file: str | None, controller: str) -> None: """Start scheduler event recording, optionally to FILE (auto-generated when omitted).""" @@ -205,7 +221,11 @@ def scheduler_record_start(file: str | None, controller: str) -> None: @scheduler_record.command("stop") @click.option( - "--controller", default=_DEFAULT_CONTROLLER, help="host:port of any controller node (2.2)." + "--controller", + default=_DEFAULT_CONTROLLER, + envvar=_CONTROLLER_ENV_VAR, + help=f"host:port of any controller node (2.2). Falls back to ${_CONTROLLER_ENV_VAR} " + "if set, then the bare default.", ) def scheduler_record_stop(controller: str) -> None: """Stop scheduler event recording.""" @@ -215,7 +235,11 @@ def scheduler_record_stop(controller: str) -> None: @scheduler_record.command("status") @click.option( - "--controller", default=_DEFAULT_CONTROLLER, help="host:port of any controller node (2.2)." + "--controller", + default=_DEFAULT_CONTROLLER, + envvar=_CONTROLLER_ENV_VAR, + help=f"host:port of any controller node (2.2). Falls back to ${_CONTROLLER_ENV_VAR} " + "if set, then the bare default.", ) def scheduler_record_status(controller: str) -> None: """Report current scheduler event-recording state.""" diff --git a/src/xceadmin/tests/test_cli.py b/src/xceadmin/tests/test_cli.py index 4a046e89..19040b6a 100644 --- a/src/xceadmin/tests/test_cli.py +++ b/src/xceadmin/tests/test_cli.py @@ -498,6 +498,55 @@ def test_scheduler_record_start_passes_filename_and_controller_arguments(monkeyp assert captured["filename"] == "myfile.replay" +# --------------------------------------------------------------------------- +# --controller falls back to $XCEADMIN_CONTROLLER, not the hardcoded +# _DEFAULT_CONTROLLER, when the env var is set (a deployment whose cluster +# doesn't happen to use localhost:9991 -- e.g. /xce-deploy-ha's +# scripts/xceadmin wrapper exports this to the actual first --peers +# candidate -- must not silently point at the wrong port). Explicit +# --controller on the command line still wins over the env var (click's +# built-in precedence: CLI flag > envvar > default). +# --------------------------------------------------------------------------- + + +def test_controller_option_falls_back_to_xceadmin_controller_env_var(monkeypatch) -> None: + captured = {} + + def _fake_record_status(controller): + captured["controller"] = controller + return RecordResult(recording=False, file="", event_count=0, started_at=0) + + monkeypatch.setattr(cli_module, "record_status", _fake_record_status) + + runner = CliRunner() + result = runner.invoke( + cli, ["scheduler", "record", "status"], env={"XCEADMIN_CONTROLLER": "localhost:9901"} + ) + + assert result.exit_code == 0, result.output + assert captured["controller"] == "localhost:9901" + + +def test_controller_flag_overrides_xceadmin_controller_env_var(monkeypatch) -> None: + captured = {} + + def _fake_record_status(controller): + captured["controller"] = controller + return RecordResult(recording=False, file="", event_count=0, started_at=0) + + monkeypatch.setattr(cli_module, "record_status", _fake_record_status) + + runner = CliRunner() + result = runner.invoke( + cli, + ["scheduler", "record", "status", "--controller", "127.0.0.1:9999"], + env={"XCEADMIN_CONTROLLER": "localhost:9901"}, + ) + + assert result.exit_code == 0, result.output + assert captured["controller"] == "127.0.0.1:9999" + + def test_scheduler_record_stop_success_prints_formatted_result(monkeypatch) -> None: def _fake_record_stop(controller): return RecordResult(recording=False, file="final.replay", event_count=14273, started_at=1000)