test(e2e): drop readiness asserts that HelmRelease Ready already covers by myasnikovdaniil · Pull Request #4652 · cozystack/cozystack

myasnikovdaniil · 2026-10-01T13:11:41Z

What this PR does

Removes e2e asserts that wait for something an earlier assert in the same test already proves. First step of #2653, not the whole issue.

qdrant and external-dns render the workload in a nested -system HelmRelease, and both suites already assert that HelmRelease Ready. The readyReplicas: 1 on the StatefulSet and the Deployments goes, the instance-label pin stays because the catch collectors select on it. I checked on a dev cluster that a parent HelmRelease waits for its nested one (a Qdrant with a broken storageClass kept the parent in Progressing until its install timeout, then it failed naming the nested HelmRelease), details in this comment.

vminstance: both PVC Bound asserts follow a DataVolume Ready assert, and a DataVolume is not Ready before its PVC is Bound.

Left alone on purpose: the qdrant PVC assert (the reclaim-on-delete step only proves anything if the PVC was seen first, same in kafka), openbao sts and PVC (they run after unseal), operator-created workloads, and the Service and Endpoints shape checks. Chainsaw isn't installed on my machine, so the suites are checked by CI only.

Screenshots

No UI changes.

Downstream repositories

The diff only edits hack/e2e-chainsaw/*/chainsaw-test.yaml. The trigger map's hack/ entry is for cozystack/ccp, whose skills anchor on hack/package.mk, hack/common-envs.mk and make generate, none of which change here. No box ticked.

Release note

test(e2e): the qdrant, external-dns and vminstance suites no longer repeat readiness checks that the HelmRelease Ready assert already covers.

Summary by CodeRabbit

Tests
- Updated end-to-end checks to use Helm release readiness for Deployment and StatefulSet scenarios, rather than separately asserting ready replica counts.
- Removed the requirement for PersistentVolumeClaims to reach Bound in the VMDisk and VMInstance scenarios; DataVolume readiness checks remain.
- Retained instance-label checks and clarified that PVC reclaim validation requires the claim to be observed first.

The qdrant and external-dns apps render their workload in a nested -system HelmRelease, and both suites already assert that HelmRelease Ready, which helm-controller reports only once the workload is up. Asserting readyReplicas on the same Deployment or StatefulSet again adds nothing, so those asserts keep only the instance-label pin. The vminstance PVC asserts follow a DataVolume Ready assert, and a DataVolume is not Ready before its PVC is Bound. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <[email protected]>

coderabbitai · 2026-10-01T13:11:48Z

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: ee65126c-5095-402f-835a-40977e567eb2

📥 Commits

Reviewing files that changed from the base of the PR and between 5415973 and 7fe7135.

📒 Files selected for processing (3)

hack/e2e-chainsaw/external-dns/chainsaw-test.yaml
hack/e2e-chainsaw/qdrant/chainsaw-test.yaml
hack/e2e-chainsaw/vminstance/chainsaw-test.yaml

💤 Files with no reviewable changes (1)

hack/e2e-chainsaw/vminstance/chainsaw-test.yaml

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

Chainsaw tests remove Deployment and StatefulSet replica-count assertions and VM scenario PVC Bound assertions. They retain the corresponding HelmRelease or DataVolume readiness checks.

Changes

Chainsaw assertion updates

Layer / File(s)	Summary
Workload readiness and label assertions `hack/e2e-chainsaw/external-dns/chainsaw-test.yaml`, `hack/e2e-chainsaw/qdrant/chainsaw-test.yaml`	External DNS Deployment and Qdrant StatefulSet assertions retain instance-label checks and remove `readyReplicas` checks. Comments point to HelmRelease readiness checks. The Qdrant comments also clarify that the PVC reclaim step requires the PVC to have been observed.
VM scenario PVC assertions `hack/e2e-chainsaw/vminstance/chainsaw-test.yaml`	The VMDisk and VMInstance scenarios remove PVC `Bound` assertions. Their DataVolume readiness assertions remain.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 7fe71

The retained readiness gates cover the removed checks in the supported test configuration, so no merge blocker remains.

Architecture Summary

Architecture risk: 🔵 Low · up to 7fe71

The change affects 1 system.

Changed systems: hack

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

observed — hack (service) was modified; 3 changed files map to changed impact.

Before / after behavior

observed — Modified behavior in hack/e2e-chainsaw/external-dns/chainsaw-test.yaml: The inmemory scenario’s Deployment assertion now checks only the test-system instance label; the readyReplicas: 1 check was removed, and a comment points to the HelmRelease readiness assertion.
observed — Modified behavior in hack/e2e-chainsaw/external-dns/chainsaw-test.yaml: The prefix scenario’s Deployment assertion now checks only the test-prefix-system instance label; the readyReplicas: 1 check was removed, and a comment points to the HelmRelease readiness assertion.
observed — Modified behavior in hack/e2e-chainsaw/qdrant/chainsaw-test.yaml: The StatefulSet assertion removes the readyReplicas: 1 status check while retaining the qdrant-test-system instance-label match. The comments clarify that HelmRelease readiness is checked separately and that the reclaim step requires the PVC to have been observed; an error on a PVC that never existed can also pass that step.
observed — Modified behavior in hack/e2e-chainsaw/vminstance/chainsaw-test.yaml: Removed the 4-minute assertion that each scenario’s PersistentVolumeClaim reaches Bound; the preceding DataVolume Ready=True assertion remains.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: removing redundant readiness assertions from end-to-end tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

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.}

github-actions Bot added size/M This PR changes 30-99 lines, ignoring generated files area/testing Issues or PRs related to testing (e2e, bats, unit tests) labels Oct 1, 2026

myasnikovdaniil marked this pull request as ready for review October 2, 2026 05:28

myasnikovdaniil requested review from IvanHunters, Andrei Kvapil (kvaps), Aleksei Sviridkin (lexfrei) and Timofei Larkin (lllamnyp) as code owners October 2, 2026 05:28

myasnikovdaniil mentioned this pull request Oct 2, 2026

Audit e2e Chainsaw post-HR-Ready asserts for redundancy under Flux v2.8 kstatus #2653

Open

Aleksei Sviridkin (lexfrei) approved these changes Oct 2, 2026

View reviewed changes

Aleksei Sviridkin (lexfrei) merged commit 42faf54 into main Oct 2, 2026
20 checks passed

Aleksei Sviridkin (lexfrei) deleted the test/e2e-drop-redundant-readiness-asserts branch October 2, 2026 06:50

Navigation Menu

Repository navigation

test(e2e): drop readiness asserts that HelmRelease Ready already covers - #4652

test(e2e): drop readiness asserts that HelmRelease Ready already covers#4652
Aleksei Sviridkin (lexfrei) merged 1 commit into
mainfrom
test/e2e-drop-redundant-readiness-asserts

myasnikovdaniil commented Oct 1, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 1, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

myasnikovdaniil commented Oct 1, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Screenshots

Downstream repositories

Release note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Architecture Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

myasnikovdaniil commented Oct 1, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 1, 2026 •

edited

Loading