fix(release): preserve tag prefixes when retagging promoted images - #3895
Open
myasnikovdaniil wants to merge 3 commits into
Open
myasnikovdaniil wants to merge 3 commits into
myasnikovdaniil wants to merge 3 commits into
Conversation
promote-retag.sh composed every destination tag from the release version
alone, so an image whose repository holds more than one variant collapsed
into a single tag. ubuntu-container-disk on the 1.5 line is exactly that
shape: one image per Kubernetes minor, six distinct digests under
ghcr.io/cozystack/cozystack/ubuntu-container-disk told apart only by a
v1.30- .. v1.35- tag prefix. All six aimed at :v1.5.4; the sort-first one
was written and the second hit the write-once guard.
That failure lands during finalize, after the stable git tag and the
GitHub release are already public, which is the worst available moment:
the release exists but its registry side is half-written.
Carry the prefix through instead. A source tag is split left to right at
each "-" and the prefix is everything consumed before the first
remainder that parses as a whole version, so v1.30-v1.5.2 promotes to
v1.30-v1.5.4 while a version-only v1.5.2 and a prerelease v1.6.0-rc.1
keep being replaced whole -- anchoring on the remainder rather than on
the head is what stops "v1.6.0-rc.1" being misread as prefix "v1.6.0-".
A tag matching nothing ("latest", "dev", a git-describe) falls back to
the old behaviour, so an unrecognised shape cannot start mis-mapping.
:latest is prefixed for the same reason. Without it every per-minor disk
copies to <repo>:latest and the tag lands on whichever digest the sort
put last -- a floating tag naming one minor's image while reading as if
it named the repository's.
Verified against the committed tree: the v1.5.4 plan is byte-identical
for all 40 unprefixed refs in both MOVE_LATEST modes, and the six
container disks now get six distinct destinations instead of one.
Signed-off-by: Myasnikov Daniil <[email protected]>
Five tests, each red before the fix:
- the destination tag keeps a source tag's prefix and nothing else --
one case per source-tag shape (version-only, prefixed, "latest", a
bare prerelease, tagless), which is the whole prefix-vs-version rule
in one table. The version-only case is the regression guard that
matters most: 40 of the branch's 46 owned refs are that shape.
- several prefixed refs in one repository get distinct destinations --
the six real ubuntu-container-disk refs verbatim. Before the fix it
failed with "duplicate destination refs in the promotion plan:
.../ubuntu-container-disk:v1.5.4".
- MOVE_LATEST keeps the prefix -- pins one floating tag per prefix
rather than six writes to one.
- two digests behind one prefixed destination still trip the
write-once guard -- proves preserving the prefix did not become a
way around the guard, and that the refusal names the prefixed tag.
- the real tree's promotion plan has no duplicate destination -- the
check that would have caught this before a release reached finalize,
now pinned against the committed tree permanently. The per-minor
count is derived from the images/*.tag files rather than hardcoded,
so branches shipping no such image assert 0 == 0 and stay green.
_make_ref_tree builds a throwaway package tree of images/*.tag files and
runs the script with it as CWD, so the real hack/lib/image-refs.sh
collection path is exercised rather than a stubbed yq. images/*.tag is
the deliberate shape: it is what ubuntu-container-disk uses and the only
one whose collected ref still carries the source tag.
Negative assertions are counted (grep -c ... -eq 0) or written as an
explicit non-empty test, never as `! grep -q`: a !-negated pipeline is
exempt from errexit and cannot fail a cozytest.sh test.
Signed-off-by: Myasnikov Daniil <[email protected]>
Six assertions were written `! grep -q X file`. POSIX and bash both exempt a `!`-negated pipeline from errexit, so under cozytest.sh -- which runs each @test as a shell function with `set -e` and takes a non-zero exit as the failure -- none of them could ever fail. shellcheck reports all six as SC2314 errors. Each is now counted, and each was mutation-proven inert first. Two mutants isolate them, both leaving every positive assertion in the same test satisfied so the negative one is the only thing that can object: - an unconditional `copy` inserted before the write-once probe. Catches the four `! grep -q '^copy '` assertions in the OCI-artifact, already-published, empty-body and 429 tests -- all four pass with the `!` form and fail when counted. - MOVE_LATEST defaulting to 1. Catches "default leaves :latest unmoved". - the empty-body diagnostic interpolating sha256sum of the empty manifest file, the shape of a plausible "helpful" debug addition. Catches the e3b0c442… assertion in the empty-body test. No guard was actually broken: all six hold once real, so this changes no behaviour and adds no coverage -- it makes the coverage the file already claimed enforceable. The harness note at the top now states the rule. Signed-off-by: Myasnikov Daniil <[email protected]>
Contributor
|
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:
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 |
12 tasks
Aleksei Sviridkin (lexfrei)
added a commit
that referenced
this pull request
Sep 24, 2026
…4460) <!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does A PR stacked on a feature branch now overlays the image refs of the line it grew from, so its E2E no longer falls back to the refs committed in the tree. The Finalize job pulls `cozystack-packages:<base branch>` and overlays its image refs onto the packages the PR did not rebuild. For a stacked PR the base is something like `fix/foo`. The slash is not valid in an OCI tag, and no workflow publishes an artifact for that branch anyway. So the pull failed, the step only printed a warning, and every unbuilt package ran its committed release ref. That is how stacked PRs got the Grafana plugin 404 described in #4391. Now `main` and `release-X.Y` bases read their own artifact. Any other base reads the artifact of whichever of main and the release lines it has the fewest commits on top of, and a notice says which one. A stack on a release line keeps that line's images, for example #3895, which sits on `release-1.5`. On a tie main wins, so a stack on a line cut from main reads main's artifact until the stack contains a commit of the line's own. Always taking main there would put main's images under the line's charts, and that is the install failure from #3437. The new bats case builds a small git fixture and checks the tag pulled for six bases, one of them a deleted branch. The other half of #4391, the stale `grafana-dashboards.tag`, is a release-managed digest pin and is not touched here. It gets re-stamped at release prep. Part of #4391 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(ci): E2E for a PR stacked on a feature branch overlays the image refs of the line the stack grew from, instead of falling back to the committed release refs. ```
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge after the release-machinery port PR, this is stacked on it because release-1.5 has no
hack/promote-retag.shuntil that lands.ubuntu-container-diskon the 1.5 line is one image per kubernetes minor, six digests under a single repository distinguished only by a tag prefix,v1.30-v1.5.2throughv1.35-v1.5.2. The retag composes its destination from the release version alone, so all six aim atubuntu-container-disk:v1.5.4. The sort-first one is written and the second fails the write-once guard, and that happens in finalize, after the stable tag and the github release are already public.Three separate things hid this. The tag is discarded at collection,
ref_repo()strips it and refs are carried as<repo>@<digest>. The guard detects collisions by manifest digest rather than by planning, so--dry-runskips the probe entirely and six copies aimed at one tag print as a clean plan. And thesort -udedup operates on<repo>@<digest>, which six distinct digests pass without complaint.Fix splits the source tag at each
-left to right and treats as prefix everything before the first remainder that is a whole version. Anchoring on the remainder is the load-bearing part:v1.6.0-rc.1leavesrc.1, which is not a version, so that tag stays whole, and an rc string is exactly what a promoted tree carries sincetags.yamlstampsIMAGE_TAG: github.ref_name. Left to right also getsv1.30.0-v1.5.2right where a longest-suffix rule would collapse it. No match means empty prefix and exactly the old behaviour.A/B of the full copy plan against the base is byte-identical for all 40 unprefixed refs in both
MOVE_LATESTmodes, and gives 46 of 46 unique destinations where the base gave 46 to 41. Also a no-op on main (42/42) and release-1.6 (43/43), neither of which carries prefixed refs, so this is safe to cherry-pick upstream unchanged.The red phase reports
duplicate destination refs in the promotion plan: ghcr.io/cozystack/cozystack/ubuntu-container-disk:v1.5.4, and there is a permanent assertion that a v1.5.4 plan contains no duplicate destination, since that is the check that would have caught this in the first place.Separate commit converts six pre-existing negated-grep assertions in the same file, each proven inert before conversion by mutating the thing it claimed to check. All six hold once they are real, so no guard was actually broken, they were just decorative.
main needs the identical change as a straight cherry-pick,
promote-retag.shandpromote-retag_test.batsare byte-identical there. One known limitation: a<prefix>-<non-semver>tag likev1.30-devfrom a local build gets no prefix and collapses as it does today. Not reachable in promotion,tags.yamlis the only workflow that commits image refs back.