Skip to content

test(e2e): hold the wait-budget guard to a start margin across four suites - #4468

Merged
Aleksei Sviridkin (lexfrei) merged 1 commit into
mainfrom
test/wait-budget-guard-margin
Oct 1, 2026
Merged

Aleksei Sviridkin (lexfrei) merged 1 commit into
mainfrom
test/wait-budget-guard-margin

Conversation

@lexfrei

@lexfrei Aleksei Sviridkin (lexfrei) commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

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

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.

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.

@github-actions github-actions Bot added area/testing Issues or PRs related to testing (e2e, bats, unit tests) size/XL This PR changes 500-999 lines, ignoring generated files labels Sep 24, 2026
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: cozystack/cozystack/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 14a30c5b-7926-4927-8887-3d1ebc0aee1d

📥 Commits

Reviewing files that changed from the base of the PR and between 08d89df and 94b328c.

📒 Files selected for processing (2)
  • docs/agents/e2e-testing.md
  • hack/mariadb-first-boot-waits.bats

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The wait-budget guard now covers MariaDB, MongoDB, RabbitMQ, and ClickHouse. It checks database waits against startup and install-timeout floors with a 60-second start margin. It also checks Chainsaw operation slack after reached waits.

Changes

E2E wait-budget guard

Layer / File(s) Summary
Define suite budgets and extract waits
docs/agents/e2e-testing.md, hack/mariadb-first-boot-waits.bats
The documentation describes the start-margin rule and identifies MBW_START_MARGIN. The guard defines configuration for four suites, extracts database waits by release prefix, and selects Chainsaw operations by script path. It detects SKIP_RESTORE=1 on the matching script command.
Check suite floors and operation slack
hack/mariadb-first-boot-waits.bats
Floor checks require exactly two waits for distinct releases and a 60-second margin above the larger applicable startup and install-timeout floor. Operation checks account for reached waits and the bucket wait, stop after a restore-skip exit, and validate each suite’s remaining-time reserve. Fixtures cover failure cases, and live checks cover all four suites.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Other · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 94b32

This change only tightens test-time checks and updates documentation, so it does not affect production behavior. No actionable merge-blocking issue was found.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 76.92% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 1 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the E2E test change, the start-margin requirement, and the four-suite scope. It matches the main changes in the pull request.
Linked Issues check ✅ Passed The PR satisfies the coding requirements in issue #4418. hack/mariadb-first-boot-waits.bats defines MBW_START_MARGIN=60 and checks each wait against floor + margin. Fixture tests cover the margi…
Out of Scope Changes check ✅ Passed The changed files contain the wait-budget guard, its automated tests, and related E2E sizing guidance. These changes directly support issue #4418. The PR does not change wait values, operation timeout…
Full details: Docstring Coverage

Explanation

Docstring coverage is 76.92% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 1 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

IvanHunters
IvanHunters previously approved these changes Sep 25, 2026

