fix(release): stop a stale release cut from freezing an rc tag on an outdated commit - #641
Merged
Merged
Conversation
…commit A release cut that materializes a git tag (action=update, tag-only, create-tag) treated a 422 'reference already exists' from POST /git/refs as an unconditional success. That only holds when the existing tag already points at the target commit. When it points elsewhere the ref create is a silent no-op, leaving the tag frozen on a stale commit while the cut reports success, which stranded an rc tag on an outdated sha for a full day. createGitTag now resolves the existing tag on a 422 and returns nil only when its target matches the requested commit; a mismatch (or an unresolvable target) fails loudly. An rc tag is immutable in the single-flight release model, so a fresh cut needing a different commit takes a new rc number rather than moving an existing tag. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
Version derivation set the rc number from recorded state alone (state.Version fed to CalculateNext, which computes rc.N+1) and never consulted existing tags. When Finalize state-writes stalled behind a 409, the recorded version stuck at rc.0 and every cut recomputed the same rc.1, which already existed as a tag at an outdated commit. Nothing guarded the collision, so a fresh cut could re-mint an rc number already published at a different sha. calculateVersion (and the component path) now resolve the candidate against the repository's tags: while a tag of the same name exists at a commit other than HEAD, the rc number advances until it is free. A tag already at HEAD is the same cut re-running and is reused unchanged, and a candidate with no rc segment or an unresolvable HEAD is left untouched, so the no-collision path is byte-identical. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
Contributor
|
All PR Validation checks passed. |
Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
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.
Problem
An rc tag froze at a stale commit for a full day: a release cut recomputed an rc number that already existed and then silently failed to move the tag. Two compounding defects.
Defect 1 - version derivation had no collision guard.
internal/orchestrate/orchestrator.gocalculateVersion(andcalculateComponentVersion) set the rc number from recorded state alone (state.Versionfed toversion.CalculateNext, which computesrc.N+1) and never consulted existing tags. When Finalize state-writes stalled behind a 409, the recorded version stuck atrc.0and every cut recomputed the samerc.1, which already existed as a tag at an outdated commit.Defect 2 - tag creation no-oped on an existing tag at a different commit.
internal/release/release.gocreateGitTagtreated a422 reference already existsfromPOST /git/refsas unconditional success. That only holds when the existing tag already points at the target commit; when it points elsewhere the ref create is a silent no-op, so the tag stayed frozen on the stale commit while the cut reported success.Fix
Defect 1 (advance):
calculateVersionnow resolves the candidate against the repository's tags. While a tag of the same name exists at a commit other than HEAD, the rc number advances until it is free. A tag already at HEAD is the same cut re-running and is reused unchanged; a candidate with no rc segment or an unresolvable HEAD is left untouched, so the no-collision path is byte-identical. Chosen semantics: advance rather than fail, because a stalled state resolving is a normal recoverable condition and the single-flight model wants the cut to make forward progress on the next free rc, not wedge.Defect 2 (fail closed): on a 422,
createGitTagnow resolves the existing tag viaGET /git/refs/tags/<name>and returns nil only when its target matches the requested commit; a mismatch (or an unresolvable target) fails loudly. Chosen semantics: fail loud, not force-move. An rc tag is immutable in the single-flight release model, so a fresh cut that needs a different commit takes a new rc number (Defect 1's job); no caller legitimately moves a tag, and there is noPATCH /git/refsforce-update in the tree. This is the fail-closed safety net that turns the original silent freeze into a visible error. Same-sha 422 stays the genuinely idempotent success.Verification
internal/release: existing tag at a different sha now errors instead of returning a silent nil;internal/orchestrate: a stuck-at-rc.0 state with an existingrc.1tag at another commit now yieldsrc.2).go build ./...,go test ./...(3651 pass),go test -raceon the affected packages, andgolangci-lint runall clean.