Skip to content

test(metallb): assert digest-pinned image form, not exact version literal - #2873

Closed
myasnikovdaniil wants to merge 1 commit into
fix/e2e-postprep-timeoutfrom
test/metallb-relax-image-assertions
Closed

test(metallb): assert digest-pinned image form, not exact version literal#2873
myasnikovdaniil wants to merge 1 commit into
fix/e2e-postprep-timeoutfrom
test/metallb-relax-image-assertions

Conversation

@myasnikovdaniil

@myasnikovdaniil myasnikovdaniil commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

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 on main but fails on every release branch.

This stayed invisible because release PRs carry the release label, which skips the Build job that runs make unit-tests. It only surfaced on #2861 (cut v1.5.0-rc.2) once that label was dropped.

This relaxes both assertions to matchRegex on the cozystack repository + @sha256 digest form, leaving the version string free. The image must still be the ghcr.io/cozystack/cozystack/metallb-{controller,speaker} build and must still be digest-pinned, so a wrong repository (e.g. an upstream quay.io image) or a floating tag with no digest still fails the suite.

Verified locally with helm unittest:

  • current values (v0.16.1) — pass
  • release-stamped values (v1.5.0-rc.2) — pass (previously failed)
  • wrong repository — fails (guard intact)
  • tag without digest — fails (guard intact)

Release note

NONE

Summary by CodeRabbit

Release Notes

  • New Features
    • Added SeaweedFS support for an object-lock “locked” BucketClass variant, including a dedicated readonly access class.
  • Improvements
    • Added a startup probe hitting /healthz to the CAPI controller.
    • Reduced the SeaweedFS master per-volume size limit, impacting allocation behavior.
  • Bug Fixes
    • Updated the SeaweedFS S3 Service resource naming when enabled.
  • Tests
    • Updated Metallb image validation to require @sha256:<64-hex> digest pinning without matching exact tag+digest literals.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Walkthrough

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

Changes

Build Cache Infrastructure Refactoring

