Skip to content

122 · T107 — the shared pod observer: one watch, reconnect, identity - #132

Merged
athvin merged 7 commits into
mainfrom
feat/t107-shared-pod-observer
Aug 2, 2026
Merged

athvin merged 7 commits into
mainfrom
feat/t107-shared-pod-observer

Conversation

@athvin

@athvin athvin commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Ticket: T107 — docs/implementation/122-T107-shared-pod-observer.md

Summary

One PodObserver per orchestrator process owns a single watch and demultiplexes
to per-attempt waiters — no per-pod watch exists. Implements T101's reconnect
discipline over the client's primitive list/watch calls: cause classification,
re-list-then-watch, bookmark handling, and stall detection, with a resync that neither
loses nor duplicates a terminal transition. Pod identity is selector-only labels
(≤63 chars) plus authoritative annotations, so a truncation collision is still
attributed correctly and a fingerprint mismatch is reported foreign. The client lives
in a new opt-in dagr-k8s crate behind a default-off feature, with a second default-off
feature quarantining the real client. No pods are ever submitted — this is an observer;
the executor is T108.

Tests-first

Confirmed — failing tests committed first in 89e1d07.

Definition of done

  • One PodObserver per orchestrator process owns a single watch and demultiplexes to per-attempt waiters; no per-pod watch exists.
  • The reconnect discipline from T101 is implemented: cause classification, re-list-then-watch, bookmark handling, and stall detection (90s); a resync neither loses nor duplicates a terminal transition.
  • Labels are ≤63 characters and selector-only; annotations carry authoritative identity; a truncation collision is still attributed correctly.
  • A pod whose annotated fingerprint does not match is reported foreign, not attributed.
  • Out-of-cluster (kubeconfig) and in-cluster configuration both work; neither present is an actionable error. Precedence: KUBECONFIG > in-cluster > $HOME/.kube/config; a named-but-absent file is an error.
  • The client lives in a new opt-in crate behind a default-off feature with no edge onto dagr-core; --no-default-features and cargo build --all pull no HTTP/TLS stack; deny.toml covers the new licences; core's runtime dependency set is still empty.
  • scripts/check-metastore-acceptance-boundary.sh passes unchanged.
  • Least-privilege RBAC (get/list/watch pods, one namespace) ships as a reviewed manifest.
  • The observer is torn down with the run and respects the shutdown budget.
  • Tests pass on ubuntu-latest and macos-latestpending, confirmed by this PR's CI (fake API surface; the real cluster test is T112).
  • CI is green on the ticket branch — pending. The local gate is GATE=PASS, verified by the orchestrator on the committed tree, but CI on the PR is the authoritative verdict.

Open questions resolved

Nine are recorded in the ticket's ## Open questions — resolved section:

  1. Fake API surface → a port the observer watches through, faked in-process, plus recorded frames through the real client's deserializer.
  2. Stall-detection bound → 90 seconds; a settable field, deliberately not an operator knob.
  3. Primitive calls vs the client's self-healing watcher → the primitive calls (see Deviations — this one has a licence consequence).
    3b. Where the async runtime livesdagr-cli, where ADR 004 already places it. dagr-k8s names no async runtime at all.
  4. Client quarantine → inside dagr-k8s, behind a second default-off feature.
  5. Node label: digest or truncation → a truncation; the ticket's own test is why.
  6. Cluster-access precedenceKUBECONFIG > in-cluster > $HOME/.kube/config; a named file that is absent is an error.
  7. What retires a waiter → a terminal phase or a disappearance, once, on the attempt key.
  8. Label/annotation key prefixdagr.io/.

Deviations

One, recorded in docs/implementation/DEVIATIONS.md (2026-08-01 · 122 (T107)).

