Skip to content

Commit 08d89df

Browse files
committed
test(e2e): hold every raised backup wait to a start margin
The wait-budget guard accepted a wait one second over the install timeout, although the wait's clock starts before the install's by the Application reconcile, the chart fetch and the helm-controller queue. A wait lowered from 660 to 601 passed every test and brought back the bare assert timeout the budgets exist to avoid. A wait now has to clear its floor by a named 60s margin. Nothing in the tree bounds those three delays, so the value is policy: the headroom every current wait already carries. The sizing guide in docs/agents/e2e-testing.md asked only for "strictly above", which the guard now rejects; it points at the margin instead of restating a number. The mongodb, rabbitmq and clickhouse walkthrough waits were raised for the same reason as mariadb's, and their Chainsaw ops grown with them, but nothing held either. Each suite is now a row in one table and gets its own floor and op tests, so a regression reds only its own suite. Floors come from the api-server install-timeout default and, for mariadb, the chart's startup probe; each suite's RD is checked for an override instead of being assumed not to carry one. Each op keeps the remainder it had before the raise over every HelmRelease wait it reaches, the bucket one included, so raising a wait and its op together keeps the remainder instead of widening the slack. The mongodb target behind SKIP_RESTORE=1 is not reached and is left out until the op stops passing it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
1 parent 564f540 commit 08d89df

2 files changed

Lines changed: 461 additions & 212 deletions

File tree

‎docs/agents/e2e-testing.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ The suite is pinned to Chainsaw **v0.2.15** (the latest release as of May 2026);
179179
3. Imperative-only waits live in a `script` step; any bare `sleep` carries a `TODO(e2e-replace-fixed-timeouts):` justification, unless it is a measurement interval, which §2 carves out by name and which takes no marker because no event can replace it.
180180
4. No test-level `EXIT`/`RETURN` trap — rely on Chainsaw cleanup; a self-contained trap is allowed inside a single `script` step (port-forward / temp dir), and inside an explicit subshell within a BATS `@test`, where it does not displace the runner's own handler. A BATS file that still carries test-level traps declares the count itself in `# EXIT-TRAP DEBT: N`; do not ask for one to be removed without checking that line, because the count is exact in both directions.
181181
5. Controller-created artifacts the test cannot reclaim are pruned explicitly; nested tenants delete child → parent with a wait-for-deletion between.
182-
6. An HR-Ready assert has a floor: the release's own install timeout, not only the app's boot time. Below it a failed install reports as a bare assert timeout, so **a number under the floor buys a shorter red run at the price of the diagnosis**. Whether to raise a given suite to clear it is a per-suite call, made against what that suite's red runs cost; most of the tree currently sits under it. Derive the floor from [Sizing an HR-Ready budget](#sizing-an-hr-ready-budget) rather than copying a neighbour's number, and justify a raise in-line. The same goes for any other wait longer than the ones around it, HR-Ready or not: it carries its reason in-line.
182+
6. An HR-Ready assert has a floor: the release's own install timeout, not only the app's boot time. Below it a failed install reports as a bare assert timeout, and above it by less than a start margin it can, so **a number under the floor buys a shorter red run at the price of the diagnosis**. Whether to raise a given suite to clear it is a per-suite call, made against what that suite's red runs cost; most of the tree currently sits under it. Derive the floor from [Sizing an HR-Ready budget](#sizing-an-hr-ready-budget) rather than copying a neighbour's number, and justify a raise in-line. The same goes for any other wait longer than the ones around it, HR-Ready or not: it carries its reason in-line.
183183
7. Failure path attaches scoped diagnostics via a `catch:` block, never a silent pass. The node-join deadline is no exception, and no longer has one: with the soft-red marker gone it fails its suite like any other deadline and its diagnostics run from the same `catch:`.
184184
8. If it touches parent-HR behavior, add the `status.history` remediation guard (`hack/e2e-chainsaw/_lib/remediation-guard.sh`).
185185
9. A test that creates a tenant `Kubernetes` cluster waits for its release to reach `Ready` **before the assertions it ends on**, with a `--timeout` the guard can read as positive and no longer than a day. Reads earlier in the run are expected and are not what this bounds. The mechanism, how to size the budget and what the guard does with it are under "Do not mask cleanup or teardown failures". One thing to carry from there rather than look up: the naming is part of the rule, not a detail of the check, so an end-assertion added under any name outside the `cozy_assert_*` / `cozy_switch_and_assert_*` convention is invisible to it and passes before the wait without reddening anything — as long as one recognised call site is left behind it. Rename every recognised call and the guard reds instead, on finding none at all, which is the one shape of this mistake it does catch.
@@ -212,9 +212,9 @@ That second point used to be much worse than a sizing note. An importer pod moun
212212

