123 · T108 — the Kubernetes node runner and the two retry budgets - #133
Merged
Merged
Conversation
…disbelieved-shard path
…ifecycle port its kube backend
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
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: T108 — docs/implementation/123-T108-kubernetes-node-runner.md
Summary
K8sNodeRunnerimplementsNodeRunnerand drives the whole remote attempt —record → submit → await → read shard → replay →
TerminalState— through theunchanged driver. Submission is a bare Pod, write-ahead-logged before creation via a
SubmissionLogthat is the run's sequence authority, with the newattempt-submittedrecord landing in
dagr-artifactfor the@1.3kind. The two retry budgets are keptdistinct: pre-start failures spend
--dagr.pod-launch-retriesand consume nouser-visible attempt, while post-start failures spend
NodePolicy::retrieswith realbackoff.
--dagr.executor=k8sstill refuses at bootstrap — lifting that is T112's.Tests-first
Confirmed — failing tests committed first in 94ee107.
Definition of done
K8sNodeRunnerimplementsNodeRunnerand drives record → submit → await → read shard → replay →TerminalState, reporting through the existing hooks; no driver change —crates/cli/src/driver.rsand all ofcrates/core/are absent from this diff, and the seam is proven bya_mixed_pipeline_runs_end_to_end_through_the_unchanged_driver, which drives a realdrive()with one placed and one local runner.attempt-submittedwriter and record type land indagr-artifactfor the@1.3kind, with a fixture-corpus artifact parsed in CI.{ uri, content_hash }inputs, both fingerprints, tool version, image digest, and intended pod name; observed name, UID, and host are recorded after creation.inputsis an empty array for a consume-nothing source and has exactly the declared arity in positional order; an arity mismatch fails before launching.attempt-submittedrecord and stays byte-identical; the fold is unperturbed by streams that contain them.seqand fold cleanly; the orchestrator is the single writer.--dagr.pod-launch-retriesand consume no user-visible attempt; post-start failures consumeNodePolicy::retrieswith real backoff.Pending+ a known-fatalwaiting.reasonunder the runner's own 60s bound — not by awaiting a terminal phase (T101 proved none arrives for an unpullable image).OOMKilled/Evicted/Unschedulable/ImagePullBackOffare diagnostics; the terminal taxonomy still has nine members.(run_id, node, attempt); cancellation deletes pods inside the shutdown budget.Payloadpayloads is a compile error (trybuild).--dagr.executor=localremains byte-identical to a pre-M10 run.scripts/check-metastore-acceptance-boundary.shpasses.ubuntu-latestandmacos-latest— pending, confirmed by this PR's CI.GATE=PASS(35 PASS, 1 SKIP: coverage-matrix, already run as criteria-matrix, 0 FAIL), verified by the orchestrator on the committed tree, but CI on the PR is the authoritative verdict.Open questions resolved
The ticket's two, plus eight decisions the implementation forced, are recorded in the
ticket's
## Open questions — resolvedsection (docs/tasks.mdenumerates M0–M4 onlyand carries no T108 entry):
backoffLimit=0? → a bare Pod.AttemptEventSinkbuffers → aSubmissionLogthat is the run's sequence authority.--dagr.executor=k8sstop refusing? → no; that is T112's, not this ticket's.Payloadbound.NodePolicy::timeout→ the cancellable half of C14, because a remote wait really is cancellable.PodLifecycleget a real backend here or in T112? → here.Deviations
None.
Notes
The runtime-placement boundary established by T107 holds:
dagr-k8sstill carries notokio (or any async-runtime) edge, and
dagr-coreremains zero-runtime-dependency. Noscripts/check-*.shallowlist was widened and no assertion weakened — the only scriptchanges add assertions.
One rustdoc lint (
redundant-explicit-linksatcrates/k8s/src/lib.rs:19) was caughtby the local gate and fixed in
82b1a8ebefore this PR opened. The bare[PodLifecycle]shorthand is safe in every feature combination becausePodLifecycleis re-exported unconditionally at the crate root andmod apicarriesno
#[cfg]— unlike the genuinely gatedclientmodule, which is deliberately leftunlinked.