diff --git a/.kiro/specs/controller-peers-env-fallback/brief.md b/.kiro/specs/controller-peers-env-fallback/brief.md new file mode 100644 index 00000000..0bb8fc66 --- /dev/null +++ b/.kiro/specs/controller-peers-env-fallback/brief.md @@ -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 ` 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). diff --git a/.kiro/specs/controller-peers-env-fallback/design.md b/.kiro/specs/controller-peers-env-fallback/design.md new file mode 100644 index 00000000..f48f1339 --- /dev/null +++ b/.kiro/specs/controller-peers-env-fallback/design.md @@ -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 ` 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. diff --git a/.kiro/specs/controller-peers-env-fallback/requirements.md b/.kiro/specs/controller-peers-env-fallback/requirements.md new file mode 100644 index 00000000..165c6389 --- /dev/null +++ b/.kiro/specs/controller-peers-env-fallback/requirements.md @@ -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. diff --git a/.kiro/specs/controller-peers-env-fallback/research.md b/.kiro/specs/controller-peers-env-fallback/research.md new file mode 100644 index 00000000..2dd4a718 --- /dev/null +++ b/.kiro/specs/controller-peers-env-fallback/research.md @@ -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. diff --git a/.kiro/specs/controller-peers-env-fallback/spec.json b/.kiro/specs/controller-peers-env-fallback/spec.json new file mode 100644 index 00000000..9225d4dc --- /dev/null +++ b/.kiro/specs/controller-peers-env-fallback/spec.json @@ -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 +} diff --git a/.wolf/anatomy-index.json b/.wolf/anatomy-index.json index a5ca4a85..cf16bc57 100644 --- a/.wolf/anatomy-index.json +++ b/.wolf/anatomy-index.json @@ -1,13 +1,13 @@ { "version": 1, "meta": { - "lastScanned": "2026-09-22T03:01:53.188Z", - "fileCount": 671, + "lastScanned": "2026-09-23T02:33:52.263Z", + "fileCount": 683, "hits": 0, "misses": 0, - "renderedHash": "02614f3b797e44059a670011f293e3047a15c20c2c31f6bcec3e18336f1f4f2d", - "storeUpdatedAt": "2026-09-22T03:01:53.196Z", - "rootHash": "e7f2dc076a00fe7b" + "renderedHash": "b057ab996dad903a1362cecdf33c671ae71dc917e01a294cf69f58208783b3fc", + "storeUpdatedAt": "2026-09-23T02:33:52.274Z", + "rootHash": "d5ef97d98b68db51" }, "files": { ".agent-shell/transcripts/2026-08-11-11-39-59.md": { @@ -10133,20 +10133,20 @@ }, ".claude/skills/xce-test-ha/run-ha-soak.sh": { "description": "run-ha-soak.sh — driver for /xce-test-ha: deploy a 3-node XCE HA cluster, then", - "tokens": 47176, - "hash": "0e99bb6e2a2a9ece", - "size": 179105, - "mtimeMs": 1789368443287.2227, - "updatedAt": "2026-09-14T06:47:23.336Z", + "tokens": 51606, + "hash": "b1e6808537161344", + "size": 196026, + "mtimeMs": 1790077156253.1445, + "updatedAt": "2026-09-22T11:39:16.297Z", "source": "hook" }, ".claude/skills/xce-test-ha/SKILL.md": { "description": "XCE HA Failover Soak Test", - "tokens": 10080, - "hash": "dcb4b066592c38ce", - "size": 40603, - "mtimeMs": 1789368549893.6606, - "updatedAt": "2026-09-14T06:49:09.930Z", + "tokens": 10827, + "hash": "f2649f6350937e0f", + "size": 43609, + "mtimeMs": 1790072383701.4255, + "updatedAt": "2026-09-22T10:19:43.755Z", "source": "hook" }, ".claude/skills/xce-deploy-ha/SKILL.md": { @@ -10271,8 +10271,8 @@ "tokens": 1359, "hash": "329a9fe468acd8ad", "size": 5093, - "mtimeMs": 1789441323455.6272, - "updatedAt": "2026-09-15T03:02:03.502Z", + "mtimeMs": 1790051284501.0596, + "updatedAt": "2026-09-22T04:28:04.545Z", "source": "hook" }, "src/lib/meson.build": { @@ -10280,8 +10280,8 @@ "tokens": 2428, "hash": "61df53cb05f969ce", "size": 9104, - "mtimeMs": 1789441310019.5066, - "updatedAt": "2026-09-15T03:01:50.064Z", + "mtimeMs": 1790051269331.7236, + "updatedAt": "2026-09-22T04:27:49.372Z", "source": "hook" }, "src/lib/vtaskmgr.c": { @@ -11782,11 +11782,11 @@ }, "src/scheduler/testsuite/test_sim_live_allocation.c": { "description": "Declares already", - "tokens": 9156, - "hash": "0234199ee3e7f8de", - "size": 34503, - "mtimeMs": 1789983083813.7146, - "updatedAt": "2026-09-21T09:31:23.860Z", + "tokens": 9204, + "hash": "43ed3515f039ccb9", + "size": 34685, + "mtimeMs": 1790130832217.2603, + "updatedAt": "2026-09-23T02:33:52.262Z", "source": "hook" }, "src/scheduler/src/sim_alloc.c": { @@ -12219,20 +12219,20 @@ }, "src/scheduler/testsuite/test_sim_verify.c": { "description": "Declares mismatch", - "tokens": 18890, - "hash": "222d4fed39c1fccc", - "size": 70834, - "mtimeMs": 1789989575780.3364, - "updatedAt": "2026-09-21T11:19:35.816Z", + "tokens": 19394, + "hash": "7fa136721cf382f0", + "size": 72727, + "mtimeMs": 1790130818284.3984, + "updatedAt": "2026-09-23T02:33:38.334Z", "source": "hook" }, "src/scheduler/src/sim_verify.c": { "description": "include ", - "tokens": 4694, - "hash": "fe98da3155c5abee", - "size": 17601, - "mtimeMs": 1789983096595.9946, - "updatedAt": "2026-09-21T09:31:36.645Z", + "tokens": 4975, + "hash": "e1a321f2179b0f34", + "size": 18653, + "mtimeMs": 1790130791318.6594, + "updatedAt": "2026-09-23T02:33:11.369Z", "source": "hook" }, "src/scheduler/src/core.c": { @@ -12261,6 +12261,114 @@ "mtimeMs": 1790044881003.8472, "updatedAt": "2026-09-22T02:41:21.052Z", "source": "hook" + }, + "src/lib/testsuite/test_xcectx_reconnect_race.c": { + "description": "", + "tokens": 3278, + "hash": "85e8fc00e51fd0ab", + "size": 12292, + "mtimeMs": 1790051122490.1106, + "updatedAt": "2026-09-22T04:25:22.540Z", + "source": "hook" + }, + ".kiro/specs/controller-peers-env-fallback/brief.md": { + "description": "Brief: controller-peers-env-fallback", + "tokens": 3006, + "hash": "cc85936278dc57d5", + "size": 12022, + "mtimeMs": 1790062563804.6033, + "updatedAt": "2026-09-22T07:36:03.845Z", + "source": "hook" + }, + ".kiro/specs/controller-peers-env-fallback/spec.json": { + "description": "", + "tokens": 133, + "hash": "b8a459e88579744d", + "size": 464, + "mtimeMs": 1790065158100.524, + "updatedAt": "2026-09-22T08:19:18.167Z", + "source": "hook" + }, + ".kiro/specs/controller-peers-env-fallback/requirements.md": { + "description": "Requirements Document", + "tokens": 1982, + "hash": "ec66cdd671d534b2", + "size": 7945, + "mtimeMs": 1790063989888.4863, + "updatedAt": "2026-09-22T07:59:49.983Z", + "source": "hook" + }, + ".kiro/specs/controller-peers-env-fallback/research.md": { + "description": "Research & Design Decisions", + "tokens": 2974, + "hash": "473fd117b1d6433b", + "size": 11940, + "mtimeMs": 1790064903181.8152, + "updatedAt": "2026-09-22T08:15:03.249Z", + "source": "hook" + }, + ".kiro/specs/controller-peers-env-fallback/design.md": { + "description": "Design Document", + "tokens": 5981, + "hash": "5399a24e35e42271", + "size": 24021, + "mtimeMs": 1790066216160.3623, + "updatedAt": "2026-09-22T08:36:56.224Z", + "source": "hook" + }, + "src/controller/testsuite/test_worker_resync.c": { + "description": "Declares that", + "tokens": 56586, + "hash": "fa29afa63d882a05", + "size": 213125, + "mtimeMs": 1790067745378.11, + "updatedAt": "2026-09-22T09:02:25.467Z", + "source": "hook" + }, + "src/include/persist.h": { + "description": "pragma once", + "tokens": 26768, + "hash": "4457cd8f549c27d1", + "size": 100687, + "mtimeMs": 1790068774367.1018, + "updatedAt": "2026-09-22T09:19:34.442Z", + "source": "hook" + }, + "src/controller/persist.c": { + "description": "define _GNU_SOURCE", + "tokens": 109748, + "hash": "64e039b00874d12e", + "size": 412236, + "mtimeMs": 1790068788133.859, + "updatedAt": "2026-09-22T09:19:48.212Z", + "source": "hook" + }, + "src/controller/resync.c": { + "description": "here: construction", + "tokens": 11276, + "hash": "201f2fd706c83d5b", + "size": 42838, + "mtimeMs": 1790068800728.621, + "updatedAt": "2026-09-22T09:20:00.803Z", + "source": "hook" + }, + ".claude/skills/xce-test-ha/soak-verdict.sh": { + "description": "soak-verdict.sh — pure (side-effect-free) verdict helpers for run-ha-soak.sh.", + "tokens": 3084, + "hash": "d6c908e6e011acd6", + "size": 11597, + "mtimeMs": 1790072270334.6003, + "updatedAt": "2026-09-22T10:17:50.384Z", + "source": "hook" + }, + ".claude/skills/xce-test-ha/soak-verdict-selftest.sh": { + "description": "soak-verdict-selftest.sh — deterministic unit test for the verdict parser in", + "tokens": 2867, + "hash": "da09b9f1bf8e6c56", + "size": 11132, + "mtimeMs": 1790072281493.8599, + "updatedAt": "2026-09-22T10:18:01.537Z", + "source": "hook" } } } \ No newline at end of file diff --git a/.wolf/anatomy.md b/.wolf/anatomy.md index 0e057b9c..ce96f65c 100644 --- a/.wolf/anatomy.md +++ b/.wolf/anatomy.md @@ -1,7 +1,7 @@ # anatomy.md -> Auto-maintained by OpenWolf. Last scanned: 2026-09-22T03:01:53.188Z -> Files: 671 tracked | Anatomy hits: 0 | Misses: 0 +> Auto-maintained by OpenWolf. Last scanned: 2026-09-23T02:33:52.263Z +> Files: 683 tracked | Anatomy hits: 0 | Misses: 0 ## ./ @@ -35,8 +35,10 @@ ## .claude/skills/xce-test-ha/ -- `run-ha-soak.sh` — run-ha-soak.sh — driver for /xce-test-ha: deploy a 3-node XCE HA cluster, then (~47176 tok) -- `SKILL.md` — XCE HA Failover Soak Test (~10080 tok) +- `run-ha-soak.sh` — run-ha-soak.sh — driver for /xce-test-ha: deploy a 3-node XCE HA cluster, then (~51606 tok) +- `SKILL.md` — XCE HA Failover Soak Test (~10827 tok) +- `soak-verdict-selftest.sh` — soak-verdict-selftest.sh — deterministic unit test for the verdict parser in (~2867 tok) +- `soak-verdict.sh` — soak-verdict.sh — pure (side-effect-free) verdict helpers for run-ha-soak.sh. (~3084 tok) ## .claude/skills/xce-test-multihost/ @@ -152,6 +154,14 @@ - `spec.json` (~387 tok) - `tasks.md` — Implementation Plan (~12616 tok) +## .kiro/specs/controller-peers-env-fallback/ + +- `brief.md` — Brief: controller-peers-env-fallback (~3006 tok) +- `design.md` — Design Document (~5981 tok) +- `requirements.md` — Requirements Document (~1982 tok) +- `research.md` — Research & Design Decisions (~2974 tok) +- `spec.json` (~133 tok) + ## .kiro/specs/controller-status/ - `design.md` — Design Document — controller-status (`ctrlstatus`) (~17072 tok) @@ -887,6 +897,8 @@ - `ha.c` — define _GNU_SOURCE (~127991 tok) - `ha.h` — ifndef HA_H (~34444 tok) - `meson.build` (~3405 tok) +- `persist.c` — define _GNU_SOURCE (~109748 tok) +- `resync.c` — here: construction (~11276 tok) ## src/controller/testsuite/ @@ -900,6 +912,7 @@ - `test_ha_role.c` — include (~13998 tok) - `test_ha_vep_teardown.c` (~8206 tok) - `test_rcfile_integration.sh` — test_rcfile_integration.sh — xce-test-rc-file integration test, shell-driven (~4117 tok) +- `test_worker_resync.c` — Declares that (~56586 tok) ## src/harness/ @@ -916,6 +929,7 @@ ## src/include/ +- `persist.h` — pragma once (~26768 tok) - `request.h` — if !defined(_REQUEST_H_) (~3288 tok) - `resreq.h` — if !defined(_RESREQ_H_) (~3310 tok) - `typemap.h` — if !defined(_TYPEMAP_H_) (~8720 tok) @@ -948,6 +962,7 @@ - `test_utils.c` — include (~4370 tok) - `test_vtaskmgr_teardown.c` — Declares int (~1668 tok) - `test_xcealloc.c` — include (~4979 tok) +- `test_xcectx_reconnect_race.c` (~3278 tok) - `test_xlist.c` — include (~1140 tok) - `test_zarray.c` — include (~1603 tok) - `test.c` — include (~1359 tok) @@ -991,7 +1006,7 @@ - `sim_alloc.c` — include (~2585 tok) - `sim_limit.c` — include (~2102 tok) - `sim_loop.c` — include (~23299 tok) -- `sim_verify.c` — include (~4694 tok) +- `sim_verify.c` — include (~4975 tok) - `sim.c` — include (~7432 tok) ## src/scheduler/src/plugins/ @@ -1035,12 +1050,12 @@ - `test_sim_e2e_limit_visibility.c` — Declares char (~8076 tok) - `test_sim_e2e_original_motivation.c` (~19514 tok) - `test_sim_equivalence.c` — Declares BOTH (~11161 tok) -- `test_sim_live_allocation.c` — Declares already (~9156 tok) +- `test_sim_live_allocation.c` — Declares already (~9204 tok) - `test_sim_reconcile_unaccounted.c` — Declares documents (~36877 tok) - `test_sim_resource_contention.c` — Declares is (~10149 tok) - `test_sim_round_boundary_reconciliation.c` (~9980 tok) - `test_sim_session_ordering.c` (~10531 tok) -- `test_sim_verify.c` — Declares mismatch (~18890 tok) +- `test_sim_verify.c` — Declares mismatch (~19394 tok) - `test_snapshot_save_load.c` (~29478 tok) - `test.c` — define UNIT_TEST 1 (~7044 tok) diff --git a/.wolf/memory.md b/.wolf/memory.md index b6c01887..b7d39328 100644 --- a/.wolf/memory.md +++ b/.wolf/memory.md @@ -5634,3 +5634,98 @@ | 19:53 | Edited src/controller/testsuite/test_ha_liveness.c | 4→4 lines | ~60 | | 19:54 | Edited src/controller/testsuite/test_ha_liveness.c | 6→4 lines | ~69 | | 20:01 | Edited src/controller/ha.c | modified XCE_TESTING() | ~235 | +| 20:40 | Created src/lib/testsuite/test_xcectx_reconnect_race.c | — | ~2575 | +| 20:41 | Edited src/lib/testsuite/test.c | 1→2 lines | ~38 | +| 20:41 | Edited src/lib/testsuite/test.c | 1→2 lines | ~21 | +| 20:41 | Edited src/lib/meson.build | 1→2 lines | ~34 | +| 20:43 | Edited src/lib/testsuite/test_xcectx_reconnect_race.c | 2→4 lines | ~79 | +| 20:47 | Edited src/lib/testsuite/test_xcectx_reconnect_race.c | expanded (+7 lines) | ~154 | +| 21:00 | Edited src/lib/testsuite/test_xcectx_reconnect_race.c | 4→8 lines | ~92 | +| 21:03 | Edited src/lib/meson.build | 2→1 lines | ~17 | +| 21:03 | Edited src/lib/testsuite/test.c | 2→1 lines | ~13 | +| 21:03 | Edited src/lib/testsuite/test.c | 2→1 lines | ~11 | +| 21:18 | Edited src/lib/testsuite/test_xcectx_reconnect_race.c | 11→12 lines | ~67 | +| 21:19 | Edited src/lib/testsuite/test_xcectx_reconnect_race.c | added 2 condition(s) | ~546 | +| 21:19 | Edited src/lib/testsuite/test_xcectx_reconnect_race.c | 10→11 lines | ~157 | +| 21:20 | Edited src/lib/meson.build | 1→2 lines | ~34 | +| 21:22 | Edited src/lib/testsuite/test.c | 2→3 lines | ~39 | +| 21:22 | Edited src/lib/testsuite/test.c | 1→2 lines | ~21 | +| 21:25 | Edited src/lib/testsuite/test_xcectx_reconnect_race.c | expanded (+7 lines) | ~158 | +| 21:27 | Edited src/lib/meson.build | 2→1 lines | ~17 | +| 21:27 | Edited src/lib/testsuite/test.c | 3→2 lines | ~26 | +| 21:28 | Edited src/lib/testsuite/test.c | 2→1 lines | ~11 | +| 00:27 | Created .kiro/specs/controller-peers-env-fallback/brief.md | — | ~1910 | +| 00:32 | Edited .kiro/specs/controller-peers-env-fallback/brief.md | modified accepts() | ~819 | +| 00:33 | Edited .kiro/specs/controller-peers-env-fallback/brief.md | 8→12 lines | ~176 | +| 00:33 | Edited .kiro/specs/controller-peers-env-fallback/brief.md | 4→3 lines | ~57 | +| 00:33 | Edited .kiro/specs/controller-peers-env-fallback/brief.md | expanded (+9 lines) | ~226 | +| 00:34 | Edited .kiro/specs/controller-peers-env-fallback/brief.md | modified xceadmin() | ~374 | +| 00:35 | Edited .kiro/specs/controller-peers-env-fallback/brief.md | expanded (+7 lines) | ~370 | +| 00:35 | Edited .kiro/specs/controller-peers-env-fallback/brief.md | modified helper() | ~281 | +| 00:35 | Edited .kiro/specs/controller-peers-env-fallback/brief.md | 8→11 lines | ~191 | +| 00:35 | Edited .kiro/specs/controller-peers-env-fallback/brief.md | 4→7 lines | ~132 | +| 00:36 | Edited .kiro/specs/controller-peers-env-fallback/brief.md | 7→9 lines | ~166 | +| 00:37 | Created .kiro/specs/controller-peers-env-fallback/spec.json | — | ~132 | +| 00:37 | Created .kiro/specs/controller-peers-env-fallback/requirements.md | — | ~802 | +| 00:44 | Created .kiro/specs/controller-peers-env-fallback/requirements.md | — | ~1826 | +| 00:45 | Edited .kiro/specs/controller-peers-env-fallback/spec.json | 8→8 lines | ~55 | +| 00:59 | Edited .kiro/specs/controller-peers-env-fallback/requirements.md | expanded (+21 lines) | ~338 | +| 01:00 | Edited .kiro/specs/controller-peers-env-fallback/spec.json | inline fix | ~12 | +| 01:15 | Created .kiro/specs/controller-peers-env-fallback/research.md | — | ~3173 | +| 01:18 | Created .kiro/specs/controller-peers-env-fallback/design.md | — | ~5408 | +| 01:18 | Edited .kiro/specs/controller-peers-env-fallback/design.md | expanded (+9 lines) | ~148 | +| 01:19 | Edited .kiro/specs/controller-peers-env-fallback/spec.json | 12→12 lines | ~74 | +| 01:27 | Edited .kiro/specs/controller-peers-env-fallback/design.md | inline fix | ~38 | +| 01:28 | Edited .kiro/specs/controller-peers-env-fallback/design.md | inline fix | ~92 | +| 01:28 | Edited .kiro/specs/controller-peers-env-fallback/design.md | inline fix | ~56 | +| 01:28 | Edited .kiro/specs/controller-peers-env-fallback/design.md | inline fix | ~19 | +| 01:28 | Edited .kiro/specs/controller-peers-env-fallback/design.md | inline fix | ~159 | +| 01:29 | Edited .kiro/specs/controller-peers-env-fallback/design.md | inline fix | ~69 | +| 01:30 | Edited .kiro/specs/controller-peers-env-fallback/design.md | inline fix | ~164 | +| 01:33 | Edited .kiro/specs/controller-peers-env-fallback/design.md | expanded (+35 lines) | ~752 | +| 01:33 | Edited .kiro/specs/controller-peers-env-fallback/design.md | 1→4 lines | ~87 | +| 01:34 | Edited .kiro/specs/controller-peers-env-fallback/design.md | 3→4 lines | ~103 | +| 01:35 | Edited .kiro/specs/controller-peers-env-fallback/design.md | 3→3 lines | ~33 | +| 01:35 | Edited .kiro/specs/controller-peers-env-fallback/design.md | 3→3 lines | ~30 | +| 01:36 | Edited .kiro/specs/controller-peers-env-fallback/design.md | 1→3 lines | ~183 | +| 01:36 | Edited .kiro/specs/controller-peers-env-fallback/design.md | 2→3 lines | ~46 | +| 01:40 | Session end: 83 writes across 21 files (core.h, core.c, sched_election.c, sim.h, sim_loop.c) | 44 reads | ~96447 tok | + +## Session: 2026-09-22 01:40 + +| Time | Action | File(s) | Outcome | ~Tokens | +|------|--------|---------|---------|--------| +| 02:01 | Edited src/controller/testsuite/test_worker_resync.c | added 1 condition(s) | ~842 | +| 02:02 | Edited src/controller/testsuite/test_worker_resync.c | 2→3 lines | ~56 | +| 02:19 | Edited src/include/persist.h | modified 1841() | ~362 | +| 02:19 | Edited src/controller/persist.c | modified persist_notify_job_updated_already_persisted() | ~295 | +| 02:20 | Edited src/controller/resync.c | added 1 condition(s) | ~313 | +| 03:15 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | modified soak() | ~703 | +| 03:15 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | modified overrides() | ~286 | +| 03:15 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | 2→3 lines | ~46 | +| 03:15 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | modified worker_slot_capacity() | ~635 | +| 03:16 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | modified poll_until_run_elapsed() | ~307 | +| 03:16 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | modified run_worker_capacity_soak_round() | ~1793 | +| 03:16 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | 8→8 lines | ~127 | +| 03:17 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | modified PASS() | ~463 | +| 03:17 | Edited .claude/skills/xce-test-ha/soak-verdict.sh | modified verdict_emit_arrayretry() | ~367 | +| 03:18 | Edited .claude/skills/xce-test-ha/soak-verdict-selftest.sh | modified verdict_emit_workercapacity() | ~293 | +| 03:19 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | modified run_worker_capacity_soak_round() | ~74 | +| 03:19 | Edited .claude/skills/xce-test-ha/SKILL.md | modified line() | ~888 | +| 04:12 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | modified run_worker_capacity_soak_round() | ~1497 | +| 04:13 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | 3→3 lines | ~27 | +| 04:37 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | modified run_worker_capacity_soak_round() | ~1173 | +| 04:38 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | 3→3 lines | ~77 | +| 04:38 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | expanded (+7 lines) | ~317 | +| 04:39 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | "${XCE_HA_WORKERCAP_SETTLE" → "${XCE_HA_WORKERCAP_SETTLE" | ~13 | +| 04:39 | Edited .claude/skills/xce-test-ha/run-ha-soak.sh | 5→5 lines | ~93 | +| 06:15 | Session end: 24 writes across 8 files (test_worker_resync.c, persist.h, persist.c, resync.c, run-ha-soak.sh) | 20 reads | ~17517 tok | + +## Session: 2026-09-22 06:15 + +| Time | Action | File(s) | Outcome | ~Tokens | +|------|--------|---------|---------|--------| +| 19:33 | Edited src/scheduler/src/sim_verify.c | modified excluded() | ~488 | +| 19:33 | Edited src/scheduler/testsuite/test_sim_verify.c | modified test_dispatch_decisions_differing_from_timestamp_still_match() | ~529 | +| 19:33 | Edited src/scheduler/testsuite/test_sim_verify.c | 2→3 lines | ~57 | +| 19:33 | Edited src/scheduler/testsuite/test_sim_live_allocation.c | modified entirely() | ~259 | diff --git a/issues/new_requirements.md b/issues/new_requirements.md new file mode 100644 index 00000000..39461a58 --- /dev/null +++ b/issues/new_requirements.md @@ -0,0 +1,133 @@ +1. Enhance to scheduler simulator + +- enhance `info job` command for specific job to: + - for pending job, show the pending reason + - for running job, show the resource usage information(include reserved and real used) +- can we add more info commands for exploring internal state of scheduler in simulation mode? +- add option to trace the full decision made by scheduler + - which job has been selected to selected and why + - trace the full process to resreq selection + - trace the full of planner to resreq selection + - trace plan part to determine the host selection + - trace the allocation details(such as resource allocation and limits applied) + - trace the reason of why host can/cannot meet resreq + - we can trace the run of all jobs or just single job + +2. Job pending reason +- for any job which is in PEND state, there should be a pend reason attached +- if the job has not been scheduled, we can set the reason to "The job is waiting to scheduler" +- if the job is not ready to be scheduled, show the reason of why job is not ready to be scheduled, it can be: + - because of queue? + - time constraint? + - job dependency? + - others? +- If the job can not be run in some host, give out the reason, such as: + - host status? + - select considition? + - resource can not be meet +- optimize the pendding reason set between scheduler and controller, old push back when pending reason changed + +3. Enhance harness tool xce_test to get detail information of job: +```sh +xce/client>jobsub -q xce -u rshen -R "rusage[mem=2048]" -a 40 -c "sleep 100 && hostname" +jobsubmit ok: JobId: 0-14-1789369200-0 +xce/client>joblist +JobId User Queue State SubmitTime StartTime FinishTime MinTasks MaxTasks Command +0-15-1789108916-0[0] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:41:56,000 2026-09-11 14:43:37,000 4 4 sleep 100 && hostname +0-15-1789108916-0[1] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:41:56,000 2026-09-11 14:43:36,000 4 4 sleep 100 && hostname +0-15-1789108916-0[2] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:41:56,000 2026-09-11 14:43:37,000 4 4 sleep 100 && hostname +0-15-1789108916-0[3] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:41:56,000 2026-09-11 14:43:37,000 4 4 sleep 100 && hostname +0-15-1789108916-0[4] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:41:56,000 2026-09-11 14:43:37,000 4 4 sleep 100 && hostname +0-15-1789108916-0[5] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:43:40,000 2026-09-11 14:45:21,000 4 4 sleep 100 && hostname +0-15-1789108916-0[6] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:43:40,000 2026-09-11 14:45:21,000 4 4 sleep 100 && hostname +0-15-1789108916-0[7] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:43:41,000 2026-09-11 14:45:21,000 4 4 sleep 100 && hostname +0-15-1789108916-0[8] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:43:41,000 2026-09-11 14:45:21,000 4 4 sleep 100 && hostname +0-15-1789108916-0[9] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:43:41,000 2026-09-11 14:45:21,000 4 4 sleep 100 && hostname +0-15-1789108940-0[0] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:42:21,000 2026-09-11 14:44:02,000 1 1 sleep 100 && hostname +0-15-1789108940-0[1] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:42:22,000 2026-09-11 14:44:02,000 1 1 sleep 100 && hostname +0-15-1789108940-0[2] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:43:43,000 2026-09-11 14:45:23,000 1 1 sleep 100 && hostname +0-15-1789108940-0[3] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:43:43,000 2026-09-11 14:45:23,000 1 1 sleep 100 && hostname +0-15-1789108940-0[4] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:43:43,000 2026-09-11 14:45:24,000 1 1 sleep 100 && hostname +0-15-1789108940-0[5] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:43:43,000 2026-09-11 14:45:23,000 1 1 sleep 100 && hostname +0-15-1789108940-0[6] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:44:04,000 2026-09-11 14:45:44,000 1 1 sleep 100 && hostname +0-15-1789108940-0[7] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:44:04,000 2026-09-11 14:45:45,000 1 1 sleep 100 && hostname +0-15-1789108940-0[8] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:45:22,000 2026-09-11 14:47:02,000 1 1 sleep 100 && hostname +0-15-1789108940-0[9] rshen xce DONE 2026-09-14 14:58:57,000 2026-09-11 14:45:22,000 2026-09-11 14:47:02,000 1 1 sleep 100 && hostname +0-14-1789369200-0[0] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:01,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[1] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:01,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[2] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:01,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[3] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:01,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[4] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:02,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[5] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:02,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[6] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:02,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[7] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:02,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[8] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:02,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[9] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:02,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[10] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:02,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[11] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:02,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[12] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:02,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[13] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:02,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[14] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:02,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[15] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:02,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[16] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:02,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[17] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:02,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[18] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:03,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[19] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:03,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[20] rshen xce RUN 2026-09-14 15:00:00,000 2026-09-14 15:00:03,000 - 1 1 sleep 100 && hostname +0-14-1789369200-0[21] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[22] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[23] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[24] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[25] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[26] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[27] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[28] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[29] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[30] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[31] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[32] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[33] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[34] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[35] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[36] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[37] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[38] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +0-14-1789369200-0[39] rshen xce PEND 2026-09-14 15:00:00,000 - - 1 1 sleep 100 && hostname +xce/client>jobinfo -j "0-14-1789369200-0[23]" +JobID: 0-14-1789369200-0[23] +state: PEND +state_raw: 0x1 +Queue: xce +User: rshen +Slots: 1,1 +Command: sleep 100 && hostname +Pend Reason: +``` + +we prefer to make jobinfo to also show resreq and pending reason information and maybe other informations + +4. re-arch plugin interface to eliminate duplicated code(such as my_alloc()). + +According the design of scheduler-allocation project we can make a general allocation function in plugin framework to make sequential and parallel allocation plugins do not need to export my_alloc(). + +5. Enhance controller to clean up jobs which meet following conditions: +- pre-defined parameter in cluster configuration for how long to keep a job before it can be clean uped +- Job is completed until not exceed a period (say job clean up period) +- No other jobs depends on this jobs (we just need a stub function to check depend jobs) + +6. job begin time & endtime +- job will not be scheduled before begin time +- job should be finished before end time + +7. job dependency + +8. verify the cache reuse in scheduler session? +This should be based on trace + +9. run job inside sandbox to decouple the submit user and execution user? +10. provision volume for input/output of job? +11. xce resource provider? resource consumer? +- worker provides: + - cpu, memory, swap? +- volume/data? +- license?