diff --git a/crates/libs/api/AGENTS.md b/crates/libs/api/AGENTS.md index 63f165a4fb..fa4599fed3 100644 --- a/crates/libs/api/AGENTS.md +++ b/crates/libs/api/AGENTS.md @@ -1,6 +1,8 @@ # `pplx-space-api` -Before changing the substrate, read `src/controllers/substrate/AGENTS.md`. +Before changing the substrate, or shared code that branches on the substrate +(`Substrate::Direct`, `managed_by()`) or names a Direct-only table, read +`src/controllers/substrate/AGENTS.md`. ## Changes diff --git a/crates/libs/api/src/controllers/substrate/AGENTS.md b/crates/libs/api/src/controllers/substrate/AGENTS.md index 6865edcbcc..216e0f12bb 100644 --- a/crates/libs/api/src/controllers/substrate/AGENTS.md +++ b/crates/libs/api/src/controllers/substrate/AGENTS.md @@ -26,6 +26,63 @@ view doesn't carry, add it to the view and its conversion; don't add a second cache or fall back to a full-object GET to get it. Annotation and label reads go through `SandboxPodKey`, which is also the set of keys the conversion keeps. +## Direct Substrate Contract + +`DirectSubstrate` (`direct.rs`) exists for single-host deployments without +Kubernetes and for builds without the `kubernetes` feature. +`Substrate::from_env_with_lifecycle` returns Kubernetes only when built with +that feature and `SPACE_K8S_CONTROL_PLANE` is truthy; an unset, false or +unrecognized value selects Direct, and a Kubernetes init failure is an error, +not a fallback. Shipped images and Kind enable the feature; a plain +`cargo build -p pplx-space-api-svc` does not. + +What Direct does today (its `Unsupported` and no-op methods, admin routes, +Kubernetes-only reconcilers and known gaps) is in the `direct.rs` module doc; +update it with any change to Direct. Don't cite a cluster-controller +reconciler as the retry path for a failed Direct execution retire or repair; +the `direct.rs` module doc lists the Kubernetes-only loops. Don't change a +known gap as a side effect of other work: rejecting or implementing one +changes Direct behavior and belongs in its own PR. + +For new work: + +- Implement new lifecycle operations for Kubernetes. On Direct, return + `SubstrateError::Unsupported` unless the operation is one of the calls + `examples/first_sandbox.py` makes (create, command session, list/inspect, + delete). +- Make a new `SubstrateAdapter` operation a required method, so the compiler + demands each substrate's implementation and its arm in + `impl SubstrateAdapter for Substrate`; Direct returns + `SubstrateError::Unsupported` explicitly. A default would let a forgotten + arm give both substrates the default and skip the Kubernetes override. Only + a hook shared code calls on every path may default to "nothing to do", as + `retire_fence_mismatch` does, and it still needs its arm. +- Where shared code needs a static fact about the substrate, add a + `SubstrateCapabilities` field, set it in both `SubstrateCapabilities::DIRECT` + and `::KUBERNETES`, and read it through `capabilities()`; where the + substrate must act, add a required method (above). Don't match + `Substrate::Direct` or branch behavior on `managed_by()`. Matching + `Substrate::Kubernetes(k)` to reach Kubernetes-only state is fine. +- Scoping a query to the rows a substrate owns is required, not debt. In new + SQL, bind the substrate value instead of a `'k8s'`/`'space'` literal: + `managed_by = $n` with `.bind(substrate.managed_by())` for the running + substrate's rows, as the transient-lifecycle sweeper does, or a fixed + `SubstrateKind::Kubernetes`/`::Space` where code targets one substrate's + rows. When a fixed-scope statement may run under another substrate, return + early in Rust instead of also ANDing the running substrate's value, as the + sweeper's failed-active-execution cleanup does. Existing literals, such as + the shared `routable_execution_or_non_k8s` fragment, are debt. Never choose + a runtime endpoint from a row's label. +- `node_direct`, `sandbox_direct_route_identities` and + `sandbox_direct_route_provenance` belong to Direct. Outside tests, write + them only from `direct.rs` and the Direct node registry; read them through + an existing query that already joins them rather than adding a join. + archcheck's `pattern_ratchet.direct_tables` rule enforces this (advisory + today). +- Other existing substrate matches, Direct-table joins in shared code and + `Unsupported` trait defaults (`list_pool_configs`, `upsert_pool_config`, + `disable_pool_config`) are debt, not precedent. + ## Tests Kubernetes pod rendering is owned by `pplx_space_common_core::kubernetes`. @@ -35,3 +92,13 @@ restore, and pooled manifests in one place. Keep narrow tests only for behavior that is hard to express clearly in the broad YAML contract test. + +Use `Substrate::Direct` only to test Direct behavior (tests under `direct/`) or +behavior that genuinely doesn't depend on the substrate. Test shared lifecycle +behavior Kubernetes deployments depend on against a `Substrate::Kubernetes` +built with the `kubernetes::tests` helpers (`substrate_with_store` and its +variants), as `routes/public/sandboxes/admission_test_fixture.rs` does. +Adapter tests inside `kubernetes/` use `scripted_test_doubles.rs`, as +`adapter_characterization_tests/` does. Both compile only with +`--features kubernetes`: CI's `--workspace --all-features` run includes them, +but `cargo test -p pplx-space-api` silently skips them. diff --git a/crates/libs/api/src/controllers/substrate/CLAUDE.md b/crates/libs/api/src/controllers/substrate/CLAUDE.md new file mode 120000 index 0000000000..47dc3e3d86 --- /dev/null +++ b/crates/libs/api/src/controllers/substrate/CLAUDE.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file diff --git a/crates/libs/api/src/controllers/substrate/direct.rs b/crates/libs/api/src/controllers/substrate/direct.rs index c2463d9f57..fd4716b9b3 100644 --- a/crates/libs/api/src/controllers/substrate/direct.rs +++ b/crates/libs/api/src/controllers/substrate/direct.rs @@ -1,3 +1,52 @@ +//! The Direct substrate: [`DirectSubstrate`] dials each worker's node-api at +//! `node_direct.node_api_endpoint`, with no sandbox pods. The rules for +//! changing it are in this directory's `AGENTS.md`. Below is what it does +//! today: a selection, not a complete classification. +//! +//! # Operations +//! +//! - Create/delete, start, suspend/restore and wake go through +//! [`SubstrateAdapter`]. Stop, kill, pause/resume and checkpoint go through +//! the shared controller over Direct's runtime clients, which never recreate +//! an execution. +//! - Warm-pool claims adopt node-side; custom vcpu/memory or encrypted claims +//! go cold. Admission-time pool slot claims always miss. +//! - [`SubstrateError::Unsupported`]: node-pool placement, forks and cluster +//! pool configs. +//! - No-ops returning `None` or nothing: `sandbox_ip`, `access_sandbox_ip`, +//! `list_inventory_nodes` (Postgres is the inventory), +//! `retire_fence_mismatch` and `observe_pod_identities`. +//! `retire_execution` ignores its fence, but still stops the sandbox and +//! clears its routing. +//! - Direct-only: the node registry and the template pull and delete routes in +//! `routes/admin/node_maintenance.rs` (501 on Kubernetes). +//! - No drain or evacuation. After drain-state validation, admin `:drain`, +//! `:undrain`, `:cordon` and `:uncordon` return 502 on a Direct node; +//! `:drain` still commits `drain_phase='draining'`, which nothing advances. +//! +//! # Kubernetes-only reconcilers +//! +//! The cluster controller's `run_reconcilers` starts its drain, orphan, +//! failed-row repair and node-sync loops only for Kubernetes. The loops it +//! starts for both substrates include the public-operation dispatcher, which +//! retries transient failures of the create, lifecycle and delete operations +//! it executes. Direct reaches `retire_execution` only through the +//! transient-lifecycle sweeper's best-effort `cleanup_after_sweep`, and the +//! sweeper's failed-cleanup retry does nothing unless the running substrate is +//! Kubernetes, so nothing retries a failed Direct retire. +//! +//! # Known gaps +//! +//! Both succeed silently today: +//! +//! - One-shot create (`use_one_shot_create`) does not forward requested vCPU +//! or memory overrides: sbx-mgr's `CreateRequest` has no fields for them, so +//! autostart uses node-side defaults and memory policy while +//! `public.sandboxes` records the requested values. Disk size is forwarded. +//! - `retire_execution` never enqueues the stopped-disk backup that +//! [`SubstrateAdapter::retire_execution`] documents; the Kubernetes teardown +//! does. + use pplx_space_common_core::sql; use pplx_space_common_svc::sql::contention; use std::fmt; @@ -3768,8 +3817,8 @@ impl SubstrateAdapter for DirectSubstrate { let (_, _, mut client) = self.worker_client_for_sandbox(sandbox_id).await?; // Unlike `delete_execution`, the stop is the whole operation here, so - // its outcome must propagate: a failure keeps the caller's retry loop - // (e.g. the cluster-controller failed-cleanup pass) re-driving it. + // its outcome must propagate to the caller. Nothing retries a failed + // Direct retire: the failed-cleanup pass skips every substrate but Kubernetes. client .stop_sandbox(pplx_space_sbx_mgr_client::proto::StopRequest { sandbox_id: sandbox_id.to_string(), diff --git a/crates/libs/api/src/controllers/substrate/mod.rs b/crates/libs/api/src/controllers/substrate/mod.rs index 7669369fdf..5f1cc8437c 100644 --- a/crates/libs/api/src/controllers/substrate/mod.rs +++ b/crates/libs/api/src/controllers/substrate/mod.rs @@ -2194,7 +2194,8 @@ pub struct SubstrateCapabilities { /// fallthrough create takes a configured or policy pool shape, and /// otherwise vCPU and memory the request left unset come from the /// template's build shape. Without it, the requested shape goes to the - /// node unchanged. + /// node unchanged, except that Direct's one-shot create cannot carry vCPU + /// or memory (see the [`direct`] module doc). pub resolves_cold_create_shape: bool, /// Create requests may carry a Kubernetes node-pool placement pin. pub supports_node_pool_placement: bool, diff --git a/crates/libs/cluster/controller/server/AGENTS.md b/crates/libs/cluster/controller/server/AGENTS.md index 6cf48e63eb..2955f7d0b6 100644 --- a/crates/libs/cluster/controller/server/AGENTS.md +++ b/crates/libs/cluster/controller/server/AGENTS.md @@ -22,6 +22,12 @@ Every table named here is in the control-plane Postgres store. - Read a knob a deployment must change with a production `EnvConfig` reader (`src/env_config.rs`); its `test_*` readers return the default unless `test-hooks` is compiled in. +- Scope SQL to a substrate's rows with `managed_by = $n`, never a + `'k8s'`/`'space'` literal, bound from `substrate.managed_by()` when it scopes + to the running substrate's rows, or a fixed `SubstrateKind` when it targets + one substrate (the Kubernetes arm of `run_reconcilers`; + `tombstone_retention`'s Direct tombstones). For what Direct lacks, see the + [Direct substrate contract](../../../api/src/controllers/substrate/AGENTS.md#direct-substrate-contract). - `stuck_container_creating_gc` reads the `orphaned_sandboxes` and transient-lifecycle sweeper configs and runs only when both would retire the rows of the pods it deletes; changing those readers changes its gate. diff --git a/crates/libs/internal/api/server/AGENTS.md b/crates/libs/internal/api/server/AGENTS.md index 5a98bcc91a..743f2d9cf6 100644 --- a/crates/libs/internal/api/server/AGENTS.md +++ b/crates/libs/internal/api/server/AGENTS.md @@ -2,6 +2,10 @@ Every table named here is in the control-plane Postgres store. +Before changing code that branches on the substrate or names a Direct-only +table (`node_direct`, `sandbox_direct_route_*`), read the +[Direct substrate contract](../../../api/src/controllers/substrate/AGENTS.md#direct-substrate-contract). + ## Changes - New ClusterApi and NodeInfo RPCs rely on the auth layers in `src/run.rs`: