diff --git a/.kiro/specs/controller-peers-env-fallback/tasks.md b/.kiro/specs/controller-peers-env-fallback/tasks.md index 2aef5aea..179725c9 100644 --- a/.kiro/specs/controller-peers-env-fallback/tasks.md +++ b/.kiro/specs/controller-peers-env-fallback/tasks.md @@ -35,7 +35,7 @@ - Done when, after rebuilding the controller, the controller suite passes and that case shows the line in the log file in all 20 runs - _Requirements: 1.3_ -- [ ] 2.3 Fall back to the environment variable in the controller and name the value's source +- [x] 2.3 Fall back to the environment variable in the controller and name the value's source - Add the controller cases to the regression script first and see them fail: env list used with no `--peers`, an odd candidate count and `--myname` naming a candidate ("HA mode: N controller candidates" in the log), so neither the even-count rule nor the own-name check in 2.4 masks the result; `--peers` winning; neither set (extended standalone line); empty variable (standalone); parse and even-count errors from each source naming that source on stderr; `--help` naming the variable and the opt-out. Each run gets scratch work, certificate and log paths, so it stops at the certificate pre-flight - Use the variable as the peers list only when no `--peers` is given, and remember whether it supplied the list - Name the source in the parse-error and even-count messages; extend the "No --peers flag" line to say the variable was unset or empty; add the variable and the opt-out note to the `--peers` help @@ -134,3 +134,4 @@ - (1.3) Drop the file-wide `shellcheck disable=SC2329,SC2034` once cases call the helpers, so real unused-variable warnings surface. - (2.2) The tlog early-exit race did not reproduce on this machine (0/20 before the fix, 0/200 ad hoc); `case_controller_log_flushed_on_early_exit` is kept as a regression guard, not proof of the fix. - (2.2 → 2.3) That case matches the text "No --peers flag; running as single-node primary"; when 2.3 extends the line (1.8), keep that text intact or update the case's pattern. +- (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. diff --git a/src/controller/main.c b/src/controller/main.c index ed3faae3..e88187b3 100644 --- a/src/controller/main.c +++ b/src/controller/main.c @@ -84,7 +84,11 @@ static struct argp_option options[] = { { "myname", 'n', "STR", 0, "The name of controller" }, { "peers", OPT_PEERS, "PEERS", 0, "HA peer list: comma-separated host:port pairs ordered by priority " - "(e.g. ctrl01:9990,ctrl02:9990). Index 0 = highest priority (primary)." }, + "(e.g. ctrl01:9990,ctrl02:9990). Index 0 = highest priority (primary). " + "When omitted, the XCE_CONTROLLER_PEERS environment variable (same " + "format) is used instead; with that variable unset or empty the " + "controller runs standalone, so unset or empty it for one invocation " + "to opt out (e.g. XCE_CONTROLLER_PEERS= xce_controller ...)." }, { "conf-keep-n", OPT_CONF_KEEP_N, "N", 0, "Versioned config files to retain (default: 2)" }, { "conf-stale-timeout", OPT_CONF_STALE_TIMEOUT, "SECS", 0, @@ -458,12 +462,24 @@ int main(int argc, char* argv[]) return -1; } - if (arguments.peers_str) { - if (xce_parse_peers(arguments.peers_str, &myconf.peers_cfg) != 0) { + /* Req 1.1-1.3, 1.7: an explicit --peers wins; otherwise a non-empty + * XCE_CONTROLLER_PEERS supplies the list (unset or empty = standalone). + * peers_src names where the list came from in every message about it + * (Req 1.4). */ + const char* peers_str = arguments.peers_str; + 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, &myconf.peers_cfg) != 0) { fprintf(stderr, - "xce_controller: --peers parse error: expected " + "xce_controller: %s parse error: expected " "host:port,... got: %s\n", - arguments.peers_str); + peers_src, peers_str); mylog_fini(); return 1; } @@ -471,27 +487,29 @@ int main(int argc, char* argv[]) /* Req 4.1: reject even-N peer counts (cannot form safe majority) */ if (validate_peer_count(myconf.peers_cfg.candidates_count) != 0) { fprintf(stderr, - "xce_controller: --peers: even peer count (%d) is not " + "xce_controller: %s: even peer count (%d) is not " "allowed; Raft quorum requires an odd number of nodes " "(1, 3, 5, ...)\n", - myconf.peers_cfg.candidates_count); + peers_src, myconf.peers_cfg.candidates_count); mylog_fini(); return 1; } strncpy(myconf.peers_cfg.identity, arguments.myname, sizeof(myconf.peers_cfg.identity) - 1); - INFO("HA mode: %d controller candidates, identity=%s", - myconf.peers_cfg.candidates_count, myconf.peers_cfg.identity); + INFO("HA mode: %d controller candidates, identity=%s%s", + myconf.peers_cfg.candidates_count, myconf.peers_cfg.identity, + peers_from_env ? " (from XCE_CONTROLLER_PEERS)" : ""); for (int i = 0; i < myconf.peers_cfg.candidates_count; i++) { if (myconf.peers_cfg.candidates[i].hostname[0] == '\0' || myconf.peers_cfg.candidates[i].port == 0) { - WARN("--peers: candidate %d has empty host or port 0 — " - "check peer string", i); + WARN("%s: candidate %d has empty host or port 0 — " + "check peer string", peers_src, i); } } } else { - INFO("No --peers flag; running as single-node primary"); + INFO("No --peers flag and XCE_CONTROLLER_PEERS unset or empty; " + "running as single-node primary"); } /* Req 4.2: cert pre-flight — fail early with a clear path in the message */ diff --git a/src/tools/verify-controller-peers-env.sh b/src/tools/verify-controller-peers-env.sh index f9c7b702..abeee2f6 100755 --- a/src/tools/verify-controller-peers-env.sh +++ b/src/tools/verify-controller-peers-env.sh @@ -320,7 +320,7 @@ case_controller_log_flushed_on_early_exit() { _show_tail "$d/stderr" return 1 fi - if ! grep -Eq -- 'No --peers flag; running as single-node primary' \ + if ! grep -Eq -- 'No --peers flag and XCE_CONTROLLER_PEERS unset or empty; running as single-node primary' \ "$d/controller.log" 2>/dev/null; then missing=$((missing + 1)) red " run $i: last startup line missing from $d/controller.log" @@ -335,6 +335,127 @@ case_controller_log_flushed_on_early_exit() { add_case case_controller_log_flushed_on_early_exit \ "controller: log flushed before the certificate pre-flight exit ($FLUSH_RUNS runs)" +# ── Task 2.3 (Req 1.1-1.5, 1.7, 1.8): controller resolves its candidate list +# from --peers, else XCE_CONTROLLER_PEERS, else runs standalone. +# +# run_controller -- [controller args...] +# Run the controller with scratch --workdir/--certdir (empty)/--logfile in +# , so it stops at the certificate pre-flight. are env(1) +# arguments (NAME=value or `-u NAME`) placed before the binary. +run_controller() { + local dir="$1"; shift + local envargs=() + while [ $# -gt 0 ] && [ "$1" != -- ]; do + envargs+=("$1"); shift + done + [ $# -gt 0 ] && shift + mkdir -p "$dir/work" "$dir/certs" + run_to_completion "$dir" 10 "${envargs[@]}" "$CONTROLLER_BIN" \ + --workdir "$dir/work" --certdir "$dir/certs" \ + --logfile "$dir/controller.log" "$@" +} + +# The cert pre-flight (or an argument error) must have stopped the run: a +# zero status or a timeout means it went further than these cases expect. +_assert_stopped_early() { + if [ "$RUN_RC" -eq 0 ] || [ "$RUN_RC" -ge 124 ]; then + red " expected the controller to stop early, exit status $RUN_RC" + _show_tail "$1/stderr" + return 1 + fi +} + +ENV3="a=localhost:1,b=localhost:2,c=localhost:3" +STANDALONE_RE='No --peers flag and XCE_CONTROLLER_PEERS unset or empty; running as single-node primary' + +case_controller_env_list() { + local d; d="$(new_scratch controller-env-list)" + run_controller "$d" "XCE_CONTROLLER_PEERS=$ENV3" -- --myname a + _assert_stopped_early "$d" || return 1 + assert_grep "$d/controller.log" 'HA mode: 3 controller candidates, identity=a' || return 1 + assert_no_grep "$d/controller.log" 'No --peers flag' || return 1 +} +add_case case_controller_env_list \ + "controller: XCE_CONTROLLER_PEERS used when --peers is absent (1.2)" + +case_controller_peers_wins() { + local d; d="$(new_scratch controller-peers-wins)" + run_controller "$d" "XCE_CONTROLLER_PEERS=$ENV3" -- \ + --myname solo --peers solo=localhost:7 + _assert_stopped_early "$d" || return 1 + assert_grep "$d/controller.log" 'HA mode: 1 controller candidates, identity=solo' || return 1 + assert_no_grep "$d/controller.log" 'HA mode: 3 controller candidates' || return 1 + assert_no_grep "$d/controller.log" 'from XCE_CONTROLLER_PEERS' || return 1 +} +add_case case_controller_peers_wins \ + "controller: explicit --peers wins over XCE_CONTROLLER_PEERS (1.1)" + +case_controller_neither_set() { + local d; d="$(new_scratch controller-neither)" + run_controller "$d" -u XCE_CONTROLLER_PEERS -- --myname ctrl-none + _assert_stopped_early "$d" || return 1 + assert_grep "$d/controller.log" "$STANDALONE_RE" || return 1 + assert_no_grep "$d/controller.log" 'HA mode' || return 1 +} +add_case case_controller_neither_set \ + "controller: neither --peers nor the variable -> standalone, line names the variable (1.3, 1.8)" + +case_controller_env_empty() { + local d; d="$(new_scratch controller-env-empty)" + run_controller "$d" "XCE_CONTROLLER_PEERS=" -- --myname ctrl-empty + _assert_stopped_early "$d" || return 1 + assert_grep "$d/controller.log" "$STANDALONE_RE" || return 1 + assert_no_grep "$d/controller.log" 'HA mode' || return 1 +} +add_case case_controller_env_empty \ + "controller: empty XCE_CONTROLLER_PEERS behaves as unset (1.7, 1.8)" + +case_controller_parse_error_source() { + local d + d="$(new_scratch controller-parse-env)" + run_controller "$d" "XCE_CONTROLLER_PEERS=a=nocolon" -- --myname a + assert_rc 1 || return 1 + assert_grep "$d/stderr" 'XCE_CONTROLLER_PEERS parse error' "stderr (env value)" || return 1 + + d="$(new_scratch controller-parse-flag)" + run_controller "$d" "XCE_CONTROLLER_PEERS=$ENV3" -- --myname a --peers a=nocolon + assert_rc 1 || return 1 + assert_grep "$d/stderr" -- '--peers parse error' "stderr (--peers value)" || return 1 + assert_no_grep "$d/stderr" 'XCE_CONTROLLER_PEERS' "stderr (--peers value)" || return 1 +} +add_case case_controller_parse_error_source \ + "controller: parse error names --peers or XCE_CONTROLLER_PEERS (1.4)" + +case_controller_even_count_source() { + local d + d="$(new_scratch controller-even-env)" + run_controller "$d" "XCE_CONTROLLER_PEERS=a=localhost:1,b=localhost:2" -- --myname a + assert_rc 1 || return 1 + assert_grep "$d/stderr" 'XCE_CONTROLLER_PEERS: even peer count \(2\)' "stderr (env value)" || return 1 + + d="$(new_scratch controller-even-flag)" + run_controller "$d" "XCE_CONTROLLER_PEERS=$ENV3" -- --myname a \ + --peers a=localhost:1,b=localhost:2 + assert_rc 1 || return 1 + assert_grep "$d/stderr" -- '--peers: even peer count \(2\)' "stderr (--peers value)" || return 1 + assert_no_grep "$d/stderr" 'XCE_CONTROLLER_PEERS' "stderr (--peers value)" || return 1 +} +add_case case_controller_even_count_source \ + "controller: even-count error names --peers or XCE_CONTROLLER_PEERS (1.4)" + +case_controller_help_names_variable() { + local d; d="$(new_scratch controller-help)" + run_to_completion "$d" 10 -u XCE_CONTROLLER_PEERS "$CONTROLLER_BIN" --help + assert_rc 0 || return 1 + # argp wraps help text, so join the lines before matching. + tr '\n' ' ' <"$d/stdout" | tr -s ' ' >"$d/help.txt" + assert_grep "$d/help.txt" 'XCE_CONTROLLER_PEERS' "--help output" || return 1 + assert_grep "$d/help.txt" 'unset or empty' "--help output" || return 1 + assert_grep "$d/help.txt" 'standalone' "--help output" || return 1 +} +add_case case_controller_help_names_variable \ + "controller: --help documents XCE_CONTROLLER_PEERS and the opt-out (1.5, 1.7)" + # ════════════════════════════════════════════════════════════════════════════ # main