test(e2e): fail install test when HelmReleases are not Ready - #2824
test(e2e): fail install test when HelmReleases are not Ready#2824myasnikovdaniil wants to merge 1 commit into
Conversation
The HR readiness gate in e2e-install-cozystack.bats was toothless in two layers: the backgrounded kubectl-wait fan-out discarded child exit codes (POSIX `wait` without args returns 0), and the final check echoed "Some HelmReleases failed to reconcile" without exiting non-zero — a regression from 1f24038, which replaced the unavailable bats `fail` helper with a bare echo. A permanently-failing platform HelmRelease (e.g. the backup-controller CRD rejection, #2822) shipped through green CI for weeks. Replace the fan-out with a single `kubectl wait hr --all -A` for trace visibility, gate on an outcome-based re-list (which also covers HRs created after the snapshot, with a 120s retry to absorb momentary drift-reconcile flaps), and dump the full Ready condition message per non-ready HR so the real error is visible in the test output. Refs #2822 Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a silent failure in the E2E installation test where non-ready HelmReleases were not causing the test to fail. By replacing unreliable backgrounded processes with a synchronous wait and adding a strict outcome-based validation gate, the test now correctly identifies and reports reconciliation failures, ensuring platform stability in CI. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe e2e test waiting logic for HelmRelease readiness is rewritten to use a single cluster-wide ChangesHelmRelease Readiness Waiting
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request improves the reliability and debugging output of the E2E installation script by replacing the backgrounded HelmRelease wait with a single kubectl wait command, and adding a retry loop that dumps detailed status conditions for any non-ready HelmReleases. The reviewer suggested a more robust way to filter non-ready HelmReleases using custom-columns and awk to prevent false negatives caused by unstructured grep matching.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if ! timeout 120 sh -ec 'while kubectl get hr -A --no-headers | grep -v " True " | grep -q .; do sleep 5; done'; then | ||
| kubectl get hr -A | ||
| # kubectl's STATUS column truncates long messages; dump the full Ready | ||
| # condition per non-ready HR so the real error (e.g. a rejected CRD) is | ||
| # visible in the test output instead of only inside the cozyreport. | ||
| kubectl get hr -A --no-headers | grep -v " True " | while read -r ns name rest; do | ||
| echo "--- Non-ready HelmRelease: $ns/$name" >&2 | ||
| kubectl get hr -n "$ns" "$name" -o jsonpath='{range .status.conditions[*]}{.type}={.status} reason={.reason}: {.message}{"\n"}{end}' >&2 | ||
| done |
There was a problem hiding this comment.
Using grep -v " True " on the unstructured output of kubectl get hr -A is fragile and prone to false negatives. If a non-ready HelmRelease has a status message containing the substring " True " (for example, in a detailed error message or description), it will be incorrectly filtered out and treated as ready.
Instead, use custom-columns to isolate the Ready condition status into a specific column, and filter using awk to ensure precise matching.
if ! timeout 120 sh -ec 'while kubectl get hr -A -o custom-columns=NS:.metadata.namespace,NAME:.metadata.name,READY:.status.conditions[?(@.type=="Ready")].status --no-headers | awk '\''$3 != "True"'\'' | grep -q .; do sleep 5; done'; then
kubectl get hr -A
# kubectl's STATUS column truncates long messages; dump the full Ready
# condition per non-ready HR so the real error (e.g. a rejected CRD) is
# visible in the test output instead of only inside the cozyreport.
kubectl get hr -A -o custom-columns=NS:.metadata.namespace,NAME:.metadata.name,READY:.status.conditions[?(@.type=="Ready")].status --no-headers | awk '$3 != "True"' | while read -r ns name ready; do
echo "--- Non-ready HelmRelease: $ns/$name (Ready=$ready)" >&2
kubectl get hr -n "$ns" "$name" -o jsonpath='{range .status.conditions[*]}{.type}={.status} reason={.reason}: {.message}{"\n"}{end}' >&2
done
|
Thanks myasnikovdaniil — your readiness-gate fix is the cleaner implementation (single |
…chema (#2823) ## What this PR does Two changes that together fix the v1.5.0 release blocker where the `cozy-backup-controller` HelmRelease failed on every reconcile. **Root cause** — the four `backups.cozystack.io` CRDs (`backupjobs`, `backups`, `plans`, `restorejobs`) embedded the `x-cozystack-options` vendor extension directly in their OpenAPI validation schema. `apiextensions` `JSONSchemaProps` only preserves the documented `x-kubernetes-*` vendor extensions — every other `x-` key is rejected by helm-controller's server-side apply (`field not declared in schema`) and dropped by the apiserver on decode. So the CRDs never applied and the controller crash-looped. This moves the field→source mapping the dashboard needs out of the schema and into CRD `metadata.annotations` (which the apiserver preserves), emitted declaratively via `+kubebuilder:metadata:annotations` markers on the Go types, and removes the post-processing awk injector (`hack/inject-cozystack-options.awk`). A helm-unittest suite pins the contract: each dropdown field carries its source in annotations and must not carry `x-cozystack-options` in the schema. The dashboard reattaches the extension client-side from these annotations. Annotation contract: `options.cozystack.io/source.<spec-relative-path>: <option-source>`, e.g. `options.cozystack.io/source.applicationRef.kind: appkind`. **Detection gap** — the install e2e let this ship green: the fanned-out `kubectl wait` discarded child exit codes (a bare POSIX `wait` returns 0) and the readiness check printed offending HelmReleases without ever exiting non-zero. The gate is now restored — a single `kubectl wait hr --all -A` for trace visibility, an outcome-based re-list that also gates late-created HelmReleases (with a short retry to absorb momentary drift-reconcile flaps) and `exit 1`s, plus a full Ready-condition dump per non-ready HelmRelease so the real failure reason is visible. This commit is by @myasnikovdaniil, consolidated here from #2824 so the root cause and the CI gap land in one PR. With the gate restored, CI on `main` stays red until the backups fix in this PR merges — intended. Closes #2822 · Dashboard counterpart: cozystack/cozystack-ui#40 ### Release note ```release-note fix(backups): fix backup-controller CRDs failing to apply — the dashboard dropdown hint was placed in the CRD schema where apiextensions rejects it; it now lives in CRD annotations ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a CRD option-source test suite and a package test target to validate annotation-based option sources. * **Bug Fixes** * Improved end-to-end install checks with consolidated readiness waits and richer failure diagnostics for releases. * **Refactor** * Migrated backup-controller CRD option metadata from legacy vendor extensions to annotation-based keys for cleaner schema metadata. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
What this PR does
Restores the HelmRelease readiness gate in
e2e-install-cozystack.bats, which was toothless in two layers:kubectl wait ... &fan-out discarded child exit codes — POSIXwaitwithout arguments returns 0 regardless of children's failures.failhelper (undefined under cozytest'sshrunner) with a bare echo.As a result, a permanently-failing platform HelmRelease shipped through green CI for weeks — see #2822 (backup-controller CRDs rejected by SSA, controller in CrashLoopBackOff, present in the cozyreport of green runs).
Changes:
kubectl wait hr --all -A --timeout=15m— same per-HR trace visibility ("condition met" / "timed out waiting" lines), no swallowed exit codes.Unknownflaps from drift reconciles, thenexit 1.Note: with this gate restored, CI on
mainwill go red until #2822 (backup-controller CRDx-cozystack-optionsrejection) is fixed — that is the intended behavior.Fixes the CI-gap half of #2822.
Release note
Summary by CodeRabbit