213213
## Sizing an HR-Ready budget
214214

215-
Give an HR-Ready wait a budget strictly above both the app's first-boot budget and the install timeout of the release it waits on, read off the ApplicationDefinition for a generated release and off the rendering template otherwise; the rest of this section is how to find those two numbers.
215+
Give an HR-Ready wait a budget that clears, by a start margin, both the app's first-boot budget and the install timeout of the release it waits on, read off the ApplicationDefinition for a generated release and off the rendering template otherwise; the rest of this section is how to find those two numbers.
216216

217-
A HelmRelease install waits through kstatus unless the ApplicationDefinition carries `release.cozystack.io/helm-install-disable-wait`. `kubernetes-rd` and `kubernetes-nodes-rd` are the only two that do, so their installs never poll at all — though the `kubernetes` chart's blocking post-install hook still holds its install open, and sizing a wait on that kind is covered under "Do not mask cleanup or teardown failures" rather than here. Two other things would take an install off kstatus and onto helm's own readiness logic, neither of them in effect: a `spec.release.waitStrategy` of `legacy`, which no ApplicationDefinition sets at all — the only two places in the tree setting a strategy are hand-written HelmRelease templates under `packages/extra/`, and both set `poller` — and the `UseHelm3Defaults` feature gate, which flips the controller-wide default to `legacy` and is not among the gates helm-controller runs with here. Every other release therefore polls, and the wait in front of it has two floors. It has to beat both, and beat them strictly. A wait equal to the install-timeout floor does not merely tie with it: the two clocks start at different moments, the wait's when the step runs and the install's once the Application has been reconciled and the chart artifact fetched, so at equal budgets the wait is the one that expires, and it expires before the release has recorded anything about the failure.
217+
A HelmRelease install waits through kstatus unless the ApplicationDefinition carries `release.cozystack.io/helm-install-disable-wait`. `kubernetes-rd` and `kubernetes-nodes-rd` are the only two that do, so their installs never poll at all — though the `kubernetes` chart's blocking post-install hook still holds its install open, and sizing a wait on that kind is covered under "Do not mask cleanup or teardown failures" rather than here. Two other things would take an install off kstatus and onto helm's own readiness logic, neither of them in effect: a `spec.release.waitStrategy` of `legacy`, which no ApplicationDefinition sets at all — the only two places in the tree setting a strategy are hand-written HelmRelease templates under `packages/extra/`, and both set `poller` — and the `UseHelm3Defaults` feature gate, which flips the controller-wide default to `legacy` and is not among the gates helm-controller runs with here. Every other release therefore polls, and the wait in front of it has two floors. It has to beat both by a start margin. A wait equal to the install-timeout floor does not merely tie with it: the two clocks start at different moments, the wait's when the step runs and the install's once the Application has been reconciled and the chart artifact fetched, so at equal budgets the wait is the one that expires, and it expires before the release has recorded anything about the failure. The margin has to cover that gap, the helm-controller queue included; nothing in the tree bounds any part of it, so it is a policy value, held in `MBW_START_MARGIN` in `hack/mariadb-first-boot-waits.bats`.
218218

219219
The first floor is the app's own first boot. For a chart that sets `startupProbe` it is `initialDelaySeconds + (failureThreshold - 1) * periodSeconds`; for a chart that does not, it is whatever its operator needs, and only a measurement will say. mariadb is the case in this tree where this floor is real rather than nominal: its chart grants a 310s startup budget, so a wait that fails earlier fails a boot the probe was still willing to wait for.
220220

0 commit comments

Comments
 (0)