@IvanHunters IvanHunters left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Verified the full guard suite (28 tests) passes on the PR head and on a test-merge with current main, where recent rabbitmq backup-demo changes (#4458, #4459) made this a "green apart, red together" candidate; the byte-identity copy check (test 11) stays green too.

The four per-suite floor/slack checks bind against real values. Each op's remainder equals its MBW_SUITES constant exactly (mariadb/rabbitmq 900, mongodb 1200 with SKIP_RESTORE, clickhouse 2700), and the mongodb/rabbitmq/clickhouse charts carry no startupProbe, so the "-" startup-floor column is a real fact and not a skipped case. Reverting the new margin predicate (-ge floor+margin back to -gt floor) reds the fixture tests 4 and 15, which confirms the start-margin semantics are held by fixtures and not just decorative on the live 660s waits.

NIT (non-blocking): the file is still named mariadb-first-boot-waits.bats and keeps the MBW_ prefix while now holding a four-suite contract; a rename is optional churn and the header already generalises the contract.

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]>
@lexfrei
Aleksei Sviridkin (lexfrei) merged commit bc400df into main Oct 1, 2026
20 of 21 checks passed
@lexfrei
Aleksei Sviridkin (lexfrei) deleted the test/wait-budget-guard-margin branch October 1, 2026 15:43
Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Oct 2, 2026
…ase they wait on (#4471)

## What this PR does

Stacked on #4468, which holds the raised backup waits to a start margin;
review that one first. This PR's own diff is the six commits on top of
it.

Most HR-Ready waits in the E2E suites gave up before the release they
were waiting on could fail its install. cozystack-api gives every
release it generates a 600s install timeout, and the Chainsaw asserts in
front of those releases sat at 5 or 6 minutes. When an install genuinely
failed, the suite reported a bare assert timeout against a release that
was still installing, and the reason the install failed, which the
release records when its own timeout fires, was never reached. Red runs
failed just as often as they should; they only said less.

I raised every HR-Ready wait above the install timeout of its own
release, by the 60s start margin #4468 introduces:

- Chainsaw asserts on releases cozystack-api generates go to 11m, across
bucket, clickhouse, etcd, external-dns, foundationdb, harbor, mongodb,
openbao, opensearch, postgres, qdrant, rabbitmq, redis, valkey and
vminstance.
- Asserts on nested `<name>-system` releases take their floor from the
template that renders them: 11m for external-dns, openbao and qdrant
(10m `spec.timeout`), 16m for `harbor-test-system` (15m). `harbor-test`
sat exactly on its 600s floor, which expires first because the assert's
clock starts before the install's; it goes to 11m too.
- The kuberture Package release takes its 10m from cozystack-operator
and goes to 11m.
- In the backup walkthroughs, the bucket waits go from 300s to 660s in
clickhouse, kafka, mariadb, mongodb, postgres, rabbitmq and redis, and
the two postgres waits go from 360s to 660s. The kafka bucket wait
passed no budget and silently took the helper's 300s default, which is
why earlier scans for literal budgets missed it. Each Chainsaw op that
runs a walkthrough grows by exactly what its waits gained (clickhouse
78m, kafka 46m, mariadb 48m, mongodb 42m, postgres 66m, rabbitmq 48m,
redis 36m), so a failure at any later wait keeps the headroom it had and
is still reported by the script instead of being cut off by the op.

Two waits stay as they are, on purpose:

- kafka: `kafka-rd` sets a 7h install timeout, which no budget inside
the 215-minute E2E job can clear. The kafka assert and the Kafka waits
in the kafka and kafka-metadata walkthroughs keep their numbers, and the
assert now says why in-line.
- `vm-instance-test`: `vm-instance-rd` disables the install wait, so its
release goes Ready as soon as helm has applied and there is no floor to
clear. Earlier lists of this problem included it by mistake.

The cost falls on red runs only: a suite whose release never goes Ready
now spends up to six more minutes before it reports, and in exchange the
report carries the release's own reason. Green runs are unchanged.
Suites run one after another in a single job capped at 215 minutes, so a
systemic failure that reddens many suites now reaches that cap sooner,
and a job cancelled at the cap loses its log and `cozyreport.tgz`. I
took that trade because a failed install that says why is worth more
than a red run that ends a few minutes earlier.

`docs/agents/e2e-testing.md` stopped being true in several places once
the waits were raised: it described most of the tree as under the floor,
mariadb as the only suite raised, the walkthrough bucket waits as under
it, and harbor's asserts as sitting on and under theirs. I rewrote those
sentences, and the one that pointed at "every wait above the band" of
5-6m, kept the start-margin wording from #4468, and added that a
walkthrough wait passing no budget takes the helper's default. The
comments in `hack/mariadb-first-boot-waits.bats` that quoted the old op
and bucket figures are restated; its existing checks and constants are
unchanged, because every op grew by exactly its bucket delta and each
remainder still sits at exactly its constant. I also added postgres to
the table that file holds the walkthroughs to, so its raised waits and
its op cannot drift back apart unnoticed, and a test that holds each
table suite's bucket wait to the bucket release's floor, read from the
api-server default with `bucket-rd` checked for an override; the op
check alone could not catch a bucket wait dropping back, since a lower
wait only widens the slack. redis stays out: its walkthrough waits on no
redis HelmRelease, only on the bucket and on the sentinel master, so the
table's floor check has nothing to read there.

How I checked it, since E2E cannot run locally:

- I re-ran the enumeration from the sizing section of
`docs/agents/e2e-testing.md` over every
`hack/e2e-chainsaw/*/chainsaw-test.yaml` and every `wait_hr_ready` call
under `examples/backups/`, literal or default, and held each against its
own release's floor. Every `Ready=True` assert and every `wait_hr_ready`
call a suite drives now clears its floor, except the kafka ones named
above. The opt-in etcd round-trip uses bare `kubectl wait hr` calls at
300s instead of that helper; they are listed in #4470. The `Ready=False`
assert on the rejected redis upgrade is not a readiness wait and is left
out.
- `make bats-unit-mariadb-first-boot-waits` passes, 31 of 31. For the
new postgres row, putting the op back to 50m fails only the postgres
slack test, and a postgres wait at 659s fails only the postgres floor
test. Setting any one table suite's bucket wait to 659s fails only the
bucket test, naming only that suite, and an install-timeout override on
`bucket-rd` fails it too. I ran the mutations under plain `bats` so a
fail-fast runner could not hide a second failure.
- `hack/select-e2e.sh` over this diff selects bucket, clickhouse, etcd,
external-dns, foundationdb, harbor, kafka, kuberture, mariadb, mongodb,
openbao, opensearch, postgres, qdrant, rabbitmq, redis, valkey and
vminstance.

Out of scope, and tracked in #4470: the tenant addon waits in
`hack/e2e-chainsaw/_lib/run-kubernetes.sh` (an edit there escalates the
whole E2E run), the redis and kafka-metadata `wait_hr_ready` helpers
that print no reason on timeout, and the opt-in etcd round-trip's bare
`kubectl wait` calls.

Known and left for later, none of them changed here:

- Only the mariadb Chainsaw asserts and the walkthroughs in the guard's
table are held by a test. The other raised asserts, and the redis and
kafka bucket waits, rest on literals with in-line reasons; #4470 covers
extending the guard.
- The Kafka waits in `examples/backups/kafka/04-create-kafka.sh`,
`07-restore-to-copy.sh` and `kafka-metadata/run-all.sh` stay at 300s
against the 7h floor without an in-line reason at the call site. The
doc's rule asks that of Chainsaw asserts only, and the kafka Chainsaw
assert carries it.

The postgres waits are the ones #3748 is about. Their budget now clears
the install-timeout floor and a test holds it there; what a CNPG first
boot costs on this platform is still unmeasured, so that issue stays
open.

Closes #3771

Closes #3753

### Screenshots

Not a UI change.

### Downstream repositories

I walked the trigger map in `docs/agents/contributing.md` against the
diff. It touches E2E suites, backup walkthrough scripts, a unit-lane
bats file and an agent doc, none of which any downstream repository
restates.

- [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
fix(tests): HelmRelease readiness waits in the E2E suites and backup walkthroughs now outlast the install timeout of the release they wait on, so a failed install reports its own reason instead of a bare timeout.
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Tests**
* Extended readiness and backup round-trip test timeouts across multiple
services to accommodate slower startup and provisioning.
* Added checks to verify that bucket readiness waits meet configured
installation-timeout requirements.

* **Documentation**
* Updated testing guidance to clarify how readiness waits relate to
release-specific timeouts and overall test budgets.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/testing Issues or PRs related to testing (e2e, bats, unit tests) size/XL This PR changes 500-999 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(e2e): the wait-budget guard accepts a 1s margin and does not pin mongodb, rabbitmq or clickhouse

3 participants