14 KiB
Implementation Plan
TDD is mandatory (project rule): add the failing test/scenario first, then the change (red → green → refactor). Almost all work is in the single test client TU
src/controller/testsuite/test.c, so tasks are sequential by design; only the soak conversion touches a different file. Behavioral tests are driven throughxce_testitself (piped scripts), gating on its exit code.
-
1. Foundation: verdict, loop wrapper, ergonomics
-
1.1 Session failure tracking + process exit code + abort/continue + comment/blank handling
- Add file-scope state (
g_session_failed,g_continue_on_error,g_last_status) and apreprocess_and_run(line)wrapper used by the main read loop: ignore blank and#-comment lines; run normal lines viaexecute_line; updateg_last_status/g_session_failedfrom the result; in default mode stop on first failure, undercontinue-on-errorkeep going.mainreturnsg_session_failed ? 1 : 0. Extendcmd_setwith acontinue-on-errortoggle. - Observable: a piped script with a bogus/failing command makes
xce_testexit non-zero; an all-valid script exits 0; withset continue-on-errora failing line does not stop later lines but the final exit is still non-zero;#/blank lines are ignored. - Requirements: 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 6.1, 6.2
- Boundary: VerdictLoopWrapper
- Add file-scope state (
-
1.2
echocommand- Add
cmd_echo(table-registered) that prints its argument message and returns success. - Observable:
echo helloemitshelloand the command succeeds (exit unaffected). - Requirements: 6.3
- Boundary: VerdictLoopWrapper
- Depends: 1.1
- Add
-
2. Shell-escape
-
2.1
!<cmd>runs a shell command and feeds$?- In
preprocess_and_run, when the first non-whitespace char is!, run the remainder viasystem(); setg_last_status = WEXITSTATUS(rc)(non-zero onsystemfailure); a non-zero status is a command failure (subject to 1.1's abort/continue + exit code). - Observable:
!truesucceeds and sets$?=0;!falsefails the session and sets$?≠0; a script can!kill .../!bash start.sh ...to drive node lifecycle. - Requirements: 2.1, 2.2, 2.3, 2.4
- Boundary: ShellEscape
- Depends: 1.1
- In
-
3. Auto-variable substitution
-
3.1 Substitute the fixed auto-vars
$?,$jobid,$jobcount- Add
substitute_autovars(line)(run before tokenizing inpreprocess_and_run): replace whole-token$?→g_last_status,$jobid→g_last_jobid,$jobcount→g_last_jobcount. A reference to an unset$jobid/$jobcountfails the command (no garbage substitution). No other variable syntax is recognized. - Observable: after a command,
echo $?prints the prior status; referencing$jobidbefore any successfuljobsubfails that command rather than substituting an empty/garbage value. - Requirements: 3.1, 3.3, 3.5, 3.6
- Boundary: AutoVars
- Depends: 1.1
- Add
-
4. Correct job-submission reporting (bug-786) +
$jobidcapture -
4.1
cmd_jobsubreports success only on a clean ack with a valid JobId- Gate the
jobsubmit ok: JobId: <id>report on a successful reply status AND a well-formed JobId; on success setg_last_jobid(feeds$jobid, 3.2). Otherwise print a typed error (redirect / no-master / not-ready / timeout) and return<0. Never print "ok" with an uninitialized/garbage id (63-15--1-...). - Observable: a submit that does not get a clean ack returns failure and does NOT set
$jobid;jobsubnever prints success with a malformed id; a successful submit sets$jobidto the acked JobId. - Requirements: 7.1, 7.2, 7.3, 3.2
- Boundary: JobsubFix
- Depends: 1.1, 3.1
- Gate the
-
5. Job-count capture and assertions
-
5.1
cmd_joblistsets$jobcount; extract a shared count/contains helper- Refactor
cmd_joblistto expose an internal helper that returns the current job count and tests membership of a JobId; setg_last_jobcountafter ajoblist. - Observable: after
joblist,echo $jobcountprints the number of jobs returned; the helper is reusable by the assertions. - Requirements: 3.4
- Boundary: JoblistCount
- Depends: 1.1, 3.1
- Refactor
-
5.2
expect-jobs Nassertion- Add
cmd_expect_jobs: pass when the current job count equalsN; otherwise print expected vs actual and return<0. - Observable:
expect-jobs <correct>succeeds;expect-jobs <wrong>fails the session and prints expected/actual. - Requirements: 5.1, 5.2
- Boundary: Assertions
- Depends: 5.1
- Add
-
5.3
expect-job <jobid>assertion- Add
cmd_expect_job: pass when the JobId is present in the job list; otherwise print the missing id and return<0. Works with$jobidsubstitution. - Observable:
expect-job $jobidpasses for a submitted job;expect-job <absent>fails the session and prints the missing id. - Requirements: 5.3, 5.4
- Boundary: Assertions
- Depends: 5.1, 3.1, 4.1
- Add
-
6. Wait for elected primary
-
6.1
wait-primary [timeout_ms]- Add
cmd_wait_primary: loop a cheap, failover-aware client op (the existingjoblist/probe path viaactive_ctx) until it succeeds or the timeout elapses (monotonic clock); default timeout when the arg is omitted; return 0 on success,<0on timeout. - Observable: against a healthy cluster
wait-primaryreturns immediately; after a primary kill it blocks until a new primary serves, then succeeds; with no primary within the timeout it fails. - Requirements: 4.1, 4.2, 4.3, 4.4
- Boundary: WaitPrimary
- Depends: 1.1
- Add
-
7. Reference E2E driver
-
7.1 Convert
test-commit-durability.shto use the primitives- Replace the prose-grep + sanitizer-filter logic with piped
xce_testscripts usingwait-primary,jobsub/$jobid,expect-jobs/expect-job, and!-shell-escape for the primary kill/restart; gate each phase on thexce_testexit code. Preserve the rolling-kill + zero-loss coverage; add a!-driven cold-restart-all +expect-jobsstep. - Observable: the converted soak runs end-to-end and reports pass/fail via
xce_testexit codes (no joblist prose parsing); rolling-kill zero-loss coverage is retained; the cold-restart step asserts all committed jobs recover. - Requirements: 8.1, 8.2, 8.3
- Boundary: ReferenceDriver
- Depends: 1.1, 2.1, 4.1, 5.2, 5.3
- Replace the prose-grep + sanitizer-filter logic with piped
-
7.2 Run the converted soak on a live
/xce-deploy-hacluster- Execute the converted soak against a real 3-node cluster (real fsync-able fs) and confirm pass.
- Observable: soak passes all rounds with the exit-code verdict; manual environment prerequisite (live cluster), same status as other live-E2E tasks.
- Requirements: 8.2, 8.3
- Boundary: ReferenceDriver
- Depends: 7.1
- Blocked: PARTIAL — the converted soak was RUN live (3-node cluster) and works: it runs end-to-end and reports pass/fail via the exit-code verdict; rolling-kill zero-loss coverage PASSES every round. Two findings: (1) IN-SCOPE & FIXED — the live run surfaced that task 4.1's bug-786 gate (
jobid != 0) was insufficient against a stale non-zero reply id (63-15--1-1015808); strengthened to requirejobid == jobsubmit.jobid(commit8609976) and re-verified (garbage now rejected loud). (2) UPSTREAM, OUT OF BOUNDARY — the new cold-restart-all step NON-DETERMINISTICALLY detects a committed-job loss after a full kill-all+restart (a job that returnedjobsubmit okand survived all rolling rounds vanishes; recovery logs show WALtruncate). This is a controller commit-durability behavior under total-cluster-outage recovery — owned by specha-commit-durability, NOT this test-primitives spec (which changes no controller code). The primitives are working correctly by surfacing it. Confirmation needs a REAL fsync-able fs (this sandbox is overlayfs, the task's stated manual prerequisite is unmet) + the ha-commit-durability owner. Full-green 7.2 is blocked on that upstream investigation + a proper live environment.
Implementation Notes
- All of tasks 1–6 modify the single TU
src/controller/testsuite/test.c; sequential by design (no(P)). Task 7 touchessrc/tools/test-commit-durability.shbut depends on the primitives, so it is also ordered last. - Exit-code verdict is only trustworthy with a clean process exit; run the soak with
ASAN_OPTIONS=detect_leaks=0(existing convention). Client leak cleanup is out of scope (separate item). - Build dir is
src/xce.build(notsrc/build). Fast incremental build:cd src && nix develop --command bash -c "meson compile -C xce.build xce_test". Binary:src/xce.build/controller/xce_test. Behavioral tests = piped scripts gating on$?withASAN_OPTIONS=detect_leaks=0; no controller needed (an unknown command is a convenient failing command).xce_testis NOT a meson-registeredtest(), someson test --suite controllerdoes not exercise it. - Pre-existing unrelated failure:
meson test --suite controllerreportstest_demotion_replay_yields_committed_onlyFAIL (incontroller_unit_test/test_commit_barrier_reply.c) — from the in-flightha-commit-durabilityfeature, in a binary that does NOT compiletest.c. Not caused by this spec; ignore it for verdicts here. - 1.1:
preprocess_and_run()is the single chokepoint where the leading-!shell-escape (2.1) and$-autovar substitution (3.1) plug in. File-scopeg_session_failed/g_continue_on_error/g_last_statusadded;mainreturnsg_session_failed ? 1 : 0;set continue-on-erroris a process-global argc==2 special-case incmd_set(not a per-contextsettings[]entry). - Feature Flag Protocol: the flag must be fully REMOVED at the end (no pinned
#define X (1)guarding a dead#else). Reviewer rejects leftover flag scaffolding as dead code. (3.1 hit this — re-mediated.) - 3.1: introduced
g_last_jobid[XCE_MAXNAME_LEN]={0}(empty = unset) andg_last_jobcount=-1(-1 = unset) file-scope state, POPULATED later by 4.1 (jobsub) / 5.1 (joblist).substitute_autovars(line,out,sz)does whole-token replace of$?/$jobid/$jobcount; unset$jobid/$jobcount→ command fails (autovar: $X referenced before set), no garbage.!shell-escape lines are NOT substituted (raw to shell — the shell owns its$). Added#include "config.h"for XCE_MAXNAME_LEN. - 4.1 (bug-786 fixed):
cmd_jobsubinits localjobid=0, printsjobsubmit ok+ capturesg_last_jobidONLY after BOTH gates:rc>=0ANDjobid!=0; else typed error (E_XCE_NOMASTER/REDIRECT/NOT_READY/timeout switch viaxce_strerror_r) +return <0.$jobidcapture usesfmt_jobid(g_last_jobid, ..., jobid)— the FULL jobid, byte-identical to howcmd_joblistformatsjobs[i]->jobid(NOTarray_jobid(), which the display-onlyok:line uses). This is REQUIRED so task 5.3expect-job $jobidmatches the joblist column for array jobs. Also moved the error check AFTER the FREEUP block → fixed a pre-existing error-path leak. Live success-path ($jobid==joblist entry) verified by code reading; runtime-verified via soak (7.2). - 5.1: extracted DRY print-free helper
static int joblist_query(const char* find_jobid, int* contains, int print_table)— singlexcecli_joblistcall + internal free; returns count (>=0) or<0; membership viastrcmp(fmt_jobid(buf,...,jobs[i]->jobid), find_jobid)(SAME full-jobid form as 4.1's$jobid).cmd_joblistdelegatesjoblist_query(NULL,NULL,1)and setsg_last_jobcount=countONLY on success (no fake count on failure). 5.2/5.3 assertion commands calljoblist_querydirectly. Live$jobcount-after-success needs a controller (soak 7.2). - 5.2/5.3/6.1:
expect-jobs,expect-job,wait-primaryare all CLIENT-context commands registered inclient_commands[](they needactive_ctxto query jobs) — a script mustclientfirst. All return<0on failure →preprocess_and_runfails the session. Implemented directly (no flag macro); RED shown via unknown-command absence. Key invariant across assertions: a FAILED joblist query is an assertion failure (distinct message), NEVER silently "0 jobs"/"absent".wait-primary: monotonicclock_gettime(CLOCK_MONOTONIC), defaultWAIT_PRIMARY_DEFAULT_MS=30000, pollWAIT_PRIMARY_POLL_MS=200, bounded (no hang) — do NOT run no-argwait-primaryagainst a dead cluster in a test (blocks 30s); pass a small explicit timeout. - 7.1 (soak conversion): all
xce_testverdicts now come from the PROCESS EXIT CODE via a singlexce_test_scriptchokepoint (setsASAN_OPTIONS=detect_leaks=0).$jobidcapture usesecho ACKED_JOBID $jobid(space-separated — whole-token substitution requires it; a gluedMARK=$jobidwould NOT expand) parsed withsed. Multi-line script accumulation must use real$'\n', NOT$(...)(command-sub strips trailing newlines → glued tokens).current_primary/node_pid/node_rolelog helpers retained ONLY for lifecycle (victim selection, strace targeting), never as a verdict. Newrun_cold_restart_recoveryphase:!pkillall →!bash start.shall →wait-primary→expect-jobs/expect-job. 7.2 (live run on/xce-deploy-ha) is the only remaining task — manual env prerequisite (needs a real 3-node cluster). - 7.2 LIVE RUN OUTCOME (ran 3 live soaks on a sandbox 3-node cluster): the converted soak works end-to-end with the exit-code verdict; rolling-kill zero-loss PASSES every round. TWO findings: (A) IN-SCOPE/FIXED — bug-786's 4.1 gate was insufficient (stale non-zero reply id
63-15--1-1015808slipped pastjobid != 0); strengthened tojobid == jobsubmit.jobid(commit8609976), garbage now rejected loud. Root cause is upstream libxce (xcectx.c handle_sub_replyleavesctx->reply.u.jobidstale on a non-jobid reply;xceapi.c:309copies it when status>=0) — the test client defends itself per Req 7.3 without touching libxce (boundary). (B) UPSTREAM/OUT-OF-SCOPE — cold-restart-all (kill ALL 3 + restart) NON-DETERMINISTICALLY loses a committed job (passed run #2, FAILED run #3: baseline job0-0-...332-0survived all rolling rounds then vanished after full restart; recovery logs show WALtruncate). This is a controller commit-durability race under total-cluster-outage — owned byha-commit-durability, surfaced (correctly) by this spec's new soak step. Sandbox is overlayfs, NOT the task's required "real fsync-able fs"; confirm on real hw + route to ha-commit-durability owner. DO NOT patch this in the test spec.