test(metallb): assert digest-pinned image form, not exact version literal - #2873
test(metallb): assert digest-pinned image form, not exact version literal#2873myasnikovdaniil wants to merge 1 commit into
Conversation
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughThis PR implements a multi-faceted infrastructure update: centralizes Docker build cache configuration via a shared Make macro and cache-warming workflow, introduces in-cluster Cilium endpoint leak remediation for e2e test stability, extends SeaweedFS with object lock and read-only bucket support, updates metallb test assertions, and adds CAPI startup probe. ChangesBuild Cache Infrastructure Refactoring
E2E Test Infrastructure and Stability Improvements
SeaweedFS Enhancements
Minor Component Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
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 fragility in the MetalLB unit tests where assertions were failing on release branches due to hardcoded image tags. By transitioning from exact-literal assertions to regex-based pattern matching, the tests now correctly validate that images are sourced from the expected repository and are pinned by a SHA256 digest, without requiring the version string to remain static during release preparation. 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
|
There was a problem hiding this comment.
Code Review
This pull request updates the MetalLB unit tests in packages/system/metallb/tests/metallb_test.yaml to assert controller and speaker image tags using regex pattern matching instead of exact-literal strings. This prevents test failures on release branches where image tags are re-stamped during release preparation. There are no review comments, so I have no additional feedback to provide.
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.
4b0938f to
e06fd55
Compare
…eral The two image-pin assertions hard-coded v0.16.1@sha256:..., but release-prep re-stamps every image tag to the platform version (e.g. v1.5.0-rc.2) and rebuilds the digest. The exact-literal assertion is green on main yet fails on every release branch; the breakage stayed hidden because the `release` label skips the Build job that runs `make unit-tests`, and only surfaced on #2861 once that label was dropped. Relax both assertions to matchRegex on the cozystack repository + @sha256 digest form, leaving the version string free. A wrong repository or a non-digest tag still fails the suite. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
e06fd55 to
092ea49
Compare
## What Consolidated landing branch for a set of interdependent platform/CI stabilization fixes. It began as the source PRs in the table below (in dependency order) and has since grown with review-driven fixes and a few production-behavior changes — the full current inventory is under **Scope** below. Landing them as one unit lets CI run against the real combined state instead of a fragile bottom-up merge train. ## Source PRs (dependency order) | # | Commit | Source PR | Addresses | |---|--------|-----------|-----------| | 1 | fix(capi): startupProbe on capi-controller-manager | #2946 | capi crashloop during cert provisioning | | 2 | test(e2e): bucket.bats port-forward + S3 client reliability | #2944 | flaky bucket test | | 3 | fix(seaweedfs): restore -lock BucketClass, s3 svc name, drop volumeSizeLimitMB | #2943 | missing bucket/access classes | | 4 | test(e2e): in-cluster Cilium endpoint-leak healer (install + apps) | #2874 | cilium "IP already in use" leak | | 5 | refactor(build): mode=max registry cache + main-only warmer | #2938 | build cache | | 6 | ci(build): isolate each PR build on its own ephemeral runner VM | #2939 | shared-buildkit contention | | 7 | fix(e2e): LINSTOR post-install waits on a single 15m deadline | #2928 | LINSTOR provisioning timeouts | | 8 | test(metallb): assert digest-pinned image form, not version literal | #2873 | brittle metallb assertion | ## Why consolidated These form a dependency DAG (verified from CI logs): e.g. 2943 needs 2946, 2938 needs 2874, 2928 needs 2938/2939, 2873 needs 2928. Tested bottom-up, each lower PR runs with none of the fixes above it and can't go green alone. This branch carries all of them, so CI runs against the real combined state. ## Verification status CI is green on the latest head (`5653c30`): full-suite E2E passes end-to-end and is reproducible (2 of 3 runs on this SHA green). The one red run was a LINSTOR tie-breaker / DRBD-metadata infra flake on the sandbox, unrelated to the diff — a different environmental subsystem failed each run (details in the comments below). The earlier 3-hour crust-gather snapshot hang is fixed in `5653c30`. ## Scope beyond the original 8 The branch has grown past the 8 source PRs above with review-driven fixes and a few production-behavior changes. For reviewer transparency, the full set: **Review fixes (@lexfrei review):** - **B1 (blocker)** — `8f41910` converts the SeaweedFS `-lock`/`-readonly` BucketClasses and the s3 service-name override into `patches/`, wired into `make update`, so a re-vendor no longer drops them. - **FU1** — `cc99230` grants the cilium leak-healer `delete` on pods (both delete remedies were RBAC-forbidden). - **FU3** — `014de3f` adds a unit test for the HelmRelease update conflict-retry path. - **FU4** — `6deddc6` makes the leak-healer refuse a disconnect for any non-terminal owner phase, not just `Running`. - **FU7** — `f3914d2` sets `persist-credentials: false` on the build-cache checkout (SHA-pinning is handled repo-wide in #2849). **Production-behavior changes (not in the table above):** - VPA `updateMode: Auto → Initial` for etcd (`6e9ff90`) and monitoring (`0997105`), plus `vmselect`/`vmstorage` `minAllowed` floors — stops install-time eviction churn. - cozystack-api: `retry.RetryOnConflict` on the Application→HelmRelease update path (`728f676`). **E2E robustness follow-ups:** per-test crust-gather snapshots (`50e5f94`), EtcdBackupSchedule wait (`763b85e`), tenant-node single-deadline wait (`9e43d45`), harbor BucketClaim 10m budget (`d7f244f`), tenant API via LoadBalancer (`9beda4e`), crust-gather pin (`95f42aa`). **Not addressed (by decision):** FU2 — the s3 Service keeps `seaweedfs.name`-based naming via patch (rendered output unchanged; cozystack renders `fullname == seaweedfs`, so it matches the `componentName`/`fullname` siblings in practice). FU6 — fork PR build push is handled by the existing mirror-to-same-repo-branch workflow. ## Relates to #2946 #2944 #2943 #2874 #2938 #2939 #2928 #2873 — if this lands, those can be closed; otherwise they remain the granular per-PR review path. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added CI build cache warming for main branch builds. * Added a best-effort in-cluster Cilium endpoint self-heal watchdog for e2e installs. * Extended SeaweedFS COSI with object-lock support plus separate readonly access. * **Improvements** * Reduced e2e install flakiness with a shared readiness deadline, safer waits, and improved cleanup/diagnostics capture. * Prevented install-time churn by switching VPA update mode to **Initial**. * Added container startup probes and updated SeaweedFS volume sizing. * **Tests** * Improved e2e robustness (etcd backup schedule waiting, digest-pinned image checks, and corrected S3 port-forwarding). <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…eral (cozystack#2873) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
superseded by #2948 |
What this PR does
The metallb chart-rendering unit test pinned the controller and speaker images to the exact literal
v0.16.1@sha256:.... Release-prep re-stamps every image tag to the platform version (e.g.v1.5.0-rc.2) and rebuilds the digest, so both the version string and the digest change on a release cut. The exact-literal assertion is green onmainbut fails on every release branch.This stayed invisible because release PRs carry the
releaselabel, which skips the Build job that runsmake unit-tests. It only surfaced on #2861 (cut v1.5.0-rc.2) once that label was dropped.This relaxes both assertions to
matchRegexon the cozystack repository +@sha256digest form, leaving the version string free. The image must still be theghcr.io/cozystack/cozystack/metallb-{controller,speaker}build and must still be digest-pinned, so a wrong repository (e.g. an upstreamquay.ioimage) or a floating tag with no digest still fails the suite.Verified locally with
helm unittest:v0.16.1) — passv1.5.0-rc.2) — pass (previously failed)Release note
Summary by CodeRabbit
Release Notes
/healthzto the CAPI controller.@sha256:<64-hex>digest pinning without matching exact tag+digest literals.