Layer / File(s) Summary
Cache macro and registry configuration
hack/common-envs.mk
Introduced CACHE_REGISTRY (defaulting to colocated build registry), WRITE_CACHE toggle for conditional cache export, and defined the cache-args macro that generates --cache-from and conditional --cache-to Docker buildx arguments.
Main branch cache-warming workflow
.github/workflows/build-main.yaml
Added new workflow triggered on main pushes, configured with Docker login, Buildx setup with docker-container driver, and make build execution using WRITE_CACHE=1 to export registry-mode cache with IMAGE_TAG=main for floating tags.
PR runner selection and build toolchain setup
.github/workflows/pull-requests.yaml
Updated PR builds to default to ephemeral Oracle VMs with conditional routing to self-hosted for debug-labeled PRs, and added Set up build toolchain step that idempotently installs helm-unittest and flux CLI.
Package Makefiles cache-args adoption
packages/*/Makefile (30+ files)
Refactored across core, apps, system, and extra packages to replace explicit --cache-from and --cache-to flags with $(call cache-args,<image>) macro, unifying Docker build cache configuration.

E2E Test Infrastructure and Stability Improvements

Layer / File(s) Summary
Cilium endpoint leak healer
hack/e2e-cilium-endpoint-leak-healer.sh, hack/e2e-cilium-leak-healer.yaml
Implemented in-cluster self-healing loop that detects stale Cilium endpoint entries via FailedCreatePodSandBox events, performs live-owner verification to avoid disconnecting legitimate endpoints, and executes targeted cilium-dbg endpoint disconnect for confirmed orphans. Deployed as cluster-wide Job with required RBAC.
E2E healer test integration
hack/e2e-install-cozystack.bats
Added setup_file() to inject cilium-leak-healer ConfigMap and teardown_file() to capture healer Job logs without deletion, ensuring watchdog runs across e2e install phase.
Test robustness improvements
hack/e2e-apps/bucket.bats, hack/e2e-post-install-prep.sh
Fixed S3 port-forward setup with direct kubectl port-forward and 127.0.0.1 polling, and improved LINSTOR readiness with shared 15-minute deadline and wait_for helper tolerating transient delays.

SeaweedFS Enhancements

Layer / File(s) Summary
Bucket class and access control
packages/system/seaweedfs/charts/seaweedfs/templates/cosi/cosi-bucket-class.yaml
Added -lock suffix BucketClass with deletionPolicy: Retain and object-lock parameters (enabled, compliance retention mode/days), added parameters.accessPolicy: readwrite to base BucketAccessClass, and introduced -readonly variant with parameters.accessPolicy: readonly.
S3 service and volume configuration
packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-service.yaml, packages/system/seaweedfs/values.yaml
Updated S3 Service naming from componentName helper to simplified template format, and reduced default volumeSizeLimitMB from 30000 to 1000 with documentation on per-volume slot allocation impact.

Minor Component Updates

Layer / File(s) Summary
Metallb test assertion relaxation
packages/system/metallb/tests/metallb_test.yaml
Relaxed controller and speaker image assertions from exact appVersion@sha256:<hash> literals to matchRegex patterns requiring digest-addressed cozystack images with @sha256:<64-hex> format.
CAPI controller startup probe
packages/system/capi-providers-core/files/core-components.yaml
Added startupProbe with HTTP GET on /healthz port, failureThreshold: 30, and periodSeconds: 10.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 Buildx caches now march in step,
Shared macros dance through thirty files,
E2E healers mend what breaks in tests,
SeaweedFS locks its treasures tight,
Infrastructure blooms! 🔨✨

Possibly related issues

  • cozystack/cozystack#1921: Implements object-lock and read-only bucket access features for SeaweedFS COSI driver via new BucketClass and BucketAccessClass templates with retention parameters and access policy variants.
  • cozystack/cozystack#2937: Directly implements Phase 1 (cache-args macro and WRITE_CACHE gating) and Phase 3 (Makefile cache-args adoption and build-main.yaml cache warmer) of the build cache refactoring epic.

Possibly related PRs

  • cozystack/cozystack#2558: Related through E2E test modifications to .github/workflows/pull-requests.yaml and hack/e2e-*.bats scripts affecting test execution and diagnostics.
  • cozystack/cozystack#2711: Related through metallb test assertion changes that depend on standardized image-tagging behavior.
  • cozystack/cozystack#2834: Related through SeaweedFS Helm chart updates (COSI bucket-class, S3 service, values configuration).

Suggested labels

area/build, area/ci-cd, area/e2e, area/seaweedfs

Suggested reviewers

  • kvaps
  • lllamnyp
  • androndo
  • sircthulhu
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. 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 PR title accurately summarizes the main change: relaxing metallb image assertion tests from exact version literals to digest-pinned form validation.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/metallb-relax-image-assertions

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 and usage tips.

@dosubot dosubot Bot added the area/testing Issues or PRs related to testing (e2e, bats, unit tests) label Jun 11, 2026
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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

  • Test Assertion Relaxation: Updated the MetalLB unit tests to use regex matching for image tags instead of exact literal string matching.
  • Release Compatibility: Ensured that unit tests remain valid across release branches by allowing the version string to vary while still enforcing digest-pinned image requirements.
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 Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment Gemini (@gemini-code-assist) Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@github-actions github-actions Bot added size/M This PR changes 30-99 lines, ignoring generated files area/networking Issues or PRs related to networking (ingress, gateway, vpn, metallb, kube-ovn) labels Jun 11, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@myasnikovdaniil
myasnikovdaniil force-pushed the test/metallb-relax-image-assertions branch from 4b0938f to e06fd55 Compare June 17, 2026 11:55
…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>
@myasnikovdaniil
myasnikovdaniil force-pushed the test/metallb-relax-image-assertions branch from e06fd55 to 092ea49 Compare June 17, 2026 16:03
@github-actions github-actions Bot added size/XL This PR changes 500-999 lines, ignoring generated files and removed size/M This PR changes 30-99 lines, ignoring generated files labels Jun 17, 2026
@myasnikovdaniil
myasnikovdaniil changed the base branch from main to fix/e2e-postprep-timeout June 17, 2026 16:03
@myasnikovdaniil myasnikovdaniil added the full-e2e Run the full E2E app suite (bypass TIA selection) label Jun 18, 2026
@github-actions github-actions Bot added size/M This PR changes 30-99 lines, ignoring generated files and removed size/XL This PR changes 500-999 lines, ignoring generated files labels Jun 18, 2026
@myasnikovdaniil
myasnikovdaniil marked this pull request as draft June 19, 2026 08:46
myasnikovdaniil added a commit that referenced this pull request Jun 22, 2026
## 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 -->
pull Bot pushed a commit to medampudi/cozystack that referenced this pull request Jun 22, 2026
…eral (cozystack#2873)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
@myasnikovdaniil

Copy link
Copy Markdown
Contributor Author

superseded by #2948

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/networking Issues or PRs related to networking (ingress, gateway, vpn, metallb, kube-ovn) area/testing Issues or PRs related to testing (e2e, bats, unit tests) full-e2e Run the full E2E app suite (bypass TIA selection) size/M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant