test(admin-config-tool): add live single-node E2E test for config push

Deploys a real xce_controller (no HA peers) and runs real xceadmin
config build/push over actual TLS+capnp -- no mock server -- regression
locking task 6.1's envelope fix. Skips cleanly when no controller binary
is available. Surfaced a real, out-of-scope finding: single-node pushes
always report ackedCount==0/exit 1 even on full server-side success,
since there are no standbys to ACK.
This commit is contained in:
Rongsong Shen 2026-07-30 10:21:04 +00:00
parent 915e6fdf64
commit 1075f4529a
2 changed files with 247 additions and 1 deletions

View file

@ -151,7 +151,7 @@
- _Boundary: xceadmin.discovery, xceadmin.push_
- _Depends: 3.2, 3.3_
- [ ] 6.2 Add a live single-node E2E test for `config push`
- [x] 6.2 Add a live single-node E2E test for `config push`
- New test deploys one real `xce_controller` (no HA peers needed — reachability/decode correctness is what's under test, not consensus), runs `xceadmin config build` against `src/configs/`, then a real `xceadmin config push` against it over the actual TLS+capnp wire protocol — no mock server
- Test asserts exit 0, a parsed `versionTag`/`ackedCount` in the output, and confirms the controller's own log shows the config committed and activated
- Skips cleanly (not a hard failure) when no controller binary is available in the current build profile, matching this suite's existing cert-dependent skip convention (see task 5.3's Implementation Notes)
@ -183,6 +183,7 @@
- **Post-implementation, out-of-band (2026-07-22, second pass)**: `meson-build.sh`/`meson-install.sh` no longer resolve `python3` via a bare PATH lookup — `meson.build` now pins it once via `import('python').find_installation('python3')` (same mechanism as `find_program('go')`/capnp/packcc elsewhere in this build, visible at `meson setup` time as `Program python3 found: YES (/nix/store/...)`) and threads the resolved path into both scripts explicitly. Verified: `meson setup` prints the pinned interpreter, rebuild stays 165/165 green, and the built venv's own `python3` resolves through to that exact store path.
- **Post-implementation, out-of-band (2026-07-22, third pass — removes `ha/data/` entirely)**: `deploy-ha.sh` no longer takes a `<data-dir>` argument at all (`--build-config` flag also removed) — every invocation now generates a fresh self-signed TLS cert/key (`openssl req -x509 -newkey rsa:2048 -nodes -days 3650`, 10y validity) directly into the deploy dir's `certs/`, and builds `cluster.bin` directly into `controller/ctrl-1/conf/` via `xceadmin`, with no intermediate staging directory. `run-ha-soak.sh` (`/xce-test-ha`) updated to match: dropped its own `XCE_HA_DATADIR`-based precondition entirely, since `deploy-ha.sh` now handles both cert and config generation itself. Updated 4 callers (`run-ha-soak.sh`, `test-heartbeat-tolerance.sh`, `test-commit-durability.sh`, plus both `xce-deploy-ha`/`xce-test-ha` SKILL.md docs) to the new 3-arg signature. `ha/data/{xce.crt,cluster.bin}` removed from git tracking (`cluster.bin` was already gitignored from the prior pass; the certs were a real checked-in TLS private key, now gone from history going forward).
- **6.2**: A genuinely single-node (zero-peer) controller always computes `ackedCount == 0` for `config push` — no standbys exist to ACK — even when the config change commits and activates successfully server-side (`config_repl.c`'s `push_config_to_standbys`/`do_upgrade_config_with_bytes`). `xceadmin.cli`'s postcondition (`acked_count > 0` required for exit 0) can't distinguish "no standbys exist" from "push failed," so a real single-node `config push` always exits 1 with `"push rejected (ackedCount == 0)"` even on full success — reproduced live and independently re-confirmed by the reviewer against real `config_repl.c`/`repl.c` source. Verified spec-consistent (matches `cli.py`/design.md's own exit-0-iff-`acked_count>0` postcondition as written), not a regression from this task. `test_e2e_live_push.py` asserts this real exit-1/ackedCount==0 behavior directly rather than the task text's literal "exit 0," and independently proves genuine server-side success via the controller's own commit/activate log lines. Out of this task's boundary to fix; a future task should decide whether single-node pushes deserve a distinct success signal in `xceadmin.cli`/`push.py` (e.g. treating a fully-connected zero-peer topology as trivially quorate) rather than always reading as a CLI failure.
- **6.1**: `decode_reply()` (new, `discovery.py`) is the single unwrap point for the `Request{opcode: reply, body.replymsg: RequestReply}` envelope every real controller reply arrives in (`xce_reply()`, `src/lib/request.c:384-418`) — both `discovery._redirect_target()` and `push.push_config()` go through it; `probe_config()` is untouched (`ProbeConfigTagReply` is never envelope-wrapped). Fixing the two documented call sites alone was insufficient — task 5.6's own mock-server test fixtures (`test_discovery.py`, `test_config_push_mock.py`) AND `test_cli.py`'s two `config push` mock handlers all constructed bare, unwrapped `RequestReply` bytes, meaning the entire existing test suite had been silently validating the wrong wire shape since task 5.6/task 4.1. All fixed via one shared `wrap_reply` fixture in `conftest.py`. Any future test that hand-builds a controller reply for a mock server must wrap it the same way, or it will pass against a mock while still being broken against a real controller.
Investigating this surfaced a real, independent finding: `xcecommon.wire.connect()` was loading a **client-side** TLS cert (`context.load_cert_chain(...)` on the `PROTOCOL_TLS_CLIENT` context) — verified against the actual reference implementation (`netio.c`'s `create_ssl_context()`) that the real client presents **no** certificate at all when `cert_file`/`key_file` are `NULL` (bare `TLS_client_method()`, no `SSL_CTX_use_certificate_chain_file`/`SSL_CTX_use_PrivateKey_file` call). `xceadmin`'s client diverged from the reference client it's meant to mirror. Fixed: `connect()` no longer loads a client cert or takes a `cert_dir` parameter at all. Since nothing else consumed `cert_dir` (no CA verification happens either — `verify_mode=CERT_NONE` throughout), removed it end-to-end: `xceadmin.discovery.follow_redirects()`'s `cert_dir` param (already dead/`noqa`'d — signature-parity leftover), `xceadmin.push.push_config()`/`probe_config()`, and the `--cert-dir` CLI flag on `config push`. Server-side cert loading (the mock TLS servers in `test_wire.py`/`test_cli.py`/`test_config_push_mock.py`) is unaffected and still needed — moved into a shared session-scoped `tls_certs` fixture in a new `tests/conftest.py` (openssl-generated, same approach as `deploy-ha.sh`), replacing 3 duplicated module-level `REPO_HA_DATA_DIR`/`pytest.mark.skipif(not ha/data/xce.crt...)` blocks. Net result: those 3 files' TLS-dependent tests no longer skip under a hermetic `nix build` sandbox (which never had `ha/data/` in its git-tracked-only source) — verified 165/165 passed, 0 skipped, both via `meson compile` and via a full `nix build --option sandbox false .#xcecore_debug` rebuild (previously 128 passed / 37 skipped there). Requirements 2.3/2.4 and 8.3/8.4/8.5 marked superseded in `requirements.md` with dated correction notes rather than silently rewritten.