Skip to content

Commit 2330d6f

Browse files
myasnikovdaniilmyasnikovdaniil
committed
fix(ci): overlay images from the PR base branch, and publish per-line artifacts
Supersedes the interim guard in this branch`s first commit, which skipped the overlay entirely for release-line PRs. Skipping fixed the wrong images but left those 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 exercised as its pre-backport binary until the next rc. The overlay now reads `cozystack-packages:<base branch>` instead of always `:main`, and build-release.yaml publishes that artifact for every maintained `release-<major>.<minor>` branch the way build-main.yaml does for main: images tagged with the branch, and the whole packages tree pushed with each reference digest-pinned to what the run just built. Each base branch therefore has its own generation to overlay from, which is what the original bug was really about — #3437 failed install because main`s cozystack-controller served an aggregated OpenAPI release-1.6`s charts could not validate against. 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 would be 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 on a release line it now emits a ::warning:: naming the branch. Silent degradation is indistinguishable from a working overlay, which is how a mis-specified branch filter would hide for a whole release cycle. Cost: one `make build` per push to a maintained line, i.e. per merged backport. 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. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent fea40d6 commit 2330d6f

3 files changed

Lines changed: 210 additions & 41 deletions

File tree

‎.github/workflows/build-release.yaml‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
name: Build release line
2+
3+
# Publishes the per-line equivalent of what build-main.yaml publishes for main:
4+
# every image tagged with the line's branch name, and the whole packages tree as
5+
# the `cozystack-packages:<branch>` OCI artifact with each image reference
6+
# digest-pinned to the images this run just pushed (`make build` ->
7+
# packages/core/installer image-packages does the `flux push artifact`).
8+
#
9+
# Why it exists: pull-requests.yaml overlays image refs for every package a PR
10+
# did NOT rebuild, so that e2e and the installer do not test last-release images
11+
# for everything outside the PR's build matrix. That overlay had only
12+
# `cozystack-packages:main` to read, which meant a release-line PR was handed
13+
# MAIN's binaries to run against its own line's charts — a cross-generation mix
14+
# that failed deterministically (see #3437: main's cozystack-controller served an
15+
# aggregated OpenAPI release-1.6's charts could not validate against). With this
16+
# artifact published per line, the overlay reads the PR's own base branch and the
17+
# generations match.
18+
#
19+
# Cost: one full `make build` per push to a maintained line, i.e. per merged
20+
# backport. That is the price of release-line PRs testing their line's tip rather
21+
# than its last release.
22+
23+
env:
24+
# Same per-CI registry as pull-requests.yaml and build-main.yaml.
25+
REGISTRY: iad.ocir.io/idyksih5sir9/cozystack
26+
27+
on:
28+
push:
29+
# Maintained LINE branches only: `release-<major>.<minor>`. The `+` quantifies
30+
# the preceding character class, and `.` is literal, so this matches
31+
# release-1.6 but NOT the per-release staging branches promote-rc.yaml creates
32+
# (release-1.6.1) nor rc staging branches (release-1.6.0-rc.4) — building
33+
# those would be pure waste, since their images are built by tags.yaml.
34+
branches: ['release-[0-9]+.[0-9]+']
35+
paths-ignore:
36+
- 'docs/**'
37+
38+
# Per line: a newer push to the same line supersedes an in-flight build, but
39+
# different lines build independently.
40+
concurrency:
41+
group: build-release-${{ github.ref_name }}
42+
cancel-in-progress: true
43+
44+
permissions:
45+
contents: read
46+
47+
jobs:
48+
build-line:
49+
name: Build ${{ github.ref_name }} images and packages artifact
50+
runs-on: oracle-vm-24cpu-96gb-x86-64
51+
timeout-minutes: 120
52+
permissions:
53+
contents: read
54+
packages: write
55+
56+
steps:
57+
- name: Checkout code
58+
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
59+
with:
60+
fetch-depth: 0
61+
fetch-tags: true
62+
# This workflow holds packages:write and an OCIR push token; it never
63+
# pushes git, so don't leave the checkout's GITHUB_TOKEN in .git/config
64+
# where a later build step could read it.
65+
persist-credentials: false
66+
67+
# Ephemeral runners lack the flux CLI the installer's image-packages step
68+
# shells out to; install if absent (idempotent).
69+
- name: Set up build toolchain
70+
run: |
71+
command -v flux >/dev/null \
72+
|| curl -fsSL https://fluxcd.io/install.sh | sudo bash
73+
74+
- name: Set up Docker config
75+
run: |
76+
if [ -d ~/.docker ]; then
77+
cp -r ~/.docker "${{ runner.temp }}/.docker"
78+
fi
79+
80+
- name: Login to OCIR
81+
uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3
82+
with:
83+
username: ${{ secrets.OCIR_USER }}
84+
password: ${{ secrets.OCIR_TOKEN }}
85+
registry: iad.ocir.io
86+
env:
87+
DOCKER_CONFIG: ${{ runner.temp }}/.docker
88+
89+
# Same isolated buildkit as build-main.yaml, so a line build never contends
90+
# with PR builds on the shared embedded builder.
91+
- name: Set up Buildx (docker-container driver)
92+
id: buildx
93+
uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3
94+
with:
95+
driver: docker-container
96+
env:
97+
DOCKER_CONFIG: ${{ runner.temp }}/.docker
98+
99+
- name: Build line images and publish the packages artifact
100+
run: make build
101+
env:
102+
DOCKER_CONFIG: ${{ runner.temp }}/.docker
103+
BUILDER: ${{ steps.buildx.outputs.name }}
104+
# READ the shared mode=max cache, never write it. CACHE_REGISTRY/<img>:
105+
# buildcache is a single ref per image and build-main.yaml is
106+
# deliberately its only writer, serialized, precisely so concurrent
107+
# builds cannot race on the cache manifest (the 409 collision class
108+
# #2711 fixed for image tags). A line build can overlap a main build, so
109+
# writing here would reintroduce exactly that race.
110+
WRITE_CACHE: '0'
111+
# Floating per-line handle: images and the packages artifact both land
112+
# under the branch name (e.g. :release-1.6). Versioned and :latest tags
113+
# belong to releases (tags.yaml / finalize), never to this workflow.
114+
IMAGE_TAG: ${{ github.ref_name }}
115+
PUBLISH_VERSIONED: '0'
116+
PUBLISH_FLOATING: '0'

‎.github/workflows/pull-requests.yaml‎

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -378,36 +378,49 @@ jobs:
378378
# (build-main not yet green) is a no-op: packages keep their committed
379379
# refs (prior behaviour). See hack/overlay-main-images.sh.
380380
#
381-
# ONLY for PRs based on main. On a release-line PR the committed refs are
382-
# not stale — they are that line's released digests, which is exactly what
383-
# its charts are written against. Overlaying main's images there deploys
384-
# main's binaries onto a release branch's charts, and the mismatch grows
385-
# with every commit main gains: #3437 (a one-line change on release-1.6)
386-
# failed install deterministically because main's cozystack-controller
387-
# served an aggregated OpenAPI its charts could not validate against
388-
# (`unknown model in reference: …v1alpha1.OptionSpec`), which reads as a
389-
# broken PR while nothing in the PR is broken. Retargeting the overlay at a
390-
# per-line artifact is not possible today: build-main.yaml publishes only
391-
# cozystack-packages:main, with no release-* equivalent.
392-
- name: Pull current-main packages tree
393-
if: github.base_ref == 'main'
381+
# Read the artifact for the PR's OWN base branch, not always main. Overlaying
382+
# main's images onto a release-line PR mixes generations: main's binaries
383+
# against that line's charts. #3437 (a one-line change on release-1.6) failed
384+
# install deterministically that way, because main's cozystack-controller
385+
# served an aggregated OpenAPI release-1.6's charts could not validate
386+
# against (`unknown model in reference: …v1alpha1.OptionSpec`) — a broken
387+
# lane reading as a broken PR. build-release.yaml publishes
388+
# cozystack-packages:<line> for every maintained release-<major>.<minor>
389+
# branch, so each base branch has its own artifact to read.
390+
#
391+
# A missing artifact stays a no-op (packages keep their committed refs), which
392+
# also covers a line whose first build-release run has not landed yet.
393+
- name: Pull base-branch packages tree
394394
run: |
395395
rm -rf _out/mainpkgs
396396
mkdir -p _out/mainpkgs
397-
flux pull artifact "oci://${REGISTRY}/cozystack-packages:main" \
398-
--output _out/mainpkgs \
399-
|| { echo "cozystack-packages:main pull failed — keeping committed refs"; rm -rf _out/mainpkgs; }
397+
if ! flux pull artifact "oci://${REGISTRY}/cozystack-packages:${BASE_REF}" \
398+
--output _out/mainpkgs; then
399+
rm -rf _out/mainpkgs
400+
# Degrading to committed refs is safe, but SAY so: on a release line a
401+
# missing artifact means build-release.yaml has not published this line
402+
# yet (first build pending, or its branch filter does not match this
403+
# branch), and the PR is then testing the line's last release instead of
404+
# its tip. Silent would look identical to a working overlay.
405+
if [ "${BASE_REF}" = "main" ]; then
406+
echo "cozystack-packages:${BASE_REF} pull failed — keeping committed refs"
407+
else
408+
echo "::warning title=No packages artifact for ${BASE_REF}::Keeping committed refs, so unbuilt packages use this line's last release rather than its tip. Check that build-release.yaml has run for ${BASE_REF}."
409+
fi
410+
fi
400411
env:
401412
DOCKER_CONFIG: ${{ runner.temp }}/.docker
402-
- name: Overlay current-main refs for unbuilt packages
413+
# Env-passed, not interpolated into the script: base_ref is repo-controlled
414+
# here (a branch name in this repo), but the workflow-injection hygiene
415+
# applies uniformly.
416+
BASE_REF: ${{ github.base_ref }}
417+
- name: Overlay base-branch refs for unbuilt packages
403418
# Best-effort: this only improves WHICH images e2e and the installer use,
404419
# so an unexpected failure must degrade to the committed refs (prior
405420
# behaviour), never block the PR. overlay-main-images.sh already handles
406-
# the expected paths (missing artifact, drift, absent unit) internally.
407-
# Guarded on the base branch for the same reason as the pull above; the
408-
# script would treat the absent artifact as a no-op anyway, but a
409-
# release-line PR should not depend on that to avoid main's images.
410-
if: github.base_ref == 'main'
421+
# the expected paths (missing artifact, drift, absent unit) internally —
422+
# including the empty directory the step above leaves when a line has no
423+
# artifact yet.
411424
run: |
412425
hack/overlay-main-images.sh _out/mainpkgs "$BUILD_MATRIX" "$PR_TOUCHED" \
413426
|| echo "::warning::overlay-main-images failed; unbuilt packages keep committed refs"

‎hack/overlay-main-images_test.bats‎

Lines changed: 59 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -149,30 +149,70 @@
149149
}
150150

151151
# ── workflow wiring ─────────────────────────────────────────────────────────
152-
# The script is only half the mechanism; WHEN the workflow invokes it decides
153-
# which branch's images a PR gets. Both steps must be gated on the base branch,
154-
# so a release-line PR keeps that line's committed digests instead of running
155-
# main's binaries against its charts.
152+
# The script is only half the mechanism; WHICH artifact the workflow hands it
153+
# decides which generation of images a PR gets. A release-line PR must read its
154+
# own line's artifact — reading main's puts main's binaries against that line's
155+
# charts, which failed install deterministically (#3437).
156156

157-
@test "the overlay steps are gated on a main base branch" {
157+
@test "the overlay reads the artifact for the PR's own base branch" {
158158
root=$(pwd)
159159
wf="$root/.github/workflows/pull-requests.yaml"
160160
[ -f "$wf" ]
161161

162-
# Executable lines only: a commented-out guard must not satisfy this.
162+
# Executable lines only: a comment mentioning the tag must not satisfy this.
163163
code="$(grep -v '^[[:space:]]*#' "$wf")"
164164

165-
# Each overlay step name must be followed by the base-branch guard before the
166-
# step's `run:` — assert per step, so adding a third unguarded step is caught.
167-
for step in "Pull current-main packages tree" "Overlay current-main refs for unbuilt packages"; do
168-
block="$(printf '%s\n' "$code" | awk -v s=" - name: $step" '
169-
$0 == s { inside = 1; next }
170-
/^ - name: / { inside = 0 }
171-
inside')"
172-
[ -n "$block" ] || { echo "step not found in $wf: $step" >&2; exit 1; }
173-
printf '%s\n' "$block" | grep -qF "if: github.base_ref == 'main'" || {
174-
echo "step '$step' is not gated on the base branch; a release-line PR would" >&2
175-
echo "get main's images overlaid onto that line's charts." >&2
176-
exit 1; }
177-
done
165+
block="$(printf '%s\n' "$code" | awk '
166+
$0 == " - name: Pull base-branch packages tree" { inside = 1; next }
167+
/^ - name: / { inside = 0 }
168+
inside')"
169+
[ -n "$block" ] || { echo "the base-branch packages pull step is missing from $wf" >&2; exit 1; }
170+
171+
# The artifact tag must come from the base branch, passed via env.
172+
printf '%s\n' "$block" | grep -qF 'cozystack-packages:${BASE_REF}' || {
173+
echo "the packages artifact tag is not the PR's base branch. Reading a fixed" >&2
174+
echo "tag (e.g. :main) hands a release-line PR another generation's images." >&2
175+
exit 1; }
176+
printf '%s\n' "$block" | grep -qF 'BASE_REF: ${{ github.base_ref }}' || {
177+
echo "BASE_REF is not wired to github.base_ref in the pull step's env." >&2; exit 1; }
178+
179+
# A hardcoded :main anywhere in the two overlay steps defeats the point.
180+
overlay="$(printf '%s\n' "$code" | awk '
181+
$0 == " - name: Overlay base-branch refs for unbuilt packages" { inside = 1; next }
182+
/^ - name: / { inside = 0 }
183+
inside')"
184+
[ -n "$overlay" ] || { echo "the overlay step is missing from $wf" >&2; exit 1; }
185+
printf '%s\n%s\n' "$block" "$overlay" | grep -qF 'cozystack-packages:main' && {
186+
echo "an overlay step still pins cozystack-packages:main" >&2; exit 1; }
187+
return 0
188+
}
189+
190+
@test "every maintained release line publishes its own packages artifact" {
191+
root=$(pwd)
192+
wf="$root/.github/workflows/build-release.yaml"
193+
[ -f "$wf" ] || {
194+
echo "build-release.yaml is missing: without a per-line artifact the overlay" >&2
195+
echo "above degrades to a no-op on release branches, so their PRs test the" >&2
196+
echo "line's last release rather than its tip." >&2
197+
exit 1; }
198+
199+
code="$(grep -v '^[[:space:]]*#' "$wf")"
200+
201+
# Line branches only. The per-release and rc staging branches promote-rc.yaml
202+
# and tags.yaml create (release-1.6.1, release-1.6.0-rc.4) must NOT trigger a
203+
# full rebuild — their images come from the tag build.
204+
printf '%s\n' "$code" | grep -qF "branches: ['release-[0-9]+.[0-9]+']" || {
205+
echo "build-release.yaml does not restrict its trigger to release-<major>.<minor>" >&2; exit 1; }
206+
207+
# The artifact and image tag must be the branch, or the overlay cannot find it.
208+
printf '%s\n' "$code" | grep -qF 'IMAGE_TAG: ${{ github.ref_name }}' || {
209+
echo "build-release.yaml does not tag images/artifact with the branch name" >&2; exit 1; }
210+
211+
# It must not write the shared mode=max cache: that ref has exactly one
212+
# serialized writer (build-main.yaml) so concurrent builds cannot race on the
213+
# cache manifest. A line build can overlap a main build.
214+
printf '%s\n' "$code" | grep -qF "WRITE_CACHE: '0'" || {
215+
echo "build-release.yaml must set WRITE_CACHE: '0' — writing the shared" >&2
216+
echo ":buildcache ref races build-main.yaml on the cache manifest." >&2
217+
exit 1; }
178218
}

0 commit comments

Comments
 (0)