Skip to content

Commit bc400df

Browse files
authored
test(e2e): hold the wait-budget guard to a start margin across four suites (#4468)
## What this PR does `hack/mariadb-first-boot-waits.bats` let two regressions of the wait budgets it guards pass green. This closes both. The change is test and doc only: no wait, op or chart value moves. A wait now has to clear its floor by a named `MBW_START_MARGIN` of 60s, not by one second. The wait's clock starts before the install's by the Application reconcile, the chart artifact fetch and the helm-controller queue. Nothing in the tree bounds those three, so the value is a policy choice and the header says so. It equals the headroom every current wait already carries (660 against a 600s floor), which is why no value has to change. A fixture pins 660 against 600 as passing and 659 and 601 as refused. The sizing guide in `docs/agents/e2e-testing.md` asked only for "strictly above" the floor, which the guard now rejects, so it now asks for a start margin and points at the constant instead of restating the number. The mongodb, rabbitmq and clickhouse walkthrough waits and their Chainsaw ops are now held to the same contract as mariadb, through one table with a row per suite. Each suite gets its own floor test and its own op test, so a regression turns only that suite's tests red. The floor comes from the api-server's install-timeout default, and each suite's ApplicationDefinition is checked for an override. The startup-probe half is computed only for mariadb, since the other three charts set no `startupProbe`. Each op must keep the remainder it had before its waits were raised, and that raise preserved it: 900s for mariadb and rabbitmq, 1200s for mongodb, 2700s for clickhouse. The remainder is taken over every HelmRelease wait the op reaches, the bucket wait included, so raising a wait together with its op keeps the remainder instead of turning the raise into slack. The mongodb target wait sits behind the `SKIP_RESTORE=1` its op passes, so it is not reached and is left out; the test turns red once the op stops passing it. The Chainsaw HR-Ready asserts of mongodb, rabbitmq and clickhouse (6m) stay out of scope. They sit under the floor by per-suite choice, and the header says so. Verification: - The file goes from 22 to 28 tests. Four mariadb-only tests are replaced by the eight per-suite ones: the example floor, the both-waits op slack, the check that both applications are waited on and the ApplicationDefinition override. Two fixture tests drive the per-suite floor and op checks through directory, suite-file and RD overrides. They cover each way of falling short that the live tree cannot produce, since every live wait sits at exactly 660 and every remainder at exactly its constant. That includes an RD that overrides or disables the install wait, and a startup budget large enough to bind over the install timeout. - `make bats-unit-mariadb-first-boot-waits` passes, and the same file passes under dash through `hack/cozytest.sh`, as CI runs it. - Mutations, each confirmed applied and each run test by test, because the runner stops at the first failure: - Any suite's database wait lowered to 601: only that suite's floor test fails. That includes the mongodb target wait the op never reaches. - Each op lowered back to its old value (30m, 30m, 30m, 60m): only that suite's op tests fail. For mariadb that is also the existing prefix-remainder test. - `SKIP_RESTORE=1` dropped from the mongodb op: only the mongodb op test fails. - Any suite's bucket wait raised to 660, or the rabbitmq one indented out of the flow: only that suite's op tests fail. Raising a bucket wait to 660 together with its op by 6m keeps all 28 tests green. - A third clickhouse wait added in a new file under `examples/backups/clickhouse/`: the clickhouse floor and op tests fail, because the waits are read from every script in the directory rather than from a listed subset. - Any suite's target wait renamed to wait on the source again, in mariadb and rabbitmq: only that suite's floor test fails. Dropping the distinct-release check, or its `sort -u`, fails only the floor fixture test. - Inside the helpers themselves, one mutation per branch of every new or changed helper, 37 in all: each turns its own fixture test red, or the live suite checks where the branch selects which waits are read at all. Two branches whose deletion changed no output were removed rather than pinned. - `MBW_START_MARGIN` set to 0: only the two fixture tests that encode the margin fail. Deleting the constant fails every test that reads it, as an unbound variable. - `hack/select-e2e.sh` selects no suite for this diff. Left for later: - The file name. `hack/mariadb-first-boot-waits.bats` now guards four suites. I kept the name because renaming it changes its make target and every reference to it, which belongs in a change of its own. - The SKIP_RESTORE stop works within one file. A suite whose waits span several scripts, as clickhouse's do, would still count the waits in later files if its op ever passed SKIP_RESTORE=1. That errs toward a false red, not a silent pass, and no such op exists today. - Measuring the margin. The delay from Application creation to the start of the install is measurable from CI reports and would turn the 60s policy value into a bound. That delay is not bounded anywhere today. Closes #4418 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note test(e2e): the wait-budget guard now requires a 60s margin over each floor and covers the mongodb, rabbitmq and clickhouse backup walkthroughs alongside mariadb. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified that HR-Ready waits should exceed both the app’s first-boot budget and the release install timeout, with a start margin for controller processing. * **Tests** * Expanded wait-budget and timeout validation across configured suites, including checks for missing or invalid settings, operation slack, restore-skip behavior, and bucket waits. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents 889bb79 + 94b328c commit bc400df

2 files changed

Lines changed: 460 additions & 211 deletions

File tree

‎docs/agents/e2e-testing.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ The suite is pinned to Chainsaw **v0.2.15** (the latest release as of May 2026);
190190
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.
191191
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.
192192
5. Controller-created artifacts the test cannot reclaim are pruned explicitly; nested tenants delete child → parent with a wait-for-deletion between.
193-
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.
193+
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.
194194
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:`.
195195
8. If it touches parent-HR behavior, add the `status.history` remediation guard (`hack/e2e-chainsaw/_lib/remediation-guard.sh`).
196196
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.
@@ -223,9 +223,9 @@ That second point used to be much worse than a sizing note. An importer pod moun
223223

224224
## Sizing an HR-Ready budget
225225

226-
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.
226+
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.
227227

228-
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.
228+
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`.
229229

230230
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.
231231

0 commit comments

Comments
 (0)