fix(ci): only overlay current-main images on main-based PRs - #3472
fix(ci): only overlay current-main images on main-based PRs#3472myasnikovdaniil wants to merge 1 commit into
Conversation
The PR finalize job pulls cozystack-packages:main and repoints every package the PR did not rebuild at current-main images. On a main-based PR that is the whole point: it stops e2e from testing last-release images for everything outside the build matrix. On a release-line PR it is wrong — the committed refs there are not stale, they are that line`s released digests, and the charts are written against exactly those. So a release-line PR was installing main`s binaries onto its own charts, and the mismatch grows with every commit main gains. #3437 is the demonstration: a one-line change on release-1.6 that deactivates an app failed install deterministically, twice, with SchemaError(...core/v1alpha1.Option.spec): unknown model in reference: "...core~1v1alpha1.OptionSpec" from main`s cozystack-controller serving an aggregated OpenAPI that branch`s charts cannot validate against. Nothing in the PR was broken; the lane was. Left alone this makes every 1.6 backport look red, which is when release-branch PRs are busiest. Both overlay steps are now gated on `github.base_ref == main`, so a release-line PR keeps its committed digests — the behaviour that predates the overlay. Retargeting the overlay at a per-line artifact would be better but is not possible today: build-main.yaml publishes only cozystack-packages:main, and the registry carries no release-* equivalent (verified against the packages repo`s tag list: `main` plus per-PR tags, nothing else). hack/overlay-main-images_test.bats pins the wiring per step, so adding a third overlay step without the guard fails the suite. Mutation-checked by removing one guard. 12/12 green; actionlint clean. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> (cherry picked from commit fea40d6)
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
|
Closing in favour of #3471, which now solves this properly rather than by turning the overlay off. This PR carried the interim fix: skip the current-main overlay for non-main base branches. That stopped main's binaries being installed against #3471 instead adds
|
… artifacts (#3471) ## What this PR does The PR finalize job overlays image refs for every package a PR did not rebuild, so e2e and the installer do not test last-release images for everything outside the PR's build matrix. It could only ever read `cozystack-packages:main`, which meant a **release-line PR was handed main's binaries to run against its own line's charts**. #3437 is the demonstration: a one-line change on `release-1.6` deactivating an app failed install deterministically, twice, with ``` helmrelease/backupstrategy-controller: Helm install failed … error validating data: SchemaError(github.com/cozystack/cozystack/pkg/apis/core/v1alpha1.Option.spec): unknown model in reference: "github.com~1cozystack~1cozystack~1pkg~1apis~1core~1v1alpha1.OptionSpec" ``` main's `cozystack-controller` served an aggregated OpenAPI that `release-1.6`'s charts could not validate against. Nothing in that PR was broken; the lane was. Both branches carry `option_types.go` and key `OptionSpec` identically in the committed generated OpenAPI, so this is a generation mismatch at runtime, not a codegen drift. This PR fixes it by giving every base branch its own artifact to overlay from, rather than by turning the overlay off. ### The change 1. **`pull-requests.yaml`** reads `cozystack-packages:${BASE_REF}` (`github.base_ref`) instead of a hardcoded `:main`. 2. **`build-release.yaml`** (new) publishes that artifact for maintained `release-<major>.<minor>` branches exactly as `build-main.yaml` does for main: every image tagged with the branch, and the whole packages tree pushed with each reference digest-pinned to what the run just built. An earlier revision of this branch simply skipped the overlay for non-main bases. That fixed the wrong-images problem but left release-line PRs testing their line's *last release*: for any package the PR did not rebuild, the committed ref is the released digest, so a component changed by an earlier backport was still exercised as its pre-backport binary until the next rc. Per-line artifacts remove that gap too, which is why the guard was replaced rather than kept. ### Three deliberate choices **The trigger matches line branches only** (`release-[0-9]+.[0-9]+`). The per-release and rc staging branches `promote-rc.yaml` and `tags.yaml` create — `release-1.6.1`, `release-1.6.0-rc.4` — must not trigger a full rebuild; their images come from the tag build and rebuilding them is waste. **`WRITE_CACHE` stays `0`.** `CACHE_REGISTRY/<img>:buildcache` is a single ref per image and `build-main.yaml` is deliberately its only, serialized writer so concurrent builds cannot race on the cache manifest — the 409 class #2711 fixed for image tags. A line build can overlap a main build, so writing here would reintroduce that race. Line builds read the cache. **A missing artifact still degrades to committed refs, but says so.** On a release line it emits a `::warning::` naming the branch. Silent degradation is indistinguishable from a working overlay, which is how a mis-specified branch filter could hide for a whole release cycle. ### Cost One `make build` per push to a maintained line — in practice per merged backport. That is the price of release-line PRs testing their line's tip instead of its last release. ### Verification `hack/overlay-main-images_test.bats` pins the artifact tag to the base branch, rejects a hardcoded `:main` in either overlay step, and pins `build-release.yaml`'s branch filter, image tag and `WRITE_CACHE: '0'`. Mutation-checked: restoring `:main`, setting `WRITE_CACHE: '1'`, and broadening the filter to `release-*` each fail a test. 13/13 green; `actionlint` and `zizmor` clean. Worth an explicit ack in review: the branch-filter pattern is the one thing no local test can prove, since only GitHub evaluates it. If it does not match, `build-release` never runs and the new `::warning::` is what surfaces it on the next release-line PR. ### Backport `release-1.6` needs this too — for `pull_request` events GitHub builds the workflow from the merge ref, so a release-line PR only stops receiving main's images once the change is on its base branch. #3472 carried the interim guard and is closed in favour of backporting this instead. ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a “Build release line” workflow that builds and publishes images and the packages artifact for maintained `release-<major>.<minor>` branches. * **Bug Fixes** * Updated PR workflow finalization to use base-branch–specific package overlays, avoiding incorrect main-branch image references when targeting release branches. * Improved fallback behavior when the base-branch packages artifact is unavailable. * **Tests** * Added workflow wiring tests to verify base-branch artifact usage and that each maintained release line publishes its own packages artifact. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
What this PR does
Backport of #3471 to
release-1.6(clean cherry-pick,-xreference in the commit).The overlay must be fixed on this branch to have any effect here: for
pull_requestevents GitHub builds the workflow from the merge ref, so a release-line PR only stops receiving main's images once the guard is on the base branch. Landing #3471 onmainalone does nothing for the 1.6 line.Why it matters here specifically
The finalize job pulls
cozystack-packages:mainand repoints every package a PR did not rebuild at current-main images. On this branch that deploys main's binaries ontorelease-1.6's charts, and the mismatch grows with every commit main gains.#3437 — a one-line change deactivating an app in a bundle — failed install deterministically, twice:
cozystack-controller:mainserved an aggregated OpenAPI this branch's charts could not validate against. Every 1.6 backport is exposed to the same thing, and the symptom reads as a broken PR rather than a broken lane.After this merges, re-running #3437's E2E picks the guard up with no rebase.
Verification
hack/overlay-main-images_test.batspins the wiring per step, mutation-checked by removing one guard. 12/12 green on this branch,actionlintclean.Release note