127 · T112 — M10 end-to-end demo, RBAC, and acceptance gate - #137
Merged
Merged
Conversation
…ptance gate The M10 milestone gate, written first. Five suites, covering the deferrals T108 and T109 both routed here plus this ticket's own Definition of done: * crates/k8s/tests/rbac_orchestrator_manifest.rs — the complete least-privilege grant (create, delete, get, list, patch, watch on pods in ONE namespace and nothing else), and that the observer's read-only manifest is not widened. * crates/k8s/tests/rbac_missing_verb.rs — a missing verb is classified and NAMED, from the platform's own message when it says and from the attempted call when it does not; a non-403 is not misreported as one. * crates/cli/tests/m10_acceptance_gate.rs — the boundary proof on the default feature set: core's runtime dependency set is empty, no build that did not ask for remote execution compiles an HTTP/TLS, Kubernetes or S3 stack, the reserved open modes are still stubs, the taxonomy still has nine members and three trigger rules, both matrices are still complete, and the demo runs locally with no cluster and no warning about one. * crates/cli/tests/m10_remote_execution.rs — the wired run path behind the default-off `k8s` feature: the lifted bootstrap refusal and the guard that replaces it, a placed node running end to end with its declared size on the pod, adoption on restart instead of resubmission, dual-mode parity, and the three RBAC failure modes. * crates/cli/tests/remote_execution_docs_claims.rs — the README and cookbook remote-execution sections say what ships and claim nothing unshipped. All fail: `dagr_k8s::rbac`, the orchestrator manifest, `dagr_cli::remote`, `RunnableFlow::run_placed`, the example, the gate script and both docs sections do not exist yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The grant an orchestrator that actually SUBMITS work needs, written down once and asserted three ways so the manifest, the code, and the diagnostic an operator reads cannot drift. * `dagr_k8s::rbac` — `PodVerb`, a CLOSED six-verb set (create, delete, get, list, patch, watch) on one resource, and the pure classification that turns a 403 into a sentence naming the missing verb and the manifest that grants it. It reads the verb out of the platform's own `cannot <verb> resource "pods"` clause and falls back to the attempted call only when the server did not say, because a guess about which line to add to a Role is worse than no guidance. Three entry points, for the three shapes a refusal actually arrives in: a typed `ApiFailure`, a rendered `LaunchExhausted` string (the launch budget keeps the words, not the type), and a terminated observer (which retries a 403 as transient and needs the actionable sentence when its budget is spent). The text form is deliberately strict — a marker AND a named verb — so a quota rejection is never misreported as a permission problem. * `crates/k8s/manifests/dagr-orchestrator-rbac.yaml` — the reviewed ServiceAccount + namespaced Role + RoleBinding. Every absence is load-bearing and commented: no `update` (a full replace races the platform's own status writes), no `deletecollection`, no `pods/log`, no `pods/exec`, no jobs (the platform must not retry alongside dagr), no configmaps or secrets (payloads travel through the blob store, never the API server), nothing cluster-scoped. T107's read-only observer manifest is untouched and still ships beside it — the read and write halves are separate ports precisely so they can be separate grants. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
T105 refused `--dagr.executor=k8s` at bootstrap, and T108 and T109 both recorded the same reason for leaving it refusing: lifting it needs the flow-level wiring that turns a `RunnableFlow`'s placed nodes into `K8sNodeRunner`s. This is that wiring. * `RunnableFlow::run_placed` — one discovery pass (T109's deferral, now in the run path rather than in a function a test calls), one watch for the whole run, a runner map that is a `K8sNodeRunner` per placed node and the flow's own local runner for everything else, then ONE `RunPlan` and ONE `drive()`. No driver change: `NodeRunner` was already the "where does this node run" seam, which is ADR 115's central claim, now exercised by the shipped path. * `crate::remote` — the operator-stated `RemoteTarget` (namespace, image, digest, blob container: deployment facts, deliberately not `--dagr.*` knobs), the two-port `RemoteCluster`, and `Pinned`, which forwards every cluster call onto the observer's runtime. A tokio I/O resource belongs to the reactor it was created on, and a client built for the observer then polled on the driver's task runtime is a failure class that only shows up against a real API server; pinning removes the class instead of documenting it. * The RETURN half of the data path. A local runner fills its output slot as the last thing an attempt does; a placed one cannot, because the value was produced in another process. So `run_placed` keeps each node's captured codec — the same `fn` pointer `prepare_attempt`'s `fill_input` uses, so the two directions cannot disagree — and the reporting wrapper fetches the produced bytes out of the blob container and decodes them into the slot. A fetch or decode that fails FAILS the node: a consumer reading an unfilled slot is a framework-defect panic, so "succeeded but nothing downstream can use it" is not an outcome worth returning. * `crate::remote_guard` — the refusal that replaced T105's. Lifting the bootstrap refusal did not lift the reason for it, so the check moved to where it can be precise: `ensure_available` now answers "did this build compile the executor in at all", and the guard answers "was this run given a cluster to place its placed nodes on". A pipeline with NO placed node runs fine under a remote executor — there is nothing to place, so nothing to substitute, and refusing it is theatre. A placed node with no codec is refused too, by name. * `DiscoveryReport` now keeps WHY a pod could not be claimed, so a caller can classify it: a missing `patch` grant reads very differently from a transient 500, and only one of them is worth telling an operator to fix. The suite drives T107's fake API surface, and its shard fixture now really stores the encoded value — a fixture that recorded a reference to bytes nobody wrote would assert the return half of the data path away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n docs The capability proof an operator can read, and the boundary proof that keeps ADR 115's carve-out at the width it was granted. * `crates/cli/examples/placed_pipeline.rs` — one pipeline, one node with a declared size (500m / 512Mi), runnable from the SAME binary under both executors. Locally it runs in-process with no cluster and, asserted by the gate, no warning about one; under `--dagr.executor=k8s` the example supplies the cluster itself, because a namespace and an image digest are facts about how a binary was DEPLOYED and do not belong in the same tier as its runtime knobs. Indexed in the examples README with the exact invocation a reader copies. * `scripts/check-m10-acceptance-boundary.sh` — the structural half, on ADR 097's precedent: crate-graph and diff facts a unit test cannot reach. Core's runtime dependency set is empty at every setting; a build that did not ask for remote execution compiles no HTTP/TLS stack, no Kubernetes client and no object-store transport, WITH a non-vacuity leg proving the same query does see them under `--features k8s` / `--features blob-s3`; the pod path links no metastore and carries no database credential (and the credential scan is narrow on purpose — the pod spec builder holds a list of credential markers it REFUSES, and a scan that read those as leaks would assert the opposite of the invariant); the reserved open modes are still `ModeNotImplemented`; the M9->M10 shipped-source diff adds no listener, server framework or scheduler type; the taxonomy is nine states and three trigger rules; both matrices are complete; the shipped Role is still namespaced and still six verbs. Composes the T107 and T104 gating scripts rather than restating them, and is wired into CI beside T88's M7 sibling, which still passes unmodified. * README `## Remote execution` and cookbook `## Placing a node on remote compute` — when to reach for it, the measured latency (0.9 s co-located to 2.3 s across a network, and a double-digit-second p99 for a cold fan-out), the shared-volume versus object-store choice, the six-verb manifest an operator applies, and the policy-not-execution-class rule that makes a resume proceed. Every flag, environment variable and default is compared against the shipped constant by `remote_execution_docs_claims.rs`, and the forbidden-claim scan fails if either document starts promising a scheduler, a control plane, or cluster-side retry. * The ticket's open questions are resolved and recorded in its own file, including the one that is NOT resolved: the shipped pod spec has no volume field, so a pod cannot be given the blob container it must write into, and the cluster half of the test plan is unimplementable without faking it. That is T108's mechanism to add, and this ticket's Out of scope forbids adding it here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* **Resume in both directions.** The DoD's dual-mode line asks that a run started under one executor resume under the other, and the claim is decidable with no cluster and no `k8s` feature — which is itself the thing being asserted. Placement feeds the POLICY HASH and never the STRUCTURAL FINGERPRINT, so `moving_a_node_between_local_and_placed_is_a_policy_diff_resume_proceeds_through` builds the same graph twice (same names, types, edges and registrar; only the placement differs), pins that the structural fingerprints match and the policy hashes do not, and drives `resume_verb` over the prior artifact in BOTH directions — asserting it proceeds with a printed diff rather than refusing. That is ADR 115 §7's payoff stated as a test rather than as prose. * **The spawn inventory.** `crates/cli/src/remote.rs` adds five spawn lines and every one of them is `Pinned` forwarding a cluster call onto the observer's runtime and awaiting it immediately — the spawn exists to put the call on the reactor its client was built on, not to make it concurrent. Recorded with that reason, because a spawn nobody can account for is the failure this inventory exists to catch. * Clippy under `-D clippy::pedantic` over the `k8s` leg: `RemoteWiring::runner` took nine arguments, so its per-node half is now a `PlacedNode` struct (the run-wide facts were already on the wiring, which is the boundary a reader has to know anyway); `run_placed` documents the two framework-defect panics it can produce; and the suite's hand-written `Task` impls use `async fn`. * The coverage matrix's SL7 note records that M10's no-server half is now held structurally by the acceptance-gate script as well as behaviourally by T65's test, and that M10 introduced no new numbered criterion — a claim the gate itself asserts rather than merely stating. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The doc said `Ok(())` covered "nothing to land"; it never did — the fill is reached only after a success, so a shard that recorded no output reference is an error. A node that succeeded and produced nothing its consumers can read has not, from the graph point of view, succeeded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two workspace conventions the new module had not met, both caught by the gate. * **Every production lock states its poisoning policy.** `Diagnostics`'s two sites are **recover**, and the reason is the point: this list is a report, not run state. A node whose attempt panicked is exactly the run that most needs its diagnostics printed, so a poisoned mutex must not escalate into a second panic and take the report down with it. * **Cross-feature intra-doc links.** `driver.rs`, `executor.rs` and `remote_guard.rs` ship in EVERY build and were linking into `crate::remote` / `crate::k8s_runner`, which a default build does not compile — so `cargo doc --workspace` (default features, `-D warnings`) broke on four unresolved links. They are now named in backticks rather than linked, with the reason stated at the one site a reader would otherwise wonder about: a link that only resolves under a non-default feature is broken in exactly the documentation most readers generate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each of these was demonstrated by planting a violation that stayed green. Every one is re-planted and confirmed to FAIL before this lands. - `crates/k8s/src/client.rs::pod_object` is the only translation of a `PodSpec` into a real `Pod`, and had no test at all. It now has two: the emitted container carries no env, envFrom, volumeMount or volume device, the pod spec no volumes, no service account and no pull secret — asserted field by field AND over the serialized object, so a future field that grows one of those surfaces fails without the test being extended. A second test pins what the object must carry, so the absences are absences. - The acceptance-boundary script scanned `executor.rs` alone for environment/secret plumbing and for credentials, so an `EnvVar` planted in `pod_object` kept both assertions green. The scan set is now every file under `crates/*/src` that references `k8s_openapi`, discovered rather than hand-listed, with the test half of each file excluded (a scan that read a gate's own absence assertions as violations would fail on its coverage). - The variant-count regex could not see a data-carrying variant, so `Displaced(String)` and `AnyOf(u8)` were invisible to a check whose own comment claimed it alone catches a tenth variant. It counts all three declaration forms now. - The RBAC assertions missed `verbs: ['*']` (single quotes), a second rule, and a `roleRef` pointing at `ClusterRole/cluster-admin` (the `^kind:` anchor cannot see an indented one). Wildcards are matched in any quoting, `kind:` anywhere on a line, and a deny-list is joined by an allow-list: exactly one rule, one resource list, one verb list, each spelled exactly. - The credential-refusal claim was a `grep -q` for a function NAME, so inverting that function's body to `return Ok(())` kept it green. It runs the behavioural test instead. - The reserved-seam scan missed libSQL's actual remote API, so `libsql::Builder::new_remote(..)` passed. The builder constructors are named now. - "The pod path links no metastore" scanned three hand-picked files, which a wrapper call defeats. The claim is narrowed to what it checks — a scan of NAMES, never a link-graph fact, since the pod re-enters the same binary — and joined by a pin of the crate-internal module edges the pod path reaches, so a new edge is a failure a reviewer clears by re-pinning. - The M9->M10 diff base fell back to `main` when the M9 marker was unreachable, silently narrowing the window while the non-vacuity guard still passed. That is a setup failure now and says so. The diff also ran `base..HEAD`, so a violation in the working tree was invisible; it runs against the tree, and the whole-tree scan runs the full pattern so an untracked file is covered too. Two alternation gaps closed: a Unix-domain listener, and a scheduler type whose name does not end in `Scheduler`. - The docs' RBAC verb loop matched substrings, so `get` was satisfied by "budget" and `delete` by the `deletecollection` in the section's own list of what is NOT granted. Two of six verbs were vacuous; all six are delimited tokens now. The remaining assertions in this commit fail, deliberately: the cookbook's pod-storage choice, the README's demo invocation, its latency attribution and the missing-verb provenance are claims the code does not support. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The ticket file and the test-module docs were honest; the user-facing docs
were not. Each of these was a claim the code cannot back.
- The cookbook offered the operator a *choice* of pod storage — an RWX volume
"mounted at the same path in the orchestrator and in every pod", or the
`blob-s3` backend — and neither is wireable. The pod side writes through a
local filesystem path and `exec-node` refuses any reference naming another
backend; the pod spec has no volume, volumeMount or environment field to
attach anything with. Offering a choice that cannot be made frames a
blocking defect as operator provisioning. The section now states the gap,
names its owner, and describes the choice as the shape the seam will take.
The README repeated the same framing ("a shared volume or an S3-compatible
bucket") and now says one of the two things an operator needs is missing.
- The README's "~0.9 s co-located, ~2.3 s across a network" read as a measured
property of the shipped executor. They are the M10 measurement spike's
numbers, for its own client, against code that has since been deleted — and
the shipped executor has never been run against a cluster. Attribution
restored, and the network figure corrected to what was measured.
- The README's `--dagr.executor=k8s` invocation omitted the four `DAGR_DEMO_*`
deployment facts the example requires, so as printed it exits
`BootstrapFailure`, contradicting the examples index. The invocation is
complete, and a test reads the variable list out of the example's own source
so a rename fails rather than drifting.
- The CI comment pointed at a `remote-cluster` workflow and a "cluster job"
that do not exist; `.github/workflows/` holds one file. It now says the
cluster coverage exists nowhere, and why.
- "Remove one and the failure names the missing verb" was backed only by a
string the tests write themselves, and the fixture claimed to be "recorded
verbatim" from a real API server with nothing backing that provenance. Both
now say what they are: one pinned fixture of the documented denial shape,
built identically on both sides, never recorded from a live cluster.
- `a_placed_pipeline_runs_end_to_end_…` quoted the ticket's "read back from
the API" while the test itself calls `write_shard()` to put the pod's shard
and output into the orchestrator's own blob directory. Renamed to what it
proves, with the gap between the two spelled out. The module doc's "a real
cluster would not make them stronger" is false for the adoption test, where
pod identity is a test-controlled struct; each claim now carries its own
strength.
- The ticket's §Open questions asserted that a `kind` `extraMounts` directory
is "a real shared filesystem across every pod". It is not: extraMounts
mounts into the NODE, and a hostPath volume in the pod spec is still
required — which is the seam that does not exist. Corrected, and the blanket
"both boxes are blocked" narrowed: adoption against real pods needs no
shared container and is provable today, while "runs to completion",
"terminal states" and "executed exactly once" from the artifact are what
genuinely need the missing seam. The cluster job stays deferred either way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The example pointed at the cookbook for "the shared-volume versus object-store choice", which no longer exists — and it is the one file a reader reaches for the remote invocation, so it should say plainly that `DAGR_DEMO_BLOBS` is a path only the orchestrator side can see today, and that the local half of the demo is entirely real. Also softens the cookbook's "runs end to end against the in-process API fake" to what the test does: drives to completion with the test supplying the shard a pod would have written. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…stub `cargo test --workspace --all-features` compiles the default-off `k8s` feature, and this branch made `ensure_available` succeed there. Two assertions in the T105 suite still read the build-refusal message, which that build never renders: one expected `Kubernetes.ensure_available()` to be an error, the other expected the `run` verb's own output to name the ticket. Both are true only of a build that compiled no remote executor, so they are gated to it; the run verb's exit code — the contract an operator's shell reads — stays asserted at both settings, as does "a placed pipeline with the remote executor selected runs no node here". With the feature on, the refusal is the placement-wiring guard's and goes to stderr, and `tests/m10_remote_execution.rs` covers it. The local executor's availability moves into its own test so it keeps running at both settings, and the file's header says which refusal belongs to which build rather than calling the executor a stub. Reproduced before and after with `RUSTFLAGS=-D warnings cargo test -p dagr-cli --all-features --test placement_and_executor_selection` (2 failed -> 17 passed); `run_gate.sh ci 127` is GATE=PASS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket: T112 — docs/implementation/127-T112-m10-acceptance-gate.md
Summary
Closes M10: the placed-pipeline demo running under both executors, the remote
executor wired into the run path (discovery, one watch, runner map, the value's
return trip into its slot), the least-privilege orchestrator RBAC, and
scripts/check-m10-acceptance-boundary.shwired into CI as the milestone'sacceptance gate.
Two of the twelve Definition-of-done items are NOT met and are recorded as
blocked in the ticket's Open questions rather than papered over. See below —
this PR is not ready to merge without an operator decision.
Tests-first
Confirmed — failing tests committed first in
df920d3.Definition of done
Met:
Role(pods: create, delete, get, list, patch, watch) +RoleBinding, noClusterRole, no wildcards, no secrets, plus the 403→verb classifier and the checked-in manifest.scripts/check-m10-acceptance-boundary.shruns in CI and enforces the M10 boundary invariants.Not met — blocked on a missing mechanism seam:
Why:
crates/k8s/src/executor.rs::PodSpechas no volume field andcrates/k8s/src/client.rs::pod_objectemits a container with image, command,resources, restartPolicy, nodeSelector and tolerations — no
volumes, novolumeMounts, noenv.crates/cli/src/exec_node.rs:304refuses any blobbackend other than
file, and output/shard writes are hardcodedLocalFsBlobPathBuf. A real pod therefore has no way to hand its result back. Addingthat seam is T108 mechanism work, which this ticket's Out of scope forbids.
Scoped precisely: adoption is provable against real pods today
(
crates/cli/src/adoption.rsuses onlyapi.list+patch_labelsand nevertouches a shard). What genuinely needs the missing seam is "runs to
completion", "terminal states", and "executed exactly once" evidenced from the
artifact.
Open questions resolved
kind load docker-imageremoves the registry need, and a fresh cluster has headroom. Recorded in the ticket's Open questions.--features k8sremote suite on both tiers.kindextraMountshost directory gives "a real shared filesystem across every pod". It does not —extraMountsmounts into the kind node; a hostPath volume in the pod spec is still required, and that seam does not exist.Deviations
None. The two unmet items are blocked, not departed from; no DEVIATIONS.md
entry was fabricated for them.
Review note — gate holes found and closed
An adversarial pass planted violations against every assertion in the new gate.
Eight stayed green and were fixed, each re-planted afterwards to confirm the
check now fails:
executor.rsonly, soclient.rs::pod_object— the sole translation into a realPod, and until now with zero test references — was unscanned. A plantedDAGR_INDEX_URL=libsql://index:token@…env var passed both. The scan now covers every file that builds a Kubernetes API object, andpod_objecthas typed field-by-field tests.Displaced(String),passed).verbs: ['*'](single-quoted), a second rule grantingpods/status+serviceaccounts+endpoints, and aroleReftoClusterRole/cluster-adminall passed.reject_credential_bearingwas a name-presence grep; inverting its body toreturn Ok(())passed. It now runs the behavioural test.libsql::Builder::new_remote).mainwhen the marker was unreachable, and scannedbase..HEADrather than the worktree.getwas satisfied by "budget" anddeleteby "deletecollection" sitting in the forbidden list.Doc claims were corrected too: the README and cookbook had offered "a shared
volume or an S3-compatible bucket" as the operator's payload choice, which
framed the blocking defect above as provisioning;
ci.ymlreferenced aremote-clusterworkflow that does not exist; and the README'sk8sinvocation omitted the
DAGR_DEMO_*variables the example requires, so asprinted it exited
BootstrapFailure.