docs(controller-peers-env-fallback): design review fixes and requirements revision
Design review (NO-GO -> GO) and iterated requirements review until clean: - Controller refuses to start (non-zero exit, stderr, before any network I/O) when an env-supplied XCE_CONTROLLER_PEERS lacks its own name (1.6); new libxce xce_peers_find_identity() replaces both identity loops in controller/cluster.c so the check and my_priority/listen port agree. - deploy-ha.sh's generated scripts/xceadmin wrapper exports XCE_CONTROLLER_PEERS so an ambient value cannot redirect it (Req 4). - Opt-out is unsetting/emptying the variable (1.7); "No --peers" log lines say it was unset or empty (1.8); docs cover sudo/systemd not inheriting it (3.1). - Requirement 1 defines explicit candidate list / resolution point / own name so harness (set masters) is covered precisely; error messages name the value's source incl. validation failures (1.4). - xceadmin: name= stripping and a clean malformed-entry error apply to every --controller source; errors name the env var source (2.2, 2.8). - Test plan fixed against real code (stderr vs stdout, log texts, xcecd_virtual, --norc, free ports, failover order test). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
5c4d101b15
commit
bd1f5175a1
4 changed files with 417 additions and 153 deletions
|
|
@ -2,21 +2,22 @@
|
|||
|
||||
## Overview
|
||||
|
||||
**Purpose**: This feature lets an operator set one environment variable, `XCE_CONTROLLER_PEERS`, once per session or deployment, instead of retyping `--peers` (controller, scheduler, worker, harness/xce_test) or `--controller` (xceadmin) on every invocation. It changes nothing about what any of these components do once their connection target is resolved — only where that target's default value comes from when the operator did not name one explicitly.
|
||||
**Purpose**: This feature lets an operator set one environment variable, `XCE_CONTROLLER_PEERS`, once per session or deployment, instead of retyping `--peers` (controller, scheduler, worker), `set masters` (harness/xce_test) or `--controller` (xceadmin) on every invocation. It changes nothing about what any of these components do once their connection target is resolved — only where that target's default value comes from when the operator did not name one explicitly.
|
||||
|
||||
**Users**: Operators running local/interactive HPC cluster tools, and deploy/test scripts (e.g. HA soak setups) that currently either hardcode `--peers`/`--controller` into every generated command or accept the single-node/no-controller default they didn't actually want.
|
||||
|
||||
**Impact**: Five independently-built binaries each gain one additional, lowest-priority source for their existing connection-target resolution. No existing explicit-flag behavior, no existing "omitted, no env var" behavior, and no existing multi-candidate failover behavior changes.
|
||||
**Impact**: Five independently-built binaries each gain one additional, lowest-priority source for their existing connection-target resolution. Explicit `--peers`/`set masters` behavior, "omitted, no env var" behavior (apart from a log line's wording, 1.8), and xceadmin's multi-candidate failover order are unchanged; xceadmin's parsing of any `--controller` value, explicit or not, gains `name=` stripping and a clean malformed-entry error (2.2, 2.8). Two guards come with the new source: the controller refuses to start, early and with a message naming the variable, when an env-supplied list does not contain its own name (that list is Raft membership, not just a target list), and `deploy-ha.sh`'s generated `scripts/xceadmin` wrapper exports the new variable so an ambient value cannot redirect it.
|
||||
|
||||
### Goals
|
||||
- One new environment variable, consulted only when the corresponding CLI option is not given, across controller, scheduler, worker, harness/xce_test (via `--peers`) and xceadmin (via `--controller`).
|
||||
- Zero behavior change when the CLI option is given, and zero behavior change when neither the option nor the variable is set.
|
||||
- Reuse xceadmin's existing multi-candidate failover (`resolve_controllers`/`follow_redirects_any`, bug-1821) and its existing `XCEADMIN_CONTROLLER` fallback unmodified.
|
||||
- One new environment variable, used only when no explicit value is given, across controller, scheduler and worker (via `--peers`), harness/xce_test (via `set masters`) and xceadmin (via `--controller`).
|
||||
- An explicit value (`--peers`, `set masters`, `--controller`) always wins over the variable. Explicit `--peers`/`set masters` behave exactly as today; an explicit `--controller` gains only `name=` stripping and a clean malformed-entry error (2.2, 2.8).
|
||||
- Zero behavior change when neither an explicit value nor the variable is set (apart from the startup log line's wording, 1.8).
|
||||
- Reuse xceadmin's existing multi-candidate failover (`follow_redirects_any`, bug-1821, unmodified) and its existing `XCEADMIN_CONTROLLER` fallback; `resolve_controllers()` is extended only as 2.2/2.8 require.
|
||||
|
||||
### Non-Goals
|
||||
- apiserver (YAML-config-based) and xcecli's `--url` (a different target) — untouched.
|
||||
- Any change to `src/tools/deploy-ha.sh`'s generated scripts' own behavior.
|
||||
- Any change to what "no `--peers`/`--controller` and no env var" means today for any component.
|
||||
- Any change to `src/tools/deploy-ha.sh`'s generated daemon scripts' behavior (the `scripts/xceadmin` wrapper's added export, Requirement 4, is the one exception).
|
||||
- Any change to what "no explicit value and no env var" does today for any component (only the log wording changes, 1.8).
|
||||
- A TSAN/new-test-infrastructure investment for `main()`-level argument parsing where none exists today (see Testing Strategy).
|
||||
|
||||
## Boundary Commitments
|
||||
|
|
@ -24,6 +25,9 @@
|
|||
### This Spec Owns
|
||||
- A single shared C primitive (`xce_controller_peers_env()`, `libxce`) that reads `XCE_CONTROLLER_PEERS`, treating an unset or empty value as absent.
|
||||
- The one-line integration of that primitive into controller, scheduler, and worker's existing `--peers`-gating `if`, and into harness's settings-table default computation at startup.
|
||||
- A second shared C primitive, `xce_peers_find_identity()` (`libxce`), holding the one rule for "which candidate is me" (candidate `name` if set, else `hostname`, compared with the controller's name). The controller's two existing copies of that loop — the `my_priority` assignment in `cluster_init_phase1()` (`src/controller/cluster.c` ~L1784-1791) and the listen-port lookup in `cluster_open_listener()` (~L1890-1906) — are switched to call it, each keeping its own existing no-match handling, so the new startup check and both existing lookups can never disagree.
|
||||
- The controller's startup identity check for an env-supplied candidate list (1.6).
|
||||
- One added line in `deploy-ha.sh`'s generated `scripts/xceadmin` wrapper: `export XCE_CONTROLLER_PEERS="$PEERS_STR"` (Requirement 4).
|
||||
- Extending xceadmin's existing `resolve_controllers()` to strip an optional `name=` prefix per candidate entry.
|
||||
- Adding `"XCE_CONTROLLER_PEERS"` ahead of the existing `"XCEADMIN_CONTROLLER"` in each `--controller` option's `envvar=` list.
|
||||
- Help-text and documentation updates naming the new variable (CLI `--peers`/`--controller` help text, `build-deployment.md`, `xce-deploy-ha`/`xce-test-multihost` skill docs).
|
||||
|
|
@ -32,27 +36,58 @@
|
|||
### Out of Boundary
|
||||
- apiserver's YAML-based peer configuration.
|
||||
- xcecli's `--url` option.
|
||||
- `deploy-ha.sh`'s generated scripts' own invocations (documentation may *mention* the variable; the generated scripts keep passing `--peers`/`--controller` explicitly).
|
||||
- Any change to `xce_parse_peers()`'s or `resolve_controllers()`/`follow_redirects_any()`'s existing parsing/failover semantics beyond the one `name=`-stripping addition.
|
||||
- Refactoring any of the four C `main()` functions to become unit-testable in general — only the new, genuinely independent primitive is unit-tested (see Testing Strategy).
|
||||
- `deploy-ha.sh`'s generated daemon scripts' own invocations (they keep passing `--peers` explicitly). The `scripts/xceadmin` wrapper's one added export (Requirement 4) is the only `deploy-ha.sh` change.
|
||||
- Changing what the controller does when an *explicit* `--peers` list lacks its name. Today `cluster_open_listener()` refuses (`ERROR` in the log, `myapp_run()` returns before any network I/O), and `main()` ignores `myapp_run()`'s result, so the process exits with status 0. That behavior is kept; see Revalidation Triggers.
|
||||
- Any change to `xce_parse_peers()`'s or `resolve_controllers()`/`follow_redirects_any()`'s existing parsing/failover semantics beyond `resolve_controllers()`'s two additions: `name=` stripping (2.2) and a clean error for a malformed entry (2.8). Both apply to an explicit `--controller` value too.
|
||||
- Refactoring any of the four C `main()` functions to become unit-testable in general — only the two new, genuinely independent primitives are unit-tested (see Testing Strategy).
|
||||
|
||||
### Allowed Dependencies
|
||||
- All four C targets already link `libxce` (verified per-component `meson.build`) — the shared primitive may live there with no new build-graph edge.
|
||||
- All four C targets already link `libxce` (verified per-component `meson.build`) — the shared primitives may live there with no new build-graph edge.
|
||||
- xceadmin's existing `resolve_controllers()`/`follow_redirects_any()` (`discovery.py`) and its existing `--controller` click options (`cli.py`) — reused and extended in place, not duplicated.
|
||||
|
||||
### Project Invariant Check
|
||||
This feature's controller/scheduler changes touch only pre-startup argument
|
||||
resolution (`main.c`, before `sched_init()`/`myapp_run()`/any daemon thread,
|
||||
event loop, or Raft/HA state machine is created) — it does not intersect
|
||||
`xce-audit-invariants`' controller dimensions (async I/O thread-role
|
||||
placement, fail-loud/never-self-exit process lifecycle, consensus/
|
||||
replication safety) or its scheduler dimension (driver/simulation
|
||||
state-equivalence). No audit re-check is required beyond this design.
|
||||
Walked against `xce-audit-invariants`' controller and scheduler dimensions:
|
||||
|
||||
- **Consensus/replication safety (controller) — applies.** For the
|
||||
controller, `--peers` is the Raft membership, and `my_priority` doubles
|
||||
as `self_idx` (`ha.c` ~L633, ~L2964). If the controller's name matches
|
||||
no candidate, `cluster_init_phase1()` (`cluster.c` ~L1783) falls back to
|
||||
the last index, which would give the node another peer's `self_idx`.
|
||||
That is already contained today: `cluster_open_listener()` (~L1902)
|
||||
refuses the same no-match case in `myapp_run()` step 3, before the
|
||||
listener, iomgr or any election starts, so no Raft traffic is sent
|
||||
under the wrong identity. An inherited value makes the case far more
|
||||
likely, though (a controller started without `--myname`, so its name is
|
||||
the hostname, or one meant to run standalone), and the existing refusal
|
||||
then blames "--peers", logs only to the log file, and exits with status
|
||||
0. Guard (1.6): when the list came from `XCE_CONTROLLER_PEERS` and
|
||||
`xce_peers_find_identity()` finds no match, `main.c` writes the error to
|
||||
stderr naming the variable and the remedies and returns 1, before
|
||||
`myapp_run()`. Because all three lookups now share one function, the
|
||||
new check cannot pass where the listener check would refuse, or the
|
||||
reverse. A bare single `host:port` value follows the same rule: its key
|
||||
is the hostname, so a controller joins a one-node HA cluster only when
|
||||
its name is literally that hostname; otherwise it refuses, rather than
|
||||
silently changing from standalone to HA mode.
|
||||
- **Fail-loud, never self-exit (controller) — not affected.** The new exit
|
||||
happens in `main()` during argument validation, the same place and shape
|
||||
as the existing `--peers` parse error and even-peer-count rejection;
|
||||
no daemon thread, event loop or HA state exists yet.
|
||||
- **Async I/O thread roles (controller) — not affected.** All reads of the
|
||||
environment happen in `main()` before any thread starts; the two
|
||||
`cluster.c` refactors are pure function calls in existing pre-election
|
||||
startup code (`cluster_init_phase1()`, `cluster_open_listener()`), with
|
||||
no blocking added.
|
||||
- **Driver/simulation state-equivalence (scheduler) — not affected.**
|
||||
`--sim`/`--interactive` return before the `--peers` block, so the
|
||||
variable can never reach replay mode.
|
||||
|
||||
### Revalidation Triggers
|
||||
- `xce_peers_find_identity()` is now the only "which candidate is me" rule for the controller (the 1.6 check, `my_priority`, listen port). Any future identity rule (e.g. matching on host:port) changes there, never in a re-added inline loop.
|
||||
- If the explicit-`--peers` no-match case is ever moved to the same early `main()` check with a non-zero exit (a reasonable follow-up, out of scope here), drop the env-only condition on the 1.6 check instead of adding a second check.
|
||||
- Any change to `xce_parse_peers()`'s accepted value format (e.g. a new candidate syntax) must be mirrored in `resolve_controllers()`'s parsing if the two are to stay consistent for `XCE_CONTROLLER_PEERS` values shared between C daemons and xceadmin.
|
||||
- Any future change to harness's `settings[]` initialization order (e.g. rc-file loading moved earlier than today) must preserve "the env-var-derived default is computed before any explicit `set masters` can run."
|
||||
- If `deploy-ha.sh` is ever updated to adopt `XCE_CONTROLLER_PEERS` (explicitly deferred here), its generated scripts' current behavior baseline (documented in this design's Out of Boundary) is the thing to re-check against.
|
||||
- If `deploy-ha.sh`'s generated daemon scripts are ever updated to rely on `XCE_CONTROLLER_PEERS` instead of an explicit `--peers` (explicitly deferred here), re-check them against the baseline in this design's Out of Boundary, and re-check the 1.6 identity rule for each generated controller script.
|
||||
|
||||
## Architecture
|
||||
|
||||
|
|
@ -62,8 +97,8 @@ Each of the five components already resolves its own connection target independe
|
|||
|
||||
| Component | Mechanism | Where "omitted" is decided | Existing fallback today |
|
||||
|---|---|---|---|
|
||||
| controller | `argp`, `OPT_PEERS` → `arguments.peers_str` | `if (arguments.peers_str)` (`main.c` ~L455) | Single-node primary (bug-1457) |
|
||||
| scheduler | `argp`, `'P'`/`--peers` → `arguments.peers` | `if (arguments.peers)` (`main.c` ~L403); **`--sim` returns before this point is reached** | "single-controller path" (logged) |
|
||||
| controller | `argp`, `OPT_PEERS` → `arguments.peers_str` | `if (arguments.peers_str)` (`main.c` ~L455) | Single-node primary (bug-1457). With a list, own entry found by name twice in `cluster.c` (~L1784 `my_priority`, ~L1890 listen port); no match is refused in `cluster_open_listener()` before any network I/O, exit status 0 |
|
||||
| scheduler | `argp`, `'P'`/`--peers` → `arguments.peers` | `if (arguments.peers)` (`main.c` ~L411); **`--sim` returns before this point is reached** | "single-controller path" (logged) |
|
||||
| worker | `getopt_long`, `OPT_PEERS` → `peers_str` | `if (peers_str)` (`main.c` ~L470) | Single-controller path via `config.remote_host`/`config.remote_port` (logged; same shape as controller/scheduler's own single-controller fallback, not an error path) |
|
||||
| harness/xce_test | `settings[]` table, key `"masters"`, no CLI flag at all | `get_setting()`'s `value ?: defval` (`test.c` ~L991) | `defval = "localhost:9990"` |
|
||||
| xceadmin | click `--controller` option, `envvar="XCEADMIN_CONTROLLER"` | click's own parameter resolution | `_DEFAULT_CONTROLLER = "localhost:9991"` |
|
||||
|
|
@ -71,17 +106,17 @@ Each of the five components already resolves its own connection target independe
|
|||
All three C daemons that gate on a `--peers` argument already treat an invalid value as a hard startup error (`xce_parse_peers() != 0` → message + `return 1`), never a silent fallback — this feature preserves that for both an invalid explicit flag and an invalid `XCE_CONTROLLER_PEERS` value.
|
||||
|
||||
### Architecture Integration
|
||||
- **Selected pattern**: one shared, stateless C primitive consulted independently by four call sites, plus one Python parsing extension consulted by click's own (unmodified) resolution order — no new service, process, or inter-component call is introduced.
|
||||
- **Selected pattern**: two shared, stateless C primitives (an env reader used by four call sites, and an identity lookup used by three controller sites), plus one Python parsing extension consulted by click's own (unmodified) resolution order — no new service, process, or inter-component call is introduced.
|
||||
- **Domain/feature boundaries**: each component keeps deciding its own resolved value; the new variable is only ever a *source*, never a coordinator.
|
||||
- **Existing patterns preserved**: `xce_parse_peers()`'s `name=`-prefix convention; xceadmin's `resolve_controllers`/`follow_redirects_any` failover; each component's existing hard-error-on-invalid-value behavior.
|
||||
- **New components rationale**: exactly one new C function, because harness's settings-table shape cannot reuse a "given `cli_value`, return resolved" signature the other three could share (see `research.md`, Design Decisions).
|
||||
- **New components rationale**: two new C functions. `xce_controller_peers_env()` is a bare env reader because harness's settings-table shape cannot reuse a "given `cli_value`, return resolved" signature the other three could share (see `research.md`, Design Decisions). `xce_peers_find_identity()` exists so the controller's new startup check and its two existing lookups (`my_priority`, listen port) share one matching rule.
|
||||
- **Steering compliance**: no new build dependency; C standard/build system (Meson) and Python (click) usage unchanged.
|
||||
|
||||
### Technology Stack
|
||||
|
||||
| Layer | Choice / Version | Role in Feature | Notes |
|
||||
|-------|------------------|-----------------|-------|
|
||||
| C CLI / libxce | gnu17, existing `libxce` static lib | New `xce_controller_peers_env()` primitive; consumed at 4 existing call sites | No new dependency |
|
||||
| C CLI / libxce | gnu17, existing `libxce` static lib | New `xce_controller_peers_env()` (4 call sites) and `xce_peers_find_identity()` (controller `main.c` + two `cluster.c` sites) | No new dependency |
|
||||
| Python CLI | click (already a `xceadmin` dependency) | Native multi-`envvar` list resolution (no new click usage pattern) | Confirmed via direct read of the installed click version's `resolve_envvar_value()` |
|
||||
|
||||
## File Structure Plan
|
||||
|
|
@ -89,22 +124,25 @@ All three C daemons that gate on a `--peers` argument already treat an invalid v
|
|||
One new file (the regression script below); every other change is to an existing file — no new directories.
|
||||
|
||||
### New Files
|
||||
- `src/tools/verify-controller-peers-env.sh` — repeatable regression script (see Testing Strategy → Scripted Regression Verification) driving the real built binaries of all 4 C components plus harness/`xce_test`, asserting on startup log/settings-dump output only (no live cluster required).
|
||||
- `src/tools/verify-controller-peers-env.sh` — repeatable regression script (see Testing Strategy → Scripted Regression Verification) driving the real built binaries of controller, scheduler, worker and harness/`xce_test`, plus `deploy-ha.sh`'s generated xceadmin wrapper, asserting on startup log/settings-dump/printed-environment output only (no live cluster required).
|
||||
|
||||
### Modified Files
|
||||
- `src/include/xce/cluster.h` — declare `const char *xce_controller_peers_env(void);` alongside the existing `xce_parse_peers()` declaration.
|
||||
- `src/lib/cluster.c` — implement `xce_controller_peers_env()`: read `getenv("XCE_CONTROLLER_PEERS")`, return it if non-empty, else `NULL`.
|
||||
- `src/lib/testsuite/test_cluster.c` — new unit tests for `xce_controller_peers_env()` (unset, empty, non-empty).
|
||||
- `src/controller/main.c` — at the existing `if (arguments.peers_str)` site (~L455), resolve `arguments.peers_str` against `xce_controller_peers_env()` before the check; on a parse failure, name the source (flag vs. environment variable) in the existing error message; add `XCE_CONTROLLER_PEERS` to the `--peers` option's help text (~L85-87).
|
||||
- `src/scheduler/src/main.c` — identical shape at the existing `if (arguments.peers)` site (~L403); help text update (~L84). No change anywhere in the `--sim`/`--interactive` path, which already returns before this point.
|
||||
- `src/worker/src/main.c` — identical shape at the existing `if (peers_str)` site (~L470); update the `-h`/usage `printf` (~L443-446) to mention `XCE_CONTROLLER_PEERS`.
|
||||
- `src/harness/test.c` — at startup, before the REPL loop and before rc-file loading, compute the `"masters"` entry's `defval` from `xce_controller_peers_env()` if set, else leave it at today's literal `"localhost:9990"`; extend the `USAGE`/`set`-command help text (~L409-421) to mention the new variable.
|
||||
- `src/xceadmin/src/xceadmin/discovery.py` — extend `resolve_controllers()`'s per-entry parsing to strip an optional `name=` prefix (mirrors `xce_parse_peers()`) before its existing `rsplit(":", 1)`.
|
||||
- `src/xceadmin/src/xceadmin/cli.py` — change each of the 4 `--controller` options' `envvar="XCEADMIN_CONTROLLER"` to `envvar=["XCE_CONTROLLER_PEERS", "XCEADMIN_CONTROLLER"]`; update each option's help string.
|
||||
- `src/xceadmin/tests/test_discovery.py` — new unit tests for `resolve_controllers()`'s `name=`-stripping.
|
||||
- `src/xceadmin/tests/test_cli.py` — new unit tests for the new env-var precedence (mirrors the existing `test_controller_option_falls_back_to_xceadmin_controller_env_var`/`test_controller_flag_overrides_xceadmin_controller_env_var` tests).
|
||||
- `.kiro/steering/build-deployment.md` — document `XCE_CONTROLLER_PEERS`: components, value format, precedence.
|
||||
- `.claude/skills/xce-deploy-ha/SKILL.md` — informational note only (no generated-script behavior change).
|
||||
- `src/include/xce/cluster.h` — declare `const char *xce_controller_peers_env(void);` and `int xce_peers_find_identity(const xce_cluster_config_t *cfg, const char *name);` alongside the existing `xce_parse_peers()` declaration.
|
||||
- `src/lib/cluster.c` — implement `xce_controller_peers_env()`: read `getenv("XCE_CONTROLLER_PEERS")`, return it if non-empty, else `NULL`. Implement `xce_peers_find_identity()` with the matching rule moved verbatim from `src/controller/cluster.c` (identical at ~L1784-1791 and ~L1890-1900).
|
||||
- `src/lib/testsuite/test_cluster.c` — new unit tests for `xce_controller_peers_env()` (unset, empty, non-empty) and `xce_peers_find_identity()` (named match, bare-hostname match, named entry not matched by its hostname, no match → -1, first of duplicate names wins, NULL name → -1, empty name matching an unnamed empty-hostname entry exactly as today).
|
||||
- `src/controller/main.c` — at the existing `if (arguments.peers_str)` site (~L455), resolve `arguments.peers_str` against `xce_controller_peers_env()` before the check, remembering in a local `peers_from_env` whether the env var supplied it; on a parse or even-count failure, name the source (flag vs. environment variable) in the existing error message; after parsing, when `peers_from_env` is true and `xce_peers_find_identity(&myconf.peers_cfg, arguments.myname) < 0`, print the 1.6 error and `return 1`; extend the "No --peers flag" `INFO` (~L486) per 1.8; add `XCE_CONTROLLER_PEERS` and the "unset it to run standalone" note (1.7) to the `--peers` option's help text (~L85-87).
|
||||
- `src/controller/cluster.c` — replace both inline identity loops with `xce_peers_find_identity(pcfg, c->myconf->name)`: in `cluster_init_phase1()` (~L1784-1791), `my_priority = idx >= 0 ? idx : pcfg->candidates_count - 1;`; in `cluster_open_listener()` (~L1890-1906), `matched = idx >= 0; my_port = matched ? pcfg->candidates[idx].port : 0;` followed by the existing refusal and default-port code. Same result for every input.
|
||||
- `src/tools/deploy-ha.sh` — in the generated `scripts/xceadmin` wrapper heredoc (~L689-694), add `export XCE_CONTROLLER_PEERS="$PEERS_STR"` next to the existing `XCEADMIN_CONTROLLER` export, and extend the comment above it to say why (Requirement 4).
|
||||
- `src/scheduler/src/main.c` — identical shape at the existing `if (arguments.peers)` site (~L411), without the identity check; name the source in both the parse-error and the `xce_cluster_config_validate()` failure messages (~L412-424, 1.4); "No --peers flag" `INFO` (~L435) per 1.8; help text update with 1.7's note (~L88). No change anywhere in the `--sim`/`--interactive` path, which already returns before this point.
|
||||
- `src/worker/src/main.c` — identical shape at the existing `if (peers_str)` site (~L470), without the identity check; name the source in both the parse-error and the `xce_cluster_config_validate()` failure messages (~L472-485, 1.4); "No --peers flag" log line (~L501) per 1.8; update the `-h`/usage `printf` (~L443-446) to mention `XCE_CONTROLLER_PEERS`, 1.7's note, and that `sudo` drops it unless preserved.
|
||||
- `src/harness/test.c` — at startup, before the REPL loop and before rc-file loading, compute the `"masters"` entry's `defval` from `xce_controller_peers_env()` if set, else leave it at today's literal `"localhost:9990"`; extend the `USAGE`/`set`-command help text (~L403-415) to mention the new variable and 1.7's note.
|
||||
- `src/xceadmin/src/xceadmin/discovery.py` — extend `resolve_controllers()`'s per-entry parsing to strip an optional `name=` prefix (mirrors `xce_parse_peers()`) before its existing `rsplit(":", 1)`; turn today's uncaught `ValueError` from `int(port_str)` (~L78) into a new `ControllerValueError` naming the bad entry (2.8).
|
||||
- `src/xceadmin/src/xcecommon/errors.py` — add `ControllerValueError(XceAdminError)` next to the existing controller-reachability errors (`ConnectionTimeoutError`, `TooManyRedirectsError`, `NoPrimaryError`).
|
||||
- `src/xceadmin/src/xceadmin/cli.py` — change `_CONTROLLER_ENV_VAR` (~L109), which all 4 `--controller` options use, to the list `["XCE_CONTROLLER_PEERS", "XCEADMIN_CONTROLLER"]`; rewrite each option's help string, which today interpolates `_CONTROLLER_ENV_VAR` with an f-string and would print a raw Python list once it becomes one, and the precedence comment above it (~L102-108). Add a helper `_controller_source_note()` that returns `" (from $NAME)"` when `click.get_current_context().get_parameter_source("controller")` is `ParameterSource.ENVIRONMENT` (NAME = the first non-empty variable in the list, the same rule click applies) and `""` otherwise; append it to the two existing `cannot connect to {controller}` messages (~L172, ~L265), and, in the existing `except XceAdminError` handlers of the `--controller`-accepting commands, to the message when the error is one of `_CONTROLLER_ERRORS = (ControllerValueError, ConnectionTimeoutError, TooManyRedirectsError, NoPrimaryError)` (2.8). Errors unrelated to the controller value (e.g. `ClusterBinFileError`) keep their text unchanged.
|
||||
- `src/xceadmin/tests/test_discovery.py` — new unit tests for `resolve_controllers()`'s `name=`-stripping and its `ControllerValueError` on a non-numeric port.
|
||||
- `src/xceadmin/tests/test_cli.py` — new unit tests for the new env-var precedence (mirrors the existing `test_controller_option_falls_back_to_xceadmin_controller_env_var`/`test_controller_flag_overrides_xceadmin_controller_env_var` tests) and for 2.8's messages.
|
||||
- `.kiro/steering/build-deployment.md` — document `XCE_CONTROLLER_PEERS`: components, value format, precedence, the "unset it to run standalone" opt-out, the controller's own-name rule (1.6), and that `sudo` (default `env_reset`) and systemd services do not inherit it (`sudo --preserve-env=XCE_CONTROLLER_PEERS`, or `Environment=` in the unit).
|
||||
- `.claude/skills/xce-deploy-ha/SKILL.md` — note the variable's availability, that the generated daemon scripts still pass `--peers`, and that `scripts/xceadmin` now also exports `XCE_CONTROLLER_PEERS` (update the existing ~L330 description and the ~L518 troubleshooting row's "check `$TOP/scripts/xceadmin`" hint).
|
||||
- `.claude/skills/xce-test-multihost/SKILL.md` — same note, at its three existing `--peers`-mentioning locations (single-node deploy step, `xcecd_virtual` flag-table note, Prerequisites' generated-script list).
|
||||
|
||||
## System Flows
|
||||
|
|
@ -112,9 +150,13 @@ One new file (the regression script below); every other change is to an existing
|
|||
```mermaid
|
||||
flowchart TD
|
||||
Start[Resolution point reached] --> HasFlag{Explicit flag or\nset command given?}
|
||||
HasFlag -- yes --> UseFlag[Use that value\nnever consult env var]
|
||||
HasFlag -- yes --> UseFlag[Use that value\nenv var not used as list]
|
||||
HasFlag -- no --> HasNewEnv{XCE_CONTROLLER_PEERS\nnon-empty?}
|
||||
HasNewEnv -- yes --> UseNewEnv[Use it\nxceadmin strips name prefix]
|
||||
HasNewEnv -- yes --> IsCtrl{Component is controller?}
|
||||
IsCtrl -- no --> UseNewEnv[Use it\nxceadmin strips name prefix]
|
||||
IsCtrl -- yes --> SelfListed{Own name matches\na candidate?}
|
||||
SelfListed -- yes --> UseNewEnv
|
||||
SelfListed -- no --> Refuse[Refuse to start\nname source and remedies]
|
||||
HasNewEnv -- no --> IsAdmin{Component is xceadmin?}
|
||||
IsAdmin -- yes --> HasLegacyEnv{XCEADMIN_CONTROLLER\nnon-empty?}
|
||||
IsAdmin -- no --> UseTodayDefault[Today's unchanged default\nstandalone / built-in literal]
|
||||
|
|
@ -122,7 +164,7 @@ flowchart TD
|
|||
HasLegacyEnv -- no --> UseTodayDefault
|
||||
```
|
||||
|
||||
Both branches that resolve to a real candidate list (`UseFlag`, `UseNewEnv`, `UseLegacyEnv`) still pass through each component's existing, unchanged validation (`xce_parse_peers()`/`resolve_controllers()`); an invalid value is a hard error either way, distinguished by source only in the error message text (`1.5`).
|
||||
All three branches that resolve to a real candidate list (`UseFlag`, `UseNewEnv`, `UseLegacyEnv`) still pass through each component's existing validation (`xce_parse_peers()` and the controller's odd-count rule, unchanged; `resolve_controllers()`, extended per 2.2/2.8); an invalid value is a hard error either way, distinguished by source only in the error message text (1.4, 2.8).
|
||||
|
||||
This resolution point is reached exactly once, at process startup, for controller/scheduler/worker/xceadmin. For harness/`xce_test` it is reached on *every* connection attempt (`enter_ctx()`), since `set masters ...` can run at any point in an interactive/scripted session — the diagram represents one such evaluation, not a startup-only decision, for that component.
|
||||
|
||||
|
|
@ -130,35 +172,43 @@ This resolution point is reached exactly once, at process startup, for controlle
|
|||
|
||||
| Requirement | Summary | Components | Interfaces | Flows |
|
||||
|-------------|---------|------------|------------|-------|
|
||||
| 1.1 | Recognize `XCE_CONTROLLER_PEERS` | controller/scheduler/worker/harness main.c/test.c | `xce_controller_peers_env()` | Flowchart: `HasNewEnv` |
|
||||
| 1.2 | Explicit `--peers` wins, env var not consulted | controller/scheduler/worker main.c | existing `if (peers)` gate | Flowchart: `HasFlag` = yes |
|
||||
| 1.3 | Env var used when flag omitted | controller/scheduler/worker main.c, harness test.c | `xce_controller_peers_env()` | Flowchart: `UseNewEnv` |
|
||||
| 1.4 | Unset/empty env var ⇒ unchanged today's behavior | all 4 | same call sites | Flowchart: `UseTodayDefault` |
|
||||
| 1.5 | Parse-error message names the source | controller/scheduler/worker main.c, harness test.c | existing error `fprintf`/`enter_ctx` message | n/a |
|
||||
| 1.6 | `--peers` help text documents the variable | controller/scheduler/worker main.c, harness test.c | argp/getopt/settings help strings | n/a |
|
||||
| 1.1 | Explicit candidate list wins, env var not used as candidate list | controller/scheduler/worker main.c; harness test.c (explicit `set masters`, interactive or rc-file) | existing `if (peers)` gate; `get_setting()` value-before-defval | Flowchart: `HasFlag` = yes |
|
||||
| 1.2 | Env var used when no explicit list (harness: at each connection attempt) | controller/scheduler/worker main.c, harness test.c | `xce_controller_peers_env()`; harness `masters` `defval` | Flowchart: `HasNewEnv` → `UseNewEnv` |
|
||||
| 1.3 | Unset/empty env var ⇒ unchanged today's behavior | all 4 | same call sites | Flowchart: `UseTodayDefault` |
|
||||
| 1.4 | Parse/validation error (incl. controller even count) names the source | controller/scheduler/worker main.c, harness test.c | existing error `fprintf`s (parse, controller even-count)/`enter_ctx` message | n/a |
|
||||
| 1.5 | `--peers` (harness: `set`) help text documents the variable | controller/scheduler/worker main.c, harness test.c | argp/getopt/settings help strings | n/a |
|
||||
| 1.6 | Controller refuses an env-supplied list without its own name | controller main.c | `xce_peers_find_identity()` | Flowchart: `SelfListed` = no |
|
||||
| 1.7 | Opt-out is "unset the variable"; help says so | controller/scheduler/worker main.c, harness test.c | help strings; `xce_controller_peers_env()` empty-as-unset | Flowchart: `HasNewEnv` = no |
|
||||
| 1.8 | "No --peers" log line says the variable was unset or empty | controller/scheduler/worker main.c | existing `INFO`/log line text | Flowchart: `UseTodayDefault` |
|
||||
| 2.1 | Derive controller list from env var | xceadmin cli.py, discovery.py | click `envvar=` list | Flowchart: `UseNewEnv` |
|
||||
| 2.2 | Strip `name=` prefix | xceadmin discovery.py | `resolve_controllers()` | n/a |
|
||||
| 2.2 | Strip `name=` prefix, any source incl. explicit `--controller` | xceadmin discovery.py | `resolve_controllers()` | n/a |
|
||||
| 2.3 | Try each candidate in order | xceadmin discovery.py | `resolve_controllers`/`follow_redirects_any` (unmodified) | n/a |
|
||||
| 2.4 | Explicit `--controller` wins | xceadmin cli.py | click parameter resolution (unmodified) | Flowchart: `HasFlag` = yes |
|
||||
| 2.5 | Legacy `XCEADMIN_CONTROLLER` fallback unchanged | xceadmin cli.py | click `envvar=` list order | Flowchart: `HasLegacyEnv` |
|
||||
| 2.6 | Both unset ⇒ hardcoded default unchanged | xceadmin cli.py | `_DEFAULT_CONTROLLER` (unmodified) | Flowchart: `UseTodayDefault` |
|
||||
| 2.7 | `--controller` help text documents the variable | xceadmin cli.py | click option help strings | n/a |
|
||||
| 3.1 | Steering doc describes the variable | `build-deployment.md` | n/a | n/a |
|
||||
| 3.2 | HA deploy guidance notes availability | `xce-deploy-ha/SKILL.md` | n/a | n/a |
|
||||
| 2.8 | Clean error for unreachable/malformed; names the env var source | xceadmin cli.py, discovery.py | `ControllerValueError`; `_controller_source_note()` | n/a |
|
||||
| 3.1 | Steering doc describes the variable: components, format, precedence, opt-out (1.7), own-name rule (1.6), `sudo`/systemd not inheriting it | `build-deployment.md` | n/a | n/a |
|
||||
| 3.2 | HA deploy guidance notes availability and the wrapper export | `xce-deploy-ha/SKILL.md` | n/a | n/a |
|
||||
| 3.3 | Multi-host guidance notes availability | `xce-test-multihost/SKILL.md` | n/a | n/a |
|
||||
| 4.1 | Wrapper exports the deployment's own list | `deploy-ha.sh` wrapper heredoc | `export XCE_CONTROLLER_PEERS="$PEERS_STR"` | n/a |
|
||||
| 4.2 | Wrapper keeps `XCEADMIN_CONTROLLER` | `deploy-ha.sh` wrapper heredoc | existing export, unchanged | n/a |
|
||||
| 4.3 | Explicit `--controller` still wins in the wrapper | xceadmin cli.py | click parameter resolution (unmodified) | Flowchart: `HasFlag` = yes |
|
||||
|
||||
## Components and Interfaces
|
||||
|
||||
| Component | Domain/Layer | Intent | Req Coverage | Key Dependencies | Contracts |
|
||||
|-----------|--------------|--------|---------------|-------------------|-----------|
|
||||
| `xce_controller_peers_env()` | libxce / C | Read `XCE_CONTROLLER_PEERS`, empty-as-unset | 1.1, 1.3, 1.4 | none (P0) | Service |
|
||||
| controller/scheduler/worker `--peers` call sites | C daemon CLI | Combine flag + env var, preserve existing validation/help | 1.2-1.6 | `xce_controller_peers_env` (P0) | — |
|
||||
| harness `masters` setting default | C CLI (settings table) | Compute `defval` from env var at startup | 1.1, 1.3, 1.4, 1.5, 1.6 | `xce_controller_peers_env` (P0) | State |
|
||||
| `resolve_controllers()` extension | xceadmin / Python | Strip `name=` prefix per entry | 2.1, 2.2 | none (P0) | Service |
|
||||
| xceadmin `--controller` options | xceadmin / Python (click) | Add `XCE_CONTROLLER_PEERS` ahead of legacy envvar | 2.1, 2.4-2.7 | `resolve_controllers` (P0), click (P0, external) | — |
|
||||
| `xce_controller_peers_env()` | libxce / C | Read `XCE_CONTROLLER_PEERS`, empty-as-unset | 1.2, 1.3, 1.7 | none (P0) | Service |
|
||||
| `xce_peers_find_identity()` | libxce / C | One rule for "which candidate is me" | 1.6 | none (P0) | Service |
|
||||
| controller/scheduler/worker `--peers` call sites | C daemon CLI | Combine flag + env var, preserve existing validation/help; controller adds the 1.6 check | 1.1-1.8 | `xce_controller_peers_env` (P0), `xce_peers_find_identity` (P0, controller only) | — |
|
||||
| harness `masters` setting default | C CLI (settings table) | Compute `defval` from env var at startup | 1.1-1.5, 1.7 | `xce_controller_peers_env` (P0) | State |
|
||||
| `resolve_controllers()` extension | xceadmin / Python | Strip `name=` prefix per entry; clean error on a malformed entry | 2.1, 2.2, 2.8 | none (P0) | Service |
|
||||
| xceadmin `--controller` options | xceadmin / Python (click) | Add `XCE_CONTROLLER_PEERS` ahead of legacy envvar; name the env source in errors | 2.1, 2.4-2.8 | `resolve_controllers` (P0), click (P0, external) | — |
|
||||
| `scripts/xceadmin` wrapper export | `deploy-ha.sh` | Pin the wrapper to its own deployment | 4.1-4.3 | none | — |
|
||||
| Documentation updates | Docs | Describe/announce the variable | 3.1-3.3 | none | — |
|
||||
|
||||
Only the two components introducing genuinely new logic — `xce_controller_peers_env()` and the `resolve_controllers()` extension — warrant a full contract block; the remaining rows are thin call-site wiring around existing, unchanged behavior.
|
||||
Only the three components introducing genuinely new logic — `xce_controller_peers_env()`, `xce_peers_find_identity()` and the `resolve_controllers()` extension — warrant a full contract block; the remaining rows are thin call-site wiring around existing, unchanged behavior.
|
||||
|
||||
### libxce
|
||||
|
||||
|
|
@ -167,7 +217,7 @@ Only the two components introducing genuinely new logic — `xce_controller_peer
|
|||
| Field | Detail |
|
||||
|-------|--------|
|
||||
| Intent | Sole read point for `XCE_CONTROLLER_PEERS`, normalizing empty-string to absent |
|
||||
| Requirements | 1.1, 1.3, 1.4 |
|
||||
| Requirements | 1.2, 1.3, 1.7 |
|
||||
|
||||
**Responsibilities & Constraints**
|
||||
- Pure function: no side effects, no caching (the environment can only change between process invocations for these components, so no staleness concern).
|
||||
|
|
@ -188,6 +238,33 @@ const char *xce_controller_peers_env(void);
|
|||
- Postconditions: returns a pointer into the process environment (valid for the process lifetime) or `NULL`.
|
||||
- Invariants: never returns a non-NULL empty string.
|
||||
|
||||
#### xce_peers_find_identity
|
||||
|
||||
| Field | Detail |
|
||||
|-------|--------|
|
||||
| Intent | Find the controller's own candidate in a parsed peers list |
|
||||
| Requirements | 1.6 |
|
||||
|
||||
**Responsibilities & Constraints**
|
||||
- Pure function over an already-parsed `xce_cluster_config_t`; no environment or global access.
|
||||
- Matching rule, moved unchanged from `src/controller/cluster.c` (~L1784-1791 and ~L1890-1900, identical today): a candidate's key is its `name` when non-empty, else its `hostname`; the first candidate whose key equals `name` (compared with `strncmp(key, name, XCE_MAXNAME_LEN - 1)`) wins.
|
||||
- Does not apply any default: "no match → last index" stays at the `my_priority` call site, "no match → refuse" at `cluster_open_listener()`, and the 1.6 check treats no match as an error.
|
||||
|
||||
**Dependencies**
|
||||
- Inbound: controller `main.c` startup check (P0), controller `cluster.c` `my_priority` assignment and listen-port lookup (P0).
|
||||
- Outbound: none.
|
||||
|
||||
**Contracts**: Service [x] / API [ ] / Event [ ] / Batch [ ] / State [ ]
|
||||
|
||||
##### Service Interface
|
||||
```c
|
||||
/* src/include/xce/cluster.h */
|
||||
int xce_peers_find_identity(const xce_cluster_config_t *cfg, const char *name);
|
||||
```
|
||||
- Preconditions: `cfg` non-NULL.
|
||||
- Postconditions: returns the index (0..`candidates_count`-1) of the first matching candidate, or -1 when `name` is NULL or nothing matches. An empty `name` is compared like any other (today's loops do the same: it matches an unnamed entry with an empty hostname, e.g. from `:5`), so the invariant below holds for every input.
|
||||
- Invariants: for every input, `idx >= 0 ? idx : candidates_count - 1` equals what `cluster.c` computes for `my_priority` today, and `idx >= 0` equals the listener's `matched` today.
|
||||
|
||||
### xceadmin
|
||||
|
||||
#### resolve_controllers (extended)
|
||||
|
|
@ -195,11 +272,13 @@ const char *xce_controller_peers_env(void);
|
|||
| Field | Detail |
|
||||
|-------|--------|
|
||||
| Intent | Parse a `--controller`-shaped value into an ordered candidate list, now tolerant of `name=` prefixes |
|
||||
| Requirements | 2.1, 2.2 |
|
||||
| Requirements | 2.1, 2.2, 2.8 |
|
||||
|
||||
**Responsibilities & Constraints**
|
||||
- Existing behavior (whitespace stripping, blank-entry skipping, bare-host default port) is unchanged.
|
||||
- New: for each comma-separated entry, if it contains `=`, discard everything up to and including the first `=` before applying the existing `rsplit(":", 1)`.
|
||||
- New: a port that is not an integer raises `ControllerValueError` (an `XceAdminError` subclass) naming the entry, instead of today's uncaught `ValueError`.
|
||||
- Both changes apply to every caller, an explicit `--controller` value included.
|
||||
|
||||
**Dependencies**
|
||||
- Inbound: xceadmin `--controller` options (P0), any future caller expecting `name=host:port` tolerance.
|
||||
|
|
@ -212,7 +291,7 @@ const char *xce_controller_peers_env(void);
|
|||
def resolve_controllers(controller: str) -> list[tuple[str, int]]: ...
|
||||
```
|
||||
- Preconditions: `controller` is a comma-separated string of `[name=]host[:port]` entries.
|
||||
- Postconditions: returns an ordered list of `(host, port)` tuples, `name=` prefixes (if any) stripped.
|
||||
- Postconditions: returns an ordered list of `(host, port)` tuples, `name=` prefixes (if any) stripped; raises `ControllerValueError` on a malformed entry.
|
||||
- Invariants: a bare `host:port` entry (today's only supported shape) parses identically to before.
|
||||
|
||||
**Implementation Notes**
|
||||
|
|
@ -223,22 +302,31 @@ def resolve_controllers(controller: str) -> list[tuple[str, int]]: ...
|
|||
## Error Handling
|
||||
|
||||
### Error Strategy
|
||||
Every existing hard-error-on-invalid-value behavior is preserved exactly; the only addition is naming the value's source in the message text, per 1.5.
|
||||
Every existing hard-error-on-invalid-value behavior is preserved exactly, with the value's source named in the message text (1.4). The one new error is the controller's own-name check for an env-supplied list (1.6).
|
||||
|
||||
### Error Categories and Responses
|
||||
- **Invalid candidate list, from an explicit `--peers`/`--controller`/`set masters` value**: unchanged existing message and exit behavior.
|
||||
- **Invalid candidate list, from `XCE_CONTROLLER_PEERS`**: same parse failure, same exit behavior, message additionally names the environment variable as the source (controller/scheduler/worker: adjust the existing `fprintf`; harness: `enter_ctx()`'s existing "invalid 'masters' setting" message, extended to check `settings[i].value == NULL` for the `"masters"` entry at the point of failure — reusing `get_setting()`'s own existing value-before-defval precedence check, not a new tracking variable — to say "default (from XCE_CONTROLLER_PEERS)" instead of "explicit `set masters`" when true).
|
||||
- **Invalid candidate list, from legacy `XCEADMIN_CONTROLLER`**: unchanged (out of scope — 1.5/2.x do not require source-naming for the legacy variable, only for `XCE_CONTROLLER_PEERS` and the explicit flag).
|
||||
- **Invalid candidate list, from an explicit `--peers` value**: unchanged exit behavior; message names `--peers` as the source (most already do; scheduler's and worker's `xce_cluster_config_validate()` messages gain it).
|
||||
- **Invalid candidate list, from an explicit `set masters`**: unchanged behavior; `enter_ctx()`'s message names the source as described in the next bullet.
|
||||
- **Invalid or unreachable value, from an explicit `--controller`**: today's unreachable messages unchanged; a malformed entry now gives the 2.8 clean error instead of a traceback. No source note is added, since the operator typed the value.
|
||||
- **Invalid candidate list, from `XCE_CONTROLLER_PEERS`**: same parse or validation failure (including the controller's even-count rejection), same exit behavior, message additionally names the environment variable as the source (controller/scheduler/worker: adjust the existing `fprintf`; harness: `enter_ctx()`'s existing "invalid 'masters' setting" message, extended to say "default (from XCE_CONTROLLER_PEERS)" when the `"masters"` entry's `settings[i].value == NULL` (no `set masters` ran, reusing `get_setting()`'s own value-before-defval precedence) **and** `xce_controller_peers_env() != NULL`; "explicit `set masters`" when `value != NULL`; and today's wording otherwise (the built-in `localhost:9990` literal, which cannot fail to parse today, is then never blamed on the variable).
|
||||
- **Controller name not in an env-supplied list (1.6)**: `main.c` prints, to stderr, e.g. `xce_controller: XCE_CONTROLLER_PEERS (a=h1:1,b=h2:2) has no candidate named 'node7'; pass --myname <one of: a, b> or unset XCE_CONTROLLER_PEERS to run standalone`, then `return 1` before the cert pre-flight and `myapp_run()`. This replaces, for the env-var case only, the later `cluster_open_listener()` refusal, which blames "--peers", writes only to the log file, and exits with status 0. Candidates without a `name=` are listed by hostname.
|
||||
- **xceadmin, unreachable candidates or a malformed entry (2.8)**: `click.ClickException` (exit 1) with today's text (`cannot connect to {controller}: ...` for `OSError`, or the `e.message` of `ConnectionTimeoutError`/`TooManyRedirectsError`/`NoPrimaryError`), or `invalid --controller entry '...'` for `ControllerValueError`, followed by ` (from $XCE_CONTROLLER_PEERS)` or ` (from $XCEADMIN_CONTROLLER)` when the value came from the environment. Today a malformed port is an uncaught `ValueError` traceback.
|
||||
|
||||
### Monitoring
|
||||
No new logging/metrics infrastructure; each component's existing startup log line (e.g. scheduler's `INFO("HA cluster config: ...")` / `INFO("No --peers flag; ...")`) is extended only to remain accurate about which source supplied the value it's reporting.
|
||||
No new logging/metrics infrastructure; each component's existing startup log line (e.g. scheduler's `INFO("HA cluster config: ...")` / `INFO("No --peers flag; ...")`) is extended only to remain accurate about which source supplied the value it's reporting. The three "No --peers flag" lines (controller `main.c` ~L486, scheduler ~L435, worker ~L501) become "No --peers flag and XCE_CONTROLLER_PEERS unset or empty; ..." (1.8). This is the operator's only clue when `sudo`'s default `env_reset` or a systemd unit drops the variable — the main risk for the production worker, which `check_user()` requires to run as root (`deploy-ha.sh` ~L470 starts it with `sudo`).
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
### Unit Tests
|
||||
- `xce_controller_peers_env()` (`src/lib/testsuite/test_cluster.c`): unset → `NULL`; empty string → `NULL`; non-empty → returned unchanged. (1.1, 1.3, 1.4)
|
||||
- `xce_controller_peers_env()` (`src/lib/testsuite/test_cluster.c`): unset → `NULL`; empty string → `NULL`; non-empty → returned unchanged. (1.2, 1.3, 1.7)
|
||||
- `xce_peers_find_identity()` (`src/lib/testsuite/test_cluster.c`): the cases listed in File Structure Plan, each written first as a failing test. The existing controller tests that go through `my_priority` and `cluster_open_listener()` must stay green after `cluster.c` switches to the helper. (1.6)
|
||||
- `resolve_controllers()` extension (`src/xceadmin/tests/test_discovery.py`): single `name=host:port`; mixed bare/named multi-candidate list; existing bare-host cases still pass unchanged. (2.2)
|
||||
- xceadmin `--controller` option precedence (`src/xceadmin/tests/test_cli.py`): `XCE_CONTROLLER_PEERS` set alone; both `XCE_CONTROLLER_PEERS` and `XCEADMIN_CONTROLLER` set (new wins); explicit `--controller` overrides both; neither set (existing default). (2.1, 2.4-2.6) All existing `test_cli.py` cases that assert `XCEADMIN_CONTROLLER`-only or no-env-var behavior (e.g. `test_controller_option_falls_back_to_xceadmin_controller_env_var`) must explicitly `monkeypatch.delenv("XCE_CONTROLLER_PEERS", raising=False)`, so the new variable's introduction cannot make them order-dependent on ambient environment state.
|
||||
- xceadmin `--controller` option precedence (`src/xceadmin/tests/test_cli.py`): `XCE_CONTROLLER_PEERS` set alone; both `XCE_CONTROLLER_PEERS` and `XCEADMIN_CONTROLLER` set (new wins); explicit `--controller` overrides both; neither set (existing default). (2.1, 2.4-2.6) For 2.3: with
|
||||
`XCE_CONTROLLER_PEERS="a=127.0.0.1:1,b=127.0.0.1:2,127.0.0.1:3"` and no
|
||||
`--controller`, mock the send function under `follow_redirects_any`, make
|
||||
the first candidate raise a connection error, and assert the candidates
|
||||
arrive in env order with `name=` stripped and that the second one is
|
||||
tried next. (2.3) For 2.8: an unreachable value from each of `XCE_CONTROLLER_PEERS`, `XCEADMIN_CONTROLLER` and an explicit `--controller` (message names the variable in the first two only), and a non-numeric port giving a clean error rather than a traceback. Run the unreachable cases through `config push` with a real, readable cluster-bin file (`push_config()` reads it before resolving controllers, so a missing file raises `ClusterBinFileError` first), or patch out the retry sleep, since the `scheduler record` commands retry `NoPrimaryError` 8 times with a 2 s pause (`scheduler.py` ~L131-132). All existing `test_cli.py` cases that assert `XCEADMIN_CONTROLLER`-only or no-env-var behavior (e.g. `test_controller_option_falls_back_to_xceadmin_controller_env_var`) must explicitly `monkeypatch.delenv("XCE_CONTROLLER_PEERS", raising=False)`, so the new variable's introduction cannot make them order-dependent on ambient environment state.
|
||||
|
||||
### Scripted Regression Verification
|
||||
None of controller/scheduler/worker/harness's `main()`-level argument or
|
||||
|
|
@ -252,32 +340,91 @@ repeatable script, `src/tools/verify-controller-peers-env.sh`, following
|
|||
and asserting on their output — but deliberately lightweight: it asserts on
|
||||
each component's **startup log/settings-dump line**, not on a live,
|
||||
reachable cluster, since resolving the candidate list happens, and is
|
||||
observable, entirely before any connection attempt succeeds or fails:
|
||||
observable, entirely before any connection attempt succeeds or fails.
|
||||
The worker cases use `xcecd_virtual`, since the production `xcecd`
|
||||
requires root (`check_user()`), and grep the worker's captured **stdout**,
|
||||
not `xcecd-<name>.log`: its "HA cluster config" / "No --peers flag" lines
|
||||
(`worker/src/main.c` ~L493-503) are logged before its log file is set up
|
||||
(~L529-544), so the logging library still writes them to stdout. The
|
||||
controller and scheduler set up their log files earlier (controller
|
||||
`main.c` ~L393, scheduler ~L332), so their cases grep the log file. Controller cases always use an odd
|
||||
candidate count and pass `--myname` naming a candidate unless the case
|
||||
is about the 1.6 check, so the even-count rule and 1.6 do not mask what
|
||||
the case is testing.
|
||||
- controller/scheduler/worker: with `XCE_CONTROLLER_PEERS` set (e.g.
|
||||
`a=host1:1,b=host2:2`) and no `--peers`, start briefly and grep the log
|
||||
for the existing "N candidate(s)" line already logged today for an
|
||||
explicit `--peers` value, asserting `N` and the candidate identity match
|
||||
the env var's content — no reachable target required. (1.1, 1.3)
|
||||
- Same three, with both `XCE_CONTROLLER_PEERS` set to one value and
|
||||
`--peers` set to a different one; assert the log reflects the `--peers`
|
||||
value, not the env var's. (1.2)
|
||||
- Same three, with neither set; assert the existing "No --peers flag"
|
||||
(or worker's "single-controller path") log line, unchanged. (1.4)
|
||||
`a=localhost:1,b=localhost:2,c=localhost:3`; controller also
|
||||
`--myname a`) and no `--peers`, start briefly and grep the log (worker:
|
||||
stdout) for the line each already logs today for an explicit `--peers`
|
||||
value — controller `HA mode: N controller candidates, identity=...`
|
||||
(`main.c` ~L476), scheduler and worker `HA cluster config: N
|
||||
candidate(s), ...` (scheduler ~L431, worker ~L493) — asserting `N`
|
||||
matches the env var's content — no reachable target required. (1.2)
|
||||
- Same three, with both `XCE_CONTROLLER_PEERS` set to one value (3
|
||||
candidates) and `--peers` set to a different one (1 candidate, which the
|
||||
controller's `--myname` names); assert the log reflects the `--peers`
|
||||
value, not the env var's (worker: stdout). (1.1)
|
||||
- Same three, with neither set; assert the "No --peers flag" (or
|
||||
worker's "single-controller path", on stdout) log line now also says
|
||||
`XCE_CONTROLLER_PEERS` was unset or empty, and that behavior is otherwise
|
||||
unchanged. (1.3, 1.8)
|
||||
- scheduler with `--sim <file>` and `XCE_CONTROLLER_PEERS` set: assert the
|
||||
process never logs an "HA cluster config"/"No --peers flag" line at all
|
||||
(the `--sim` path returns before that code is reached — this is a
|
||||
by-construction guarantee already, not new logic; the script asserts it
|
||||
stays true). (Non-Goals cross-check)
|
||||
- harness/`xce_test`: run non-interactively with a script containing only
|
||||
- harness/`xce_test` (always with `--norc`, so a developer's own rc file
|
||||
cannot change the result): run non-interactively with a script containing only
|
||||
`set` (no arguments) — `cmd_set`'s existing argc==1 branch prints every
|
||||
setting's current value/default with **no connection attempt** — first
|
||||
with `XCE_CONTROLLER_PEERS` set and no prior `set masters`, asserting the
|
||||
printed `masters` default reflects the env var; then with an explicit
|
||||
`set masters ...` line before the bare `set`, asserting the printed
|
||||
*value* (not default) reflects the explicit override. (1.1, 1.3)
|
||||
- Trigger a parse error via each of an invalid explicit flag and an invalid
|
||||
`XCE_CONTROLLER_PEERS` value on one component (e.g. controller); assert
|
||||
the two error messages are distinguishable by source. (1.5)
|
||||
*value* (not default) reflects the explicit override. (1.1, 1.2)
|
||||
- For each of controller, scheduler and worker, trigger a parse error
|
||||
once with an invalid explicit `--peers` and once with an invalid
|
||||
`XCE_CONTROLLER_PEERS` and no `--peers`; assert on **stderr** (all three
|
||||
write these errors with `fprintf(stderr, ...)`) that the two messages
|
||||
name different sources. For the controller, repeat with a 2-candidate
|
||||
value from each source (even-count message); for scheduler and worker,
|
||||
repeat with `a=localhost:0`, which parses but fails
|
||||
`xce_cluster_config_validate()` (port 0). (1.4)
|
||||
- harness/`xce_test --norc`, each script ending in a connecting command
|
||||
(`client`): (a) `set masters bad` with `XCE_CONTROLLER_PEERS` unset →
|
||||
message says explicit `set masters`; (b) no `set masters`,
|
||||
`XCE_CONTROLLER_PEERS=bad` → message says default from
|
||||
`XCE_CONTROLLER_PEERS`; (c) `set masters bad` with
|
||||
`XCE_CONTROLLER_PEERS` set to a valid value → still says explicit
|
||||
`set masters` (explicit wins, 1.1). (1.1, 1.4)
|
||||
- controller with `XCE_CONTROLLER_PEERS=a=localhost:1,b=localhost:2,c=localhost:3`:
|
||||
with `--myname zz`, assert a non-zero exit and the 1.6 message (names
|
||||
`zz`, the variable, and the remedies); with `--myname b`, assert the
|
||||
"HA mode: 3 controller candidates" line and no 1.6 message; with
|
||||
`--myname zz --peers a=localhost:1,b=localhost:2,c=localhost:3`, assert
|
||||
no 1.6 message (explicit-flag behavior unchanged). (1.1, 1.6)
|
||||
- controller with `XCE_CONTROLLER_PEERS=""` and no `--peers`: assert the
|
||||
"No --peers flag ... running as single-node primary" line. (1.7)
|
||||
- Help text (1.5, 1.7): run `xce_controller --help`, `xce_scheduler --help`
|
||||
and `xcecd_virtual -h`, and feed `help set` to `xce_test --norc`; assert
|
||||
each output names `XCE_CONTROLLER_PEERS` and says that unsetting or
|
||||
emptying it opts out for one invocation. For xceadmin, `test_cli.py`
|
||||
asserts the same name in each `--controller` option's `--help` output
|
||||
(2.7).
|
||||
- `deploy-ha.sh` wrapper: run `deploy-ha.sh` with the built controller
|
||||
binary and a peers list on free high ports (it refuses ports already in
|
||||
use, `deploy-ha.sh` ~L204-225) into a scratch `$TOP` (generation only,
|
||||
nothing started), copy
|
||||
`$TOP/scripts/xceadmin` with its final `exec` line replaced by
|
||||
`printenv XCE_CONTROLLER_PEERS XCEADMIN_CONTROLLER`, and run the copy
|
||||
with `XCE_CONTROLLER_PEERS=bogus:1` in the caller's environment; assert
|
||||
it prints the deployment's own `--peers` list and first candidate. If
|
||||
the xceadmin wheel is missing (so no wrapper is generated), this case
|
||||
fails with a message naming `/xce-build xceadmin`, rather than being
|
||||
skipped. (4.1, 4.2)
|
||||
- xceadmin (`test_cli.py`): with `XCE_CONTROLLER_PEERS` and
|
||||
`XCEADMIN_CONTROLLER` both set and an explicit `--controller`, the flag
|
||||
wins. (4.3; this runs the CLI directly rather than through the
|
||||
generated wrapper, which is sufficient because the wrapper only exports
|
||||
variables and passes its arguments through unchanged)
|
||||
|
||||
This script is a new addition to the File Structure Plan and is intended to
|
||||
be re-run on any future change to these five components' startup argument
|
||||
|
|
|
|||
|
|
@ -2,15 +2,17 @@
|
|||
|
||||
## Introduction
|
||||
|
||||
Operators and deploy/test scripts must retype the full `--peers` candidate
|
||||
list on every invocation of controller, scheduler, worker, and
|
||||
harness/xce_test, and the `--controller` target on every invocation of
|
||||
xceadmin — there is no way to set a default once per session or
|
||||
environment for the four `--peers`-based components today. `--peers` is
|
||||
optional for all four, and omitting it intentionally selects each
|
||||
component's existing standalone/single-controller behavior (confirmed:
|
||||
each component's own startup log already names this an intentional path,
|
||||
not an error) — that behavior must keep working unchanged. xceadmin
|
||||
Operators and deploy/test scripts must retype the full controller
|
||||
candidate list on every invocation of controller, scheduler and worker
|
||||
(their `--peers` option) and of harness/xce_test (its `set masters`
|
||||
setting) — there is no way to set a default once per session or
|
||||
environment for these four peers-based components today. The candidate
|
||||
list is optional for all four,
|
||||
and leaving it out intentionally selects each component's existing
|
||||
standalone/single-controller behavior (confirmed: the controller,
|
||||
scheduler and worker startup logs already name this an intentional path,
|
||||
not an error) — that behavior must keep working unchanged, apart from
|
||||
added wording in its startup log line (1.8). The fifth tool, xceadmin,
|
||||
already solved the equivalent problem for its own `--controller` option via
|
||||
the `XCEADMIN_CONTROLLER` environment variable (bug-1566), and, separately,
|
||||
xceadmin's `--controller` option can already accept a comma-separated list
|
||||
|
|
@ -20,69 +22,129 @@ rather than only a single target.
|
|||
This feature introduces one new environment variable,
|
||||
`XCE_CONTROLLER_PEERS`, read as a fallback default by all five components
|
||||
(controller, scheduler, worker, harness/xce_test, and xceadmin) whenever
|
||||
their respective connection-target option (`--peers` or `--controller`) is
|
||||
not explicitly given on the command line — and only then; an explicit
|
||||
option value is always used exactly as it is today, with the environment
|
||||
variable never consulted. xceadmin's existing `XCEADMIN_CONTROLLER`
|
||||
variable keeps working exactly as it does today, as a lower-priority
|
||||
fallback beneath the new variable.
|
||||
no explicit value is given (`--peers`, `set masters`, or `--controller`)
|
||||
— and only then; an explicit value always wins, and the environment
|
||||
variable is never used as the candidate list when one is given. Explicit `--peers` and
|
||||
`set masters` values behave exactly as today, apart from the error wording
|
||||
in 1.4; an explicit `--controller`
|
||||
value gains the two parsing changes described under Adjacent expectations
|
||||
(2.2, 2.8). xceadmin's existing `XCEADMIN_CONTROLLER` variable keeps
|
||||
working as a lower-priority fallback beneath the new variable, unchanged
|
||||
apart from the parsing changes in 2.2 and 2.8 (which apply to every source
|
||||
of the `--controller` value).
|
||||
|
||||
For the controller specifically, `--peers` is not just a list of servers to
|
||||
try: it defines the HA (Raft) cluster membership, and the controller finds
|
||||
its own place in that list by matching its name. A value inherited from the
|
||||
environment, rather than typed next to `--myname`, must therefore never let
|
||||
a controller join a cluster it cannot identify itself in.
|
||||
|
||||
## Boundary Context
|
||||
|
||||
- **In scope**: A single new environment variable, `XCE_CONTROLLER_PEERS`,
|
||||
providing the default connection target for controller, scheduler,
|
||||
worker, harness/xce_test (via their existing `--peers` option) and
|
||||
xceadmin (via its existing `--controller` option), consulted only when
|
||||
the corresponding command-line option is omitted.
|
||||
- **Out of scope**: The apiserver (its controller connection comes from a
|
||||
configuration file, not a command-line option, and is unaffected) and
|
||||
xcecli's `--url` option (a different target — the apiserver, not a
|
||||
controller — and unaffected). Any change to `src/tools/deploy-ha.sh`'s
|
||||
generated scripts, which continue to pass `--peers`/`--controller`
|
||||
explicitly exactly as they do today. Any change to what each component
|
||||
does today when its connection-target option is omitted and no
|
||||
environment variable applies either (today's standalone/single-controller
|
||||
behavior is preserved exactly).
|
||||
- **Adjacent expectations**: xceadmin's existing multi-candidate,
|
||||
try-until-reachable behavior for `--controller` (bug-1821) and its
|
||||
existing `XCEADMIN_CONTROLLER` fallback (bug-1566) are both reused
|
||||
unmodified — this feature only changes what value feeds into that
|
||||
already-existing behavior, never how that behavior itself works.
|
||||
providing the default connection target for controller, scheduler and
|
||||
worker (via their existing `--peers` option), harness/xce_test (via its
|
||||
existing `masters` setting) and xceadmin (via its existing `--controller`
|
||||
option), used only when no explicit value is given.
|
||||
- **Out of scope**:
|
||||
- The apiserver (its controller connection comes from a configuration
|
||||
file, not a command-line option) and xcecli's `--url` option (a
|
||||
different target — the apiserver, not a controller). Both are
|
||||
unaffected.
|
||||
- Any change to `src/tools/deploy-ha.sh`'s generated daemon scripts,
|
||||
which continue to pass `--peers` explicitly exactly as they do today.
|
||||
One exception: the generated `scripts/xceadmin` wrapper selects its
|
||||
deployment's controllers only through the `XCEADMIN_CONTROLLER`
|
||||
environment variable, which the new variable outranks, so it also
|
||||
exports `XCE_CONTROLLER_PEERS` (Requirement 4).
|
||||
- Any change to what each component does today when no explicit value
|
||||
is given and no environment variable applies either (today's
|
||||
standalone/single-controller behavior is preserved, apart from the log
|
||||
wording in 1.8).
|
||||
- **Adjacent expectations**: xceadmin's existing in-order,
|
||||
try-until-reachable failover for `--controller` (bug-1821) and its
|
||||
existing `XCEADMIN_CONTROLLER` fallback (bug-1566) keep working as they
|
||||
do today, apart from the parsing changes below. xceadmin's parsing of a `--controller`-shaped value changes in
|
||||
two ways, whichever source the value comes from (an explicit
|
||||
`--controller` included): a `name=` prefix on an entry is removed (2.2),
|
||||
and a malformed entry is reported as an error rather than a crash (2.8).
|
||||
Today an explicit `--controller ctrl-1=host:port` fails as an
|
||||
unresolvable hostname; after this feature it reaches `host:port`.
|
||||
|
||||
## Requirements
|
||||
|
||||
### Requirement 1: Default peers candidate list for --peers-based components
|
||||
### Requirement 1: Default candidate list for peers-based components
|
||||
|
||||
**Objective:** As an operator running controller, scheduler, worker, or
|
||||
harness/xce_test, I want a default `--peers` candidate list available via
|
||||
harness/xce_test, I want a default controller candidate list available via
|
||||
an environment variable, so that I don't have to retype it on every
|
||||
invocation while still being able to run any single invocation without it.
|
||||
|
||||
**Terms used below:**
|
||||
- *Peers-based component*: controller, scheduler, worker, or
|
||||
harness/xce_test.
|
||||
- *Explicit candidate list*: for controller, scheduler and worker, the
|
||||
`--peers` argument; for harness/xce_test, a `set masters ...` command,
|
||||
whether typed interactively, in a script, or in its rc file.
|
||||
- *Resolution point*: for controller, scheduler and worker, process
|
||||
startup; for harness/xce_test, each connection attempt (`client`,
|
||||
`worker` and similar commands), since `set masters` can run at any time
|
||||
in a session.
|
||||
- *Own name* (controller only): the `--myname` value, or the host name
|
||||
the controller uses when `--myname` is not given.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. The peers-based component (controller, scheduler, worker, or
|
||||
harness/xce_test) shall recognize `XCE_CONTROLLER_PEERS` as the
|
||||
environment variable providing its default `--peers` candidate list.
|
||||
2. When the peers-based component starts with an explicit `--peers`
|
||||
argument, the peers-based component shall use that argument's value and
|
||||
shall not consult `XCE_CONTROLLER_PEERS`.
|
||||
3. When the peers-based component starts without an explicit `--peers`
|
||||
argument and `XCE_CONTROLLER_PEERS` is set to a non-empty value, the
|
||||
peers-based component shall use that value as its `--peers` candidate
|
||||
list, exactly as if it had been supplied via `--peers`.
|
||||
4. When the peers-based component starts without an explicit `--peers`
|
||||
argument and `XCE_CONTROLLER_PEERS` is unset or empty, the peers-based
|
||||
component shall behave exactly as it does today when started with no
|
||||
`--peers` argument (today's standalone/single-controller behavior,
|
||||
unchanged).
|
||||
5. If the candidate-list value in use — from either `--peers` or
|
||||
`XCE_CONTROLLER_PEERS` — fails to parse as a valid candidate list, the
|
||||
peers-based component shall report a parse-error message, and that
|
||||
message shall identify whether the invalid value came from the
|
||||
`--peers` argument or from the `XCE_CONTROLLER_PEERS` environment
|
||||
variable.
|
||||
6. The peers-based component's `--peers` help/usage text shall document
|
||||
`XCE_CONTROLLER_PEERS` as an environment-variable fallback for that
|
||||
option.
|
||||
1. When the peers-based component reaches its resolution point with an
|
||||
explicit candidate list, the peers-based component shall use that list
|
||||
and shall not use `XCE_CONTROLLER_PEERS` as its candidate list (it may
|
||||
still read the variable, e.g. to show it as harness/xce_test's `masters`
|
||||
default in `set` output).
|
||||
2. When the peers-based component reaches its resolution point without an
|
||||
explicit candidate list and `XCE_CONTROLLER_PEERS` is set to a
|
||||
non-empty value, the peers-based component shall use that value as its
|
||||
candidate list, exactly as if it had been given as an explicit
|
||||
candidate list, except for the error-source wording in 1.4, the
|
||||
controller's own-name check in 1.6, and, for harness/xce_test, that the
|
||||
value is shown as the `masters` setting's default rather than its value
|
||||
in `set` output.
|
||||
3. When the peers-based component reaches its resolution point without an
|
||||
explicit candidate list and `XCE_CONTROLLER_PEERS` is unset or empty,
|
||||
the peers-based component shall behave exactly as it does today with no
|
||||
explicit candidate list (controller: standalone single-node primary;
|
||||
scheduler and worker: their single-controller path; harness/xce_test:
|
||||
its built-in `masters` default), apart from the log wording in 1.8.
|
||||
4. If the candidate list in use, from either an explicit candidate list
|
||||
or `XCE_CONTROLLER_PEERS`, fails parsing or the component's own
|
||||
validation of it (including the controller's rule that the number of
|
||||
candidates must be odd), the peers-based component shall report an
|
||||
error message that identifies whether the value came from the explicit
|
||||
candidate list (`--peers`, or `set masters` for harness/xce_test) or
|
||||
from the `XCE_CONTROLLER_PEERS` environment variable, and shall keep
|
||||
today's failure behavior for that error.
|
||||
5. The peers-based component's help/usage text for its candidate list
|
||||
(`--peers` for controller, scheduler and worker; the `masters` setting
|
||||
in harness/xce_test's `set` help) shall document `XCE_CONTROLLER_PEERS`
|
||||
as an environment-variable fallback for it.
|
||||
6. If the controller takes its candidate list from `XCE_CONTROLLER_PEERS`
|
||||
and its own name matches no candidate in that list, the controller
|
||||
shall exit with a non-zero status before opening any network
|
||||
connection, writing an error message to stderr that states the
|
||||
controller's own name, names `XCE_CONTROLLER_PEERS` as the source, and the two remedies: pass
|
||||
`--myname` naming one of the candidates, or unset
|
||||
`XCE_CONTROLLER_PEERS` to run standalone. When the candidate list comes
|
||||
from an explicit `--peers` argument, the controller's identity handling
|
||||
is unchanged.
|
||||
7. The peers-based component shall treat an empty `XCE_CONTROLLER_PEERS`
|
||||
the same as an unset one, so that unsetting or emptying the variable for
|
||||
one invocation gives that invocation the behavior in 1.3; no separate
|
||||
opt-out option is added, and the help text from 1.5 shall describe this
|
||||
way of opting out.
|
||||
8. When the controller, scheduler or worker starts with neither an
|
||||
explicit `--peers` argument nor a non-empty `XCE_CONTROLLER_PEERS`, its
|
||||
existing startup log line for that case shall also say that
|
||||
`XCE_CONTROLLER_PEERS` was unset or empty, so an operator whose variable was
|
||||
dropped (e.g. by `sudo`) can tell from the log why the standalone or
|
||||
single-controller path was taken.
|
||||
|
||||
### Requirement 2: Default controller candidate list for xceadmin
|
||||
|
||||
|
|
@ -95,9 +157,10 @@ variable configures every tool's default connection target.
|
|||
explicit `--controller` option and `XCE_CONTROLLER_PEERS` is set to a
|
||||
non-empty value, xceadmin shall derive its controller candidate list
|
||||
from `XCE_CONTROLLER_PEERS`.
|
||||
2. Where a candidate entry in `XCE_CONTROLLER_PEERS` carries a `name=`
|
||||
prefix, xceadmin shall remove the prefix before treating that entry as a
|
||||
controller candidate.
|
||||
2. Where a candidate entry carries a `name=` prefix, xceadmin shall remove
|
||||
the prefix before treating that entry as a controller candidate. This
|
||||
applies to every source of the `--controller` value, an explicit
|
||||
`--controller` included.
|
||||
3. When xceadmin derives its controller candidate list from
|
||||
`XCE_CONTROLLER_PEERS`, xceadmin shall attempt each resulting candidate
|
||||
in the same order the variable lists them, moving to the next candidate
|
||||
|
|
@ -105,11 +168,13 @@ variable configures every tool's default connection target.
|
|||
already provides for an explicit multi-candidate `--controller` value.
|
||||
4. When an xceadmin subcommand that accepts `--controller` runs with an
|
||||
explicit `--controller` option, xceadmin shall use that option's value
|
||||
and shall not consult `XCE_CONTROLLER_PEERS` or `XCEADMIN_CONTROLLER`.
|
||||
(parsed as in 2.2 and 2.8) and shall not consult `XCE_CONTROLLER_PEERS`
|
||||
or `XCEADMIN_CONTROLLER`.
|
||||
5. When an xceadmin subcommand that accepts `--controller` runs without an
|
||||
explicit `--controller` option, and `XCE_CONTROLLER_PEERS` is unset or
|
||||
empty, and `XCEADMIN_CONTROLLER` is set to a non-empty value, xceadmin
|
||||
shall use `XCEADMIN_CONTROLLER` exactly as it does today.
|
||||
shall use `XCEADMIN_CONTROLLER` exactly as it does today, apart from
|
||||
the parsing changes in 2.2 and 2.8.
|
||||
6. When an xceadmin subcommand that accepts `--controller` runs without an
|
||||
explicit `--controller` option, and both `XCE_CONTROLLER_PEERS` and
|
||||
`XCEADMIN_CONTROLLER` are unset or empty, xceadmin shall fall back to
|
||||
|
|
@ -117,6 +182,14 @@ variable configures every tool's default connection target.
|
|||
7. The `--controller` option's help text shall document
|
||||
`XCE_CONTROLLER_PEERS` as its preferred environment-variable fallback,
|
||||
alongside the existing `XCEADMIN_CONTROLLER` mention.
|
||||
8. If xceadmin cannot reach any controller candidate, or a candidate entry
|
||||
is malformed (e.g. a non-numeric port), xceadmin shall exit with an
|
||||
error message rather than an unhandled exception. When the
|
||||
`--controller` value came from `XCE_CONTROLLER_PEERS` or
|
||||
`XCEADMIN_CONTROLLER` rather than an explicit `--controller` option,
|
||||
that message shall name the environment variable it came from. For a
|
||||
malformed entry, whatever the source, the message shall quote the
|
||||
malformed entry.
|
||||
|
||||
### Requirement 3: Documentation of the new environment variable
|
||||
|
||||
|
|
@ -126,15 +199,41 @@ I can discover and use it without reading source code.
|
|||
|
||||
#### Acceptance Criteria
|
||||
1. The project's build/deployment steering documentation shall describe
|
||||
`XCE_CONTROLLER_PEERS`: which components read it, its value format, and
|
||||
its precedence relative to each component's connection-target option
|
||||
(`--peers` or `--controller`).
|
||||
`XCE_CONTROLLER_PEERS`: which components read it, its value format, its
|
||||
precedence relative to each component's explicit value (`--peers`,
|
||||
`set masters`, or `--controller`), how to opt out for one invocation
|
||||
(1.7), the controller's own-name rule (1.6), and that it is not
|
||||
inherited by a
|
||||
process started through `sudo` (default `env_reset`) or by a systemd
|
||||
service, with the way to pass it in each case (e.g.
|
||||
`sudo --preserve-env=XCE_CONTROLLER_PEERS ...`, or `Environment=` in
|
||||
the unit). This matters most for the production worker, which must run
|
||||
as root.
|
||||
2. The HA cluster deployment guidance shall note that
|
||||
`XCE_CONTROLLER_PEERS` is available as an alternative to passing
|
||||
`--peers`/`--controller` explicitly on every generated script
|
||||
invocation, without stating or implying any change to what those
|
||||
generated scripts themselves pass.
|
||||
`--peers`/`--controller` explicitly, that the generated daemon scripts
|
||||
still pass `--peers` explicitly, and that the generated
|
||||
`scripts/xceadmin` wrapper exports `XCE_CONTROLLER_PEERS` for its own
|
||||
deployment (Requirement 4).
|
||||
3. The multi-host verification guidance shall note the same availability
|
||||
wherever it currently documents driving deployment via `--peers`,
|
||||
without stating or implying any change to that verification's own
|
||||
generated invocations.
|
||||
|
||||
### Requirement 4: Generated xceadmin wrapper is not shadowed by an ambient variable
|
||||
|
||||
**Objective:** As an operator who has `XCE_CONTROLLER_PEERS` set in my
|
||||
shell for another cluster, I want a deployment's generated
|
||||
`scripts/xceadmin` wrapper to keep reaching that deployment's own
|
||||
controllers, so that the new variable does not bring back bug-1566's
|
||||
wrong-controller symptom.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. The `scripts/xceadmin` wrapper generated by `deploy-ha.sh` shall export
|
||||
`XCE_CONTROLLER_PEERS` set to the deployment's own full `--peers`
|
||||
candidate list before starting xceadmin, overriding any value inherited
|
||||
from the caller's environment.
|
||||
2. The generated wrapper shall keep exporting `XCEADMIN_CONTROLLER` exactly
|
||||
as it does today.
|
||||
3. An explicit `--controller` option passed to the generated wrapper shall
|
||||
still take precedence over both exported variables.
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@
|
|||
## Research Log
|
||||
|
||||
### Does `--peers`-omitted behavior differ by component, and is it safe to default it via an env var?
|
||||
- **Context**: Requirement 1.4 requires that omitting `--peers` with no env var set behaves exactly as today; needed to confirm this "today" behavior is genuinely a supported mode (not an error) for all four components, and that an env-var-supplied default cannot unsafely interact with an unrelated mode.
|
||||
- **Context**: Requirement 1.3 requires that omitting `--peers` with no env var set behaves exactly as today; needed to confirm this "today" behavior is genuinely a supported mode (not an error) for all four components, and that an env-var-supplied default cannot unsafely interact with an unrelated mode.
|
||||
- **Sources Consulted**: `src/controller/main.c` (~L403-429), `src/scheduler/src/main.c` (~L380-429), `src/worker/src/main.c` (~L389-475), buglog.json bug-1457.
|
||||
- **Findings**:
|
||||
- controller: `if (arguments.peers_str) {...}` — omitting it is controller's own supported single-node/standalone primary path (bug-1457 fixed a startup-ordering defect in this exact path in 2026-07; the path itself is intentional, not an error).
|
||||
|
|
@ -67,6 +67,24 @@
|
|||
- **Rationale**: Click already implements exactly the required precedence (first non-empty of an ordered list, empty treated as unset) — reinventing it in a custom `default=` callable would duplicate already-correct, already-tested library behavior for no benefit.
|
||||
- **Trade-offs**: None identified; strictly less code than the alternative.
|
||||
|
||||
### Decision: Controller refuses an env-supplied peers list that lacks its own name (design review, 2026-10-02)
|
||||
- **Context**: For the controller, `--peers` is the Raft membership. `src/controller/cluster.c` ~L1780-1791 finds `my_priority` (which is also Raft `self_idx`) by matching the controller's name against each candidate's `name` (or `hostname` when unnamed), and falls back to the last index when nothing matches. `cluster_open_listener()` (~L1902) already refuses that no-match case before any network I/O, so Raft safety is not at risk, but the refusal blames "--peers", goes only to the log file, and the process exits with status 0 (`main()` ignores `myapp_run()`'s result). An inherited `XCE_CONTROLLER_PEERS` makes the case far more likely: a controller started without `--myname` (name = hostname), or one meant to run standalone.
|
||||
- **Alternatives Considered**:
|
||||
1. The controller ignores `XCE_CONTROLLER_PEERS`; only scheduler, worker, harness and xceadmin read it.
|
||||
2. The controller reads it, but refuses to start when its own name is not a candidate.
|
||||
3. Make the no-match case an error for explicit `--peers` too.
|
||||
- **Selected Approach**: Option 2, with the matching rule moved into a shared `xce_peers_find_identity()` that both existing `cluster.c` lookups also call, so the three cannot drift.
|
||||
- **Rationale**: Keeps one variable for every component (the brief's goal) and turns the most likely new misconfiguration into an immediate, clear, non-zero-exit error instead of a log-only refusal that blames the wrong source. Option 3 changes shipped explicit-flag behavior, which the requirements rule out; it is recorded as a follow-up in the design's Revalidation Triggers.
|
||||
- **Opt-out for one invocation** (the brief's open question): unset the variable or set it empty for that run; no separate flag. `xce_controller_peers_env()` already treats empty as unset.
|
||||
|
||||
### Decision: Generated `scripts/xceadmin` wrapper exports `XCE_CONTROLLER_PEERS` (design review, 2026-10-02)
|
||||
- **Context**: `deploy-ha.sh` ~L692 points the wrapper at its own deployment only through `XCEADMIN_CONTROLLER`. Because the new variable outranks it, an operator with `XCE_CONTROLLER_PEERS` set for another cluster would see the wrapper reach the wrong controller (bug-1566's symptom), while the generated daemon scripts stay safe because they pass `--peers` explicitly.
|
||||
- **Alternatives Considered**:
|
||||
1. Keep `deploy-ha.sh` untouched and document the shadowing.
|
||||
2. Add `export XCE_CONTROLLER_PEERS="$PEERS_STR"` to the wrapper.
|
||||
- **Selected Approach**: Option 2. One line, and the wrapper gains failover across the deployment's full candidate list instead of only the first candidate.
|
||||
- **Trade-offs**: Narrows the brief's "deploy-ha unchanged" scope decision to "generated daemon scripts unchanged".
|
||||
|
||||
## Risks & Mitigations
|
||||
- **Risk**: A future `--peers`/`--controller`/`masters` value could legitimately contain `=` in a way that collides with the `name=` stripping convention. **Mitigation**: Not possible in practice — DNS hostnames and IP literals cannot contain `=`; both `xce_parse_peers()` (existing, proven) and the extended `resolve_controllers()` share the identical, narrow "split on the first `=`" rule.
|
||||
- **Risk**: harness's `defval` mutation could be perceived as "changing settings without the operator asking," surprising a `set` command's own listing output. **Mitigation**: `defval` has always been the table's documented fallback slot (`get_setting()` already falls through to it); computing it from an explicit, documented environment variable at startup is more transparent than silently keeping a stale hardcoded literal while an env var the operator deliberately set is ignored.
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
{
|
||||
"feature_name": "controller-peers-env-fallback",
|
||||
"created_at": "2026-09-22T07:37:10Z",
|
||||
"updated_at": "2026-09-22T08:18:57Z",
|
||||
"updated_at": "2026-10-03T04:05:28Z",
|
||||
"language": "en",
|
||||
"phase": "design-generated",
|
||||
"approvals": {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue