Added initial sbtest framework to build / compose long-running tests - #445
Open
noctarius wants to merge 3 commits into
Open
Added initial sbtest framework to build / compose long-running tests#445noctarius wants to merge 3 commits into
noctarius wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces the initial sbtest framework under test/framework/ to support building, composing, running, collecting, and offline re-analyzing long-running (soak) migration tests via a component+detector architecture.
Changes:
- Adds a plugin-based core (
Runner,RunContext, config loading) that runs enabled components, collects artifacts, and evaluates enabled detectors into a structured report. - Adds bundled components for cluster interaction and evidence collection (migration driver, log streaming/collection, NVMe sampling/snapshots, cluster events, dmesg) plus archive/live evidence adapters.
- Adds initial suites and a CLI/Makefile workflow for
run,collect, andanalyze, with unit tests covering core behavior and key driver/workload semantics.
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/framework/tests/test_driver.py | Unit tests for migration driver policies, grouping, polling semantics, persistence, and fio workload artifact layout. |
| test/framework/tests/test_core.py | Unit tests for config resolution, runner lifecycle ordering, judging/report behavior, archive reading tolerances, and log-grabber naming/reuse expectations. |
| test/framework/sbtest/suites/migration-soak.yaml | “Observation-only” soak suite emphasizing collection + detector thresholds without driving workload/migrations. |
| test/framework/sbtest/suites/migration-full.yaml | Full driven migration suite combining fio workload, migration driver, collection components, and detector set. |
| test/framework/sbtest/suites/corruption-hunt.yaml | Narrow analysis suite focused on integrity/corruption signals and key log patterns. |
| test/framework/sbtest/suites/analyze-only.yaml | Suite expressing “analyze existing artifacts only” defaults (no components, all detectors). |
| test/framework/sbtest/detectors/security.py | Adds secret/credential exposure scanning over collected logs. |
| test/framework/sbtest/detectors/nvme.py | Adds NVMe fabric detectors (stale controllers, ctrl_loss_tmo checks). |
| test/framework/sbtest/detectors/migration.py | Adds migration outcome/error-shape detectors over migration timelines. |
| test/framework/sbtest/detectors/meta.py | Adds evidence-quality detectors (log coverage, blind spots, inventory). |
| test/framework/sbtest/detectors/logs.py | Adds configurable regex-based log pattern detector with a bundled catalogue. |
| test/framework/sbtest/detectors/fio.py | Adds fio detectors for job errors, checksum corruption, outage windows, throughput outliers. |
| test/framework/sbtest/detectors/control.py | Adds control-plane event-log detectors (node flaps, stuck tasks, health, retry storms, node-agent signals). |
| test/framework/sbtest/detectors/ana.py | Adds ANA/path-based detectors and shared freeze window helpers. |
| test/framework/sbtest/detectors/init.py | Registers bundled detectors. |
| test/framework/sbtest/core/runner.py | Implements lifecycle execution and detector judging, producing structured findings and output. |
| test/framework/sbtest/core/plugin.py | Implements component/detector base types and global registries/builders. |
| test/framework/sbtest/core/findings.py | Defines Finding/Report models, severity/attribution, and verdict logic. |
| test/framework/sbtest/core/evidence.py | Defines Evidence protocol and shared data types + helper functions. |
| test/framework/sbtest/core/context.py | Defines RunContext, Timeline, and logging/window recording behavior. |
| test/framework/sbtest/core/config.py | Implements suite loading (YAML/JSON), selection resolution, and CLI toggles. |
| test/framework/sbtest/core/init.py | Exposes core public API surface. |
| test/framework/sbtest/components/sbctl.py | Adds thin sbctl JSON client used by components for authoritative backend state. |
| test/framework/sbtest/components/nvme.py | Adds NVMe controller snapshotting and ANA sampling components via CSI node pods. |
| test/framework/sbtest/components/migration.py | Adds VolumeMigration CR driver component + timeline/JSON persistence. |
| test/framework/sbtest/components/logs.py | Adds live log streaming, post-run log collection, and host dmesg collection. |
| test/framework/sbtest/components/kube.py | Adds minimal kubectl subprocess helpers and pod discovery. |
| test/framework/sbtest/components/events.py | Adds cluster event-log collection via sbctl. |
| test/framework/sbtest/components/init.py | Registers bundled components. |
| test/framework/sbtest/cli.py | Adds sbtest CLI (detectors, components, analyze, collect, run). |
| test/framework/sbtest/adapters/live.py | Provides live evidence adapter overlaying archive reader with in-memory run state. |
| test/framework/sbtest/adapters/archive.py | Provides archive evidence reader compatible with existing harness artifact layouts. |
| test/framework/sbtest/adapters/init.py | Exposes evidence adapters. |
| test/framework/sbtest/main.py | Enables python -m sbtest entry. |
| test/framework/sbtest/init.py | Package init registering bundled plugins and exporting core API. |
| test/framework/requirements-dev.txt | Adds optional dev dependencies (PyYAML, ruff). |
| test/framework/pyproject.toml | Defines sbtest package metadata, dependencies, Ruff/Mypy config. |
| test/framework/Makefile | Adds venv bootstrapping + workflows for test/lint/types/analyze/collect/run. |
| test/framework/.gitignore | Ignores venv, caches, and run artifacts under framework directory. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+385
to
+387
| nodes = {p.node for p, _c, _n in plan if p.node} | ||
| self._grabbers = self._start_grabbers(ctx, sorted(nodes), int(self.opt("ttl_s"))) | ||
|
|
Comment on lines
+153
to
+160
| self._window_start = getattr(self, "_window_start", None) or start or now_utc() | ||
| if end: | ||
| self._window_end = end | ||
| self.save_json("run.json", { | ||
| "run_id": self.run_id, | ||
| "start": iso(self._window_start), | ||
| "end": iso(end) if end else None, | ||
| }) |
Comment on lines
+77
to
+80
| evidence={"pattern": label, "total": total, | ||
| "per_log": {k: len(v) for k, v in per_log.items()}}, | ||
| artifacts=sorted(per_log), | ||
| note="The value is deliberately not quoted here — findings.json travels at " |
…e came back Ported from the fio harness's own fix (058bb00), which found three things wrong with how a stopped volume was read. All three were here too, and the last one silently. fio's log offsets are milliseconds since *that job* started, so the only base that is right by construction is the job's own start. The workload component used the run's start instead — stamped before the PVCs, the pods and the fio processes existed, so ahead of every pod's fio by a different amount: 207s and 210s in the two archived runs that still carry a pre-fix timeseries.csv. That is not merely a shifted chart. The same base decides which migration an outage overlaps, so a wrong base names a wrong migration. ArchiveEvidence re-derives the base rather than trusting the wall_clock column, because the column is only as right as whatever wrote it, and a detector that cannot be re-run against the run that motivated it is a detector nobody trusts. Replaying fio-mig-1787557273 with the base corrected moves 90 of its 160 outage windows onto a different migration, every one of them from the migration after the one that caused it. A window was then attributed by its first second, which fails whenever the host goes dry before the operator records the migration as started — those gaps came out belonging to no migration at all, which reads as "the cluster is unwell" rather than "the cutover cost this". attribute_window measures the overlap instead and names the migration holding most of the window, so a long gap touching two of them lands on the one it mostly sits in. Last, fio.outage called both kinds of stopped volume an outage. A gap that recovered is a freeze: every write the application issued was eventually taken, and what the length measures is how long an application had to survive with the volume gone — a cutover is one by design. A gap still open when fio stopped is a loss: I/O the volume was supposed to accept and never did. Both are CRITICAL, both fail the run, but they need different next steps, and a report that gives them one name hands the reader that work. The tests were written against the unchanged code first: the clock tests failed on the three-minute shift, the attribution tests on ['mig-a'] != ['mig-b'] and [] != ['mig-1'], and the freeze/loss tests on the missing `kind`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e a finding is in Three findings from the review on #445, each with the test that would have caught it written and run red first. logs.collect started a privileged pod on every node it wanted a log from and never looked at ctx.shared["logs.grabbers"], which logs.stream publishes for exactly this purpose — so a run carried two privileged pods per node doing the same job. Worse, it recorded nothing in `_own`, and `_own` is all teardown is allowed to delete, so every pod it started outlived the run until its TTL expired. It now borrows what has been published and owns only what it had to start. The test that was supposed to cover this passed throughout, because it re-implemented the reuse arithmetic in its own body and never called collect(). It now drives the real collect() with the cluster faked at the component's boundary, which is what made both defects visible: the first case fails on ['vm02','vm03','vm04'] != ['vm04'], the second on [[]] != [['own-vm03','own-vm04']]. mark_window persisted its `end` argument rather than the end it had stored, so any later call without one rewrote run.json with end: null. Nothing in the current sequence calls it that way, which is precisely why it would have gone unnoticed: the window is what bounds a ring-buffer detector to this run, and an open window silently lets evidence from after the run count against it. security.secret-exposure listed log names where the run directory holds <name>.txt, so the one field that says where to look did not name a file. Every other detector reporting a container log names it with its extension. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
noctarius
force-pushed
the
main
branch
2 times, most recently
from
September 9, 2026 10:21
60dceb7 to
fbaabe4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.