chore: persist OpenWolf state, controller-peers-env-fallback spec, and scheduler backlog notes
- .wolf/anatomy-index.json, anatomy.md, memory.md: OpenWolf hook-maintained project index and session log, refreshed by this session's activity. - .kiro/specs/controller-peers-env-fallback/: spec scaffold (brief, requirements, design, research) from prior spec-init work. - issues/new_requirements.md: scheduler simulator enhancement backlog (info-command improvements, pend-reason detail, decision tracing). Excluded as non-persistent: issues/Issues.md (empty placeholder), src/lib/testsuite/test_xcectx_reconnect_race.c (self-documented as an ASAN stress reproduction only, not a committed regression test, and not wired into meson.build), scheduler-replay-20260922-095207.replay (4MB binary debug capture, no .replay files are tracked in this repo). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
7901db57d4
commit
668e3f6893
9 changed files with 1130 additions and 41 deletions
214
.kiro/specs/controller-peers-env-fallback/brief.md
Normal file
214
.kiro/specs/controller-peers-env-fallback/brief.md
Normal file
|
|
@ -0,0 +1,214 @@
|
|||
# Brief: controller-peers-env-fallback
|
||||
|
||||
## Problem
|
||||
|
||||
Operators and deploy/test scripts must retype the full `--peers` candidate
|
||||
list (or, for xceadmin, `--controller`) on every single invocation of every
|
||||
XCE component -- there is no way to set a default once per session/
|
||||
environment. xceadmin already solved this for its own `--controller` flag
|
||||
(`XCEADMIN_CONTROLLER` env var, bug-1566), but the four `--peers`-based
|
||||
daemons (controller, scheduler, worker, harness/xce_test) have no
|
||||
equivalent: `--peers` is optional (omitting it runs standalone/non-HA), but
|
||||
there is no way to supply a default candidate list via the environment for
|
||||
someone who wants the convenience of not retyping it, as opposed to someone
|
||||
deliberately omitting it to opt out of HA.
|
||||
|
||||
## Current State
|
||||
|
||||
- xceadmin (Python, click): `--controller` already falls back to
|
||||
`$XCEADMIN_CONTROLLER`, then a hardcoded `localhost:9991` default. Fully
|
||||
implemented and tested (bug-1566), used consistently by every
|
||||
`--controller`-accepting subcommand (`config push`, `config probe`,
|
||||
`scheduler record start/stop/status`) -- confirmed via source read,
|
||||
`src/xceadmin/src/xceadmin/cli.py`.
|
||||
- **Correction, discovered mid-session (bug-1821, 2026-09-17)**: `--controller`
|
||||
is NOT single-target-only -- it already accepts a comma-separated list of
|
||||
bare `host:port` candidates (`xceadmin.discovery.resolve_controllers()` ->
|
||||
`follow_redirects_any()`), tried in order until one is reachable, mirroring
|
||||
the client-side master-rotation failover `xcecli`/`xce_test` already use.
|
||||
`resolve_controllers()` does a plain `part.rsplit(":", 1)` per
|
||||
comma-separated entry with **no** `name=` prefix handling -- a
|
||||
`name=host:port`-shaped entry (the `--peers`/`xce_parse_peers()` format)
|
||||
would mis-parse as a literal (unresolvable) hostname `"name=host"`. This
|
||||
directly shapes the merge design below: xceadmin cannot consume a
|
||||
`name=host:port,...` value as-is, but stripping the `name=` prefixes and
|
||||
handing the WHOLE resulting candidate list to its own existing
|
||||
`resolve_controllers`/`follow_redirects_any` gets proper multi-candidate
|
||||
failover for free, rather than arbitrarily trusting just one entry.
|
||||
- controller, scheduler, worker (C, argp/getopt_long) and harness/xce_test
|
||||
(C, custom `flagspec.c` parser) each take an optional
|
||||
`--peers <name=host:port,...>` flag with **no** environment-variable
|
||||
fallback at all -- confirmed via source grep, zero `getenv()` calls for
|
||||
this anywhere in these four components.
|
||||
- All four of these C binaries already link `libxce` (`src/lib/`) --
|
||||
confirmed via each component's `meson.build` (`libxce_dep`).
|
||||
- apiserver (Go) takes its controller peer list from a YAML config file
|
||||
(`app.yaml`'s `backend.peers`), not a CLI flag -- no `--peers`/
|
||||
`--controller` equivalent exists there.
|
||||
- xcecli (Go) takes `--url` to reach the apiserver -- a different target
|
||||
("which apiserver") than "which controller(s)" entirely.
|
||||
|
||||
## Desired Outcome
|
||||
|
||||
- **A single, merged environment variable, `XCE_CONTROLLER_PEERS`**,
|
||||
provides the default target for all five components. Its value format is
|
||||
exactly what `xce_parse_peers()` already accepts (`src/lib/cluster.c`):
|
||||
either a bare `host:port` (single candidate, no name) or a full
|
||||
`name=host:port,name=host:port,...` candidate list -- confirmed by
|
||||
reading `xce_parse_peers()` itself, which already tolerates both shapes
|
||||
with no name required (a strict superset relationship, not an assumption).
|
||||
- controller, scheduler, worker, harness/xce_test: when `--peers` is not
|
||||
passed, use `$XCE_CONTROLLER_PEERS` as-is (no extra parsing needed --
|
||||
it's already exactly `xce_parse_peers()`'s native input shape).
|
||||
Precedence: `--peers` flag > `$XCE_CONTROLLER_PEERS` > today's no-peers
|
||||
behavior (standalone/non-HA).
|
||||
- xceadmin: when `--controller` is not passed, read the *same*
|
||||
`$XCE_CONTROLLER_PEERS`, strip any `name=` prefix from each
|
||||
comma-separated entry (producing the bare `host:port,host:port,...` shape
|
||||
`resolve_controllers()` already natively accepts), and feed the WHOLE
|
||||
resulting string through xceadmin's existing `--controller` value path --
|
||||
i.e. functionally as if the user had typed
|
||||
`--controller "host1:port1,host2:port2,..."` themselves. This inherits
|
||||
xceadmin's own already-tested multi-candidate failover
|
||||
(`resolve_controllers` + `follow_redirects_any`, bug-1821) for free,
|
||||
rather than arbitrarily trusting a single entry regardless of whether
|
||||
it's alive. The existing `XCEADMIN_CONTROLLER` env var (bare `host:port`
|
||||
or comma-list, unchanged) keeps working as a lower-priority legacy
|
||||
fallback -- no breaking change to the already-shipped, tested,
|
||||
`deploy-ha.sh`-integrated behavior. Precedence: `--controller` flag >
|
||||
`$XCE_CONTROLLER_PEERS` (name= stripped, full candidate list) >
|
||||
`$XCEADMIN_CONTROLLER` (legacy) > `localhost:9991` bare default.
|
||||
- This merges what were originally proposed as two separate new env vars
|
||||
(`XCE_CONTROLLER_PEERS` and `XCE_CONTROLLER`) into one, since the
|
||||
peers-list format is a strict superset of the single-target format --
|
||||
reduces the operator-facing surface to one variable to set, shared by
|
||||
every component.
|
||||
|
||||
## Approach
|
||||
|
||||
Add one small shared helper in `src/lib/` (all four C targets already link
|
||||
`libxce`) that resolves "the CLI-provided value, or else `$XCE_CONTROLLER_PEERS`"
|
||||
-- called once by each of `controller/main.c`, `scheduler/src/main.c`,
|
||||
`worker/src/main.c`, and `harness/test.c` right after their own (unchanged)
|
||||
argp/getopt_long/flagspec parsing of `--peers`, replacing four
|
||||
near-identical inline duplications with one shared, TDD-covered
|
||||
implementation. This helper does no format-specific parsing itself -- it
|
||||
just returns whichever raw string wins precedence, which then flows into
|
||||
each component's existing `xce_parse_peers()` call exactly as `--peers`
|
||||
would have.
|
||||
|
||||
xceadmin's click-based `--controller` option gains a new small conversion
|
||||
helper (Python): given `$XCE_CONTROLLER_PEERS`'s raw value, strip an
|
||||
optional `name=` prefix from EVERY comma-delimited entry (not just the
|
||||
first) and return the resulting bare `host:port,host:port,...` string
|
||||
unchanged in shape from what `--controller`/`resolve_controllers()` already
|
||||
parses today. Wired in ahead of the existing `envvar="XCEADMIN_CONTROLLER"`
|
||||
in click's precedence chain (click's `envvar=` only reads a raw string per
|
||||
name, so the name-stripping conversion needs a small `callback=`/default
|
||||
resolution wrapper around the option rather than relying on click's
|
||||
built-in multi-envvar list mechanism, since the two source vars now have
|
||||
different value shapes and only one of them needs transformation). No
|
||||
change to `resolve_controllers()`/`follow_redirects_any()` themselves --
|
||||
they already do exactly the right thing once handed a bare-host candidate
|
||||
list, regardless of whether that list came from an explicit `--controller`
|
||||
flag or this new env-var conversion.
|
||||
|
||||
## Scope
|
||||
|
||||
- **In**:
|
||||
- New `XCE_CONTROLLER_PEERS` env var fallback for `--peers` in
|
||||
controller, scheduler, worker, harness/xce_test (used as-is, no
|
||||
transformation).
|
||||
- The same `XCE_CONTROLLER_PEERS` env var, `name=`-stripped and handed to
|
||||
xceadmin's existing `resolve_controllers`/`follow_redirects_any`
|
||||
multi-candidate failover, as a new, higher-precedence fallback for
|
||||
xceadmin's `--controller` option, ahead of the existing
|
||||
`XCEADMIN_CONTROLLER` (kept working, noted as legacy).
|
||||
- TDD test coverage for all five call sites (4 C components' own
|
||||
testsuites + xceadmin's existing click-based test file), including the
|
||||
`name=`-stripping conversion specifically (bare `host:port` input,
|
||||
`name=host:port` input, multi-candidate input with a mix of named and
|
||||
bare entries, and that the resulting failover still tries every
|
||||
candidate in order).
|
||||
- **Out**:
|
||||
- apiserver (YAML-config-based, no CLI flag for this) -- explicit user
|
||||
decision.
|
||||
- xcecli's `--url` (different target: apiserver, not controller) --
|
||||
explicit user decision.
|
||||
- `src/tools/deploy-ha.sh` and its generated wrapper scripts -- stay
|
||||
exactly as they are today (explicit `--peers`/`--controller` baked into
|
||||
every generated script); this feature is purely additive, available for
|
||||
other use, not adopted into the deploy skill itself -- explicit user
|
||||
decision.
|
||||
- Any change to what happens today when `--peers`/`--controller` is
|
||||
omitted and no env var is set either (standalone/bare-default behavior
|
||||
is unchanged).
|
||||
|
||||
## Boundary Candidates
|
||||
|
||||
- Shared libxce helper -- one implementation, one seam all 4 C daemons call
|
||||
through.
|
||||
- xceadmin's click option definitions -- already-existing seam in `cli.py`,
|
||||
just add an env var name to the existing list.
|
||||
- Per-component test coverage -- controller/scheduler/worker/harness each
|
||||
already have their own testsuite; this feature adds one test per
|
||||
component to each, not new test infrastructure.
|
||||
|
||||
## Out of Boundary
|
||||
|
||||
- apiserver and xcecli, per user decision (different flag shapes/targets,
|
||||
not part of this feature).
|
||||
- `deploy-ha.sh` adoption, per user decision (kept purely additive; no
|
||||
change to existing deploy/test skill behavior).
|
||||
- Any redesign of what "no `--peers` and no env var" means today
|
||||
(standalone mode semantics untouched).
|
||||
|
||||
## Upstream / Downstream
|
||||
|
||||
- **Upstream**: none -- this is additive to existing, independent
|
||||
argument-parsing in each of the 5 components.
|
||||
- **Downstream**: future deploy/test tooling (including, later, an opt-in
|
||||
update to `deploy-ha.sh`, explicitly deferred here) could adopt
|
||||
`XCE_CONTROLLER_PEERS` once this ships; not a requirement of this spec.
|
||||
|
||||
## Existing Spec Touchpoints
|
||||
|
||||
- **Extends**: none directly -- `admin-config-tool` is where xceadmin's CLI
|
||||
(including `XCEADMIN_CONTROLLER`) originally shipped, but this change is
|
||||
additive/small enough not to require reopening that spec; noted here for
|
||||
cross-reference only. `xceadmin.discovery.resolve_controllers()`/
|
||||
`follow_redirects_any()` (bug-1821, 2026-09-17, not tied to any open spec)
|
||||
is reused as-is, not modified -- this feature only needs to produce a
|
||||
value in the shape those functions already accept.
|
||||
- **Adjacent**: `structure.md`/`build-deployment.md` steering (component
|
||||
build/CLI conventions) -- no changes expected, just awareness.
|
||||
|
||||
## Constraints
|
||||
|
||||
- TDD mandatory per project rules: a test in each of the 4 C components'
|
||||
`testsuite/` dirs, and an addition to xceadmin's existing click-based test
|
||||
coverage, before/alongside the implementation.
|
||||
- Must not change behavior when `--peers`/`--controller` IS explicitly
|
||||
passed on the command line (env var is purely a fallback, never an
|
||||
override).
|
||||
- Open question, not resolved in discovery: what happens for an operator
|
||||
who has `XCE_CONTROLLER_PEERS` set globally in their shell but wants ONE
|
||||
specific invocation to run standalone/non-HA? Needs an explicit answer at
|
||||
requirements/design time (e.g. is "unset the env var for that one
|
||||
invocation" considered sufficient, or does a component need an explicit
|
||||
opt-out?).
|
||||
- Naming: a single `XCE_CONTROLLER_PEERS` env var shared by all five
|
||||
components (merged from an earlier two-variable proposal, per explicit
|
||||
user decision, once `xce_parse_peers()`'s existing bare-`host:port`
|
||||
tolerance confirmed the peers-list format is a strict superset of the
|
||||
single-target format); `XCEADMIN_CONTROLLER` stays supported as a
|
||||
legacy, lower-priority fallback, not removed.
|
||||
- The `name=`-stripping conversion (xceadmin side only) must be exercised
|
||||
by tests covering: bare `host:port`, `name=host:port`, a multi-candidate
|
||||
list with a mix of named and bare entries, and that the converted value
|
||||
still drives `resolve_controllers`/`follow_redirects_any`'s existing
|
||||
in-order failover across every candidate (not just the first) -- this is
|
||||
new logic (not just a new `envvar=` name) and is the one place this
|
||||
design has room for a subtle bug (e.g. mis-splitting on an `=` inside a
|
||||
hostname, though none of `xce_parse_peers()`'s own accepted characters
|
||||
make that ambiguous in practice).
|
||||
284
.kiro/specs/controller-peers-env-fallback/design.md
Normal file
284
.kiro/specs/controller-peers-env-fallback/design.md
Normal file
|
|
@ -0,0 +1,284 @@
|
|||
# Design Document
|
||||
|
||||
## 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.
|
||||
|
||||
**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.
|
||||
|
||||
### 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.
|
||||
|
||||
### 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.
|
||||
- A TSAN/new-test-infrastructure investment for `main()`-level argument parsing where none exists today (see Testing Strategy).
|
||||
|
||||
## Boundary Commitments
|
||||
|
||||
### 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.
|
||||
- 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).
|
||||
- A repeatable regression script (`src/tools/verify-controller-peers-env.sh`) proving the above across all 5 components, intended for ongoing re-use, not a one-time implementation check.
|
||||
|
||||
### 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).
|
||||
|
||||
### 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.
|
||||
- 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.
|
||||
|
||||
### Revalidation Triggers
|
||||
- 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.
|
||||
|
||||
## Architecture
|
||||
|
||||
### Existing Architecture Analysis
|
||||
|
||||
Each of the five components already resolves its own connection target independently, with no shared code path today:
|
||||
|
||||
| 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) |
|
||||
| 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"` |
|
||||
|
||||
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.
|
||||
- **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).
|
||||
- **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 |
|
||||
| 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
|
||||
|
||||
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).
|
||||
|
||||
### 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).
|
||||
- `.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
|
||||
|
||||
```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 -- no --> HasNewEnv{XCE_CONTROLLER_PEERS\nnon-empty?}
|
||||
HasNewEnv -- yes --> UseNewEnv[Use it\nxceadmin strips name prefix]
|
||||
HasNewEnv -- no --> IsAdmin{Component is xceadmin?}
|
||||
IsAdmin -- yes --> HasLegacyEnv{XCEADMIN_CONTROLLER\nnon-empty?}
|
||||
IsAdmin -- no --> UseTodayDefault[Today's unchanged default\nstandalone / built-in literal]
|
||||
HasLegacyEnv -- yes --> UseLegacyEnv[Use it, unchanged behavior]
|
||||
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`).
|
||||
|
||||
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.
|
||||
|
||||
## Requirements Traceability
|
||||
|
||||
| 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 |
|
||||
| 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.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 |
|
||||
| 3.3 | Multi-host guidance notes availability | `xce-test-multihost/SKILL.md` | n/a | n/a |
|
||||
|
||||
## 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) | — |
|
||||
| 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.
|
||||
|
||||
### libxce
|
||||
|
||||
#### xce_controller_peers_env
|
||||
|
||||
| Field | Detail |
|
||||
|-------|--------|
|
||||
| Intent | Sole read point for `XCE_CONTROLLER_PEERS`, normalizing empty-string to absent |
|
||||
| Requirements | 1.1, 1.3, 1.4 |
|
||||
|
||||
**Responsibilities & Constraints**
|
||||
- Pure function: no side effects, no caching (the environment can only change between process invocations for these components, so no staleness concern).
|
||||
- Treats an empty string identically to an unset variable, matching the same convention `resolve_controllers()`'s per-entry blank-skip already uses.
|
||||
|
||||
**Dependencies**
|
||||
- Inbound: controller/scheduler/worker `main.c` `--peers` call sites (P0), harness `test.c` startup `masters`-default computation (P0).
|
||||
- Outbound: none.
|
||||
|
||||
**Contracts**: Service [x] / API [ ] / Event [ ] / Batch [ ] / State [ ]
|
||||
|
||||
##### Service Interface
|
||||
```c
|
||||
/* src/include/xce/cluster.h */
|
||||
const char *xce_controller_peers_env(void);
|
||||
```
|
||||
- Preconditions: none.
|
||||
- Postconditions: returns a pointer into the process environment (valid for the process lifetime) or `NULL`.
|
||||
- Invariants: never returns a non-NULL empty string.
|
||||
|
||||
### xceadmin
|
||||
|
||||
#### resolve_controllers (extended)
|
||||
|
||||
| Field | Detail |
|
||||
|-------|--------|
|
||||
| Intent | Parse a `--controller`-shaped value into an ordered candidate list, now tolerant of `name=` prefixes |
|
||||
| Requirements | 2.1, 2.2 |
|
||||
|
||||
**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)`.
|
||||
|
||||
**Dependencies**
|
||||
- Inbound: xceadmin `--controller` options (P0), any future caller expecting `name=host:port` tolerance.
|
||||
- Outbound: none new.
|
||||
|
||||
**Contracts**: Service [x] / API [ ] / Event [ ] / Batch [ ] / State [ ]
|
||||
|
||||
##### Service Interface
|
||||
```python
|
||||
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.
|
||||
- Invariants: a bare `host:port` entry (today's only supported shape) parses identically to before.
|
||||
|
||||
**Implementation Notes**
|
||||
- Integration: `follow_redirects_any()` is untouched — it only ever consumes `resolve_controllers()`'s output.
|
||||
- Validation: existing `test_discovery.py` coverage for bare-host/list parsing must stay green; new tests add `name=`-prefixed and mixed-prefix cases.
|
||||
- Risks: none identified (see `research.md` Risks & Mitigations for the `=`-in-hostname non-issue).
|
||||
|
||||
## 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.
|
||||
|
||||
### 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).
|
||||
|
||||
### 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.
|
||||
|
||||
## 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)
|
||||
- `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.
|
||||
|
||||
### Scripted Regression Verification
|
||||
None of controller/scheduler/worker/harness's `main()`-level argument or
|
||||
settings-table resolution has existing unit-test coverage today (confirmed
|
||||
via source search; `test_cluster_parse.c` tests a standalone duplicated
|
||||
reference parser, not live `main()` wiring) — refactoring any of the four
|
||||
into a unit-testable shape is out of this feature's boundary (see
|
||||
Non-Goals). Rather than one-time manual checking, this is verified by a new
|
||||
repeatable script, `src/tools/verify-controller-peers-env.sh`, following
|
||||
`xce-test-multihost`'s established pattern of driving real built binaries
|
||||
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:
|
||||
- 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)
|
||||
- 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
|
||||
`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)
|
||||
|
||||
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
|
||||
handling, not a one-time implementation check.
|
||||
140
.kiro/specs/controller-peers-env-fallback/requirements.md
Normal file
140
.kiro/specs/controller-peers-env-fallback/requirements.md
Normal file
|
|
@ -0,0 +1,140 @@
|
|||
# Requirements Document
|
||||
|
||||
## 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
|
||||
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
|
||||
of candidates and will try each in order until one is reachable (bug-1821),
|
||||
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.
|
||||
|
||||
## 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.
|
||||
|
||||
## Requirements
|
||||
|
||||
### Requirement 1: Default peers 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
|
||||
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.
|
||||
|
||||
#### 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.
|
||||
|
||||
### Requirement 2: Default controller candidate list for xceadmin
|
||||
|
||||
**Objective:** As an xceadmin operator, I want `--controller` to fall back
|
||||
to the same environment variable the other XCE components use, so that one
|
||||
variable configures every tool's default connection target.
|
||||
|
||||
#### Acceptance Criteria
|
||||
1. When an xceadmin subcommand that accepts `--controller` runs without an
|
||||
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.
|
||||
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
|
||||
only when the current one is unreachable — the same behavior xceadmin
|
||||
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`.
|
||||
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.
|
||||
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
|
||||
its existing hardcoded default controller target, unchanged.
|
||||
7. The `--controller` option's help text shall document
|
||||
`XCE_CONTROLLER_PEERS` as its preferred environment-variable fallback,
|
||||
alongside the existing `XCEADMIN_CONTROLLER` mention.
|
||||
|
||||
### Requirement 3: Documentation of the new environment variable
|
||||
|
||||
**Objective:** As an operator or future contributor, I want the project's
|
||||
build/deployment documentation to describe `XCE_CONTROLLER_PEERS`, so that
|
||||
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`).
|
||||
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.
|
||||
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.
|
||||
78
.kiro/specs/controller-peers-env-fallback/research.md
Normal file
78
.kiro/specs/controller-peers-env-fallback/research.md
Normal file
|
|
@ -0,0 +1,78 @@
|
|||
# Research & Design Decisions
|
||||
|
||||
## Summary
|
||||
- **Feature**: `controller-peers-env-fallback`
|
||||
- **Discovery Scope**: Extension (existing CLI argument-parsing across 5 already-shipped binaries)
|
||||
- **Key Findings**:
|
||||
- Finding 1: harness/`xce_test` does **not** take a `--peers` CLI flag at all — it reads a `masters` *setting* (a `{key, value, defval}` table entry, set only via the interactive/scripted `set masters ...` command or rc-file loading). This is architecturally different from controller/scheduler/worker and requires a different insertion point.
|
||||
- Finding 2: click's `envvar=` parameter already accepts an ordered list of names and natively returns "the first non-empty value," treating an empty string exactly like unset — verified directly in the installed click library's `Parameter.resolve_envvar_value()`. This eliminates the need for any custom default-resolution code on the xceadmin side.
|
||||
- Finding 3: `xce_parse_peers()` (`src/lib/cluster.c`) already strips an optional `name=` prefix per comma-separated entry before parsing `host:port` — `xceadmin.discovery.resolve_controllers()` does not. Porting the same, already-proven convention into `resolve_controllers()` is a minimal, consistent, backward-compatible extension rather than a new parsing scheme.
|
||||
|
||||
## 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.
|
||||
- **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).
|
||||
- scheduler: `if (arguments.peers) {...} else { INFO("No --peers flag; using single-controller path, ...") }` — same shape, explicitly logged as intentional. Critically, scheduler's `--sim` mode `return`s **before** this block is ever reached (`main.c` ~L380-400) — an env-var-supplied peers default therefore cannot affect `--sim`/`--interactive` replay mode at all, by construction, with no special-casing needed.
|
||||
- worker: `if (peers_str) {...}` — identical shape.
|
||||
- All three: an invalid `--peers` value is a hard parse-error (`xce_parse_peers() != 0` → `fprintf(stderr, ...); return 1;`), never a silent fallback.
|
||||
- **Implications**: The env-var fallback only needs to be consulted at the single point each component already gates on `peers_str`/`peers` being non-NULL — no new mode-interaction risk exists, and `--sim` is unaffected without any extra guard.
|
||||
|
||||
### harness/xce_test's actual connection-target mechanism
|
||||
- **Context**: Requirements assumed harness took a `--peers`-shaped CLI argument like the other three; needed to verify before designing its integration point.
|
||||
- **Sources Consulted**: `src/harness/test.c` (`settings[]` ~L265-273, `get_setting()` ~L991-1005, `enter_ctx()` ~L1007-1035), `src/harness/CLAUDE.md`.
|
||||
- **Findings**: harness has no `--peers`/`--masters` top-level CLI flag. `settings[]` is a static `{key, value, defval}` array; the `"masters"` entry ships `value=NULL, defval="localhost:9990"`. `get_setting()` returns `value` if the operator has run `set masters ...` (interactively or via an rc-file, at any point before `enter_ctx()`'s connection attempt) — otherwise it falls through to `defval`. `enter_ctx()` parses whatever `get_setting("masters")` returns through the same `xce_parse_peers()` used everywhere else, and hard-errors on an invalid value.
|
||||
- **Implications**: harness's integration point is computing `defval` once at process startup (before the REPL loop and rc-file loading) from the env var, not gating on a CLI argument. This preserves the existing `value`-before-`defval` precedence untouched — an explicit `set masters ...` (interactive or rc-file) still always wins over the env-var-derived default, satisfying the same "explicit override always wins" property the other three components have, just through a different existing seam.
|
||||
|
||||
### click's multi-envvar precedence (xceadmin side)
|
||||
- **Context**: Needed to confirm whether xceadmin's `--controller` option can prioritize `XCE_CONTROLLER_PEERS` over the existing `XCEADMIN_CONTROLLER` without custom callback code.
|
||||
- **Sources Consulted**: Installed click library, `click/core.py`, `Parameter.resolve_envvar_value()` (read directly, not from memory).
|
||||
- **Findings**: `envvar` accepts `str | Sequence[str]`. When given a sequence, click iterates in order and "return[s] the first non-empty value of the list" — an empty string is treated identically to the variable being absent. This is already exactly the "new var first, legacy var second, empty-or-unset falls through" precedence Requirement 2 (2.4-2.6) specifies.
|
||||
- **Implications**: No custom `default=`/`callback=` wiring is needed. Each `--controller` option's existing `envvar="XCEADMIN_CONTROLLER"` becomes `envvar=["XCE_CONTROLLER_PEERS", "XCEADMIN_CONTROLLER"]`. This is strictly simpler than the two-separate-variable design considered during discovery.
|
||||
|
||||
### Where to strip a `name=` prefix for xceadmin
|
||||
- **Context**: `XCE_CONTROLLER_PEERS` may carry `name=host:port` entries (the daemons' native format); `xceadmin.discovery.resolve_controllers()` (bug-1821) does a plain `part.rsplit(":", 1)` with no such handling, so a named entry would mis-parse as a literal unresolvable hostname.
|
||||
- **Sources Consulted**: `src/xceadmin/src/xceadmin/discovery.py` (`resolve_controllers()`, `follow_redirects_any()`), `src/lib/cluster.c` (`xce_parse_peers()`'s own `name=` stripping, for convention parity).
|
||||
- **Findings**: `xce_parse_peers()` already strips everything before the first `=` in each entry, unconditionally, before its own `host:port` parse. No production hostname can legally contain `=` (not permitted in DNS names or IP literals), so this is unambiguous and safe.
|
||||
- **Implications**: Extend `resolve_controllers()` itself with the same one-line convention, rather than adding a separate conversion function ahead of it. This keeps the `name=`-tolerant behavior available to *any* caller of `resolve_controllers()` (not just the new env-var path) and mirrors an already-proven pattern instead of inventing a second one.
|
||||
|
||||
## Architecture Pattern Evaluation
|
||||
|
||||
| Option | Description | Strengths | Risks / Limitations | Notes |
|
||||
|--------|-------------|-----------|---------------------|-------|
|
||||
| Single shared C helper (`xce_controller_peers_env()`) in `libxce`, called individually by each daemon's existing `--peers`-gating `if` | One pure function (read env var, treat empty as unset); each call site keeps its own existing control flow | Minimal surface, trivially unit-testable, no new abstraction | None identified — all 4 C targets already link `libxce` | **Selected** |
|
||||
| Per-component duplicate `getenv()` calls, no shared helper | Simplest possible per-file change | No shared bug surface to fix once | Duplicates the exact same 2-line logic 4 times; harder to keep the "empty string == unset" rule consistent if one copy drifts | Rejected — violates Simplification/DRY with zero offsetting benefit given the shared helper is trivial |
|
||||
| Custom click `default=`/`callback=` wrapper for xceadmin's `--controller` | Full control over resolution order and transformation | None over the adopted option | Reinvents what click's own multi-`envvar` list already does natively | Rejected once click's native behavior was confirmed (see Research Log) |
|
||||
| Extend `resolve_controllers()` to strip `name=` prefixes | One-line change to an existing, already-tested function | Reuses existing, well-tested `follow_redirects_any` failover; no new function; benefits any future caller | None identified | **Selected** |
|
||||
|
||||
## Design Decisions
|
||||
|
||||
### Decision: One shared C primitive, not a per-component copy or a `--peers`-value-transforming helper
|
||||
- **Context**: controller/scheduler/worker each already gate on `peers_str`/`peers` being non-NULL at one call site; harness has no such call site (see settings-table finding above).
|
||||
- **Alternatives Considered**:
|
||||
1. A helper taking `(cli_value)` and returning the resolved value, changing each call site's `if (arguments.peers)` to `if ((resolved = xce_resolve_peers(arguments.peers)))`.
|
||||
2. A bare `xce_controller_peers_env(void)` returning the raw env value (or NULL), left for each call site to combine with its own local variable using its own existing `if`.
|
||||
- **Selected Approach**: Option 2 — `const char *xce_controller_peers_env(void)` in `src/lib/cluster.c`, declared in `src/include/xce/cluster.h` alongside `xce_parse_peers()`.
|
||||
- **Rationale**: harness has no `cli_value` to pass in at all (its "explicit override" is a settings-table write, not a function argument), so a helper shaped around "given a CLI value, return the resolved one" doesn't fit uniformly across all 4 components. A bare env-reader fits every call site's own existing control flow without forcing a shape mismatch.
|
||||
- **Trade-offs**: Each of the 4 call sites writes its own 1-3 line combination (`arguments.peers_str ? arguments.peers_str : xce_controller_peers_env()`, or, for harness, a startup-time `defval` computation) instead of one identical call everywhere — a small amount of repetition, accepted because the alternative (option 1) does not actually fit harness's shape.
|
||||
|
||||
### Decision: Reuse click's native multi-`envvar` list instead of a custom default resolver
|
||||
- **Context**: See Research Log.
|
||||
- **Alternatives Considered**:
|
||||
1. A new `controller_peers_from_env()` Python helper performing `name=` stripping, wired in via a custom `default=` callable ahead of click's own `envvar=` chain.
|
||||
2. Extend `resolve_controllers()` to tolerate `name=` prefixes directly, and add `"XCE_CONTROLLER_PEERS"` to each option's existing `envvar=` list.
|
||||
- **Selected Approach**: Option 2.
|
||||
- **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.
|
||||
|
||||
## 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.
|
||||
|
||||
## References
|
||||
- `src/lib/cluster.c` — `xce_parse_peers()` (existing `name=` convention, reused).
|
||||
- `src/xceadmin/src/xceadmin/discovery.py` — `resolve_controllers()`/`follow_redirects_any()` (bug-1821).
|
||||
- `src/xceadmin/src/xceadmin/cli.py` — existing `--controller` option definitions (bug-1566).
|
||||
- click `core.py`, `Parameter.resolve_envvar_value()` — multi-`envvar` precedence, read directly from the installed library.
|
||||
22
.kiro/specs/controller-peers-env-fallback/spec.json
Normal file
22
.kiro/specs/controller-peers-env-fallback/spec.json
Normal file
|
|
@ -0,0 +1,22 @@
|
|||
{
|
||||
"feature_name": "controller-peers-env-fallback",
|
||||
"created_at": "2026-09-22T07:37:10Z",
|
||||
"updated_at": "2026-09-22T08:18:57Z",
|
||||
"language": "en",
|
||||
"phase": "design-generated",
|
||||
"approvals": {
|
||||
"requirements": {
|
||||
"generated": true,
|
||||
"approved": true
|
||||
},
|
||||
"design": {
|
||||
"generated": true,
|
||||
"approved": false
|
||||
},
|
||||
"tasks": {
|
||||
"generated": false,
|
||||
"approved": false
|
||||
}
|
||||
},
|
||||
"ready_for_implementation": false
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue