From ad27147a59f7f5e6f4a81a608715f52402b3d1b2 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Wed, 22 Jul 2026 09:45:11 +0000 Subject: [PATCH] feat(admin-config-tool): task 4.2 - deploy-ha.sh --build-config Add --build-config (parsed anywhere in the arg list) that rebuilds /cluster.bin from configs/ via the venv's xceadmin before deploying; a missing cluster.bin auto-builds the same way even without the flag. An existing cluster.bin with no flag passed is left byte-for-byte untouched, matching prior behavior. xceadmin and the configs source are resolved relative to the script's own location, not the caller's CWD. Verified live: missing-file auto-build, untouched-when-present (same mtime/SHA), and forced rebuild reproducing byte-identical output. --- .kiro/specs/admin-config-tool/tasks.md | 3 ++- src/tools/deploy-ha.sh | 36 ++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/.kiro/specs/admin-config-tool/tasks.md b/.kiro/specs/admin-config-tool/tasks.md index 2bebc898..12420659 100644 --- a/.kiro/specs/admin-config-tool/tasks.md +++ b/.kiro/specs/admin-config-tool/tasks.md @@ -91,7 +91,7 @@ - _Boundary: xceadmin.cli_ - _Depends: 3.1, 3.3_ -- [ ] 4.2 Build cluster configuration automatically during HA deployment +- [x] 4.2 Build cluster configuration automatically during HA deployment - Deploying a cluster with the build-config option produces a fresh binary from the current YAML before startup - Deploying with no binary present and no flag still produces one automatically - Deploying with an existing binary and no flag leaves it untouched, matching prior behavior @@ -155,3 +155,4 @@ - **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 ` 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). +- **4.2**: `deploy-ha.sh` resolves both `xceadmin` (`$SCRIPT_DIR/../xceadmin/.venv/bin/xceadmin`) and the YAML configs source (`$SCRIPT_DIR/../configs`) relative to the script's own location (`$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)`), not the caller's CWD — matches the project's other relative-path scripts and keeps the deploy working regardless of where it's invoked from. `--build-config` is parsed out of `"$@"` before positional-arg assignment so it can appear anywhere in the argument list, not just a fixed position. Verified live against the real `xceadmin` build and `src/configs/`: missing `cluster.bin` auto-builds (8.4), an existing one with no flag is left byte-for-byte untouched — same mtime, same SHA (8.5), and `--build-config` against an existing `cluster.bin` forces a rebuild that reproduces byte-identical content (8.3, confirming 9.1's determinism guarantee holds through this path too). diff --git a/src/tools/deploy-ha.sh b/src/tools/deploy-ha.sh index b446d304..4753af32 100755 --- a/src/tools/deploy-ha.sh +++ b/src/tools/deploy-ha.sh @@ -1,19 +1,45 @@ #!/bin/bash set -e -BINARY=$1 -DATADIR=$2 -TOP=$3 -PEERS=$4 +SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) + +# --build-config may appear anywhere among the positional args. +BUILD_CONFIG=0 +ARGS=() +for arg in "$@"; do + if [ "$arg" = "--build-config" ]; then + BUILD_CONFIG=1 + else + ARGS+=("$arg") + fi +done + +BINARY=${ARGS[0]} +DATADIR=${ARGS[1]} +TOP=${ARGS[2]} +PEERS=${ARGS[3]} if [ -z "$BINARY" ] || [ -z "$DATADIR" ] || [ -z "$TOP" ] || [ -z "$PEERS" ]; then - echo "Usage: $0 " + echo "Usage: $0 [--build-config] " + echo " --build-config: rebuild /cluster.bin from configs/ before deploying," + echo " even if one already exists" echo " peers: comma-separated host:port list (e.g. host1:port1,host2:port2)" exit 1 fi BINARY=$(realpath "$BINARY") +# Requirements 8.3/8.4/8.5: rebuild cluster.bin when --build-config is +# passed or none exists yet; otherwise leave an existing one untouched. +if [ "$BUILD_CONFIG" = "1" ] || [ ! -f "$DATADIR/cluster.bin" ]; then + XCEADMIN="$SCRIPT_DIR/../xceadmin/.venv/bin/xceadmin" + if [ ! -x "$XCEADMIN" ]; then + echo "Error: $XCEADMIN not found -- run 'xce-build.sh xceadmin' first" + exit 1 + fi + "$XCEADMIN" config build "$SCRIPT_DIR/../configs" -o "$DATADIR/cluster.bin" +fi + IFS=',' read -ra PEER_ARRAY <<< "$PEERS" N=${#PEER_ARRAY[@]}