The T101 spike's downstream-corrections list told T107 to add backoff to the client's
runtime::watcher and to allow Zlib in deny.toml. Two of those five are done
differently: the reconnect discipline is implemented over the client's primitive
list/watch calls, so kube's runtime feature stays off — and with it the
kube-runtime → hashbrown → foldhash path that was the sole justification for the
Zlib licence id. deny.toml therefore gains no new SPDX id; it gains the audit
that found none needed. The other three corrections (bound silence, reconcile by
periodic LIST, never resume from a 410's resourceVersion) are implemented exactly as
written.

The reasoning: T107's own DoD requires dagr to implement cause classification,
re-list-then-watch, bookmark handling, and stall detection. A self-healing watcher
consumes exactly the observations that classification is defined over — an expiry
arrives inside it as a decoded frame and is never re-emitted — so with it in the loop,
three of those four DoD clauses would be claims about somebody else's code with no way
to assert them. It also takes a concrete Api backed by a real client, so faking it
would mean standing up an HTTP fixture inside a project whose boundary claim is that it
opens no inbound port. cargo deny check passes on the new ~140-crate tree with the
allow-list unchanged, and cargo audit is clean.

Notes

A runtime-placement violation was caught by the local gate and fixed before this PR
opened: an earlier revision put tokio on dagr-k8s, which ADR 004 places on
dagr-cli (and ADR 097 §5 on dagr-metastore). It failed both
check-timeout-and-permit-adr.sh and check-async-runtime-adr.sh. It was resolved by
removing the edge, not widening the allowlist: PodWatch became a trait returning
an anonymous future, the kube adapter holds the client's Stream directly, the fake is
a queue behind a std::sync::Mutex with std::task::Wakers, and the task half moved
verbatim to dagr_cli::pod_observer behind the same default-off k8s feature with its
14 paused-clock suites intact. dagr-k8s's whole dependency table is now the four
optional client crates — no tokio, not even as a dev-dependency.

scripts/check-k8s-feature-gating.sh gained the positive half of that invariant
(the driver exists, is feature-gated, and is where the runtime is used), so a future
ticket cannot re-add tokio to dagr-k8s and delete the split silently.

athvin and others added 7 commits August 1, 2026 14:59
…discipline, and pod identity

Translates the ticket's Test plan into five suites plus a crate-graph check,
before any implementation. The crate skeleton (manifest, README, RBAC manifest,
an empty lib) lands with them so the test targets are discovered and fail on the
items they name rather than on a missing package.

- crates/k8s/tests/pod_observer.rs — the load-bearing suite, against a fake API
  surface: an in-stream 410 re-LISTs (never resuming from the 410's own
  resourceVersion) and delivers a gap terminal exactly once; a watch that stops
  delivering WITHOUT erroring is detected inside the stall bound and reconnected;
  repeated termination backs off with strictly increasing delays, is bounded, and
  surfaces a CLASSIFIED failure on the waiter rather than retrying quietly
  forever; a terminal delivered three times notifies the waiter once; two
  attempts of one node each receive only their own; a foreign structural
  fingerprint and an unidentifiable pod are reported, never attributed; a pod
  that vanishes during a gap is reported once; a transport end resumes from the
  bookmarked version while a 410 always re-lists; a transient 403 is retried.
- crates/k8s/tests/pod_identity.rs — every emitted label value is inside the
  63-character ceiling and syntactically valid for a 36-character run id and an
  85-character node name; two node names CONSTRUCTED to collide under label
  truncation stay distinguishable by annotation (the test that justifies
  annotations existing); annotations round-trip all six authoritative fields; a
  missing annotation, a missing label, a malformed attempt and a label that
  disagrees with its annotation are each a named error.
- crates/k8s/tests/cluster_access.rs — kubeconfig resolves out-of-cluster,
  service-account files resolve in-cluster, the precedence between them is
  asserted in both directions, neither present names every path and variable it
  probed, and an explicit KUBECONFIG that does not exist is refused by name
  rather than silently substituted.
- crates/k8s/tests/observer_shutdown.rs — the watch is torn down with the run and
  no task outlives it; teardown from mid-stall completes inside the budget; every
  outstanding waiter is closed; registering after shutdown is refused.
- crates/k8s/tests/rbac_manifest.rs — the shipped manifest grants get/list/watch
  on pods in ONE namespace, and nothing cluster-wide, nothing that writes, and no
  log/exec/portforward subresource.
- scripts/check-k8s-feature-gating.sh — the boundary a unit test cannot reach:
  no edge onto dagr-core or dagr-cli in either direction, the whole-workspace
  default AND --no-default-features resolutions compile NO HTTP/TLS stack, and
  the cli's k8s feature is default-off, optional, and non-vacuous.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The observer, in two halves that are separated on purpose.

`ObserverCore` is a DETERMINISTIC state machine: it takes an input and a
monotonic offset and returns the deliveries to route plus the next action. It
does no I/O, reads no clock, and spawns nothing, so every reconnect scenario is a
sequence of function calls. It implements the whole of T101's recipe:

  * A four-class termination taxonomy. An expiry arrives as a DECODED `type:
    ERROR` frame rather than as a transport failure, so a handler that only
    inspects transport errors misses every one; it is classified here, in the
    delivery path.
  * Re-LIST on an expiry, and never the resource version inside its message —
    that number is the watch cache's oldest retained bound, not the head, so
    resuming from it would skip or replay transitions. A stale version on a LIST
    is accepted and returns current state, which is why the re-list cannot lose
    anything.
  * Resume from the last version actually seen on a transport end. Bookmarks
    advance it (their point), and so does every delivered object; nothing is
    scheduled off their arrival, because their cadence tracks cluster activity.
  * Exponential bounded backoff with a classified failure at the end of it. An
    unwrapped watch loop issues hundreds of reconnects per second at an API
    server that is trying to come back; the alternative to a bound is an infinite
    quiet retry, which presents as a hung run with nothing in the record.
  * A stall bound. Silence from a broken watch is indistinguishable from silence
    from an idle cluster, and bookmarks are far too slow to close the gap, so the
    only liveness signal is a client-side bound that forces a reconciliation.

`PodObserver` is the task that wires that to a `PodApi` and a timer: ONE watch,
demultiplexed to per-attempt waiters. Delivery is idempotent on the attempt key
and not on the event, so a duplicate from the API, a repeat across a reconnect,
and a re-read during a resync all collapse to one notification. A pod that a
reconciliation no longer reports is delivered as vanished and retires its waiter
rather than leaving it to hang. The watch is a type that aborts its pump on drop,
and the observer aborts its task on drop, so "no task outlives the run" is a
property rather than a discipline.

Identity (`identity`): labels are lossy selectors, annotations are
authoritative. The node label is a sanitized TRUNCATION rather than a digest,
deliberately — a digest is equally irreversible but looks unique, and code that
reads a unique-looking label eventually trusts it. Truncation is visibly lossy,
which forces every path through the annotation, and it makes a collision
constructible so the test that justifies annotations can be written at all.

The port (`api`) is `list` + `watch`, the two primitive calls — the level T101's
taxonomy is written in terms of, and the level a fake can stand in for. `fake` is
that fake: expiry, silence, duplicate delivery and a queue of failures, all
scripted, deterministic, no cluster. `access` resolves out-of-cluster and
in-cluster as a pure decision with an actionable failure that names every path
and variable it probed.

`client` is the quarantine: the kube-rs adapter, behind a default-off feature, so
`cargo build --all` and `--no-default-features` compile no HTTP or TLS crate at
all. It installs the TLS crypto provider explicitly, because enabling a backend
does not install one and the omission surfaces as a panic on first handshake. Its
tests push RECORDED frames — including the verbatim 410 body the spike captured —
through kube's own deserializer and this crate's classifier, so the fake proves
the discipline and the frames prove the classification is aimed at real bytes.

`dagr-cli` gains a default-off `k8s` feature taking the crate with
`default-features = false, features = ["client"]`, so a pipeline binary never
links the fake. The executor that submits pods is still the recognized stub.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…surface

The ticket's two open questions are resolved in its own file with their
evidence, alongside six decisions the implementation forced.

  * The fake API surface is a PORT plus an in-process implementation of it, not
    an HTTP fixture — an HTTP fixture would stand up a listener inside a project
    whose boundary claim is that it opens none, and would test the client's
    recovery rather than dagr's. Recorded frames through the real deserializer
    keep it honest about the wire.
  * The stall bound is 90 seconds, from the spike's two measurements: above the
    worst observed idle bookmark gap (59 s) so an idle graph does not churn, and
    ~3.7x the worst measured cold-fan-out startup tail (24.2 s p99) so real work
    always produces something inside it. A settable field, not a flag — it
    becomes one only if the demo shows it needs to be.

Also recorded: why the discipline is built over the client's primitive calls
rather than its self-healing watcher, why the quarantine needed a SECOND
default-off feature inside the crate (a workspace member is built by `cargo build
--all`, so an unconditional client dependency would make the ticket's own
sentence false), why the node label is a truncation, the cluster-access
precedence and why an explicit-but-missing KUBECONFIG is refused by name, what
retires a waiter, and the key prefix.

`deny.toml` gains an AUDIT rather than a licence. The ~140-crate client tree was
checked crate by crate: every one resolves into the existing five-id allow-list,
no copyleft anywhere, and `cargo audit` is clean. The `Zlib` id the spike
predicted comes from `kube-runtime`, which is not in the tree because that
feature is off — recorded at the setting so the next ticket to reach for it finds
the cost written down. The divergence from the spike's correction list is in
DEVIATIONS.md with the reasoning.

CI gains three steps on the tier-1 test leg: the client suite, clippy over the
client (the default clippy pass never sees the one module a default build does
not compile — the gap that cost two earlier tickets a fix round), and the
crate-boundary script. The observer's own suites need no feature and so already
run on both tiers, which is where the ubuntu + macOS line is met.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…at gates it

`cargo doc --workspace --no-deps` runs with DEFAULT features, where the `client`
module does not exist — so an intra-doc link to it is a broken link in exactly
the resolution the quarantine exists to produce, and `rustdoc::broken_intra_doc_links`
is denied. The module index names it in backticks instead and says why. The
`[`PodApi`](api::PodApi)` form beside it resolved to the same destination as its
own label, which the denied `rustdoc::redundant_explicit_links` rejects.

Also records, at `note_progress`, the one consequence of clearing the failure run
on a successful list: a server that expired every watch immediately after the
list that fed it would re-list without backing off. It needs the watch cache to
turn over entirely between a list and the watch microseconds later, and the
alternative — not clearing on a successful list — would let an ordinary run's
isolated transient failures accumulate over hours until the elapsed bound tripped
on a healthy cluster. The likelier failure is the one that is prevented.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ErrorResponse::code` is a `u16`, so the defensive `u16::try_from(...)` around it
converts a type to itself — `clippy::useless_conversion`, denied. Found by the
clippy leg this ticket adds over the quarantined client, which is the whole
reason that leg exists: the default clippy pass never compiles this module.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s the runtime

`dagr-k8s` took a `tokio` edge, and ADR 004 places tokio in the crate that owns
the run loop — `scripts/check-async-runtime-adr.sh` and
`check-timeout-and-permit-adr.sh` express that as an allowlist over every
workspace member (cli, plus metastore per ADR 097 §5) and require every other
crate to justify a runtime edge or not have one.

The allowlist is not widened. The crate is split at the boundary its own module
documentation already described, and both checkers pass unchanged.

`dagr-k8s` now names no async runtime, and its whole dependency table is the four
optional client crates:

  * `PodWatch` is a TRAIT whose `next` returns an anonymous future, instead of a
    struct wrapping a `tokio::sync::mpsc::Receiver` and a `JoinHandle`; `PodApi`
    gained an associated `Watch` type.
  * the kube adapter holds the client's `Stream` directly rather than pumping it
    into a channel from a spawned task — which deletes a task, an abort-on-drop
    and the buffer that went with them, and keeps teardown a property of the type.
  * `fake` is a queue behind a `std::sync::Mutex` with `std::task::Waker`s. The
    silent stall — the scenario the ticket is built around — becomes its simplest
    case: a parked waker nobody wakes.
  * `ObserverCore` was already deterministic and is untouched.

`dagr_cli::pod_observer` is the task, behind the same default-off `k8s` feature:
the spawn, the stall/backoff timer, the `select!`, the command inbox, the routing
table, `PodObserver`/`ObserverHandle`/`AttemptWaiter`, and the fourteen suites
that drive all of it against the fake under a paused clock — moved verbatim, all
still green. `tokio/macros` rides on the `k8s` feature so the default surface
keeps its minimal feature set, and `test-util` is a dev-dependency so the shipped
binary never sees it.

"One watch per orchestrator PROCESS" is a property of the process, so the
singleton belongs to the process rather than to a library it drives; the library
half is now usable from any runtime. The DoD is met literally — it requires the
CLIENT to live in the new opt-in crate, and it does.

Also: the spawn inventory gains its row (and `crates/k8s/src` leaves it entirely),
CI runs the task half's suites and clippy on both tiers, and
`check-k8s-feature-gating.sh` gains the positive half of the boundary — that the
driver the split produced exists, is gated, and is where the runtime is used.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A new resolved open question (3b) in the ticket file: the runtime placement
decision, the alternative that was rejected (widening ADR 004's allowlist), what
moved, and why the split is better than the thing it replaced rather than merely
compliant. The stale claims that the crate's default surface "compiles with tokio
and nothing else" and that CI compiles the quarantined half two ways are corrected.

No DEVIATIONS.md entry: no DoD line is departed from. The DoD requires the CLIENT
to live in the new opt-in crate behind a default-off feature with no edge onto
dagr-core, and it does; it does not say which crate hosts the task.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@athvin
athvin merged commit 1c3ea47 into main Aug 2, 2026
16 checks passed
@athvin
athvin deleted the feat/t107-shared-pod-observer branch August 2, 2026 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant