feat(config-upgrade-snapshot): add configcompare tool, extend snapverify (task 8.4)

Build configcompare_paths(), an offline sibling to snapdiff/walcompare that
recomputes each member's active-config sha256 fresh from its own on-disk
workdir/name/conf bytes and reports identical vs. divergent (Req 4.2).
Wire it as a cmd_configcompare CLI subcommand in test.c, matching its
siblings, after review caught it shipped as an unreachable library function
in round 1.

Extend snapverify to match the current on-disk checkpoint format: bump
SNAP_FORMAT_VERSION to 2, replace the stale raw queue-frame decode with the
capnp length-prefixed walk, add host-frame and config-bytes-section
structural walks, and decode (not just presence-check) the header's
cfgref tag/sha. Without this fix snapverify rejected every checkpoint
written by the current codebase.
This commit is contained in:
Rongsong Shen 2026-07-25 09:06:24 +00:00
parent c5d8ecbd7d
commit 1d09631df0
10 changed files with 997 additions and 47 deletions

View file

@ -106,7 +106,7 @@
- _Depends: 6.2_
- _Boundary: Activation Fence (test_upgrade_activation_fence.c)_
- [ ] 8.4 (P) Build a cross-member active-configuration comparator and extend checkpoint verification tooling
- [x] 8.4 (P) Build a cross-member active-configuration comparator and extend checkpoint verification tooling
- Build a new offline, file-based tool that compares each member's active configuration content hash, recomputed fresh from each member's own on-disk configuration file, mirroring the existing checkpoint-comparison tool's shape
- Extend the existing checkpoint-structure verification tool to decode and assert the embedded configuration reference's tag and hash (not merely report their presence), and to report host-frame count and configuration-bytes-section presence
- Observable: the comparator correctly reports two members with matching configuration as identical and two with differing configuration as divergent; the extended verification tool reports a concrete tag/hash value and section counts for a real checkpoint file
@ -149,6 +149,8 @@
## Implementation Notes
- **Task 8.4 round-1 rejection (real gap, caught by review):** the first pass built `configcompare_paths()` (correct, well-tested against `persist.c`/`config_repl.c`'s real `workdir/name/conf/current`+`conf/cluster.bin.<hex>` resolution convention) and linked `configcompare.c` into both `xce_test_src` and `controller_unit_test_src` in meson, but never registered it as a `cmd_configcompare` subcommand in `testsuite/test.c` — unlike its siblings `walcompare`/`snapverify`/`snapdiff`, each of which has a `cmd_*` wrapper in BOTH `top_commands[]` and `client_commands[]`. Left as delivered, the tool was a tested library function with no shell entry point — unusable by task 8.6/8.9's later HA soak scenarios, which design.md explicitly says will invoke `configcompare` directly against each node's data directory. Fixed with a 25-line, `test.c`-only remediation mirroring `cmd_snapdiff`'s exact shape; verified end-to-end via `echo 'configcompare <dir1> <dir2>' | xce_test` (checked `$?` for both IDENTICAL/exit-0 and DIVERGENT/exit-1 cases, not just printed text). **Heads-up for any future task in this spec (or elsewhere) that adds a new offline testsuite tool in this family**: linking the `.c` file into the meson source lists is NOT sufficient — it must also get a `cmd_*` wrapper + `top_commands[]`/`client_commands[]` entries in `test.c`, or it silently has no way to be invoked outside the unit test binary.
- **Task 8.3: no new tests needed** — the 10 tests already in `test_config_repl.c` from tasks 6.1/6.2 fully satisfy both bullets and Requirements 3.2/3.3/3.4/4.3/5.5 (verified independently by implementer and reviewer; zero-diff task). The two "fence sets" tests genuinely toggle different halves of `upgrade_materialization_verify`'s condition (materialized vs. bytes_verified), not duplicates. Retry-trampoline automatic-firing (delaymgr actually running) is untested, matching the exact precedent depth of `config_activate_retry_fn`/`config_schedule_activate_retry` (also untested at that depth anywhere in this codebase) — judged out of scope, not a gap.
- **Task 8.2: no new tests needed** — task 3's `test_upgrade_snapshot_restore.c` already satisfies all 3 bullets and Requirements 1.10/1.11/5.8/7.1 (verified independently by both implementer and reviewer; zero-diff task). Requirement 7.1's full scenario is compositionally covered across two files (`test_upgrade_snapshot_restore.c`'s real-rejoin test + `test_persist_snapshot_entities.c`'s `test_snapent_load_snapshot_restores_hosts_and_queues_end_to_end`'s real checkpoint-decode test), not by one continuous test — judged adequate since `restore_host_state` is a single invocation-site-independent function. design.md's Requirements Traceability table (row 7.1) under-credits this and should eventually be corrected to cite both files, not just `test_upgrade_snapshot_restore.c` — docs-only, not a testing gap.