Draft : E2e/recording v2 - #193
Draft
AviadHayumi wants to merge 36 commits into
Draft
Conversation
Introduce the two-sided conformance testing for the Karta library.
test/conformance: the offline replay engine. TestGolden replays every recorded
workload CR through the current library and fails if a reading changed;
TestRecordsEveryDistinctCR guards that every distinct CR is recorded, not one
per state, so an intermediate reading change is caught. Replay is the one read
path shared with the recorder, so record and replay can never diverge. The
package's own unit tests live in *_internal_test.go, apart from the golden guard.
test/e2e: the live recorder (a separate Go module so its cluster deps stay out of
the published library). observeTransitions watches a real workload from its
creation resourceVersion with a RetryWatcher, judges each state from the
workload's own fields (never Karta, to avoid circular validation), and records
every distinct settled CR plus what Karta reads. Files split by role: suite /
predicates / runner / recorder, with the offline recorder unit tests in
recorder_internal_test.go. The transition order is verified in both make test-e2e
and make record-e2e; only the latter writes fixtures.
Pod is the worked example (a built-in, so a reviewer needs no operator install):
Initializing -> Running -> {Completed, Failed}, recorded and replayed offline
against docs/catalog/core-pod-v1.yaml. Further workload types land in follow-up
PRs, each adding a cases_<type>_test.go, testdata/<type>/, and fixtures/<operator>/.
Refs #137, #139, #141
Signed-off-by: aviadh <aviad.hayumi@gmail.com>
BatchJob and JobSet cases, each with running, completed, failed, and resumed flows, recorded against a live cluster. Pull the case registry into cases.go so each cases_<type>.go defines one case. Resume via a merge patch so the action does not race the controller's reconcile. Document online, offline, and regolden in the README. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Add 11 operator cases (LeaderWorkerSet, RayCluster, RayJob, PyTorchJob, MPIJob, KnativeService, KServe, Milvus, Grove, Dynamo, NIMService) alongside pod/batch-job/jobset, recovered from the earlier suite and recorded against a live cluster; register them in cases.go. Correct the mpijob and milvus kartaNames to match their defs, and move the mpijob workload to kubeflow.org/v2beta1 (served by the mpi-operator). Declare reasonFieldName explicitly in the lws/pytorch/milvus/nimservice defs; they relied on the CRD default, which the offline replay cannot apply, so the recorded reason diverged from replay. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Replace the sanitized-full-CR-per-snapshot fixture with a single recording.yaml per flow: the first CR in full, a merge-patch (RFC 7386) per state change, and the state each reaches. The recorder dedups on the classified state (read from the workload's own fields), so no denylist is needed to tell a real change from resourceVersion churn - sanitize.go, replay.go, golden_test.go, and hack/regolden are removed. The offline TestTransitions rebuilds each CR from first-CR + patches, checks Karta reads the recorded state at every step, and checks the sequence is legal (terminals only at the end, ending at want). Merge-patch is implemented inline (no new dependency). Re-record all 14 cases into the new format. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
…, restore regolden Each recorded step now stores what Karta read of the CR (matched statuses, phase, conditions, per-component extraction) next to the CR, both as a first value plus a per-state merge-patch, so the file holds only what changed. TestGolden rebuilds each CR and reading, re-runs the current Karta, and checks it matches the recorded state (judged from the workload's own fields), that its reading has not drifted, and that the transition is legal. No sanitize: the golden rebuilds the exact CR, so Karta reads the same bytes back and the diff is stable without a denylist. - Flatten fixtures to <operator>/<version>/<definition>/<flow>.yaml. - The e2e run no longer checks Karta live; it only records. Karta is asserted only offline, against the fixture. - Restore hack/regolden (make regolden) to refresh the stored reading offline after an intended library change; the own-fields anchor still fails if Karta stops reading a recorded state, so a refresh cannot hide drift. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
…d failed FLOW="<name>" focuses one flow by its spec title, so a record can target a single operator's single flow - make record-e2e WORKLOADS="kubeflow" FLOW="completed" - the fast loop while adding cases. Add the completed and failed flows to the PyTorchJob case (its definition already maps Succeeded and Failed), keeping the running flow as is, and add the Degraded status const for the replicated-workload cases to come. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
…d flows mpijob failed (launcher exits non-zero, backoffLimit 0 -> Failed), raycluster suspended (spec.suspend true -> Suspended, no pods), and rayjob failed (failing entrypoint, backoffLimit 0 -> Failed). Each definition already maps the state; only the flow, its manifest, and the recording are new. The running/completed flows are untouched. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Each definition maps a not-yet-ready state (grove availableReplicas < replicas, dynamo phase pending, nim state NotReady). A workload pointed at a nonexistent image stays there, so Karta must read it as Initializing, not Running. The running flows are untouched; only the new initializing flow and its manifest are added per operator. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
A Milvus pointed at external nonexistent etcd/storage with a bad image holds status.status=Pending and creates no in-cluster helm dependencies (nothing to leak), so Karta must read it as Initializing. The running flow is untouched. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Add a position-driven recorder path for journeys that gate a step on the workload's own fields (step.settle): it captures one CR per step, so a journey can list the same state twice and record each. A scale flow is Running -> Running -> Running gated by replicasReady at 1, 3, 1, and the golden diffs each reading because the extracted scale changes. The default dedup path (one CR per state change) is unchanged for every existing flow. Adds scaleReplicas (merge-patch spec.replicas), replicasReady/fullyAvailable predicates, and a built-in Deployment case that scales 1 -> 3 -> 1. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Signed-off-by: aviadh <aviad.hayumi@gmail.com>
…end) Signed-off-by: aviadh <aviad.hayumi@gmail.com>
…uspended) Signed-off-by: aviadh <aviad.hayumi@gmail.com>
…efulset degraded, jobset suspended Signed-off-by: aviadh <aviad.hayumi@gmail.com>
…ng; add resume + deployment initializing Ported from e2e-karta: mpijob running (sleeping launcher), kserve failed (custom predictor container with a bad-registry image; also add reasonFieldName:reason to the kserve definition so the recorder and offline golden read the conditions the same way), rayjob suspended and running. New flows: an unsuspendRunPolicy action (patches spec.runPolicy.suspend) drives pytorch and mpijob resumed; rayjob resumed uses the existing unsuspend; deployment initializing (bad image, no progress deadline, so Progressing stays True with Available False) via a restored allOf predicate. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Signed-off-by: aviadh <aviad.hayumi@gmail.com>
…icitly The team asked to keep the order check strict rather than waive it. Instead of a mayGoBackwards flag, the two flows that genuinely revisit a state now declare it: batch-job and jobset completed and resumed read active-not-ready for a tick as the pod terminates, so Running dips back to Initializing before the terminal - the journeys list Initializing twice. batch-job degraded and mpijob completed/failed only had the flag defensively (their recorded order is forward); the flag is dropped with no journey change. Removed the mayGoBackwards field and its branch in the order check. Confirmed live: batch-job and jobset completed/resumed pass the strict check. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Move the workload case definitions and their helpers out of package e2e into a new test/e2e/cases package (import github.com/run-ai/karta/test/e2e/cases): the WorkloadCase/Flow/ Step/NamedState types, Steps/Classify/Validate, the 19 state predicates, and the 4 actions, all exported. The actions take a client injected by the suite via cases.SetClient. The thin package e2e harness (suite_test.go, runner_test.go, recorder.go) imports cases and drives cases.All. The state consts stay lowercase so the lowercase flow-name strings are untouched. Pure move + rename - no behavior, fixtures, or testdata changed; files moved with git mv so history is preserved. Also fix make test-e2e: run the pure cases unit tests plainly and pass the -ginkgo args only to the suite package (.), since the no-Ginkgo cases package rejected them under ./... Signed-off-by: aviadh <aviad.hayumi@gmail.com>
The recorder keeps every distinct CR a workload passes through (dedup on the CR, not the classified state), capturing intermediate scale ramps, and advances a phase only once observedGeneration >= generation. Add an undeclared-state guard: a settled CR that no predicate matches but Karta reads a real state for fails the run with the whole CR, so no Karta reading is skipped unvalidated. Karta's Undefined sentinel counts as no match. Split observeTransitions into watchWorkload, driveByState, driveByPosition, and recordUntil. Fix the scale predicates to mirror Karta (statefulset, lws, grove, deployment, cronjob), declare initializing for the replica and job workloads, and rename NamedState.Ready to Match. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Re-record every flow with the dense recorder and the corrected predicates. milvus/running keeps its prior fixture (it times out reaching Healthy on kind) and still passes the golden replay. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Scale flows now run the observed-order check too, not only ordinary flows. A scale journey declares the transient dips it passes through (Initializing, Degraded) as Optional marker steps: the subsequence check tolerates a fast cluster skipping them, and driveByPosition skips them since they are not drive stops. This drops the journeyGated skip in the runner, so every flow's state order is checked. Verified the declared dip order stable across three record runs. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Re-record the scale flows now that they declare their dips; verified stable across three record runs and green in the golden replay. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Bring both READMEs in line with dense recording and the golden format. test/conformance: document what a fixture holds (header fields, per-step fields), note the definition and library are not stored, add a concrete batch-job example, and fix the one-step-per-state claim to one per distinct CR. test/e2e: replace the stale only-stable-states, one-CR-per-state claim with dense recording plus the undeclared-state guard, update the code examples to exported names and the scale/Optional journey, refresh the How it works section, and correct the coverage table. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Consolidate the whole record and replay subsystem under test/e2e so the main module is just the library. The recorder already lived here; this moves the format, the reading, the golden, and the fixtures next to it. - test/conformance -> test/e2e/conformance, keeping the package name, so only the import path changes, not the conformance.X calls. - hack/regolden -> test/e2e/cmd/regolden. A main-module binary cannot import the e2e-module package, so regolden moves in with it. - make test, and so make check, now runs the golden with cd test/e2e && go test ./conformance/..., so every PR still replays every fixture offline with no cluster. - Path depth follows the move: the golden's repoRoot goes from two up to three up (go test runs in the package dir); the recorder and regolden resolve fixtures and kartaFile from test/e2e. - go-cmp was used only by the golden, so it drops to indirect in the main module; NOTICE and THIRD_PARTY_LICENSES regenerated to match. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Move the online record engine into test/e2e/recorder/ so the module's two halves are sibling directories: recorder/ (online, drives workloads and writes recordings) next to conformance/ (offline, the golden that replays them). The top level is now directories only, each one a clear part, which is what a newcomer needs to see what does what. - git mv recorder.go, runner_test.go, suite_test.go, recorder_internal_test.go into recorder/, package e2e -> package recorder. Nothing imported the old root package, so no other imports change. - go test ./recorder runs one level deeper, so the case paths (KartaFile, WorkloadFile), the operator-version file, and the fixtures dir - all declared relative to test/e2e - resolve through a single e2eRoot = ".." anchor, the same pattern the golden uses with repoRoot. The cases package is untouched. - Makefile test-e2e runs ./recorder instead of the root package. - README gains a Layout section mapping each directory to its role and pinning the three data locations: cases = Go definitions, testdata = input manifests, conformance/fixtures = recorded outputs. Verified: e2e module builds and vets, the golden and unit tests pass, and a live record of batch-job/running on a kind cluster writes to the right fixtures path. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
Remove the offline reading-refresh tool (test/e2e/cmd/regolden) and its wiring for now; it can return in its own change. Nothing imports it, so this touches only the tool, the Makefile regolden target, and the refresh wording. Refreshing a recording's expected reading after an intended library change now means a live re-record (make record-e2e); the golden's drift message and both READMEs point there instead of make regolden. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
From the 10-lens review, the safe high-value batch (no behavior change to a passing run): - Recorder failure messages carry context now. The watch-closed Fail captured no context and dropped the watch.Error frame that explains it; it now prints the case/flow, recorded order, the last watch error, and the last status. The order-check messages print the observed sequence. kartaState surfaces a failed Karta read (GinkgoWriter) instead of swallowing it, so the undeclared-state guard cannot silently under-report. - The golden fails loudly on a mis-pathed fixtures dir: it surfaced neither the WalkDir error nor a zero-fixture walk (a silent pass). Both are now t.Fatalf with the absolute directory searched. - WorkloadCase.Validate rejects an empty WorkloadFile, an empty Operator/KartaFile/KartaName, and a journey ending on an Optional step. - Nits: drop the redundant fl := fl loop copy (Go 1.22+); rename IntAtLeast's min parameter off the builtin. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
…ng dedup From the 10-lens review (Tier 1 context bound and Tier 2 dedup simplification): - Give each flow a single deadline. Every RPC ran on the package-level context.Background() and only the watch select was bounded by time.After, so a stuck seed Get, action Patch, or watcher setup could hang past the per-case timeout to the suite budget. observeTransitions now derives a context.WithTimeout and threads it through watchWorkload, the actions, and recordUntil, which selects on ctx.Done(); a timeout cancels the in-flight RPCs and tears the watcher down. Cleanup keeps the background context so a timed-out flow still deletes its workload. - Simplify the recording dedup: drop the lastRaw field and cache the last kept CR's significant form, so keep computes significantCR once per event instead of twice via sameCR. Behavior-preserving - dedup and order are unchanged. Verified: the e2e module builds and vets, the unit tests and the golden pass, and the full batch-job suite (both drivers, Unsuspend and ScaleParallelism actions) is green on a kind cluster. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
…olden Q1 - the online suite always records. record-e2e was KARTA_RECORD=1 over a separate test-e2e target, and writeFixture was gated on recordEnabled(); since every online run should record, fold the two targets into one record-e2e, drop the gate and the KARTA_RECORD env, and always write. The one tradeoff, a plain drive-without-writing, is gone; discard a verification run's volatile churn with git checkout. Q2 - the transition-order check runs offline too. observedOrderErr moves into the conformance package as ObservedOrderErr(declared, observed, want); the live recorder wraps it, and TestGolden now runs it on every recorded flow, deriving the declared journey from cases.All by operator, definition, and flow. This moves the last live-only check offline, so a fixture whose order was hand-edited or has drifted from its case fails on every PR with no cluster. Verified: build, vet, gofmt, the unit tests, the golden across all 56 fixtures (order check green - every fixture maps to a case and follows its journey), and a live always-on record of batch-job/running on a kind cluster. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
The golden now runs the transition-order check (ObservedOrderErr against the case's journey from cases.All), so update both READMEs: the conformance 'What the golden checks' list and the e2e README's offline bullet described only the old terminal-last / ends-at-want check. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
…ourneys batch-job and jobset completed already declare an Initializing revisit (a Job reads active-not-ready for a tick as its pod terminates). Kubeflow has the same shape: it keeps the Created (init) condition set for the job's whole life, so if Running flips off a tick before Succeeded/Failed flips on, the CR reads Initializing again before the terminal. mpijob/pytorch completed and failed did not declare that revisit, so a re-record that caught the dip would fail the order check with an undeclared state. Declare it (Steps ... initializing, terminal), the same pattern batch-job/jobset use. Rayjob and pod stay as-is: their states are monotonic phase strings that never revert, so the dip cannot occur. No fixture change; the golden tolerates the revisit whether or not a recording caught it. Signed-off-by: aviadh <aviad.hayumi@gmail.com>
…onal ones ObservedOrderErr now uses each step's Optional flag. A required (non-Optional) step must appear in the observed order; an Optional step - a state a fast cluster may miss - may be absent, but if seen must be in place. A state that recurs in the journey (a scale flow's repeated Running, a completed flow's Initializing dip) is tolerated automatically: compaction collapses it, so the check can never see the later occurrences. Undeclared or out-of-order states still fail. Before, the check was a plain subsequence that tolerated skipping any step, so a skipped required transition passed silently. Now a workload that never reaches a required state fails, online at record time and offline in the golden. The declared journey with its flags is shared: JourneyStep carries State and Optional, the recorder builds it from fl.Journey, and the golden derives it from cases.All. Marked Optional the states a fast run legitimately skips: rayjob Initializing/Running (jobStatus phase jumps), lws running Initializing, and mpijob/pytorch completed/failed Running (instant launchers). Verified: unit tests (required-skipped fails, optional-skipped ok, repeat dip ok), the golden across all 56 fixtures, and a live batch-job suite (ordinary, scale, dip) on a kind cluster. Signed-off-by: aviadh <aviad.hayumi@gmail.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.
What does this PR do?
Related issue(s)
Fixes #
Checklist
git commit -s)make check)