diff --git a/cmd/BRAINWIRE-FINDINGS.md b/cmd/BRAINWIRE-FINDINGS.md new file mode 100644 index 0000000..7998f48 --- /dev/null +++ b/cmd/BRAINWIRE-FINDINGS.md @@ -0,0 +1,310 @@ +# T1 — three stale refusals retired, one seam at a time + +`cmd/WIRING-FINDINGS.md` §6.2 and §6.3 refused two things because the substrate +did not exist. It does now (`pkg/api/SUBSTRATE-FINDINGS.md`), so those refusals +were stale — they told an operator to go and build something that was already +built. This unit retires them. + +**Nothing here deletes a refusal.** The one in §6.2 moved into `pkg/api` as +`ErrNoHistory` and `ErrHistoryTooShort`, and the whole job was making it +reachable from a command line with a non-zero exit rather than making it go +away. + +``` +cmd/kilter/brainsource.go open-the-brain-from---db, written once for three verbs +cmd/kilter/backtest.go --cluster reaches Brain.Backtest; --from/--to now required +cmd/kilter/explain.go --db/--cluster added to BOTH verbs, beside --kube-snapshot +cmd/kilter/brainsourcewire_test.go the refusals, the replay, and the two agreements +cmd/kilter/sourcewire_test.go source selection and every flag refused by name +cmd/kilter/actuatorwire_test.go the actuators are still unreachable, asserted structurally +``` + +**Status:** `gofmt -l ./cmd` empty, `go vet ./...`, `go build ./...`, +`go test -race -count=1 ./cmd/...` and `go test -race -short ./...` all green. +**`go.mod` and `go.sum` are unchanged** — every import added is stdlib or +intra-repo, and no file outside `cmd/kilter/` was touched. + +| | | +|---|---| +| New production code | 207 lines (`brainsource.go`), plus the rewiring in two files | +| New tests | 16 test functions, 1,017 lines across three files | +| Coverage, `cmd/kilter` | **58.1 %**, from 45.1 % | + +--- + +## 1. What each verb reaches now, and over which flag + +| Verb | Flag | Reaches | Refuses via | +|---|---|---|---| +| `backtest --cluster ID --db P --from T --to T` | `--db` | `api.Brain.Backtest` over `store.Snapshots` | `api.ErrNoHistory`, `api.ErrHistoryTooShort` | +| `why-cost --db P --cluster ID --from T --to T` | `--db` | `api.Brain.WhyCost` (calls `Verify`) | unknown cluster; `notEnoughEvidence`; `unverifiable` | +| `explain --db P --cluster ID --workload W --container C` | `--db` | `api.Brain.Explain` (calls `Verify`) | unknown cluster; `notEnoughEvidence`; `unverifiable` | + +Every one of these is a **sibling** of the file-backed flag, which is +unchanged: `--kube-snapshot` (repeatable) and `--ledger` still answer without a +database, still need no brain, and are still the only way to explain snapshots +that were never ingested anywhere. §3 of the substrate findings is followed as +written; the three places this unit went beyond it are in §5 below and the two +places it declined to are in §6. + +The shared open-the-brain helper is `brainsource.go`, used by all three verbs. +It carries four operator-facing facts that are properties of bbolt and of the +brain, not of these commands, and that a reader of the code cannot see: + +1. **The database must already exist.** `bolt.Open` *creates* the file it is + given, so a mistyped `--db` would otherwise make an empty database and + answer "no history for that cluster" — a true statement about the file it + had just created, and a completely misleading one about the cluster. All + three verbs `os.Stat` first and refuse by name, and + `TestReadVerbsRefuseAMissingDatabaseRatherThanCreatingOne` asserts the file + is still absent afterwards. +2. **The lock is exclusive, so these verbs cannot read a running brain's + database.** `store.Open` takes bbolt's file lock with a 5-second timeout. + The failure surfaces as `timeout`, which does not say why, so the wrapper + offers the cause without asserting it (a permission or corruption failure + arrives at the same line). The operational recipe is: stop the brain, or + point `--db` at a copy. +3. **Opening writes.** `store.Open` creates missing buckets inside an `Update` + transaction, so even these read-only verbs touch the file. Nothing here + ingests, plans or actuates. +4. **The freshness of the two substrates differs, and one of them lags.** See + §2 — this is the finding most likely to cost somebody an afternoon. + +## 2. The two substrates do not go stale together, and it shows + +`SaveSnapshotAt` runs inside **every** `Ingest`, so `backtest --cluster` sees +everything the retention kept, up to the last snapshot the brain received. +`evidence.Memory` is persisted only every `BrainConfig.CheckpointEvery` +snapshots (default 10) and on graceful shutdown, so `why-cost --db` and +`explain --db` see the substrate **as of the last checkpoint**. + +Measured, not reasoned about — a brain given three snapshots at the default +checkpoint cadence, then read by the binary: + +``` +$ kilter why-cost --db brain.db --cluster prod --from … --to … +kilter why-cost: why-cost --cluster prod: the evidence substrate holds 0 timeline +point(s) for "prod" inside […); ΔCost needs two observations to be a measurement. + +$ kilter backtest --cluster prod --db brain.db --from … --to … # SAME database +kilter backtest — cluster prod, replayed from brain.db + policy 69955a2cdd2004fd + scored 12 decisions 4 … +``` + +Both answers are correct and they are about the same database. The failure mode +to be aware of is that "not enough evidence" reads as a statement about the +cluster and is partly a statement about **when the brain was last stopped**. +`kilter brain` exposes no `--checkpoint-every`, so the mitigation is a graceful +shutdown (`Serve` checkpoints on `ctx.Done()`), and the tests that need a +persisted substrate build their brain with `CheckpointEvery: 1` for exactly +this reason. + +This is deliberately **not** pinned by a `cmd/` test. The cadence is +`pkg/api`'s policy; a cmd test asserting it would fail the day `pkg/api` +changed its own default, which is not a fact about the command line. + +## 3. The refusals that survived, and the test pinning each + +| Refusal | Where it lives | Pinned by | +|---|---|---| +| `ErrHistoryTooShort`, **`Instants == 0`** — a real history that replays nothing | `pkg/api/backtest.go` | `TestBacktestOverADatabaseRefusesAHistoryThatWouldScoreNothing` | +| `ErrHistoryTooShort`, **count < 2** — the one-snapshot case §6.2 was written about | `pkg/api/backtest.go` | `TestBacktestLiveHistoryRefusesRatherThanScoringOneSnapshot` (re-pointed), `TestBacktestOverADatabaseRefusesASingleSnapshot` | +| the same, with **zero** snapshots — a database that never saw the cluster | `pkg/api/backtest.go` | `TestBacktestOverADatabaseRefusesAClusterWithNoHistory` | +| `--from`/`--to` required for `--cluster` | `cmd/kilter/backtest.go` | `TestBacktestClusterRequiresAFixedWindow` | +| unknown cluster, the CLI form of the routes' 404 | `cmd/kilter/brainsource.go` | `TestBrainBackedVerbsRefuseAnUnknownCluster` | +| a missing `--db` is not created | `cmd/kilter/brainsource.go` | `TestReadVerbsRefuseAMissingDatabaseRatherThanCreatingOne` | +| two sources at once | `cmd/kilter/explain.go` | `TestTheTwoSourcesAreExclusiveAndSaySo` | +| a flag the source cannot honour | `cmd/kilter/brainsource.go` | `TestFlagsThatCannotBeHonouredAreRefusedNotIgnored` | + +Every refusal test asserts **no scorecard was printed** (`assertNoScorecard` +checks for `regret`, `oracleGap`, `flipRate` and `safety` in the output), not +merely that an error came back. The error is the easy half; the trap is output +that reads as a verdict. + +### 3.1 Building a history that is real and still unscoreable + +The `Instants == 0` case took three attempts to reach and the two dead ends are +worth recording, because a future reader will otherwise write the same test and +believe it. + +- **Two snapshots two hours apart, window `[t0, t0+2h)`.** The window is + half-open, so the snapshot at exactly `t0+2h` is outside it and only one + snapshot is in range: this hits the *count* branch, not the coverage branch. +- **Widening to `[t0, t0+3h)` with a 24h horizon.** Now `backtest.Run`'s own + guard fires first — `horizon 24h0m0s exceeds the replay window 3h0m0s` — + and `Instants` is never computed. +- **What works:** a window WIDER than the horizon whose snapshots all land too + late in it. Two snapshots at `t0+30h` and `t0+31h` in `[t0, t0+48h)` with a + 24h horizon: `decisionInstants` snaps every grid point forward to the first + snapshot at or after it, then rejects it because `snapshot + horizon > to`. + Two retained snapshots, zero instants, `Scorecard.Snapshots == 2`. + +That third shape is not contrived. It is a brain that started recording late in +the window an operator asked about — the single most likely way to hit this in +production, which is precisely why a count check would have been the wrong +gate. + +## 4. Where the two sources are proven to agree + +`TestBrainAndFileSourcesAgreeOnWhyCost` runs the same three snapshots through +both sources over the same window and requires the `--json` attribution to be +**byte-identical**. JSON rather than prose, because JSON carries the evidence +IDs: the terms, the residual and every citation have to match, not just the +dollar amounts. + +"Where both can answer" is a real qualifier and **two** things narrow it. Both +were found by running the test, not by reading the code: + +- **Spacing.** `pkg/store` thins the history to one snapshot per hour, so a + 5-minute series leaves the brain with fewer composition edges than the file + series holds. The fixture is spaced 12 hours apart, where the retention keeps + everything. +- **Events.** The brain's substrate also records deploys and OOMKills; the file + path observes timeline points only. With a workload whose requests change + across the window, the brain's answer carries two extra citations + (`evt/…/deploy@…`) that the file's cannot. + +That second one is pinned rather than hidden, by +`TestTheBrainCitesMoreThanAFileCanAndTheMoneyIsUnchanged`: every term kind and +every amount is identical, the brain's citations are a strict **superset**, and +at least one of the extras is a deploy event. If the amounts ever diverged, one +of the two sources would be wrong about what a cost change is; if the citations +were equal, the brain would be discarding evidence it holds. + +A third agreement is asserted for `explain`, against a different oracle: +`TestExplainOverADatabaseMatchesTheHTTPRoute` compares +`kilter explain --db` with `GET /api/v1/clusters/{id}/explain` over the same +database, through `Brain.Handler()` and an `httptest.NewRecorder` (no socket is +opened). This exists because `pkg/api`'s `defaultExplainWindow` is unexported +and `cmd` therefore carries its own `brainExplainWindow` constant — a constant +copied by hand is a constant that drifts. The test pins the span *and* the +one-second edge to the route rather than to a comment. + +## 5. Where this went beyond §3, and why + +§3.1's sketch is `api.NewBrain(api.BrainConfig{}, catalog, st)` and +`return err`. That is what the code does. Three things it does not mention had +to be decided: + +1. **The demo-only flags are refused by name on the `--cluster` path** + (`--days`, `--workloads`, `--noise`, `--derive-costs`, `--policy`, + `--compare`, `--enforce-refusals`, `--fail-on-regression`), and the live + flags (`--db`, `--from`, `--to`) are refused on the `--demo` path. Silently + ignoring them was the alternative, and `loadPolicy` already records why that + is unacceptable: a knob that is dropped produces a report for a + configuration nobody ran. `flag.FlagSet.Visit` is what makes this exact — it + distinguishes an explicit `--days 7` from an unmentioned `--days`. +2. **`--policy`/`--compare` are refused rather than plumbed.** + `Brain.Backtest` scores *this brain's* policy, which is the question a live + replay answers; an A/B needs a policy argument `pkg/api` deliberately does + not take. The seam, if it is ever wanted, is `api.BrainConfig`'s `Recommend` + and `Plan` fields, and the reason it was not taken here is that + `Brain.Backtest` builds its harness with no `Decision` config at all, so + `--enforce-refusals` — the flag behind `pkg/backtest`'s headline result — + could not be honoured either way. Half an A/B is worse than none. + *Related fact, checked:* `kilter brain` passes only `Token`, `ReadToken` and + `ForecasterURL` to `BrainConfig`, so `BrainConfig{}` here scores the same + recommender and planner a served brain runs. That equivalence breaks the day + `kilter brain` grows a policy flag. +3. **The unknown-cluster check is in `cmd`, for two verbs and not the third.** + `why-cost` and `explain` call `requireCluster` (the CLI form of the routes' + 404 — "never ingested" and "ingested but thin" are different facts). + `backtest --cluster` deliberately does **not**: its refusals belong to + `pkg/api`, and a pre-check would shadow `ErrHistoryTooShort` — including in + the re-pointed test, whose fixture is a database with exactly one snapshot. + +Nothing in §3 was diverged from. §3.2's "nothing to do in `cmd/`" was read as a +statement about the *routes*, which are indeed complete; the same paragraph +then says `cmd` should use `Brain.WhyCost`/`Brain.Explain` rather than +duplicate them, which is what the `--db` flags do. + +### 5.1 `cmd/kilter/whycost.go` was not created + +The scope named it; it does not exist and did not before. `kilter why-cost` +lives in `explain.go` beside `kilter explain`, and the brain-backed path was +added next to the file-backed one it must agree with. Splitting one branch of +one verb into its own file would put the two answers to the same question in +two files, which is the arrangement most likely to let them drift. + +## 6. What is still unreachable after this unit + +- **The actuators.** `pkg/ec2/actuate*.go` and `pkg/rds/actuate*.go` remain + unreachable and this unit added no path toward them. Asserting that is + awkward, because both packages are *already linked into the binary* for their + collectors (`cmd/kilter/rds.go`, `cmd/kilter/rdslive.go`), so an import-graph + check answers the wrong question. `TestNoActuatorIsReachableFromTheBinary` + instead derives the forbidden set from the source — every exported identifier + declared in an `actuate*.go` file of either package, 212 of them today — and + parses every non-test file in `cmd/kilter` for a selector naming one. A new + actuator entry point is covered the moment it is written, with no list to + maintain; three canaries (`NewActuator`, `Actuator`, `ActuatorConfig`) fail + the test if the scan ever finds nothing. `TestNoCloudSDKReachesTheNewSources` + adds the narrower structural check on the three files this unit wrote or + rewired. +- **A workload-level `explain` subject.** `Brain.Explain` accepts + `Kind/namespace/name` as well as the four-segment container form, but + `kilter explain` has required `--workload` AND `--container` since it + shipped, and making `--container` optional would change what an existing + command line means. The workload subject stays reachable over HTTP only. +- **An A/B against live history.** See §5, item 2. Needs a policy argument on + `Brain.Backtest`, and needs `pkg/api` to accept a `Decision` config before + the comparison worth running (`--enforce-refusals`) is expressible. +- **`Explanation.Verdict` is still nil, so `Action` is `unknown`.** Unchanged + and for the unchanged reason: `pkg/recommend` does not import + `pkg/decision`, so production has no verdict to read out + (cmd/WIRING-FINDINGS.md §6.4, SUBSTRATE-FINDINGS.md §6). This is visible on + the `--db` path exactly as it is on the `--kube-snapshot` path — the prose + reads `Verdict: none recorded.` +- **The live RDS collector** (§6.1) and **`pkg/rds`'s `StorageParity` seam** + (§6.4) are untouched: both are `go.mod` questions and this unit changed no + module. + +### 6.1 One observed gap in §3.2's status taxonomy + +§3.2 says a subject with no history is a **422**. It is not, in the case a +typo produces: `explain.BuildExplain` returns a *payload* for an unknown +subject rather than an error, carrying the note "no evidence is stored for this +subject in this window; the payload states the decision but grounds none of +it". `Brain.Explain` passes it through, the route answers 200, and +`kilter explain --db` prints it and **exits 0**. + +Observed directly against the built binary: + +``` +$ kilter explain --db brain.db --cluster prod \ + --workload Deployment/default/app-0 --container app # real name is app-00 +kilter explain — Deployment/default/app-0/app over [2026-01-09T23:00:01Z, …] + note: no evidence is stored for this subject in this window; the payload + states the decision but grounds none of it +$ echo $? +0 +``` + +The payload is honest — it says it grounds nothing — so this is a question of +whether "you asked about a subject that does not exist" deserves a non-zero +exit, not a question of a misleading answer. It is left alone because the fix +belongs in `pkg/explain` or `pkg/api`, both outside this unit, and because the +CLI's job here is to be faithful to the route: it is, byte for byte (§4). + +## 7. Determinism + +- **No clock in any answer.** `backtestEpoch` still fixes the `--demo` window; + `--from`/`--to` are now required for `--cluster`, so a live scorecard's + window is an argument rather than a drifting default; `explain --db`'s + default window is resolved from the **latest ingested snapshot** in the + database, never `time.Now()`. `TestBacktestOverADatabaseScoresTheRetainedHistory` + asserts two replays of the same database are byte-identical. +- **One renderer per artefact.** `writeBacktestReport` and + `renderAttribution`/`renderExplanation` are shared by both sources, so the + same answer prints the same bytes whichever substrate produced it — which is + what makes the agreement tests in §4 a comparison of answers rather than of + layouts. Only the header line differs, and it names the source. +- **No new ordering.** Every sort that matters (`basisFrom`'s groups and + namespaces, `ledgerActions`' `(At, Fingerprint)`, `writeScorecard`'s refusal + codes) is unchanged and now runs inside `pkg/api` for the `--db` path, where + SUBSTRATE-FINDINGS §4 covers it. +- **No network, no cloud call, no credential** in any test. The one HTTP + comparison uses `httptest.NewRecorder` against `Brain.Handler()` in-process, + so not even a loopback socket is opened. Databases are built in `t.TempDir()` + through the real `api.Brain.Ingest`. diff --git a/cmd/kilter/actuatorwire_test.go b/cmd/kilter/actuatorwire_test.go new file mode 100644 index 0000000..f9d105b --- /dev/null +++ b/cmd/kilter/actuatorwire_test.go @@ -0,0 +1,192 @@ +package main + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "sort" + "strconv" + "strings" + "testing" +) + +// The actuators stay unreachable from the binary. +// +// pkg/ec2/actuate*.go and pkg/rds/actuate*.go are shipped, tested and +// DELIBERATELY not wired: stopping an instance or modifying a database's +// storage is a decision with an owner, and that decision has not been made. +// Both packages are already linked into `kilter` for their COLLECTORS +// (cmd/kilter/rds.go, cmd/kilter/rdslive.go), so "does the binary import +// pkg/rds" cannot be the question — it does, and must. The question is whether +// any command names an actuation symbol, and this asserts that none does. +// +// The check derives its own forbidden list from the source rather than +// carrying a hand-written one: every exported identifier declared in a file +// called actuate*.go is off limits. A new actuator entry point is therefore +// covered the moment it is written, with no list to remember to update. +// +// This unit added brainsource.go, backtest --cluster, why-cost --db and +// explain --db, all of which reach a bbolt file and a pricing catalog. None of +// them can reach a mutating cloud path, and this is how that is known rather +// than asserted in a comment. +func TestNoActuatorIsReachableFromTheBinary(t *testing.T) { + forbidden := actuatorSymbols(t) + // A canary: if the scan ever stops finding declarations — a rename, a + // moved file, a parse failure swallowed — this test would pass over an + // empty set and prove nothing. + for _, canary := range []string{"NewActuator", "Actuator", "ActuatorConfig"} { + if !forbidden[canary] { + t.Fatalf("the actuator symbol scan found no %q; it is looking in the wrong place", canary) + } + } + + files, err := filepath.Glob("*.go") + if err != nil { + t.Fatal(err) + } + sort.Strings(files) + var scanned int + for _, path := range files { + if strings.HasSuffix(path, "_test.go") { + continue + } + scanned++ + f, err := parser.ParseFile(token.NewFileSet(), path, nil, 0) + if err != nil { + t.Fatalf("parse %s: %v", path, err) + } + // The local name each actuator-bearing package is imported under, + // which is not always the last path segment. + locals := map[string]string{} + for _, imp := range f.Imports { + p, err := strconv.Unquote(imp.Path.Value) + if err != nil { + t.Fatal(err) + } + pkg, ok := actuatorPackage(p) + if !ok { + continue + } + name := filepath.Base(p) + if imp.Name != nil { + name = imp.Name.Name + } + locals[name] = pkg + } + if len(locals) == 0 { + continue + } + ast.Inspect(f, func(n ast.Node) bool { + sel, ok := n.(*ast.SelectorExpr) + if !ok { + return true + } + id, ok := sel.X.(*ast.Ident) + if !ok { + return true + } + pkg, ok := locals[id.Name] + if !ok { + return true + } + if forbidden[sel.Sel.Name] { + t.Errorf("%s names %s.%s: that is an actuator, and no mutating AWS path may be "+ + "reachable from the binary", path, pkg, sel.Sel.Name) + } + return true + }) + } + if scanned == 0 { + t.Fatal("no command source was scanned") + } +} + +// actuatorPackage reports whether an import path is a package that carries +// actuators, and is the single place the two are named. +func actuatorPackage(path string) (string, bool) { + switch path { + case "github.com/agenticode/kilter/pkg/ec2": + return "pkg/ec2", true + case "github.com/agenticode/kilter/pkg/rds": + return "pkg/rds", true + } + return "", false +} + +// actuatorSymbols is every exported identifier declared in an actuate*.go file +// of an actuator-bearing package. +func actuatorSymbols(t *testing.T) map[string]bool { + t.Helper() + out := map[string]bool{} + for _, dir := range []string{"../../pkg/ec2", "../../pkg/rds"} { + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + for _, e := range entries { + name := e.Name() + if !strings.HasPrefix(name, "actuate") || + !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { + continue + } + f, err := parser.ParseFile(token.NewFileSet(), filepath.Join(dir, name), nil, 0) + if err != nil { + t.Fatalf("parse %s: %v", name, err) + } + for _, d := range f.Decls { + switch d := d.(type) { + case *ast.FuncDecl: + if d.Recv == nil && d.Name.IsExported() { + out[d.Name.Name] = true + } + case *ast.GenDecl: + for _, spec := range d.Specs { + switch s := spec.(type) { + case *ast.TypeSpec: + if s.Name.IsExported() { + out[s.Name.Name] = true + } + case *ast.ValueSpec: + for _, n := range s.Names { + if n.IsExported() { + out[n.Name] = true + } + } + } + } + } + } + } + } + return out +} + +// TestNoCloudSDKReachesTheNewSources. +// +// The narrower, structural half of the same rule: the files this unit added or +// rewired must not import a cloud SDK at all. backtest --cluster, why-cost --db +// and explain --db read one bbolt file and one pricing catalog; if any of them +// ever grew an AWS client, the seam this prohibition protects would be open +// regardless of which symbols were named. +func TestNoCloudSDKReachesTheNewSources(t *testing.T) { + for _, path := range []string{"brainsource.go", "backtest.go", "explain.go"} { + f, err := parser.ParseFile(token.NewFileSet(), path, nil, 0) + if err != nil { + t.Fatalf("parse %s: %v", path, err) + } + for _, imp := range f.Imports { + p, err := strconv.Unquote(imp.Path.Value) + if err != nil { + t.Fatal(err) + } + if strings.Contains(p, "aws-sdk-go") || strings.Contains(p, "k8s.io") { + t.Errorf("%s imports %s; these verbs read a database, not a cloud or a cluster", path, p) + } + if _, isActuator := actuatorPackage(p); isActuator { + t.Errorf("%s imports %s, which carries actuators", path, p) + } + } + } +} diff --git a/cmd/kilter/backtest.go b/cmd/kilter/backtest.go index d18d776..fa24dc7 100644 --- a/cmd/kilter/backtest.go +++ b/cmd/kilter/backtest.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "github.com/agenticode/kilter/pkg/api" "github.com/agenticode/kilter/pkg/backtest" "github.com/agenticode/kilter/pkg/decision" "github.com/agenticode/kilter/pkg/plan" @@ -23,31 +24,37 @@ import ( // and on its first run it falsified the shipped engine. Until now nothing in // the binary could call it. // -// # What is wired, and the one thing that is not +// # Both sources of history are wired // -// The TRACE path is wired: four synthetic archetypes whose oracles are known -// in closed form, replayed through the same observe-then-ask sequence -// pkg/api's Ingest/Plan runs. Policy files, the A/B comparison, Gate, the -// JSON scorecard and the CI exit code all work over it. +// The TRACE path (--demo) replays four synthetic archetypes whose oracles are +// known in closed form, through the same observe-then-ask sequence pkg/api's +// Ingest/Plan runs. Policy files, the A/B comparison, Gate, the JSON scorecard +// and the CI exit code all work over it. // -// The LIVE path is NOT wired, and refuses rather than pretending. §4.4's -// headline feature is "replay this cluster's own history", and it needs a seam -// that does not exist: pkg/store keeps only the LATEST snapshot per cluster -// (SaveSnapshot/LoadSnapshot are keyed by cluster, not by time), and -// recommend.ObserveSnapshot and plan.Build both take a *model.ClusterSnapshot, -// so a replay through the production code path needs topology over time. +// The LIVE path (--cluster --db) replays a cluster's OWN retained history and +// was refused until pkg/store grew a time-keyed snapshot bucket. It is wired +// to api.Brain.Backtest and nothing about the refusal was deleted: it MOVED. +// Brain.Backtest still refuses, by type, in the two cases that matter — +// api.ErrNoHistory and api.ErrHistoryTooShort — and this command returns those +// errors unaltered, so they keep their non-zero exit and no scorecard is +// printed. The second case is the one a count check misses: backtest.Run over +// a history with no scoreable instant does not fail, it returns a Scorecard +// with the same shape, the same field names and the same confident tone as a +// real one, and `regret $0.00` over nothing reads as a perfect policy. That +// check lives in pkg/api because it is expressed in terms of backtest's own +// coverage report (Scorecard.Instants), not in terms of anything cmd can see. // -// Running the harness against a single snapshot would produce a scorecard — -// one instant, no horizon, every number a rounding artefact — that looks -// exactly like a real one. That is the failure mode this command refuses to -// have: --cluster names the missing seam and exits non-zero. See -// backtestLiveRefusal. +// --from and --to are REQUIRED with --cluster, for the reason backtestEpoch is +// a constant for --demo and why-cost requires its window: a replay window that +// drifts with wall-clock time makes two runs over the same configuration +// disagree, and a scorecard whose whole value is comparability cannot be +// computed over a moving window. const backtestUsage = `kilter backtest — replay a policy against history and score it Usage: - kilter backtest --demo [flags] score the shipped policy over a synthetic trace - kilter backtest --cluster [flags] replay a cluster's own history (see below) + kilter backtest --demo [flags] score the shipped policy over a synthetic trace + kilter backtest --cluster --db PATH --from T --to T replay a cluster's own retained history Archetypes (--demo), each with a closed-form oracle: steady every sample at the base level @@ -55,24 +62,33 @@ Archetypes (--demo), each with a closed-form oracle: bursty 12 spikes/day — narrower than the 5%% CPU tail, wide enough for the memory peak regime-change a level shift at the midpoint, on a decision instant +The two sources are different substrates for the same question and exactly one +may be given. --cluster reads the snapshot history a running brain persisted; +it is refused, by name and with a non-zero exit, when that history is absent or +too short to yield a single scoreable decision instant. + Flags: --demo KIND synthetic archetype to score - --cluster ID replay a live cluster's history (refused: see --help output) - --days N trace length in days (default 7) - --workloads N containers in the trace (default 2) - --noise PCT deterministic jitter, e.g. 0.05 for +/-5%% (default 0) + --cluster ID replay this cluster's own retained history (needs --db) + --db PATH brain database written by "kilter brain --db PATH" + --from RFC3339 replay window start, inclusive (required with --cluster) + --to RFC3339 replay window end, EXCLUSIVE (required with --cluster) + --days N trace length in days (default 7, --demo only) + --workloads N containers in the trace (default 2, --demo only) + --noise PCT deterministic jitter, e.g. 0.05 for +/-5%% (default 0, --demo only) --horizon DUR how far ahead each decision is scored (default 24h) --interval DUR spacing of decision instants (default 24h) --starvation F CPU violation threshold: future p95 > request x F (default 1.0) --incident-usd N price of one violated container-window (default 50) - --derive-costs derive CPU/memory rates from the catalog and the trace's nodes + --derive-costs derive CPU/memory rates from the catalog and the trace's nodes (--demo only) --catalog PATH pricing catalog JSON (default: embedded) - --policy PATH policy triple JSON; omitted means the shipped default - --compare PATH score a second policy and print Gate's verdict + --policy PATH policy triple JSON; omitted means the shipped default (--demo only) + --compare PATH score a second policy and print Gate's verdict (--demo only) --enforce-refusals run pkg/decision's refusal predicates (models the pending wiring); a policy file's "enforceDecisionRefusals" overrides it per policy + (--demo only) --json emit the scorecard verbatim (byte-stable, CI-diffable) - --fail-on-regression exit non-zero when Gate rejects the candidate + --fail-on-regression exit non-zero when Gate rejects the candidate (--demo only) ` func runBacktest(args []string) error { return runBacktestTo(os.Stdout, args) } @@ -81,6 +97,9 @@ func runBacktest(args []string) error { return runBacktestTo(os.Stdout, args) } type backtestFlags struct { demo string cluster string + db string + from string + to string days int workloads int noise float64 @@ -105,7 +124,10 @@ func runBacktestTo(w io.Writer, args []string) error { fs.SetOutput(w) var bf backtestFlags fs.StringVar(&bf.demo, "demo", "", "synthetic archetype (steady|diurnal|bursty|regime-change)") - fs.StringVar(&bf.cluster, "cluster", "", "replay a live cluster's own history") + fs.StringVar(&bf.cluster, "cluster", "", "replay a live cluster's own retained history") + fs.StringVar(&bf.db, "db", "", "brain database holding the snapshot history") + fs.StringVar(&bf.from, "from", "", "replay window start (RFC3339, required with --cluster)") + fs.StringVar(&bf.to, "to", "", "replay window end (RFC3339, required with --cluster)") fs.IntVar(&bf.days, "days", 7, "trace length in days") fs.IntVar(&bf.workloads, "workloads", 2, "containers in the trace") fs.Float64Var(&bf.noise, "noise", 0, "deterministic jitter fraction") @@ -128,46 +150,124 @@ func runBacktestTo(w io.Writer, args []string) error { case bf.cluster != "" && bf.demo != "": return fmt.Errorf("backtest: --demo and --cluster are different sources of history; pass one") case bf.cluster != "": - return backtestLiveRefusal(bf.cluster) + return runBacktestLive(w, &bf, setFlagNames(fs)) case bf.demo == "": fmt.Fprint(w, backtestUsage) return fmt.Errorf("backtest: --demo or --cluster is required") } - return runBacktestDemo(w, &bf) + return runBacktestDemo(w, &bf, setFlagNames(fs)) } -// backtestLiveRefusal is the honest half of this command. +// runBacktestLive replays a cluster's own retained history through the brain +// that recorded it. // -// It names the seam, says what it would take, and exits non-zero. It does NOT -// fall back to the one snapshot pkg/store holds: a scorecard computed over a -// single instant has the same shape, the same field names and the same -// confident tone as a real one, and an operator reading "regret $0.00" has no -// way to tell that it means "nothing was replayed". -func backtestLiveRefusal(cluster string) error { - return fmt.Errorf(`backtest --cluster %s: refused — snapshot history is not persisted. - -pkg/store keeps only the LATEST snapshot per cluster (SaveSnapshot/LoadSnapshot -are keyed by cluster, not by time), and pkg/evidence stores per-subject usage -series and events rather than pod/node topology. A replay that goes through the -production code path needs topology over time, because recommend.ObserveSnapshot -and plan.Build both take a *model.ClusterSnapshot. - -Scoring the one snapshot that does exist would produce a scorecard that looks -exactly like a real one, so this refuses instead. - -What it needs (pkg/backtest/FINDINGS.md, "Seams this unit needed and did not -find", item 1): a time-keyed snapshot bucket in pkg/store — SaveSnapshotAt(snap) -and Snapshots(cluster, from, to) — bounded the way the rest of the substrate is, -plus an adapter implementing backtest.SnapshotSource. At a 5-minute cadence a -30-day window is 8,640 snapshots per cluster, so the realistic shape is a -keyframe-plus-delta encoding or a reduced replay snapshot carrying only what -recommend and plan read. - -Meanwhile: kilter backtest --demo regime-change`, cluster) +// Every refusal below is a flag this source cannot honour, refused by name +// rather than ignored — the same rule loadPolicy applies with +// DisallowUnknownFields, and for the same reason: a knob that is silently +// dropped produces a scorecard for a configuration nobody ran. What is NOT +// here is a check on the history itself. api.Brain.Backtest owns that, it +// refuses with typed errors, and this function returns them unaltered so the +// message the operator sees is the one pkg/api wrote and the exit code is +// non-zero. +func runBacktestLive(w io.Writer, bf *backtestFlags, set map[string]bool) error { + // The trace-shaped flags describe a synthetic trace and mean nothing over + // a recorded history. + if err := refuseUnusable("backtest --cluster", "describe a synthetic trace, not a recorded history; "+ + "the history is whatever the brain retained", set, + "days", "workloads", "noise"); err != nil { + return err + } + // The policy flags would answer a different question. api.Brain.Backtest + // scores THIS BRAIN'S policy — the recommender and planner the database's + // brain actually runs with — so "how good is what is running here" is the + // question a live replay answers. Scoring a policy that never ran against + // a history it never saw is an A/B, and it needs a policy argument + // pkg/api deliberately does not take; the seam if it is ever wanted is + // api.BrainConfig's Recommend and Plan fields. + if err := refuseUnusable("backtest --cluster", "belong to an A/B between two candidate policies; "+ + "a live replay scores the policy this brain runs, and --demo is where a policy comparison "+ + "belongs", set, + "policy", "compare", "enforce-refusals", "fail-on-regression"); err != nil { + return err + } + // --derive-costs reads the trace's own nodes. Over a live history the + // equivalent is the retained snapshots, which is a cost model this + // command does not build; refusing beats deriving a different one. + if err := refuseUnusable("backtest --cluster", "derives a cost model from a synthetic trace's "+ + "nodes and has no defined meaning over a recorded history", set, "derive-costs"); err != nil { + return err + } + if bf.db == "" { + fmt.Fprint(w, backtestUsage) + return fmt.Errorf("backtest --cluster %s: --db is required — the snapshot history lives in the "+ + "brain's database, and there is no history without one", bf.cluster) + } + if bf.from == "" || bf.to == "" { + fmt.Fprint(w, backtestUsage) + return fmt.Errorf("backtest --cluster %s: --from and --to are required; the replay window is an "+ + "argument, and a window that drifts with wall-clock time makes two runs over the same "+ + "configuration disagree", bf.cluster) + } + from, err := time.Parse(time.RFC3339, bf.from) + if err != nil { + return fmt.Errorf("--from: %w", err) + } + to, err := time.Parse(time.RFC3339, bf.to) + if err != nil { + return fmt.Errorf("--to: %w", err) + } + + catalog, err := loadCatalog(bf.catalog) + if err != nil { + return err + } + src, err := openBrainSource("backtest", bf.db, catalog) + if err != nil { + return err + } + defer src.Close() + brain, err := src.brain(api.BrainConfig{}) + if err != nil { + return err + } + + scoring := backtest.DefaultConfig() + scoring.DecisionInterval = bf.interval + if bf.starvation > 0 { + scoring.StarvationFactor = bf.starvation + } + if bf.incidentUSD > 0 { + scoring.Cost.IncidentUSD = bf.incidentUSD + } + + // THE REFUSAL, reached from the command line. api.ErrNoHistory and + // api.ErrHistoryTooShort come back verbatim: nothing is printed, the error + // becomes a non-zero exit in main, and the operator is told how many + // snapshots there were and how many instants they yielded rather than + // being handed a scorecard whose zeros read as a verdict. + sc, err := brain.Backtest(bf.cluster, from, to, bf.horizon, scoring) + if err != nil { + return err + } + header := fmt.Sprintf("kilter backtest — cluster %s, replayed from %s\n"+ + "window %s .. %s horizon %s interval %s\n\n", + bf.cluster, bf.db, + from.UTC().Format(time.RFC3339), to.UTC().Format(time.RFC3339), + bf.horizon, scoring.DecisionInterval) + return writeBacktestReport(w, bf, header, sc, nil, false, nil) } // runBacktestDemo scores one or two policies over a synthetic trace. -func runBacktestDemo(w io.Writer, bf *backtestFlags) error { +func runBacktestDemo(w io.Writer, bf *backtestFlags, set map[string]bool) error { + // The live source's flags are refused here for the same reason the trace's + // flags are refused there: the trace's window is backtestEpoch plus --days + // by construction, so a --from that was quietly ignored would produce a + // scorecard over a window the operator did not ask for. + if err := refuseUnusable("backtest --demo", "belong to --cluster; a trace's window is "+ + "backtestEpoch plus --days and its history is generated, not stored", set, + "db", "from", "to"); err != nil { + return err + } kind, err := parseArchetype(bf.demo) if err != nil { return err @@ -253,6 +353,23 @@ func runBacktestDemo(w io.Writer, bf *backtestFlags) error { gateOK, gateReasons = backtest.Gate(current, candidate, backtest.DefaultTolerance()) } + header := fmt.Sprintf("kilter backtest — %s trace, %d days, %d workloads\n"+ + "window %s .. %s horizon %s interval %s\n\n", + kind, bf.days, bf.workloads, + trace.Start.UTC().Format(time.RFC3339), trace.End.UTC().Format(time.RFC3339), + bf.horizon, scoring.DecisionInterval) + return writeBacktestReport(w, bf, header, current, candidate, gateOK, gateReasons) +} + +// writeBacktestReport renders one or two scorecards, in whichever form --json +// asked for, then applies --fail-on-regression. +// +// Shared by both sources so a scorecard over a live history and a scorecard +// over a trace are the same bytes in the same layout — they are meant to be +// compared, and a second renderer is a second layout waiting to drift. Only +// the header line differs, and it names which source produced the numbers. +func writeBacktestReport(w io.Writer, bf *backtestFlags, header string, + current, candidate *backtest.Scorecard, gateOK bool, gateReasons []string) error { if bf.jsonOut { if candidate == nil { // Verbatim: Scorecard.Encode is the byte-stable, CI-diffable form. @@ -274,11 +391,7 @@ func runBacktestDemo(w io.Writer, bf *backtestFlags) error { } var b strings.Builder - fmt.Fprintf(&b, "kilter backtest — %s trace, %d days, %d workloads\n", - kind, bf.days, bf.workloads) - fmt.Fprintf(&b, "window %s .. %s horizon %s interval %s\n\n", - trace.Start.UTC().Format(time.RFC3339), trace.End.UTC().Format(time.RFC3339), - bf.horizon, scoring.DecisionInterval) + b.WriteString(header) writeScorecard(&b, "policy", current) if candidate != nil { b.WriteString("\n") diff --git a/cmd/kilter/backtest_test.go b/cmd/kilter/backtest_test.go index 374866b..f2ecda4 100644 --- a/cmd/kilter/backtest_test.go +++ b/cmd/kilter/backtest_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/agenticode/kilter/pkg/backtest" ) @@ -106,25 +107,38 @@ func TestOracleGapIsRenderedInTheUnitsTheScorecardUses(t *testing.T) { // TestBacktestLiveHistoryRefusesRatherThanScoringOneSnapshot. // -// This is the honest half of the command. pkg/store keeps only the LATEST -// snapshot per cluster, so a live replay has no history to replay. Running the -// harness against that one snapshot would yield a scorecard with the same -// shape, the same field names and the same confident tone as a real one — the -// worst possible failure, because the number looks fine. +// This is the honest half of the command, and it survives the wiring: the +// refusal MOVED, it was not removed. When this was written pkg/store kept only +// the latest snapshot per cluster, so a live replay had no history at all and +// the command refused by naming the missing seam. pkg/store now keeps a +// time-keyed history and `--cluster` reaches api.Brain.Backtest — which +// refuses, with api.ErrHistoryTooShort, in exactly the case this test has +// always been about: a cluster whose retained history holds one snapshot. +// +// Running the harness against that one snapshot would yield a scorecard with +// the same shape, the same field names and the same confident tone as a real +// one — the worst possible failure, because the number looks fine. The +// assertion that no scorecard is printed is therefore unchanged and is still +// the point of the test. func TestBacktestLiveHistoryRefusesRatherThanScoringOneSnapshot(t *testing.T) { + // A brain that has ingested exactly once: a real database, a real cluster, + // one retained snapshot. This is the state a freshly started brain is in. + db := brainDB(t, 10, fleetSnapshot(whyCostT0, 4, 0, 500)) + var b strings.Builder - err := runBacktestTo(&b, []string{"--cluster", "prod"}) + err := runBacktestTo(&b, []string{ + "--cluster", "why-cost-demo", "--db", db, + "--from", rfc(whyCostT0), "--to", rfc(whyCostT0.Add(48 * time.Hour)), + }) if err == nil { t.Fatalf("a live backtest was accepted:\n%s", b.String()) } msg := err.Error() for _, want := range []string{ - "snapshot history is not persisted", - "pkg/store", - "SaveSnapshotAt", - "Snapshots(cluster, from, to)", - "backtest.SnapshotSource", - "--demo", + "refused", + "1 snapshot(s)", + "scorecard shaped exactly like a real one", + "empty replay", } { if !strings.Contains(msg, want) { t.Errorf("the refusal does not mention %q:\n%s", want, msg) diff --git a/cmd/kilter/brainsource.go b/cmd/kilter/brainsource.go new file mode 100644 index 0000000..ef038fb --- /dev/null +++ b/cmd/kilter/brainsource.go @@ -0,0 +1,207 @@ +package main + +import ( + "flag" + "fmt" + "io" + "log/slog" + "os" + "strings" + "time" + + "github.com/agenticode/kilter/pkg/api" + "github.com/agenticode/kilter/pkg/model" + "github.com/agenticode/kilter/pkg/pricing" + "github.com/agenticode/kilter/pkg/store" +) + +// Answering from a brain's own database. +// +// `kilter backtest --cluster`, `kilter why-cost` and `kilter explain` all +// needed the same thing and none of them had it: the snapshot history and the +// evidence substrate a running brain accumulates. pkg/store now keeps a +// time-keyed history (SaveSnapshotAt/Snapshots) and api.Brain now holds an +// *evidence.Memory, so the three verbs open the database the brain writes and +// ask the brain itself. Written once, here, because three independent +// definitions of "open the brain" is three places for them to disagree. +// +// # This is a SIBLING of the file-backed sources, not a replacement +// +// --kube-snapshot (a recorded snapshot series) and --ledger (a recorded +// ledger) keep working exactly as before and are still the only source that +// needs no database at all. A user with recorded snapshots and no brain is +// unaffected by everything in this file. The two sources answer the same +// question from different substrates and are proven to agree where both can +// answer (TestBrainAndFileSourcesAgreeOnWhyCost). +// +// # Four things an operator has to know, all of them consequences of bbolt +// +// 1. THE DATABASE MUST ALREADY EXIST. bolt.Open creates the file it is given, +// so a mistyped --db would otherwise produce an empty database and an +// answer that reads as "this cluster has no history yet" rather than "that +// path is wrong". openBrainSource stats first and refuses by name. +// 2. THE LOCK IS EXCLUSIVE. bbolt takes a file lock for the process lifetime, +// and pkg/store opens with a 5-second timeout, so these verbs cannot read +// the database of a brain that is currently running. That failure is +// wrapped with what it means rather than surfaced as "timeout". +// 3. OPENING WRITES. store.Open creates missing buckets in an Update +// transaction, so even these read-only verbs touch the file. Nothing here +// ingests, plans or actuates. +// 4. THE EVIDENCE IS AS FRESH AS THE LAST CHECKPOINT. The substrate is +// persisted every BrainConfig.CheckpointEvery snapshots (default 10) and +// on graceful shutdown; a brain killed between checkpoints loses the tail. +// The snapshot HISTORY has no such lag — SaveSnapshotAt runs inside every +// Ingest — so `backtest --cluster` sees everything the retention kept +// while `why-cost`/`explain` may not see the newest few snapshots' usage. +// +// # No mutating path is reachable from here +// +// This file imports pkg/api, pkg/store, pkg/pricing and pkg/model. It reaches +// no cloud SDK and no actuator: pkg/ec2's and pkg/rds's actuators stay +// unreachable from the binary, which TestNoActuatorIsReachableFromTheBinary +// asserts over the whole command package rather than trusting this comment. + +// brainSource is an opened brain database plus the pricing catalog to read it +// with. +// +// The store is held open for the lifetime of the command and the brain is +// built separately, because the two answer different questions: "is there a +// database, and does it know this cluster" is a store read costing one bbolt +// lookup, while restoring a brain replays every cluster's recommender state +// and the whole evidence checkpoint. A mistyped --cluster should not pay for +// the second to be told about the first. +type brainSource struct { + verb string + path string + st *store.Store + catalog *pricing.Catalog +} + +// brainExplainWindow is the trailing span `kilter explain --db` resolves to +// when no --from/--to is given. It mirrors pkg/api's own defaultExplainWindow +// (unexported there) and is resolved against the LATEST INGESTED SNAPSHOT, +// never against a clock. TestExplainOverADatabaseMatchesTheHTTPRoute pins the +// two together by comparing this command's payload against the route's, so a +// change to either side that made them disagree fails rather than drifts. +const brainExplainWindow = 24 * time.Hour + +// openBrainSource opens an existing brain database for reading. +// +// verb is the command name, so every refusal below names the command the +// operator actually typed. +func openBrainSource(verb, dbPath string, catalog *pricing.Catalog) (*brainSource, error) { + if strings.TrimSpace(dbPath) == "" { + return nil, fmt.Errorf("%s: --db is required to answer from a brain's own history", verb) + } + // Stat before open: bolt.Open would CREATE this path, and a database + // created by a typo answers "no history for that cluster" — which is true + // of the empty file it just made and says nothing about the cluster. + fi, err := os.Stat(dbPath) + switch { + case os.IsNotExist(err): + return nil, fmt.Errorf("%s --db %s: no such database. This verb reads a brain's database and "+ + "does not create one; the brain writes it (kilter brain --db %s). An empty database created "+ + "here would answer \"no history\" for every cluster, which would be a fact about the file "+ + "rather than about the cluster", verb, dbPath, dbPath) + case err != nil: + return nil, fmt.Errorf("%s --db %s: %w", verb, dbPath, err) + case fi.IsDir(): + return nil, fmt.Errorf("%s --db %s: is a directory, not a bbolt database", verb, dbPath) + } + st, err := store.Open(dbPath) + if err != nil { + // The overwhelmingly likely cause is the file lock, and "timeout" on + // its own does not say so. It is offered rather than asserted because + // a permission or corruption failure arrives here too, and claiming + // the wrong cause is worse than naming the likely one. + return nil, fmt.Errorf("%w\n\nIf that is a timeout: a bbolt database is locked by one process "+ + "at a time, so this cannot read the database of a brain that is currently running. Stop the "+ + "brain, or point --db at a copy of the file", err) + } + return &brainSource{verb: verb, path: dbPath, st: st, catalog: catalog}, nil +} + +// Close releases the database lock. +func (s *brainSource) Close() error { return s.st.Close() } + +// brain restores a brain over the open store. +// +// cfg carries whatever the caller needs to set; the logger is always replaced. +// A restore logs "brain restored" at INFO and the persistence paths log at +// ERROR, and neither belongs on the stderr of a command whose entire output is +// one report — a CLI that prints a log line before its answer teaches the +// operator to ignore log lines. +func (s *brainSource) brain(cfg api.BrainConfig) (*api.Brain, error) { + cfg.Logger = slog.New(slog.NewTextHandler(io.Discard, nil)) + b, err := api.NewBrain(cfg, s.catalog, s.st) + if err != nil { + return nil, fmt.Errorf("%s --db %s: %w", s.verb, s.path, err) + } + return b, nil +} + +// requireCluster refuses a cluster id the database has never seen, and returns +// the latest snapshot ingested for it. +// +// This is the command-line form of the 404 the HTTP routes answer with +// (pkg/api/SUBSTRATE-FINDINGS.md §3.2): "never ingested" and "ingested but +// there is not enough evidence" are different facts, and a single message +// covering both sends an operator to look for the wrong problem. The known ids +// are listed because the overwhelmingly common cause is a typo or a +// disagreement about what the agent calls this cluster. +// +// backtest --cluster deliberately does NOT call this: its refusals are +// api.ErrNoHistory and api.ErrHistoryTooShort, which pkg/api owns, and a +// pre-check here would shadow them. +func (s *brainSource) requireCluster(cluster string) (*model.ClusterSnapshot, error) { + if strings.TrimSpace(cluster) == "" { + return nil, fmt.Errorf("%s: --cluster is required with --db", s.verb) + } + snap, err := s.st.LoadSnapshot(cluster) + if err != nil { + return nil, fmt.Errorf("%s --cluster %s: %w", s.verb, cluster, err) + } + if snap == nil { + known, kerr := s.st.Clusters() + if kerr != nil { + return nil, fmt.Errorf("%s --cluster %s: %w", s.verb, cluster, kerr) + } + list := "none — nothing has been ingested into this database" + if len(known) > 0 { + list = strings.Join(known, ", ") + } + return nil, fmt.Errorf("%s --cluster %s: no snapshot for that cluster in %s. Known clusters: %s", + s.verb, cluster, s.path, list) + } + return snap, nil +} + +// setFlagNames reports which flags were named on the command line, as opposed +// to left at their default value. flag.FlagSet.Visit walks exactly the flags +// that were set, which is the only way to tell an explicit `--days 7` from an +// unmentioned --days — and telling them apart is what lets a command refuse a +// flag it cannot honour instead of ignoring it. +func setFlagNames(fs *flag.FlagSet) map[string]bool { + out := map[string]bool{} + fs.Visit(func(f *flag.Flag) { out[f.Name] = true }) + return out +} + +// refuseUnusable refuses the flags a source cannot honour, by name. +// +// The alternative is to ignore them, and pkg/backtest's policy loader already +// records why that is unacceptable: a knob that is silently dropped produces a +// report for a configuration nobody ran. The same argument applies to a flag +// that belongs to a different source of history. +func refuseUnusable(verb, why string, set map[string]bool, names ...string) error { + var named []string + for _, n := range names { + if set[n] { + named = append(named, "--"+n) + } + } + if len(named) == 0 { + return nil + } + return fmt.Errorf("%s: %s %s", verb, strings.Join(named, ", "), why) +} diff --git a/cmd/kilter/brainsourcewire_test.go b/cmd/kilter/brainsourcewire_test.go new file mode 100644 index 0000000..444fb5f --- /dev/null +++ b/cmd/kilter/brainsourcewire_test.go @@ -0,0 +1,530 @@ +package main + +import ( + "encoding/json" + "errors" + "io" + "log/slog" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/agenticode/kilter/pkg/api" + "github.com/agenticode/kilter/pkg/backtest" + "github.com/agenticode/kilter/pkg/explain" + "github.com/agenticode/kilter/pkg/model" + "github.com/agenticode/kilter/pkg/pricing" + "github.com/agenticode/kilter/pkg/store" +) + +// These tests drive the REAL api.Brain over a REAL bbolt database written by +// the REAL ingest path. Nothing is stubbed: the history under replay is the +// history SaveSnapshotAt retained, thinning and all, and the substrate the +// explanations are built from is the one Ingest populated and checkpointed. +// +// No network and no cloud call anywhere. The one HTTP comparison below uses +// httptest.NewRecorder against Brain.Handler() in-process, so not even a +// loopback socket is opened. + +func testCatalog(t *testing.T) *pricing.Catalog { + t.Helper() + cat, err := loadCatalog("") + if err != nil { + t.Fatal(err) + } + return cat +} + +// brainDB writes a brain database the way `kilter brain` writes one: through +// api.Brain.Ingest, which is what fills both the time-keyed snapshot history +// and the evidence substrate. +// +// checkpointEvery is explicit because it is the freshness bound on everything +// why-cost and explain can see. A brain at the default 10 has not persisted +// the last nine snapshots' evidence, so a test that ingested five and then +// read the file would see an EMPTY substrate — which is a property of the +// brain, not of this helper, and is recorded in cmd/BRAINWIRE-FINDINGS.md. +func brainDB(t *testing.T, checkpointEvery int, snaps ...*model.ClusterSnapshot) string { + t.Helper() + path := filepath.Join(t.TempDir(), "brain.db") + st, err := store.Open(path) + if err != nil { + t.Fatal(err) + } + brain, err := api.NewBrain(api.BrainConfig{ + CheckpointEvery: checkpointEvery, + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + }, testCatalog(t), st) + if err != nil { + st.Close() + t.Fatal(err) + } + for _, s := range snaps { + if err := brain.Ingest(s); err != nil { + st.Close() + t.Fatalf("ingest %s: %v", s.Timestamp, err) + } + } + if err := st.Close(); err != nil { + t.Fatal(err) + } + return path +} + +func rfc(t time.Time) string { return t.UTC().Format(time.RFC3339) } + +// ---------------------------------------------------------------- the trap + +// TestBacktestOverADatabaseRefusesAHistoryThatWouldScoreNothing. +// +// THE trap this unit exists to avoid. The database below holds a REAL history: +// two retained snapshots, readable, priced, with pods and nodes, inside the +// requested window. Nothing about it is empty. It simply cannot be replayed — +// both snapshots land 30 hours into a 48-hour window, so no decision instant +// has a full 24-hour future left inside it — and backtest.Run does NOT fail on +// that. It returns a Scorecard reading `snapshots 2`, `regret $0.00`, with the +// same field names and the same confident tone as a scorecard over a month of +// history, and an operator cannot tell "nothing was replayed" from "the policy +// is perfect". +// +// api.Brain.Backtest checks backtest's OWN coverage report (Scorecard.Instants, +// not a predicate re-derived in cmd) and refuses. This asserts the refusal is +// reachable from the command line, exits non-zero, and prints NO SCORECARD. +// +// The window is deliberately WIDER than the horizon: backtest.Run has its own +// "horizon exceeds the replay window" guard, and a window narrower than the +// horizon would be caught by that instead, leaving Instants == 0 untested. +func TestBacktestOverADatabaseRefusesAHistoryThatWouldScoreNothing(t *testing.T) { + t0 := whyCostT0 + db := brainDB(t, 10, + fleetSnapshot(t0.Add(30*time.Hour), 4, 0, 500), + fleetSnapshot(t0.Add(31*time.Hour), 5, 1, 700), + ) + + var b strings.Builder + err := runBacktestTo(&b, []string{ + "--cluster", "why-cost-demo", "--db", db, + "--from", rfc(t0), "--to", rfc(t0.Add(48 * time.Hour)), + }) + if err == nil { + t.Fatalf("a scorecard over an unscoreable history was accepted:\n%s", b.String()) + } + // The refusal names what was there and what it yielded, because "not + // enough history" without numbers is indistinguishable from a bug. + for _, want := range []string{"refused", "2 snapshot(s)", "no decision instant", "24h"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("the refusal does not mention %q:\n%s", want, err) + } + } + assertNoScorecard(t, b.String()) + + // And the typed error survives the trip through cmd: rendering it is the + // command's job, classifying it is pkg/api's. + var tooShort api.ErrHistoryTooShort + if !errors.As(err, &tooShort) { + t.Fatalf("want api.ErrHistoryTooShort, got %T: %v", err, err) + } + if tooShort.Snapshots != 2 || tooShort.Instants != 0 { + t.Errorf("ErrHistoryTooShort reported snapshots=%d instants=%d, want 2 and 0", + tooShort.Snapshots, tooShort.Instants) + } +} + +// TestBacktestOverADatabaseRefusesASingleSnapshot pins the other half of +// api.ErrHistoryTooShort: a history below the two-snapshot floor, where the +// count check is the one that fires. Scoring the one snapshot that exists is +// exactly what cmd/WIRING-FINDINGS.md §6.2 refused to do. +func TestBacktestOverADatabaseRefusesASingleSnapshot(t *testing.T) { + t0 := whyCostT0 + db := brainDB(t, 10, fleetSnapshot(t0, 4, 0, 500)) + + var b strings.Builder + err := runBacktestTo(&b, []string{ + "--cluster", "why-cost-demo", "--db", db, + "--from", rfc(t0), "--to", rfc(t0.Add(48 * time.Hour)), + }) + if err == nil { + t.Fatalf("a scorecard over one snapshot was accepted:\n%s", b.String()) + } + for _, want := range []string{"refused", "1 snapshot(s)", "empty replay"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("the refusal does not mention %q:\n%s", want, err) + } + } + assertNoScorecard(t, b.String()) +} + +// TestBacktestOverADatabaseRefusesAnEmptyBrain pins api.ErrNoHistory's +// sibling: a database that exists and has never been ingested into. The +// cluster is unknown, so the history is empty, and the count check fires with +// zero — not a scorecard of zeros. +func TestBacktestOverADatabaseRefusesAClusterWithNoHistory(t *testing.T) { + db := brainDB(t, 10) + var b strings.Builder + err := runBacktestTo(&b, []string{ + "--cluster", "never-ingested", "--db", db, + "--from", rfc(whyCostT0), "--to", rfc(whyCostT0.Add(48 * time.Hour)), + }) + if err == nil { + t.Fatalf("a scorecard over an empty database was accepted:\n%s", b.String()) + } + if !strings.Contains(err.Error(), "0 snapshot(s)") { + t.Errorf("the refusal does not say how much history there was:\n%s", err) + } + assertNoScorecard(t, b.String()) +} + +// assertNoScorecard is the assertion every refusal above shares: the point is +// not that an error was returned, it is that no scorecard-shaped output +// reached the operator to be read as a verdict. +func assertNoScorecard(t *testing.T, out string) { + t.Helper() + for _, forbidden := range []string{"regret", "oracleGap", "flipRate", "safety"} { + if strings.Contains(out, forbidden) { + t.Errorf("a scorecard was printed for a refused replay (%q):\n%s", forbidden, out) + } + } +} + +// ---------------------------------------------------------------- the wiring + +// traceDB generates a realistic multi-day history with pkg/backtest's own +// trace builder and ingests it through api.Brain.Ingest, so the database holds +// what a running brain would have kept — including the hourly thinning +// pkg/store applies on the way in. +func traceDB(t *testing.T, kind backtest.TraceKind, days int) (path, cluster string, start time.Time) { + t.Helper() + spec := backtest.TraceSpec{ + Cluster: "replay-" + string(kind), Kind: kind, + Start: backtestEpoch, Days: days, Workloads: 2, + // Hourly rather than the 5-minute default: pkg/store thins the + // history to an hour anyway, so a denser trace would cost a thousand + // ingests to store exactly the same rows. + Interval: time.Hour, + } + trace, err := spec.Build() + if err != nil { + t.Fatal(err) + } + return brainDB(t, 10, trace.Snapshots...), trace.Cluster, trace.Start +} + +// TestBacktestOverADatabaseScoresTheRetainedHistory. +// +// The other side of the refusal: given a history long enough to replay, the +// command produces a real scorecard over the cluster's OWN snapshots. This is +// the capability cmd/WIRING-FINDINGS.md §6.2 refused and pkg/store's +// time-keyed bucket unblocked, asserted end to end through the CLI. +func TestBacktestOverADatabaseScoresTheRetainedHistory(t *testing.T) { + db, cluster, start := traceDB(t, backtest.TraceRegimeChange, 6) + args := []string{ + "--cluster", cluster, "--db", db, + "--from", rfc(start), "--to", rfc(start.Add(6 * 24 * time.Hour)), + } + + var b strings.Builder + if err := runBacktestTo(&b, append(append([]string{}, args...), "--json")); err != nil { + t.Fatalf("kilter backtest --cluster: %v\n%s", err, b.String()) + } + var sc backtest.Scorecard + if err := json.Unmarshal([]byte(b.String()), &sc); err != nil { + t.Fatalf("decode scorecard: %v\n%s", err, b.String()) + } + if sc.Cluster != cluster { + t.Errorf("scorecard is for %q, want %q", sc.Cluster, cluster) + } + // The refusal's own gate, from the other direction: a scorecard only + // escapes when something was actually replayed. + if sc.Instants == 0 { + t.Fatal("a scorecard escaped with zero decision instants — the refusal is not gating") + } + if sc.Snapshots < 2 { + t.Errorf("replayed %d snapshots, want the retained history", sc.Snapshots) + } + // The history is what SaveSnapshotAt retained, not what was ingested: the + // trace is hourly and so is the retention, so all of it survived. + if want := 6 * 24; sc.Snapshots != want { + t.Errorf("replayed %d snapshots, want the %d the hourly retention kept", sc.Snapshots, want) + } + + // Byte-identical across runs, over a database: no clock reaches the + // replay, and the history read is not order-dependent. + var again strings.Builder + if err := runBacktestTo(&again, append(append([]string{}, args...), "--json")); err != nil { + t.Fatal(err) + } + if again.String() != b.String() { + t.Error("two replays of the same database disagreed byte for byte") + } + + // And the text rendering names the source, so a scorecard pasted into a + // ticket says which history produced it. + var text strings.Builder + if err := runBacktestTo(&text, args); err != nil { + t.Fatal(err) + } + for _, want := range []string{"cluster " + cluster, "regret", rfc(start)} { + if !strings.Contains(text.String(), want) { + t.Errorf("the scorecard header does not mention %q:\n%s", want, text.String()) + } + } +} + +// ---------------------------------------------------------------- agreement + +// TestBrainAndFileSourcesAgreeOnWhyCost. +// +// The brain-backed source is a SIBLING of --kube-snapshot, and the claim that +// makes it a sibling rather than a second implementation is this one: over the +// same snapshots and the same window, the two produce the SAME ANSWER, byte +// for byte, in the form a machine reads. +// +// It is asserted over the JSON rather than the prose because JSON carries the +// evidence IDs: the terms, the residual and every citation have to match, not +// just the dollar amounts. +// +// "Where both can answer" is a real qualifier and TWO things narrow it. +// +// 1. SPACING. pkg/store thins the history to one snapshot per hour, so a +// 5-minute series would leave the brain with fewer composition edges than +// the file series holds. Twelve hours apart, the retention keeps +// everything and there is nothing left to disagree about. +// 2. EVENTS. The brain's substrate also records deploys and OOMKills, which +// the file path never observes, and pkg/explain cites them where they fall +// inside a term. The fixture here therefore holds its container sizing +// CONSTANT — the subtest below changes it on purpose and pins what the +// difference is. +func TestBrainAndFileSourcesAgreeOnWhyCost(t *testing.T) { + t0 := whyCostT0 + const steadySizing = 500 + snaps := []*model.ClusterSnapshot{ + fleetSnapshot(t0, 4, 0, steadySizing), + fleetSnapshot(t0.Add(12*time.Hour), 5, 1, steadySizing), + fleetSnapshot(t0.Add(24*time.Hour), 4, 2, steadySizing), + } + fromFile, fromBrain := bothWhyCostSources(t, snaps, rfc(t0), rfc(t0.Add(25*time.Hour))) + if fromBrain != fromFile { + t.Errorf("the two sources disagree over the same history\n--kube-snapshot:\n%s\n--db:\n%s", + fromFile, fromBrain) + } + + // Not vacuous: the answer both produced is a real decomposition. + var att explain.Attribution + if err := json.Unmarshal([]byte(fromBrain), &att); err != nil { + t.Fatal(err) + } + if len(att.Terms) == 0 { + t.Fatal("the agreed answer has no terms, so agreeing about it proves nothing") + } + var sum explain.Micro + for _, term := range att.Terms { + sum += term.Micro + } + if sum+att.Residual.Micro != att.DeltaMicro { + t.Errorf("sum(terms)=%d + residual=%d != delta=%d", sum, att.Residual.Micro, att.DeltaMicro) + } +} + +// TestTheBrainCitesMoreThanAFileCanAndTheMoneyIsUnchanged. +// +// The one way the two sources legitimately differ, pinned so it cannot drift +// into a difference of substance. A brain sees DEPLOYS — it holds the previous +// snapshot's declared sizing and diffs it — while --kube-snapshot observes +// timeline points only. Where a deploy falls inside a term, pkg/explain cites +// it. +// +// So the brain's citations are a strict SUPERSET and every amount is +// identical. If the amounts ever differed, one of the two would be wrong about +// what a cost change is; if the citations were equal, the brain would be +// throwing away evidence it holds. +func TestTheBrainCitesMoreThanAFileCanAndTheMoneyIsUnchanged(t *testing.T) { + t0 := whyCostT0 + snaps := []*model.ClusterSnapshot{ + fleetSnapshot(t0, 4, 0, 500), + fleetSnapshot(t0.Add(12*time.Hour), 5, 1, 700), // the workload was resized + fleetSnapshot(t0.Add(24*time.Hour), 4, 2, 900), // and resized again + } + rawFile, rawBrain := bothWhyCostSources(t, snaps, rfc(t0), rfc(t0.Add(25*time.Hour))) + if rawFile == rawBrain { + t.Fatal("the brain recorded no deploy event, so this asserts nothing") + } + + var file, brain explain.Attribution + if err := json.Unmarshal([]byte(rawFile), &file); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal([]byte(rawBrain), &brain); err != nil { + t.Fatal(err) + } + if file.DeltaMicro != brain.DeltaMicro || file.Residual.Micro != brain.Residual.Micro { + t.Errorf("the two sources disagree about the money: delta %d vs %d, residual %d vs %d", + file.DeltaMicro, brain.DeltaMicro, file.Residual.Micro, brain.Residual.Micro) + } + if len(file.Terms) != len(brain.Terms) { + t.Fatalf("term counts differ: %d vs %d", len(file.Terms), len(brain.Terms)) + } + var extra int + for i := range file.Terms { + ft, bt := file.Terms[i], brain.Terms[i] + if ft.Kind != bt.Kind || ft.Micro != bt.Micro { + t.Errorf("term %d: %s=%d (file) vs %s=%d (brain)", i, ft.Kind, ft.Micro, bt.Kind, bt.Micro) + } + extra += compareCitations(t, ft, bt) + if len(ft.Of) != len(bt.Of) { + t.Errorf("term %q: sub-term counts differ, %d vs %d", ft.Kind, len(ft.Of), len(bt.Of)) + continue + } + for j := range ft.Of { + if ft.Of[j].Kind != bt.Of[j].Kind || ft.Of[j].Micro != bt.Of[j].Micro { + t.Errorf("sub-term %d of %q disagrees: %s=%d vs %s=%d", j, ft.Kind, + ft.Of[j].Kind, ft.Of[j].Micro, bt.Of[j].Kind, bt.Of[j].Micro) + } + extra += compareCitations(t, ft.Of[j], bt.Of[j]) + } + } + if extra <= 0 { + t.Error("the brain cited nothing extra, so its deploy events reached no term") + } + // The extra citations are deploy events, which is the whole claim: the + // brain saw a rollout the snapshot files never described. + if !strings.Contains(rawBrain, "/deploy@") { + t.Error("the brain's extra citations are not deploy events") + } +} + +// compareCitations asserts the brain kept every citation the file source +// carries and reports how many it added. +func compareCitations(t *testing.T, file, brain explain.Term) int { + t.Helper() + have := map[explain.ID]bool{} + for _, id := range brain.Evidence { + have[id] = true + } + for _, id := range file.Evidence { + if !have[id] { + t.Errorf("term %q: the brain dropped the citation %q the file source carries", file.Kind, id) + } + } + return len(brain.Evidence) - len(file.Evidence) +} + +// bothWhyCostSources runs the same window through both sources over the same +// snapshots: once from a brain database that ingested them, once from the JSON +// files. CheckpointEvery is 1 because the substrate has to be ON DISK for +// another process to read it, and a brain at the default 10 would have +// persisted none of a three-snapshot history — a property of the brain, not of +// this test, and recorded in cmd/BRAINWIRE-FINDINGS.md. +func bothWhyCostSources(t *testing.T, snaps []*model.ClusterSnapshot, from, to string) (file, brain string) { + t.Helper() + db := brainDB(t, 1, snaps...) + dir := t.TempDir() + fileArgs := []string{} + for i, s := range snaps { + fileArgs = append(fileArgs, "--kube-snapshot", + writeSnapshot(t, dir, "snap-"+string(rune('0'+i))+".json", s)) + } + window := []string{"--from", from, "--to", to, "--json"} + return runWhyCostOK(t, append(append([]string{}, fileArgs...), window...)...), + runWhyCostOK(t, append([]string{"--db", db, "--cluster", snaps[0].ClusterID}, window...)...) +} + +// TestExplainOverADatabaseMatchesTheHTTPRoute. +// +// `kilter explain --db` and `GET /api/v1/clusters/{id}/explain` must be the +// same answer, because they are the same brain answering the same question — +// and the thing most likely to make them differ is the DEFAULT WINDOW. +// pkg/api's defaultExplainWindow is unexported, so cmd carries its own +// brainExplainWindow constant, and a constant copied by hand is a constant +// that drifts. This pins them to each other: both sides resolve the window +// from the latest ingested snapshot (never a clock), and if either changed the +// span or the +1s edge, the payloads would stop matching. +// +// The route is exercised through Brain.Handler() with httptest.NewRecorder, so +// no socket is opened. +func TestExplainOverADatabaseMatchesTheHTTPRoute(t *testing.T) { + snap := loadFixtureSnapshot(t) + db := brainDB(t, 1, snap) + + // The CLI, with NO --from/--to: the default window is what is under test. + var cli strings.Builder + if err := runExplainTo(&cli, []string{ + "--db", db, "--cluster", snap.ClusterID, + "--workload", "Deployment/default/api", "--container", "api", "--json", + }); err != nil { + t.Fatalf("kilter explain --db: %v\n%s", err, cli.String()) + } + + // The route, over the same database, opened after the command released + // the lock. + st, err := store.Open(db) + if err != nil { + t.Fatal(err) + } + defer st.Close() + brain, err := api.NewBrain(api.BrainConfig{ + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + }, testCatalog(t), st) + if err != nil { + t.Fatal(err) + } + rec := httptest.NewRecorder() + brain.Handler().ServeHTTP(rec, httptest.NewRequest("GET", + "/api/v1/clusters/"+snap.ClusterID+"/explain?subject=Deployment/default/api/api", nil)) + if rec.Code != 200 { + t.Fatalf("route answered %d: %s", rec.Code, rec.Body.String()) + } + + if got, want := canonicalJSON(t, cli.String()), canonicalJSON(t, rec.Body.String()); got != want { + t.Errorf("the command and the route disagree\ncommand: %s\nroute: %s", got, want) + } + + // Not vacuous: the agreed payload is a grounded explanation. + var payload explain.Explanation + if err := json.Unmarshal([]byte(cli.String()), &payload); err != nil { + t.Fatal(err) + } + if len(payload.Drivers) == 0 || len(payload.Citations) == 0 { + t.Fatal("the agreed payload has no drivers or no citations") + } + // The window came from the snapshot, not from a clock: it ends one second + // after the latest ingested snapshot, exactly as the route defines it. + if want := snap.Timestamp.Add(time.Second); !payload.To.Equal(want) { + t.Errorf("window ends %s, want %s (latest snapshot + 1s)", payload.To, want) + } + if want := snap.Timestamp.Add(time.Second).Add(-brainExplainWindow); !payload.From.Equal(want) { + t.Errorf("window starts %s, want %s", payload.From, want) + } +} + +// loadFixtureSnapshot reads the recorded cluster snapshot the file-backed +// explain tests already use, so both sources are fed identical bytes. +func loadFixtureSnapshot(t *testing.T) *model.ClusterSnapshot { + t.Helper() + raw, err := os.ReadFile(readFixture(t, "cluster.json")) + if err != nil { + t.Fatal(err) + } + var snap model.ClusterSnapshot + if err := json.Unmarshal(raw, &snap); err != nil { + t.Fatal(err) + } + return &snap +} + +// canonicalJSON re-encodes a payload so two encoders' whitespace choices are +// not mistaken for a disagreement about the answer. +func canonicalJSON(t *testing.T, raw string) string { + t.Helper() + var v any + if err := json.Unmarshal([]byte(raw), &v); err != nil { + t.Fatalf("decode %q: %v", raw, err) + } + out, err := json.Marshal(v) + if err != nil { + t.Fatal(err) + } + return string(out) +} diff --git a/cmd/kilter/explain.go b/cmd/kilter/explain.go index 9a42d57..2e295d9 100644 --- a/cmd/kilter/explain.go +++ b/cmd/kilter/explain.go @@ -23,12 +23,27 @@ import ( // `kilter explain` and `kilter why-cost` — the explanation plane, made // runnable. // -// Both commands read a SEQUENCE of recorded cluster snapshots, the format -// `kilter analyze --dump-snapshot` already writes. That is the honest input: -// pkg/explain has no clock and no network, its window is an argument, and both -// entry points need history that pkg/store does not keep (see -// cmd/WIRING-FINDINGS.md). Feeding them recorded snapshots is the same -// discipline `kilter domains` and `kilter simulate` already use. +// Both commands answer from one of TWO sources, and the choice is explicit: +// +// - --kube-snapshot (repeatable), a recorded snapshot series, plus --ledger +// for the recorded audit trail. This is the original source and it is +// unchanged: no database, no brain, nothing to run first. It is the same +// discipline `kilter domains` and `kilter simulate` already use. +// - --db PATH --cluster ID, a brain's own database. api.Brain now holds the +// evidence substrate these payloads are built from and Brain.WhyCost / +// Brain.Explain are exported, so a brain that has been ingesting can be +// asked about itself instead of being re-fed its own snapshots by hand. +// Both entry points call Verify internally, so neither can hand back an +// unverified payload. +// +// The second source is a SIBLING of the first, not a replacement. Passing both +// is refused rather than resolved by precedence — two substrates that disagree +// should surface as a question, not as a silent winner — and the two are +// proven to agree where both can answer, in cmd/kilter/brainsourcewire_test.go. +// +// See cmd/kilter/brainsource.go for what an operator must know about reading a +// brain's database: it must exist, it cannot be read while the brain holds the +// lock, and the substrate is only as fresh as the last checkpoint. // // # The publish gate is not optional // @@ -45,16 +60,24 @@ const explainUsage = `kilter explain — why the engine would resize this contai Usage: kilter explain --kube-snapshot PATH [--kube-snapshot PATH ...] \ --workload Kind/namespace/name --container NAME [flags] + kilter explain --db PATH --cluster ID \ + --workload Kind/namespace/name --container NAME [flags] Snapshots are replayed in timestamp order through the real recommender, so at least two are needed before anything can be said. Nothing here calls a cluster. +--db reads a brain's own evidence substrate instead; the two sources are +siblings and exactly one may be given. Flags: --kube-snapshot PATH cluster snapshot JSON (repeatable, any order) + --db PATH brain database written by "kilter brain --db PATH" + --cluster ID cluster inside --db (required with --db) --workload REF Kind/namespace/name, e.g. Deployment/default/api --container NAME container within the workload - --from RFC3339 evidence window start (default: the first snapshot) - --to RFC3339 evidence window end (default: the last snapshot) + --from RFC3339 evidence window start (default: the first snapshot; + with --db, 24h back from the latest ingested snapshot) + --to RFC3339 evidence window end (default: the last snapshot; + with --db, one second after the latest ingested snapshot) --catalog PATH pricing catalog JSON (default: embedded) --json emit the payload instead of the prose ` @@ -64,6 +87,7 @@ const whyCostUsage = `kilter why-cost — an additive, individually-citable cost Usage: kilter why-cost --kube-snapshot PATH [--kube-snapshot PATH ...] \ --from RFC3339 --to RFC3339 [flags] + kilter why-cost --db PATH --cluster ID --from RFC3339 --to RFC3339 [flags] --from and --to are REQUIRED. There is no default window: the window is an argument, and a wall-clock default makes a stored answer unreplayable. @@ -75,10 +99,13 @@ construction. Flags: --kube-snapshot PATH cluster snapshot JSON (repeatable, any order) + --db PATH brain database written by "kilter brain --db PATH" + --cluster ID cluster inside --db (required with --db) --from RFC3339 window start, inclusive (required) --to RFC3339 window end, EXCLUSIVE (required) — [from, to), matching pkg/evidence's window convention --ledger PATH kilter ledger --json output, for the kilter-action term + (--kube-snapshot only: with --db the brain reads its own) --catalog PATH pricing catalog JSON (default: embedded) --json emit the attribution instead of the prose ` @@ -93,6 +120,8 @@ func runWhyCostTo(w io.Writer, args []string) error { fs.SetOutput(w) var snaps repeatedFlag fs.Var(&snaps, "kube-snapshot", "cluster snapshot JSON (repeatable)") + dbPath := fs.String("db", "", "brain database holding the evidence substrate") + clusterID := fs.String("cluster", "", "cluster inside --db") from := fs.String("from", "", "window start (RFC3339, required)") to := fs.String("to", "", "window end (RFC3339, required)") ledgerPath := fs.String("ledger", "", "kilter ledger --json output") @@ -101,9 +130,19 @@ func runWhyCostTo(w io.Writer, args []string) error { if err := fs.Parse(args); err != nil { return err } - if len(snaps) == 0 { - fmt.Fprint(w, whyCostUsage) - return fmt.Errorf("why-cost: at least one --kube-snapshot is required") + set := setFlagNames(fs) + if err := chooseSource(w, "why-cost", whyCostUsage, len(snaps) > 0, *dbPath, *clusterID); err != nil { + return err + } + // The brain reads its OWN ledger (api.Brain.ledgerActions, the same + // projection loadLedgerActions performs over the JSON form). Splicing a + // second, file-supplied one in would give the attribution two sources for + // "which actions moved money" and no rule for reconciling them. + if *dbPath != "" { + if err := refuseUnusable("why-cost --db", "read a recorded ledger file; a brain attributes "+ + "actions from the audit ledger it kept itself", set, "ledger"); err != nil { + return err + } } if *from == "" || *to == "" { fmt.Fprint(w, whyCostUsage) @@ -125,6 +164,9 @@ func runWhyCostTo(w io.Writer, args []string) error { if err != nil { return err } + if *dbPath != "" { + return whyCostFromBrain(w, *dbPath, *clusterID, start, end, catalog, *jsonOut) + } series, err := loadSnapshotSeries(snaps) if err != nil { return err @@ -198,13 +240,84 @@ func runWhyCostTo(w io.Writer, args []string) error { return fmt.Errorf("why-cost: the answer has a citation that does not resolve, so it is not "+ "publishable: %w", err) } - if *jsonOut { + return renderAttribution(w, att, *jsonOut) +} + +// whyCostFromBrain answers from a brain's own substrate. +// +// Everything the file-backed path above does by hand — build a substrate, +// observe a timeline point per snapshot, price both window edges, project the +// ledger, call Verify — api.Brain.WhyCost already does over the substrate the +// brain accumulated while it was running. This function therefore contains no +// projection of its own: a second definition of any of those steps is a second +// answer waiting to disagree with the first, which is precisely why pkg/api +// lifted loadLedgerActions rather than re-deriving it. +// +// The one thing done here and not there is the unknown-cluster check, which is +// the command-line form of the route's 404. Brain.WhyCost answers "not enough +// evidence" for a cluster that was never ingested, and that is true but sends +// the operator to look for the wrong problem. +func whyCostFromBrain(w io.Writer, dbPath, cluster string, start, end time.Time, + catalog *pricing.Catalog, jsonOut bool) error { + src, err := openBrainSource("why-cost", dbPath, catalog) + if err != nil { + return err + } + defer src.Close() + if _, err := src.requireCluster(cluster); err != nil { + return err + } + brain, err := src.brain(api.BrainConfig{}) + if err != nil { + return err + } + // Verify is called inside WhyCost, so an unverifiable answer arrives here + // as an error and never as a payload. + att, err := brain.WhyCost(cluster, start, end) + if err != nil { + return fmt.Errorf("why-cost --cluster %s: %w", cluster, err) + } + return renderAttribution(w, att, jsonOut) +} + +// renderAttribution is the single rendering of an attribution, so the two +// sources print the same bytes for the same answer — which is what lets a test +// compare them directly. +func renderAttribution(w io.Writer, att *explain.Attribution, jsonOut bool) error { + if jsonOut { return writeJSON(w, att) } - _, err = io.WriteString(w, att.Prose()+"\n") + _, err := io.WriteString(w, att.Prose()+"\n") return err } +// chooseSource enforces "exactly one source of history", for both verbs. +// +// Refusing beats a precedence rule. A user who passes both has two substrates +// in mind and no way to know which one answered; if they disagree — and a +// thinned snapshot history and a hand-picked snapshot series can — the silent +// winner is the wrong artefact. Naming the conflict costs one run and removes +// the ambiguity permanently. +func chooseSource(w io.Writer, verb, usage string, haveFiles bool, dbPath, cluster string) error { + switch { + case haveFiles && dbPath != "": + return fmt.Errorf("%s: --kube-snapshot and --db are two different substrates for the same "+ + "question; pass one. A recorded snapshot series and a brain's retained history can "+ + "legitimately disagree, so neither is preferred silently", verb) + case dbPath == "" && cluster != "": + return fmt.Errorf("%s: --cluster names a cluster inside a brain database; it needs --db PATH. "+ + "With --kube-snapshot the cluster comes from the snapshots themselves", verb) + case dbPath != "" && cluster == "": + return fmt.Errorf("%s: --db needs --cluster ID; a database holds every cluster that ever "+ + "reported to that brain", verb) + case !haveFiles && dbPath == "": + fmt.Fprint(w, usage) + return fmt.Errorf("%s: a source of history is required — either --kube-snapshot PATH "+ + "(repeatable) or --db PATH --cluster ID", verb) + } + return nil +} + // basisFrom prices one snapshot's fleet composition. // // Three things pkg/explain/FINDINGS.md says the wiring must get right, all @@ -323,6 +436,8 @@ func runExplainTo(w io.Writer, args []string) error { fs.SetOutput(w) var snaps repeatedFlag fs.Var(&snaps, "kube-snapshot", "cluster snapshot JSON (repeatable)") + dbPath := fs.String("db", "", "brain database holding the evidence substrate") + clusterID := fs.String("cluster", "", "cluster inside --db") workload := fs.String("workload", "", "Kind/namespace/name") container := fs.String("container", "", "container name") from := fs.String("from", "", "evidence window start (RFC3339)") @@ -332,9 +447,12 @@ func runExplainTo(w io.Writer, args []string) error { if err := fs.Parse(args); err != nil { return err } - if len(snaps) == 0 || *workload == "" || *container == "" { + if err := chooseSource(w, "explain", explainUsage, len(snaps) > 0, *dbPath, *clusterID); err != nil { + return err + } + if *workload == "" || *container == "" { fmt.Fprint(w, explainUsage) - return fmt.Errorf("explain: --kube-snapshot, --workload and --container are required") + return fmt.Errorf("explain: --workload and --container are required") } ref, err := parseWorkloadRef(*workload) if err != nil { @@ -344,6 +462,11 @@ func runExplainTo(w io.Writer, args []string) error { if err != nil { return err } + if *dbPath != "" { + return explainFromBrain(w, *dbPath, *clusterID, + model.ContainerKey{Workload: ref, Container: *container}, + *from, *to, catalog, *jsonOut) + } series, err := loadSnapshotSeries(snaps) if err != nil { return err @@ -421,7 +544,69 @@ func runExplainTo(w io.Writer, args []string) error { return fmt.Errorf("explain: the answer has a citation that does not resolve, so it is not "+ "publishable: %w", err) } - if *jsonOut { + return renderExplanation(w, key, start, end, payload, *jsonOut) +} + +// explainFromBrain answers from a brain's own substrate. +// +// The window is the only thing this function decides, and it decides it the +// way the HTTP route does: 24 hours ending ONE SECOND AFTER the latest +// ingested snapshot, resolved from the store rather than from a clock, so two +// runs against an idle database produce the same window and therefore the same +// bytes. pkg/api's defaultExplainWindow is unexported, so the constant is +// mirrored here and pinned to the route by +// TestExplainOverADatabaseMatchesTheHTTPRoute rather than by comment. +// +// The subject is the four-segment container form. The three-segment workload +// subject api.Brain.Explain also accepts stays reachable over HTTP only — +// `kilter explain` has taken --workload AND --container since it shipped, and +// making --container optional would change what an existing command line +// means. +func explainFromBrain(w io.Writer, dbPath, cluster string, key model.ContainerKey, + fromRaw, toRaw string, catalog *pricing.Catalog, jsonOut bool) error { + src, err := openBrainSource("explain", dbPath, catalog) + if err != nil { + return err + } + defer src.Close() + latest, err := src.requireCluster(cluster) + if err != nil { + return err + } + defTo := latest.Timestamp.Add(time.Second) + start, end := defTo.Add(-brainExplainWindow), defTo + if fromRaw != "" { + if start, err = time.Parse(time.RFC3339, fromRaw); err != nil { + return fmt.Errorf("--from: %w", err) + } + } + if toRaw != "" { + if end, err = time.Parse(time.RFC3339, toRaw); err != nil { + return fmt.Errorf("--to: %w", err) + } + } + brain, err := src.brain(api.BrainConfig{}) + if err != nil { + return err + } + // Kind/namespace/name/container is api.Brain.Explain's container form. + subject := string(key.Workload.Kind) + "/" + key.Workload.Namespace + "/" + + key.Workload.Name + "/" + key.Container + // Verify is called inside Explain, so a payload with a dangling citation + // arrives here as an error. + payload, err := brain.Explain(cluster, subject, start, end) + if err != nil { + return fmt.Errorf("explain --cluster %s: %w", cluster, err) + } + return renderExplanation(w, key, start, end, payload, jsonOut) +} + +// renderExplanation is the single rendering of an explanation, shared by both +// sources so the same answer prints the same bytes whichever substrate it came +// from. +func renderExplanation(w io.Writer, key model.ContainerKey, start, end time.Time, + payload *explain.Explanation, jsonOut bool) error { + if jsonOut { return writeJSON(w, payload) } var b strings.Builder @@ -429,7 +614,7 @@ func runExplainTo(w io.Writer, args []string) error { start.UTC().Format(time.RFC3339), end.UTC().Format(time.RFC3339)) b.WriteString(payload.Prose()) b.WriteString("\n") - _, err = io.WriteString(w, b.String()) + _, err := io.WriteString(w, b.String()) return err } diff --git a/cmd/kilter/sourcewire_test.go b/cmd/kilter/sourcewire_test.go new file mode 100644 index 0000000..a86248f --- /dev/null +++ b/cmd/kilter/sourcewire_test.go @@ -0,0 +1,295 @@ +package main + +import ( + "io" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +// The rules that keep two sources of history from becoming one ambiguous one. +// +// Every assertion here is about a REFUSAL, and every refusal exists because +// the alternative is a plausible-looking answer: a database created by a typo +// answering "no history", a flag silently dropped, a window that moves with +// the wall clock, or two substrates resolved by an undocumented precedence. + +type verbFn func(io.Writer, []string) error + +func verbs() map[string]verbFn { + return map[string]verbFn{ + "backtest": runBacktestTo, + "why-cost": runWhyCostTo, + "explain": runExplainTo, + } +} + +// dbArgs is a minimal, otherwise-valid command line for each verb pointed at +// dbPath, so the only thing under test is the database. +func dbArgs(verb, dbPath, cluster string) []string { + t0 := rfc(whyCostT0) + t1 := rfc(whyCostT0.Add(48 * time.Hour)) + switch verb { + case "backtest": + return []string{"--cluster", cluster, "--db", dbPath, "--from", t0, "--to", t1} + case "why-cost": + return []string{"--db", dbPath, "--cluster", cluster, "--from", t0, "--to", t1} + default: + return []string{"--db", dbPath, "--cluster", cluster, + "--workload", "Deployment/default/api", "--container", "api"} + } +} + +// TestReadVerbsRefuseAMissingDatabaseRatherThanCreatingOne. +// +// bolt.Open CREATES the file it is given. Left alone, `--db /tpm/kilter.db` +// would therefore succeed, make an empty database, and answer "no history for +// that cluster" — a true statement about the file it had just created and a +// completely misleading one about the cluster. All three verbs stat first and +// refuse, and the file must still not exist afterwards. +func TestReadVerbsRefuseAMissingDatabaseRatherThanCreatingOne(t *testing.T) { + for verb, run := range verbs() { + t.Run(verb, func(t *testing.T) { + missing := filepath.Join(t.TempDir(), "typo.db") + var b strings.Builder + err := run(&b, dbArgs(verb, missing, "prod")) + if err == nil { + t.Fatalf("a missing database was accepted:\n%s", b.String()) + } + if !strings.Contains(err.Error(), "no such database") { + t.Errorf("error = %q, want it to name the missing database", err) + } + if _, statErr := os.Stat(missing); !os.IsNotExist(statErr) { + t.Errorf("a read verb created %s", missing) + } + }) + } +} + +// TestBrainBackedVerbsRefuseAnUnknownCluster. +// +// The command-line form of the routes' 404. "never ingested" and "ingested, +// but there is not enough evidence" are different facts, and merging them +// sends an operator to look for the wrong problem — so the refusal names the +// clusters the database does hold, because the usual cause is a typo or a +// disagreement about what the agent calls this cluster. +// +// backtest is deliberately absent: its refusals belong to api.Brain.Backtest, +// and a pre-check in cmd would shadow ErrHistoryTooShort. +func TestBrainBackedVerbsRefuseAnUnknownCluster(t *testing.T) { + db := brainDB(t, 1, fleetSnapshot(whyCostT0, 4, 0, 500)) + for _, verb := range []string{"why-cost", "explain"} { + t.Run(verb, func(t *testing.T) { + var b strings.Builder + err := verbs()[verb](&b, dbArgs(verb, db, "not-this-one")) + if err == nil { + t.Fatalf("an unknown cluster was accepted:\n%s", b.String()) + } + for _, want := range []string{"no snapshot for that cluster", "Known clusters", "why-cost-demo"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error = %q, want it to mention %q", err, want) + } + } + }) + } +} + +// TestTheTwoSourcesAreExclusiveAndSaySo. +// +// A precedence rule would be worse than a refusal here: a recorded snapshot +// series and a brain's thinned history can legitimately disagree, and the +// operator who passed both has no way to learn which one answered. +func TestTheTwoSourcesAreExclusiveAndSaySo(t *testing.T) { + db := brainDB(t, 1, fleetSnapshot(whyCostT0, 4, 0, 500)) + snapPath := writeSnapshot(t, t.TempDir(), "snap.json", fleetSnapshot(whyCostT0, 4, 0, 500)) + window := []string{"--from", rfc(whyCostT0), "--to", rfc(whyCostT0.Add(48 * time.Hour))} + + for _, tc := range []struct { + name, verb string + args []string + want string + }{ + {"why-cost both sources", "why-cost", + append([]string{"--kube-snapshot", snapPath, "--db", db, "--cluster", "why-cost-demo"}, window...), + "two different substrates"}, + {"why-cost cluster without db", "why-cost", + append([]string{"--kube-snapshot", snapPath, "--cluster", "why-cost-demo"}, window...), + "it needs --db"}, + {"why-cost db without cluster", "why-cost", + append([]string{"--db", db}, window...), + "--db needs --cluster"}, + {"why-cost no source", "why-cost", window, + "a source of history is required"}, + {"explain both sources", "explain", []string{ + "--kube-snapshot", snapPath, "--db", db, "--cluster", "why-cost-demo", + "--workload", "Deployment/shop/web", "--container", "web"}, + "two different substrates"}, + {"explain db without cluster", "explain", []string{ + "--db", db, "--workload", "Deployment/shop/web", "--container", "web"}, + "--db needs --cluster"}, + {"explain no source", "explain", []string{ + "--workload", "Deployment/shop/web", "--container", "web"}, + "a source of history is required"}, + {"backtest both sources", "backtest", []string{ + "--cluster", "why-cost-demo", "--demo", "steady", "--db", db}, + "different sources of history"}, + } { + t.Run(tc.name, func(t *testing.T) { + var b strings.Builder + if err := verbs()[tc.verb](&b, tc.args); err == nil { + t.Fatalf("accepted:\n%s", b.String()) + } else if !strings.Contains(err.Error(), tc.want) { + t.Errorf("error = %q, want it to mention %q", err, tc.want) + } + }) + } +} + +// TestBacktestClusterRequiresAFixedWindow. +// +// The second trap. `--demo` gets its window from backtestEpoch, a constant, +// because a replay window that drifts with wall-clock time makes two runs over +// the same configuration disagree. `--cluster` has no such constant available +// — its history is whatever was retained — so the window becomes an argument, +// exactly as why-cost already requires. Defaulting it to "the last 30 days" +// would make a scorecard's headline number depend on the day it was run, and +// comparability is the entire value of a scorecard. +func TestBacktestClusterRequiresAFixedWindow(t *testing.T) { + db := brainDB(t, 1, fleetSnapshot(whyCostT0, 4, 0, 500)) + for _, tc := range []struct { + name string + args []string + }{ + {"no window", []string{"--cluster", "why-cost-demo", "--db", db}}, + {"only from", []string{"--cluster", "why-cost-demo", "--db", db, "--from", rfc(whyCostT0)}}, + {"only to", []string{"--cluster", "why-cost-demo", "--db", db, "--to", rfc(whyCostT0)}}, + } { + t.Run(tc.name, func(t *testing.T) { + var b strings.Builder + err := runBacktestTo(&b, tc.args) + if err == nil { + t.Fatalf("a drifting replay window was accepted:\n%s", b.String()) + } + if !strings.Contains(err.Error(), "--from and --to are required") { + t.Errorf("error = %q", err) + } + assertNoScorecard(t, b.String()) + }) + } + // And --db is required before the window is even parsed: there is no + // history without a database. + var b strings.Builder + err := runBacktestTo(&b, []string{"--cluster", "why-cost-demo", + "--from", rfc(whyCostT0), "--to", rfc(whyCostT0.Add(48 * time.Hour))}) + if err == nil || !strings.Contains(err.Error(), "--db is required") { + t.Errorf("error = %v, want --db to be required", err) + } +} + +// TestFlagsThatCannotBeHonouredAreRefusedNotIgnored. +// +// loadPolicy already refuses an unknown key in a policy file, and states why: +// a knob that is silently dropped produces a report for a configuration nobody +// ran. A flag belonging to the other source is the same failure with a +// friendlier face, so each source refuses the other's flags by name. +func TestFlagsThatCannotBeHonouredAreRefusedNotIgnored(t *testing.T) { + db := brainDB(t, 1, fleetSnapshot(whyCostT0, 4, 0, 500)) + live := []string{"--cluster", "why-cost-demo", "--db", db, + "--from", rfc(whyCostT0), "--to", rfc(whyCostT0.Add(48 * time.Hour))} + policyPath := writePolicy(t, `{"recommend":{"cpuHeadroom":1.5}}`) + + for _, tc := range []struct { + name, verb string + args []string + want string + }{ + {"live rejects trace shape", "backtest", append(live, "--days", "3"), "--days"}, + {"live rejects noise", "backtest", append(live, "--noise", "0.1"), "--noise"}, + {"live rejects a policy file", "backtest", append(live, "--policy", policyPath), "--policy"}, + {"live rejects a comparison", "backtest", append(live, "--compare", policyPath), "--compare"}, + {"live rejects refusal enforcement", "backtest", append(live, "--enforce-refusals"), "--enforce-refusals"}, + {"live rejects derived costs", "backtest", append(live, "--derive-costs"), "--derive-costs"}, + {"demo rejects a database", "backtest", + []string{"--demo", "steady", "--db", db}, "--db"}, + {"demo rejects a window", "backtest", + []string{"--demo", "steady", "--from", rfc(whyCostT0)}, "--from"}, + } { + t.Run(tc.name, func(t *testing.T) { + var b strings.Builder + if err := verbs()[tc.verb](&b, tc.args); err == nil { + t.Fatalf("a flag that cannot be honoured was accepted:\n%s", b.String()) + } else if !strings.Contains(err.Error(), tc.want) { + t.Errorf("error = %q, want it to name %q", err, tc.want) + } + assertNoScorecard(t, b.String()) + }) + } +} + +// TestWhyCostOverADatabaseRefusesALedgerFile. +// +// A brain attributes actions from the audit ledger it kept itself +// (api.Brain.ledgerActions, the projection lifted from loadLedgerActions). +// Splicing a file-supplied ledger in as well would give one attribution two +// answers to "which actions moved money" and no rule for reconciling them. +func TestWhyCostOverADatabaseRefusesALedgerFile(t *testing.T) { + db := brainDB(t, 1, fleetSnapshot(whyCostT0, 4, 0, 500)) + ledger := filepath.Join(t.TempDir(), "ledger.json") + if err := os.WriteFile(ledger, []byte(`{"entries":[]}`), 0o644); err != nil { + t.Fatal(err) + } + var b strings.Builder + err := runWhyCostTo(&b, append(dbArgs("why-cost", db, "why-cost-demo"), "--ledger", ledger)) + if err == nil { + t.Fatalf("accepted:\n%s", b.String()) + } + if !strings.Contains(err.Error(), "--ledger") { + t.Errorf("error = %q, want it to name --ledger", err) + } +} + +// TestTheFileBackedSourcesStillNeedNoDatabase. +// +// The compatibility claim, asserted rather than assumed: a user with recorded +// snapshots and no database keeps working exactly as before. Nothing about +// --kube-snapshot opens, creates or requires a bbolt file, and the working +// directory is untouched — a --db that defaulted to "kilter.db" would have +// made that false without changing a single command line. +func TestTheFileBackedSourcesStillNeedNoDatabase(t *testing.T) { + dir := t.TempDir() + snaps := []string{ + "--kube-snapshot", writeSnapshot(t, dir, "a.json", fleetSnapshot(whyCostT0, 4, 0, 500)), + "--kube-snapshot", writeSnapshot(t, dir, "b.json", fleetSnapshot(whyCostT0.Add(12*time.Hour), 6, 0, 500)), + } + out := runWhyCostOK(t, append(append([]string{}, snaps...), + "--from", rfc(whyCostT0), "--to", rfc(whyCostT0.Add(13*time.Hour)))...) + if !strings.Contains(out, "node-count") { + t.Errorf("the file-backed why-cost stopped explaining:\n%s", out) + } + + var b strings.Builder + if err := runExplainTo(&b, []string{ + "--kube-snapshot", readFixture(t, "cluster.json"), + "--workload", "Deployment/default/api", "--container", "api", + }); err != nil { + t.Fatalf("the file-backed explain stopped working: %v\n%s", err, b.String()) + } + if !strings.Contains(b.String(), "usage-history") { + t.Errorf("the file-backed explain lost its drivers:\n%s", b.String()) + } + + // No database was created anywhere the commands could have put one. + for _, dir := range []string{dir, "."} { + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + for _, e := range entries { + if strings.HasSuffix(e.Name(), ".db") { + t.Errorf("a file-backed run created %s", filepath.Join(dir, e.Name())) + } + } + } +}