feat(controller-peers-env-fallback): fall back to XCE_CONTROLLER_PEERS in the worker (task 3.2)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-10-03 07:07:19 +00:00
parent 55193fcb4b
commit 05a7e6dd0c
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
3 changed files with 203 additions and 17 deletions

View file

@ -57,7 +57,7 @@
- Done when, after rebuilding the scheduler, the scheduler suite passes and every scheduler case in the regression script passes
- _Requirements: 1.1, 1.2, 1.3, 1.4, 1.5, 1.7, 1.8_
- [ ] 3.2 Fall back to the environment variable in the worker
- [x] 3.2 Fall back to the environment variable in the worker
- Add the worker cases to the regression script first and see them fail, using the virtual worker: env list used ("HA cluster config: N candidate(s)" on stdout, since these lines are logged before the worker's log file exists); `--peers` winning; neither set (extended no-peers line); parse error and port-0 validation error from each source naming that source on stderr; `-h` naming the variable, the opt-out, and that `sudo` drops it unless preserved
- Use the variable only when no `--peers` is given; name the source in both error messages; extend the no-peers line; update the usage text
- Done when, after rebuilding the worker, the worker suite passes and every worker case in the regression script passes
@ -137,3 +137,5 @@
- (2.3) The 1.8 wording cannot contain the old "No --peers flag; running ..." substring, so the 2.2 flush case now matches the full new line. Env-sourced "HA mode" lines end " (from XCE_CONTROLLER_PEERS)"; explicit-`--peers` lines and error texts are unchanged apart from naming the source. Reword the script header's "while the case list is empty" comment in 3.x when the shellcheck disable is dropped.
- (3.1 → 3.2) The scheduler's validation message now quotes the value, names the source and hints "(empty host or port 0?)" (the old "missing identity?" was wrong: identity is always set there, and `xce_parse_peers` caps the candidate count itself). Use the same shape for the worker's `xce_cluster_config_validate()` message.
- (3.1 → 5.2) On this machine `test-version-startup-consistency.sh` and `test-dirty-fingerprint-startup-log-unchanged.sh` exit 1 for an environmental reason: their apiserver `go build` fails with "module lookup disabled by GOPROXY=off" (no Go module cache). Their scheduler checks pass. Task 5.2's "both scripts pass" needs a machine with the Go module cache, or the result must be reported as environment-limited with the scheduler checks shown passing.
- (3.2) The worker's early stdout lines are not buffered (visible within 0.3s while it runs). `xcecd_virtual` ignores SIGTERM (pre-existing), so `start_worker` SIGKILLs it after the asserted line; the virtual worker leaves only its scratch log. Its peers parse/validation early returns now free `logdir` (they leaked under ASAN). Out of scope, pre-existing: `-l <dir>` combined with `-h`/`--version` still leaks `logdir`.
- (3.2 → 3.3) The file-wide `shellcheck disable=SC2329` must stay (cases and helpers run via the `"$fn"` registry and `trap`); the SC2034 disable now covers only the `HARNESS_BIN` line — remove it once the harness cases use it.

View file

@ -1,7 +1,8 @@
#!/usr/bin/env bash
# Helpers and the *_BIN variables are used by the cases later tasks add (and
# on_exit via trap), which shellcheck cannot see while the case list is empty.
# shellcheck disable=SC2329,SC2034
# main() runs every case through the registry ("$fn"), an indirect call that
# the linter cannot follow, so it reports each case_* function -- and every
# helper only cases call -- as never invoked (SC2329).
# shellcheck disable=SC2329
# verify-controller-peers-env.sh -- controller-peers-env-fallback regression
# script (.kiro/specs/controller-peers-env-fallback, design.md "Testing
# Strategy -> Scripted Regression Verification").
@ -95,6 +96,8 @@ unset XCE_CONTROLLER_PEERS
CONTROLLER_BIN=""
SCHEDULER_BIN=""
WORKER_BIN=""
# Set by preflight; the harness cases (task 3.3) are its only reader.
# shellcheck disable=SC2034
HARNESS_BIN=""
# component:relative-path:variable
@ -650,6 +653,150 @@ case_scheduler_sim_ignores_variable() {
add_case case_scheduler_sim_ignores_variable \
"scheduler: --sim never reaches the peers block, even with the variable set"
# ── Task 3.2 (Req 1.1-1.5, 1.7, 1.8): the worker (xcecd_virtual, which needs
# no root) resolves its candidate list from --peers, else XCE_CONTROLLER_PEERS,
# else the single-controller path. Its startup line is logged before the log
# file is set up, so it lands on STDOUT. tlog writes it there unbuffered (the
# line is in the redirected stdout file within ~0.3s while the worker keeps
# running), so it is waited for while the worker runs, like the scheduler's.
# The worker ignores SIGTERM for longer than stop_proc's 3s grace, so it is
# SIGKILLed once the line is there. Argument errors exit on their own, before
# the log directory is created; in an ASAN build those early returns must not
# leak (the -l directory string), which the env-value error cases check.
WORKER_ANCHOR_RE='HA cluster config|No --peers flag'
WORKER_STANDALONE_RE='No --peers flag and XCE_CONTROLLER_PEERS unset or empty; using single-controller path \(host=localhost port=9990\), version '
# _split_env_args: shared by start_worker/run_worker -- fills WENV with the
# env(1) arguments before `--` and WARGS with the worker arguments after it.
WENV=()
WARGS=()
_split_env_args() {
WENV=(); WARGS=()
while [ $# -gt 0 ] && [ "$1" != -- ]; do
WENV+=("$1"); shift
done
[ $# -gt 0 ] && shift
WARGS=("$@")
}
# start_worker <dir> <env-args...> -- [worker args...]
# Start xcecd_virtual in the background with -l <dir>/logs, wait for its
# startup line on stdout, then kill it. Fails (with the output tails) when
# the line never appears.
start_worker() {
local dir="$1" pid; shift
_split_env_args "$@"
start_bg "$dir" "${WENV[@]}" "$WORKER_BIN" -l "$dir/logs" \
"${WARGS[@]+"${WARGS[@]}"}"
pid="$LAST_PID"
if ! wait_for_line "$dir/stdout" "$WORKER_ANCHOR_RE" 10 "$pid"; then
kill -KILL "$pid" 2>/dev/null || true
stop_proc "$pid"
red " worker never printed its startup line"
_show_tail "$dir/stdout"
_show_tail "$dir/stderr"
return 1
fi
kill -KILL "$pid" 2>/dev/null || true
stop_proc "$pid"
}
# run_worker <dir> <env-args...> -- [worker args...]
# Run xcecd_virtual to completion (argument errors, -h) with -l <dir>/logs.
run_worker() {
local dir="$1"; shift
_split_env_args "$@"
run_to_completion "$dir" 10 "${WENV[@]}" "$WORKER_BIN" -l "$dir/logs" \
"${WARGS[@]+"${WARGS[@]}"}"
}
case_worker_env_list() {
local d; d="$(new_scratch worker-env-list)"
start_worker "$d" "XCE_CONTROLLER_PEERS=$ENV3" -- -n w-env || return 1
assert_grep "$d/stdout" \
'HA cluster config: 3 candidate\(s\), identity=w-env \(from XCE_CONTROLLER_PEERS\), version ' "stdout" || return 1
assert_no_grep "$d/stdout" 'No --peers flag' "stdout" || return 1
}
add_case case_worker_env_list \
"worker: XCE_CONTROLLER_PEERS used when --peers is absent (1.2)"
case_worker_peers_wins() {
local d; d="$(new_scratch worker-peers-wins)"
start_worker "$d" "XCE_CONTROLLER_PEERS=$ENV3" -- -n w-flag \
--peers solo=localhost:7 || return 1
assert_grep "$d/stdout" \
'HA cluster config: 1 candidate\(s\), identity=w-flag, version ' "stdout" || return 1
assert_no_grep "$d/stdout" '3 candidate|from XCE_CONTROLLER_PEERS' "stdout" || return 1
}
add_case case_worker_peers_wins \
"worker: explicit --peers wins over XCE_CONTROLLER_PEERS (1.1)"
case_worker_neither_set() {
local d; d="$(new_scratch worker-neither)"
start_worker "$d" -u XCE_CONTROLLER_PEERS -- -n w-none || return 1
assert_grep "$d/stdout" "$WORKER_STANDALONE_RE" "stdout" || return 1
assert_no_grep "$d/stdout" 'HA cluster config' "stdout" || return 1
}
add_case case_worker_neither_set \
"worker: neither --peers nor the variable -> single-controller path, line names the variable (1.3, 1.8)"
case_worker_env_empty() {
local d; d="$(new_scratch worker-env-empty)"
start_worker "$d" "XCE_CONTROLLER_PEERS=" -- -n w-empty || return 1
assert_grep "$d/stdout" "$WORKER_STANDALONE_RE" "stdout" || return 1
assert_no_grep "$d/stdout" 'HA cluster config' "stdout" || return 1
}
add_case case_worker_env_empty \
"worker: empty XCE_CONTROLLER_PEERS behaves as unset (1.7, 1.8)"
case_worker_parse_error_source() {
local d
d="$(new_scratch worker-parse-env)"
run_worker "$d" "XCE_CONTROLLER_PEERS=a=nocolon" -- -n w-perr
assert_rc 1 || return 1
assert_grep "$d/stderr" 'xcecd: XCE_CONTROLLER_PEERS parse error' "stderr (env value)" || return 1
assert_no_grep "$d/stderr" 'LeakSanitizer' "stderr (ASAN build: early return must free)" || return 1
d="$(new_scratch worker-parse-flag)"
run_worker "$d" "XCE_CONTROLLER_PEERS=$ENV3" -- -n w-perr --peers a=nocolon
assert_rc 1 || return 1
assert_grep "$d/stderr" 'xcecd: --peers parse error' "stderr (--peers value)" || return 1
assert_no_grep "$d/stderr" 'XCE_CONTROLLER_PEERS' "stderr (--peers value)" || return 1
}
add_case case_worker_parse_error_source \
"worker: parse error names --peers or XCE_CONTROLLER_PEERS (1.4)"
case_worker_validate_error_source() {
local d
d="$(new_scratch worker-port0-env)"
run_worker "$d" "XCE_CONTROLLER_PEERS=a=localhost:0" -- -n w-verr
assert_rc 1 || return 1
assert_grep "$d/stderr" 'xcecd: XCE_CONTROLLER_PEERS: invalid cluster config .* in "a=localhost:0"' "stderr (env value)" || return 1
assert_no_grep "$d/stderr" 'LeakSanitizer' "stderr (ASAN build: early return must free)" || return 1
d="$(new_scratch worker-port0-flag)"
run_worker "$d" "XCE_CONTROLLER_PEERS=$ENV3" -- -n w-verr --peers a=localhost:0
assert_rc 1 || return 1
assert_grep "$d/stderr" 'xcecd: --peers: invalid cluster config .* in "a=localhost:0"' "stderr (--peers value)" || return 1
assert_no_grep "$d/stderr" 'XCE_CONTROLLER_PEERS' "stderr (--peers value)" || return 1
}
add_case case_worker_validate_error_source \
"worker: cluster-config validation error (port 0) names --peers or XCE_CONTROLLER_PEERS (1.4)"
case_worker_help_names_variable() {
local d; d="$(new_scratch worker-help)"
run_to_completion "$d" 10 -u XCE_CONTROLLER_PEERS "$WORKER_BIN" -h
assert_rc 0 || return 1
tr '\n' ' ' <"$d/stdout" | tr -s ' ' >"$d/help.txt"
assert_grep "$d/help.txt" 'XCE_CONTROLLER_PEERS' "-h output" || return 1
assert_grep "$d/help.txt" 'unset or empty' "-h output" || return 1
assert_grep "$d/help.txt" 'single-controller' "-h output" || return 1
assert_grep "$d/help.txt" 'sudo --preserve-env=XCE_CONTROLLER_PEERS' "-h output" || return 1
}
add_case case_worker_help_names_variable \
"worker: -h documents XCE_CONTROLLER_PEERS, the opt-out and that sudo drops it (1.5, 1.7)"
# ════════════════════════════════════════════════════════════════════════════
# main

View file

@ -346,7 +346,7 @@ int main(int argc, char* argv[])
* (dirty-content-fingerprint task 3.2). */
static char version_display_buf[128];
xce_cluster_config_t cluster_cfg = {0};
int use_ha = 0; /* 1 when --peers given */
int use_ha = 0; /* 1 with a candidate list */
signal(SIGPIPE, SIG_IGN);
@ -443,7 +443,22 @@ int main(int argc, char* argv[])
default:
printf("Usage: %s [-n name] [-l log_dir] [-v] "
"[--peers ctrl-1=host:port,...] "
"[-h help]\n",
"[-h help]\n"
" --peers LIST HA controller candidate list: "
"[name=]host:port,...\n"
" When omitted, the XCE_CONTROLLER_PEERS "
"environment variable (same\n"
" format) is used instead; with that "
"variable unset or empty the\n"
" worker takes the single-controller path "
"(localhost:9990), so\n"
" unset or empty it for one invocation to "
"opt out\n"
" (e.g. XCE_CONTROLLER_PEERS= xcecd ...).\n"
" sudo drops XCE_CONTROLLER_PEERS (default "
"env_reset) unless it is\n"
" preserved, e.g. "
"sudo --preserve-env=XCE_CONTROLLER_PEERS xcecd ...\n",
argv[0]);
return 0;
}
@ -467,39 +482,60 @@ int main(int argc, char* argv[])
logdir = strdup("/tmp");
}
/* P2.5-5: populate and validate xce_cluster_config_t when --peers given */
/* P2.5-5: populate and validate xce_cluster_config_t when a candidate
* list is given. controller-peers-env-fallback Req 1.1-1.3, 1.7: an
* explicit --peers wins; otherwise a non-empty XCE_CONTROLLER_PEERS
* supplies the list (unset or empty = single-controller path).
* peers_src names where the list came from in every message about it
* (Req 1.4). */
int peers_from_env = 0;
if (peers_str == NULL) {
peers_str = xce_controller_peers_env();
peers_from_env = (peers_str != NULL);
}
const char* peers_src = peers_from_env ? "XCE_CONTROLLER_PEERS" : "--peers";
if (peers_str) {
if (xce_parse_peers(peers_str, &cluster_cfg) != 0) {
fprintf(stderr,
"xcecd: --peers parse error: expected "
"xcecd: %s parse error: expected "
"\"[name=]host:port,...\" got \"%s\"\n",
peers_str);
peers_src, peers_str);
FREEUP(logdir);
return 1;
}
strncpy(cluster_cfg.identity, config.virutal_name,
sizeof(cluster_cfg.identity) - 1);
/* identity comes from -n, which can be given as an empty string;
* xce_parse_peers() caps the candidate count itself, so an empty
* host, a port of 0 or an empty -n name is what validation rejects. */
if (xce_cluster_config_validate(&cluster_cfg) != 0) {
fprintf(stderr,
"xcecd: invalid cluster config (missing identity?). "
"Use -n.\n");
"xcecd: %s: invalid cluster config (empty host or port "
"0, or empty -n name?) in \"%s\"\n",
peers_src, peers_str);
FREEUP(logdir);
return 1;
}
if (cluster_cfg.candidates_count > N_MAX_MASTERS) {
fprintf(stderr,
"xcecd: WARN: --peers has %d candidates; "
"xcecd: WARN: %s has %d candidates; "
"only <=%d validated for HA\n",
cluster_cfg.candidates_count, N_MAX_MASTERS);
peers_src, cluster_cfg.candidates_count, N_MAX_MASTERS);
}
/* The version stays last on the line, as on the scheduler's. */
XCECD_LOG(XCE_LOG_INFO,
"HA cluster config: %d candidate(s), identity=%s, "
"HA cluster config: %d candidate(s), identity=%s%s, "
"version %s",
cluster_cfg.candidates_count, cluster_cfg.identity,
peers_from_env ? " (from XCE_CONTROLLER_PEERS)" : "",
version_buf);
use_ha = 1;
} else {
XCECD_LOG(XCE_LOG_INFO,
"No --peers flag; using single-controller path "
"(host=%s port=%d), version %s",
"No --peers flag and XCE_CONTROLLER_PEERS unset or empty; "
"using single-controller path (host=%s port=%d), "
"version %s",
config.remote_host, config.remote_port, version_buf);
}
/* P2.2 fix (component-failover-adoption, task 2.2, Req 1.4): assign
@ -509,7 +545,8 @@ int main(int argc, char* argv[])
* this was assigned directly onto cc_ctx AFTER xcecd_start() returned,
* which was already too late -- the first join had already run against
* an empty (zero-initialized) cluster_cfg, silently ignoring --peers on
* startup. When no --peers was given, cluster_cfg stays {0}
* startup. With no candidate list (no --peers, no XCE_CONTROLLER_PEERS),
* cluster_cfg stays {0}
* (candidates_count == 0), which is exactly the "no HA list configured"
* case the synthesize-single-candidate path already handles. */
config.cluster_cfg = cluster_cfg;