diff --git a/.kiro/specs/scheduler-core/brief.md b/.kiro/specs/scheduler-core/brief.md index 53f1452f..c2504736 100644 --- a/.kiro/specs/scheduler-core/brief.md +++ b/.kiro/specs/scheduler-core/brief.md @@ -35,7 +35,7 @@ Operators need configurable scheduling policies, and developers need a way to de ### 1. Pluggable Job Picker -The core scheduling loop calls `picker->next_job()` to get the next schedulable job, with zero knowledge of how jobs are selected. The picker runs synchronously in the core's scheduling thread — no dedicated picker thread, no `jbuf` cbuf. +The core scheduling loop calls `picker->next_job()` to get the next schedulable job, with zero knowledge of how jobs are selected. The picker retains today's dedicated thread, communicating with the allocator thread via the existing `jbuf` cbuf (superseded by the full three-thread producer/consumer contract in requirements.md Req 1 AC 3-4 — this brief predates that decision). Picker plugin interface (`xce_job_picker_iface_t`): @@ -54,7 +54,7 @@ Scheduler core is a passive, event-driven engine with zero controller I/O depend | Mode | Driver | Input | Output | |------|--------|-------|--------| | Normal | Controller-connected | capnp push/notify → core events | Core decisions → controller | -| Simulation | Replay file | JSONL event file → core events | Core decisions → log | +| Simulation | Replay file | `.replay` capnp binary stream → core events | Core decisions → log | - **Mode-agnostic core**: no mode flag, no mode-specific code paths - **Virtualized clock**: `xce_core_now(core)` with two-base + speed formula @@ -124,7 +124,7 @@ Normal: bases=0, speed=1/1 → returns wall clock. Simulation discrete: speed=0/ - `xce_sched_core_t` / `xce_sched_driver_t` / `xce_sched_sim_driver_t` - `xce_core_event_t` — typed event interface - `xce_core_decision_cb_t` — decision callback -- JSONL replay format + snapshot/restore +- `.replay` capnp binary stream format + snapshot/restore - `xce_core_now()` — virtualized clock accessor - `builtin_fcfs` — single builtin picker - `job_pick_plugin` on scheduler + per-queue override @@ -139,7 +139,7 @@ Normal: bases=0, speed=1/1 → returns wall clock. Simulation discrete: speed=0/ - Core/driver separation: passive core, normal driver, simulation driver - Virtualized clock (`xce_core_now()` with two-base + speed formula) - Discrete event-driven virtual clock -- JSONL replay format (HEADER + SNAPSHOT + events + DECISIONS) +- `.replay` capnp binary stream format (HEADER + SNAPSHOT + events + DECISIONS) - State snapshot/restore - Deterministic replay guarantee - Runtime recording control via xceadmin commands diff --git a/.kiro/specs/scheduler-core/requirements.md b/.kiro/specs/scheduler-core/requirements.md index cbcf7c86..97cca25b 100644 --- a/.kiro/specs/scheduler-core/requirements.md +++ b/.kiro/specs/scheduler-core/requirements.md @@ -36,7 +36,7 @@ These are fundamentally different concerns: ## Boundary Context - - **In scope**: Pluggable job picker interface (`xce_job_picker_iface_t` with `next_job()`) hiding all selection logic behind a single plugin; single builtin picker (`builtin_fcfs`) replicating today's behavior through the new interface; configurable picker plugin with per-queue override (`queue->job_pick_plugin`); **core/driver separation** with passive event-driven core interface, mode-agnostic core (no mode flag, no mode-specific code paths), normal-mode driver (controller-connected), simulation-mode driver as a **standalone investigation tool** (replay recorded events with discrete event-driven virtual clock, zero controller dependencies, self-contained `.replay` file — single capnp binary stream with header + snapshot + events + decisions + periodic checkpoints in one artifact, what-if editing via event file substitution, diff-based comparison of simulated vs recorded decisions via `--sim-output`), virtualized clock access (`xce_core_now()` with two-base + speed formula), runtime recording control via xceadmin commands, `.replay` capnp stream format, state snapshot/restore for replay efficiency, **interactive REPL** with single-step, breakpoints, state inspection, and snapshot capture via `--interactive`, and deterministic replay guarantee for debugging/tuning; **normal↔sim equivalence guarantee** (same config + same events → identical decisions regardless of mode, AC 7.1); `xceadmin scheduler dump ` for human-readable replay file inspection; documentation of picker plugin interface contract (hooks, cancel-safe, session-aware); unit tests for picker hook behavior, **event recording/replay, snapshot/restore, and deterministic replay**; HA soak regression for picker changes. + - **In scope**: Pluggable job picker interface (`xce_job_picker_iface_t` with `next_job()`) hiding all selection logic behind a single plugin; single builtin picker (`builtin_fcfs`) replicating today's behavior through the new interface; configurable picker plugin with per-queue override (`queue->job_pick_plugin`); **core/driver separation** with passive event-driven core interface, mode-agnostic core (no mode flag, no mode-specific code paths), normal-mode driver (controller-connected), simulation-mode driver as a **standalone investigation tool** (replay recorded events with discrete event-driven virtual clock, zero controller dependencies, self-contained `.replay` file — single capnp binary stream with header + snapshot + events + decisions + periodic checkpoints in one artifact, what-if editing via event file substitution, diff-based comparison of simulated vs recorded decisions via `--sim-output`), virtualized clock access (`xce_core_now()` with two-base + speed formula), runtime recording control via xceadmin commands, including the **controller-side changes this requires** (new `OPCode_schedAdmin` + `SchedAdminRequest`/`SchedAdminReply` capnp types in `controller.capnp`, and pass-through dispatch logic added to the controller's `myreq_handler` per AC 10 — this spec's boundary extends into `src/controller/` for this one opcode, it is not scheduler-only), `.replay` capnp stream format, state snapshot/restore for replay efficiency, **interactive REPL** with single-step, breakpoints, state inspection, and snapshot capture via `--interactive`, and deterministic replay guarantee for debugging/tuning; **normal↔sim equivalence guarantee** (same config + same events → identical decisions regardless of mode, AC 7.1); `xceadmin scheduler dump ` for human-readable replay file inspection; documentation of picker plugin interface contract (hooks, cancel-safe, session-aware); unit tests for picker hook behavior, **event recording/replay, snapshot/restore, and deterministic replay**; HA soak regression for picker changes. - **Out of scope**: Allocation iterators and parjob allocator (→ `scheduler-allocation` spec); preemption policy and SIGSTP pipeline (→ `scheduler-preemption` spec); new job selection policies beyond the builtin FCFS picker — the pluggable picker interface enables them, but none ship in this spec beyond `builtin_fcfs`; hierarchical project resource quotas (deferred to `project-resource-ownership` spec — only `project_name` on queue and `xce_sched_project_t` scaffolding ship here); decision payload capnp schema for `DispatchDecision`/`PendDecision`/`PreemptDecision` (deferred to design phase — replay file format schema is in AC 20); distributed/cluster-mode simulation (v1 is single-scheduler replay only); visual inspection / UI tool for replay files (deferred — `xceadmin scheduler dump` provides human-readable output, `diff` of dumps is the primary comparison workflow for v1). @@ -78,6 +78,7 @@ The spec is implemented in three sequential phases. Each phase ships independent - Implement `on_decision()` callback — per-decision emission to driver (AC 6) - Build normal-mode driver: capnp → core events → decisions → xcecli (AC 8) - Replace today's `main.c` decision switch with driver callback +- Audit `kvmgr`-backed iteration sites (jobs, queues, users, projects) for order-stability and fix any found unstable (AC 7); hardcode `jbuf` capacity to 1 inside `xce_sched_core_init()`, do not expose it as a parameter (AC 7) **What does NOT change**: - Picker interface and `builtin_fcfs` plugin (Phase 1 already done) @@ -505,8 +506,8 @@ if (arguments.sim_file) { 6. **Decision output callback**: The core shall emit all scheduling decisions (`DISPATCH`, `PEND`, `PREEMPT`) through a `xce_core_decision_cb_t` callback registered by the driver at init time. Signature: `int (*on_decision)(void* driver_ctx, xce_decision_t* decision)`. This replaces the current internal `decisions` cbuf poll loop in `main.c`. 7. **Deterministic guarantee**: Given identical initial state and identical event sequences with identical timestamps, the core shall produce identical decision sequences. The core is multi-threaded (picker + allocator threads communicating via cbuf). Deterministic replay is achieved through: - - **cbuf capacity=1 serialization**: The `jbuf` cbuf between picker and allocator has capacity 1 (`cbuf_init(&jbuf, 1, sizeof(uint64_t))`). This acts as a rendezvous — the picker blocks on `cbuf_put` until the allocator takes the current jobid. Jobs flow through the pipeline one at a time in fully determined order. - - **Stabilized hash table iteration**: All hash table / key-value store iteration that affects scheduling output order shall be stabilized (sorted by key before iterating). + - **cbuf capacity=1 serialization**: The `jbuf` cbuf between picker and allocator has capacity 1 (`cbuf_init(&jbuf, 1, sizeof(uint64_t))`). This acts as a rendezvous — the picker blocks on `cbuf_put` until the allocator takes the current jobid. Jobs flow through the pipeline one at a time in fully determined order. **This capacity is a determinism invariant, not a tuning knob**: today's code sets it via `int njobs = 1;` with a `/* FIXME: make this configurable */` comment (`scheduler.c:384`). That FIXME shall NOT be resolved to any value other than 1 without re-deriving the determinism argument in this AC — raising `jbuf` capacity turns the rendezvous into a buffer, and the picker may enqueue jobs faster than the allocator drains them, making allocator-observed job order depend on scheduling/timing rather than on the event sequence alone. Phase 2 (Req 5 AC 1-8, core extraction) shall either hardcode this capacity to 1 inside `xce_sched_core_init()` or leave the `njobs` FIXME untouched; it shall not expose `jbuf` capacity as a driver- or config-supplied parameter. + - **Stabilized hash table iteration**: All hash table / key-value store iteration that affects scheduling output order shall be stabilized (sorted by key before iterating). **Phase 2 ownership**: as part of extracting `xce_sched_core_t` (Req 5 AC 1), the implementer shall audit every `kvmgr`-backed iteration site reachable from the scheduling loop (jobs, queues, users, projects — `scheduler.c:404-412`) and confirm each is already order-stable as a function of (keys, insert/delete sequence) with no process-random seeding; any site found unstable shall be fixed in Phase 2, before Phase 3 simulation work depends on it. This audit is a Phase 2 deliverable, not an implicit byproduct. - **PRNG for tie-breaking**: Any required randomness (e.g., tie-breaking) shall use a PRNG seeded by the driver at init time (`core->prng_seed`). - **Same events → same picker state**: Since `core_push_events()` is called between sessions (picker idle), the picker's internal data structures (`ordq_t queues`, `ordq_t pendJobs`) are populated identically given identical event sequences. The picker iteration then produces jobids in identical order. @@ -557,7 +558,10 @@ if (arguments.sim_file) { struct SchedAdminRequest $C.mapname("sched_admin_req_t") { action @0: SchedAdminAction; - recordFile @1: Text; # path for recordStart; empty = auto-generate + recordFile @1: Text; # filename (not a path) for recordStart; empty = auto-generate. + # See AC 10 "Record file path validation" — the scheduler + # resolves this beneath a configured recording directory; + # it is never used as a caller-controlled absolute path. } struct SchedAdminReply $C.mapname("sched_admin_reply_t") { @@ -598,10 +602,19 @@ if (arguments.sim_file) { The controller is a **pass-through** — it owns no recording state, interprets no fields beyond the opcode dispatch. All recording logic lives in the scheduler's normal-mode driver. In the new architecture, recording is a driver-layer concern — the core has zero knowledge of whether recording is active. + **Record file path validation**: `recordFile` is admin-authenticated but caller-supplied, and the scheduler process opens it for writing — it shall NOT be treated as a trusted absolute path. The scheduler shall: + - Read a configured recording directory from scheduler config (e.g. `record_dir`, default `/var/log/xce/`). + - Reject any `recordFile` value containing a path separator (`/`) or a `..` path segment — only a bare filename is accepted, never a path. + - Resolve the final write target as `record_dir/recordFile` (or `record_dir/` when `recordFile` is empty), and additionally verify via `realpath()` (or equivalent) that the resolved path stays within `record_dir` before `fopen()`. + - Reply `{ok=false, error="invalid record file name"}` if validation fails — no file is opened, no directory is created. + + This mirrors the "never trust external data" input-validation boundary applied elsewhere in this codebase (e.g. RBAC/JWT-gated config endpoints) — `recordFile` is user input crossing a process boundary (xceadmin → controller → scheduler) and is validated at that boundary, not assumed safe because the caller is already admin-authenticated. + **Edge cases**: - **Scheduler not connected** (`sched.ep == NULL`): Controller replies `{ok=false, error="scheduler not connected"}` immediately. No capnp round-trip needed. - **Scheduler disconnects mid-request**: The 5s timeout triggers; controller replies `{ok=false, error="request timeout"}`. - **Controller is not primary**: xceadmin's existing redirect-following logic handles this — if the contacted controller is not the primary, it redirects to the primary (same as `config push`). The scheduler is a singleton attached to the primary controller, so recording commands always reach the primary's scheduler. + - **Invalid `recordFile`** (path separator, `..` segment, or resolves outside `record_dir`): scheduler replies `{ok=false, error="invalid record file name"}` before touching the filesystem (see "Record file path validation" above). **xceadmin CLI surface**: ``` @@ -641,6 +654,7 @@ if (arguments.sim_file) { | Scheduler not connected to this controller | false | `"scheduler not connected"` | | Request timeout (5s) | false | `"request timeout"` | | Disk full / write error (start) | false | `"cannot write to : "` | + | `recordFile` contains `/` or `..`, or resolves outside `record_dir` | false | `"invalid record file name"` | Status output (xceadmin formats the capnp reply): ``` @@ -1092,8 +1106,14 @@ if (arguments.sim_file) { **Implementation note**: The REPL is a single `fgets` loop in the simulation driver. It is not a separate thread — when the REPL blocks on stdin, the simulation loop is parked. This is correct and intentional: the operator is in control, and the simulation has no wall-clock deadlines. +### Requirement 6: Object sharing forward-compatibility + +**Objective:** As a maintainer, I want the core's data-ownership patterns to leave room for future resreq/pend-reason object sharing, so that a later optimization spec can introduce sharing without retrofitting these invariants under load-bearing code. + Full object sharing (resreq template registry, pend reason dedup pool) is deferred to a follow-up spec. However, the architecture must not block sharing from being added later: +#### Acceptance Criteria + 1. **Resreq immutability**: Resreq objects shall be treated as immutable once created. No code path shall mutate a resreq in place. This ensures sharing via `const` pointer is safe to introduce later. 2. **Pend reason factory**: All pend reason allocation shall go through `make_pendreason()` (the single factory in `lib/preason.c`). Callers never `calloc(sizeof(xce_pendreason_t))` directly. @@ -1192,6 +1212,8 @@ typedef struct { xce_cluster_config_t cluster_cfg; dataq_t* pend_updates[2]; // Recording state (driver-layer only — core is unaware) + char* record_dir; // configured recording directory (AC 10 path validation); + // loaded from scheduler config at startup, default "/var/log/xce/" char* record_file_path; // strdup'd path for status queries (AC 10) FILE* record_file; // replay file handle (NULL when not recording) bool recording; diff --git a/.kiro/steering/roadmap.md b/.kiro/steering/roadmap.md index 25201541..16b04b8d 100644 --- a/.kiro/steering/roadmap.md +++ b/.kiro/steering/roadmap.md @@ -79,8 +79,8 @@ Smaller items that likely don't warrant a full spec; pick up opportunistically o - [ ] **investigate: job_ctrl direct-mutate path (admin suspend/resume) — migrate to commit-then-apply?** — **recorded 2026-07-13, surfaced during job-array-single-wal-record's `persist_job_hook_fun` investigation.** `raft-apply-decoupling` deliberately scoped `job_ctrl`'s `mark_job_update`/`mark_job_fail` (admin SIGCONT/SIGSTOP) OUT-OF-BOUNDARY (task 4.1/6.1 Implementation Notes) — confirmed the ONLY remaining live call site that still mutates state directly, THEN fires the `AFTER_EVENT` hook, with `persist_job_hook_fun` doing the real WAL append + async commit from inside the hook (not through `apply_committed_entry()`). This is the same mutate-before-commit shape as the bug-795/828/851/1060 divergence family raft-apply-decoupling was built to eliminate for job submit + worker facts, narrowly scoped to admin suspend/resume. Not yet known to have caused a live incident — rare, synchronous, admin-only traffic. **Investigate before committing to a fix**: (a) actual blast radius if a commit fails/times out post-mutation (does it ever get observed/notified before commit — violates criterion 2.4-style provisional-state-export concerns?); (b) live-apply vs replay-apply asymmetry for this one WAL entry type — does replay actually diverge from live in any reachable scenario, or is it benign in practice; (c) cost of migrating vs. accepted-residual status quo. Promote to a spec (likely a small `raft-apply-decoupling` follow-up, not standalone) only if the investigation finds real risk, not proactively. - [ ] **WAL-trim cursor-bound guard (bug-1147; surfaced 2026-06-25 by raft-drive-async design review)** — `trim_wal_segments` (`persist.c:1374`) bounds deletion ONLY by the open segment (`if (seg_start_lsn < active_seg_lsn) unlink()`) — it consults **no** standby `acked_lsn`, **no** `committed_lsn`, **no** in-flight snapshot boundary. With no working snapshot backfill (`write_snapshot` emits a 16-byte sentinel only), a standby lagging by more than ~one open segment hits a log range that exists nowhere → permanent committed-prefix gap (the bug-795/828 family). This is **strategy-A retention discipline that should exist regardless**. Stopgap fix: compute `floor = min(all standby acked_lsn, in-flight snapshot boundaries)` under `p->lock` and refuse to delete at/above `floor`. **Folded into `raft-drive-async` task 3.2** (which moves checkpoint/trim onto a concurrent lane, worsening the race) — but stands alone if that slips. Does NOT fix empty/far-behind join (cursor=0); that needs `entity-bearing snapshots` (strategy B, backlog). Related: bug-795, bug-828, bug-1060. - [ ] OP_heartbeat peers cross-validation — include a hash of the local `--peers` string in heartbeat requests; log ERROR on mismatch between candidates. Detects misconfigured nodes at election time instead of silently risking dual-primary split-brain. _(FUTURE_WORK.md §3.)_ -- [ ] **peer-authentication-hardening** — **NEW 2026-07-06 from admin-config-tool discovery** (`/kiro-discovery`). XCE currently operates under a **trusted-LAN model** — any CA-signed SSL peer can connect to the controller and perform ANY operation. Three gaps identified: **(a) Admin→Controller**: `validate_host()` is a stub returning success for all peers (roadmap § below). `OP_upgradeConfig` has no real authorization gate today. **(b) Worker/Scheduler→Controller**: workers and schedulers connect via `controller_connector.c` using the same TLS context as controller peers, with no role differentiation — a compromised worker could send `OP_upgradeConfig` or read full config via `OP_replConfig`. **(c) Cross-component trust**: no distinction between controller peers, admin tools, workers, and schedulers — they share the same X.509 identity pool. **(d) xceadmin login (future)**: xceadmin currently uses zero-config TLS (same as xce_test). Production deployments likely need an admin login requirement — token-based session (`xceadmin login → token → config push`) or password-per-command. X.509 CN alone proves identity but not intent; a login step provides audit trail, session expiry, and operator accountability. **Needs design**: role-based X.509 CN validation (controller/admin/worker/scheduler roles), per-role OP code allowlists, admin login/session model, audit logging for admin operations. Separate from the existing `validate_host()` hardening (below) which is the minimal gate for `OP_upgradeConfig` specifically. Discuss + design before implementing; likely promotes to a spec via `/kiro-discovery`. -- [ ] validate_host() real authorization — currently a stub returning success for all peers; MUST be implemented before `CONF_ENABLE_UPGRADE_CONFIG=1`. Minimum: verify peer X.509 CN against a configured allowlist (`--admin-cn` flag or `admin_cn_list` config), reject non-allowlisted `OP_upgradeConfig` with clear error, WARN-log rejected attempts with CN+IP. Future: IP allowlist, role-based access, admin-specific CA. _(FUTURE_WORK.md §6; security-critical.)_ **Part of the broader `peer-authentication-hardening` item above.** +- [ ] **peer-authentication-hardening** — **NEW 2026-07-06 from admin-config-tool discovery** (`/kiro-discovery`). XCE currently operates under a **trusted-LAN model** — any CA-signed SSL peer can connect to the controller and perform ANY operation. Three gaps identified: **(a) Admin→Controller**: `validate_host()` is a stub returning success for all peers (roadmap § below). `OP_upgradeConfig` has no real authorization gate today. **Same gap confirmed 2026-08-02 for `OPCode_schedAdmin`** (`scheduler-core` design, `cluster.c`'s `case OPCode_upgradeConfig` was checked as the closest precedent and has only a primary/standby routing gate, no RBAC/auth check) — `scheduler-core` ships `OPCode_schedAdmin` at parity with that existing gap (project owner decision: not a `scheduler-core` blocker), so this item now covers both opcodes; when `validate_host()` is implemented, wire it to both `OP_upgradeConfig` and `OPCode_schedAdmin` in the same pass. **(b) Worker/Scheduler→Controller**: workers and schedulers connect via `controller_connector.c` using the same TLS context as controller peers, with no role differentiation — a compromised worker could send `OP_upgradeConfig` or read full config via `OP_replConfig`. **(c) Cross-component trust**: no distinction between controller peers, admin tools, workers, and schedulers — they share the same X.509 identity pool. **(d) xceadmin login (future)**: xceadmin currently uses zero-config TLS (same as xce_test). Production deployments likely need an admin login requirement — token-based session (`xceadmin login → token → config push`) or password-per-command. X.509 CN alone proves identity but not intent; a login step provides audit trail, session expiry, and operator accountability. **Needs design**: role-based X.509 CN validation (controller/admin/worker/scheduler roles), per-role OP code allowlists (now including `OPCode_schedAdmin`), admin login/session model, audit logging for admin operations. Separate from the existing `validate_host()` hardening (below) which is the minimal gate for `OP_upgradeConfig`/`OPCode_schedAdmin` specifically. Discuss + design before implementing; likely promotes to a spec via `/kiro-discovery`. +- [ ] validate_host() real authorization — currently a stub returning success for all peers; MUST be implemented before `CONF_ENABLE_UPGRADE_CONFIG=1`. Minimum: verify peer X.509 CN against a configured allowlist (`--admin-cn` flag or `admin_cn_list` config), reject non-allowlisted `OP_upgradeConfig` **and `OPCode_schedAdmin`** (added 2026-08-02, `scheduler-core`) with clear error, WARN-log rejected attempts with CN+IP. Future: IP allowlist, role-based access, admin-specific CA. _(FUTURE_WORK.md §6; security-critical.)_ **Part of the broader `peer-authentication-hardening` item above.** - [ ] OP_replConfig access control hardening — currently serves the full config blob to any CA-signed SSL peer (trusted-LAN model). Consider: verify requester is a known `--peers` controller, rate-limit per peer, `minimum_role` check (controllers only). _(FUTURE_WORK.md §7.)_ **Part of the broader `peer-authentication-hardening` item above.** - [ ] lost-and-found drain command — `xcecli lost-and-found drain`: reassign or cancel sidelined jobs, reset `lost_and_found_job_count`. Automates operator cleanup after config upgrade. _(FUTURE_WORK.md §4.)_ - [ ] OP_upgradeConfig standby redirect verification — test that a standby returns the primary's address to an admin tool and the tool retries against the primary. Note: redirect semantics change under ha-consistency-invariants Req 1 (no transitional-state redirect); verify against the new behavior. _(FUTURE_WORK.md §5.)_