diff --git a/.kiro/specs/controller-peers-env-fallback/tasks.md b/.kiro/specs/controller-peers-env-fallback/tasks.md index 3ded7878..2b3fa72e 100644 --- a/.kiro/specs/controller-peers-env-fallback/tasks.md +++ b/.kiro/specs/controller-peers-env-fallback/tasks.md @@ -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 ` 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. diff --git a/src/tools/verify-controller-peers-env.sh b/src/tools/verify-controller-peers-env.sh index 9c437832..502e1898 100755 --- a/src/tools/verify-controller-peers-env.sh +++ b/src/tools/verify-controller-peers-env.sh @@ -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 -- [worker args...] +# Start xcecd_virtual in the background with -l /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 -- [worker args...] +# Run xcecd_virtual to completion (argument errors, -h) with -l /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 diff --git a/src/worker/src/main.c b/src/worker/src/main.c index bd9919e6..420ecfc5 100644 --- a/src/worker/src/main.c +++ b/src/worker/src/main.c @@ -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;