Repository navigation
Conversation
A fix that lands in an older stable branch while still missing from a newer one is an upgrade regression: upgrades always move from an older to a newer minor, so the user loses the fix on the way up. Today nothing catches this. A backporter can open the vX.15 backport PR, a bugfix then merges carrying needs-backport for both X.15 and X.14, and the subsequently-opened X.14 backport picks it up. The fix ships in X.14 but not X.15, and the only signal is the implicit "please stop merging backport PRs" message before a release. Add a check to the pre-check step that enforces the invariant. A violation is a merged upstream PR carrying backport-done/<A> together with needs-backport/<B> or backport-pending/<B>, where B is newer than A. The check runs relative to the branch being released and covers both directions: the released branch lagging behind an older branch, and the released branch being ahead of a newer one. That way releasing any branch surfaces both sides of the invariant rather than only the ones behind it. Violations hard-block the release, matching the existing release-blocker check rather than the softer opened-backports prompt, since shipping a known upgrade regression is not something to confirm past by pressing Y. --force still overrides it, and the offending PRs are printed with their search URLs so they can be triaged. Only the --maintained-minors (default 3) most-recent branches are compared, plus the released one. Cilium maintains a fixed window of minors, and EOL branches can still carry stale needs-backport labels that would otherwise be reported as violations nobody intends to fix. Note this detects a lagging branch only when it carries an explicit needs-backport or backport-pending label. A fix never labelled for the newer branch at all is invisible to a label-pair query and would need an absence-of-label enumeration to catch. AIL:3 Signed-off-by: André Martins <andre@cilium.io>
aanm
force-pushed
the
pr/enforce-backport-ordering-invariant
branch
from
July 29, 2026 09:02
7c064c9 to
eb92049
Compare
Member
Author
|
Funny enough I had these changes locally and, when running with the v1.20.0, it flagged some PRs that were merged in 1.18 were still open for the v1.20 😄 |
aanm
marked this pull request as ready for review
July 29, 2026 09:08
joestringer
reviewed
Sep 2, 2026
Comment on lines
+133
to
+142
| allStableBranches, err := ghClient.getStableBranches(ctx, c.cfg.Owner, c.cfg.Repo) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Cilium only actively maintains the most-recent minors. Comparing against | ||
| // EOL branches (which may still carry stale needs-backport labels) would | ||
| // produce false positives, so restrict the comparison to the maintained set | ||
| // (plus the branch being released). | ||
| stableBranches := maintainedStableBranches(allStableBranches, c.cfg.TargetVer, c.cfg.MaintainedMinors) |
Member
There was a problem hiding this comment.
Instead of listing all the version branches then narrowing it down, did you consider pulling the recent five or so releases from https://github.com/cilium/releases and derive the stable branches from there? This seems like it would be more efficient.
Comment on lines
+203
to
+208
| // Ensure the released branch is always part of the set. | ||
| if relMM != "" { | ||
| if _, ok := seen[relMM]; !ok { | ||
| branches = append(branches, relMM) | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
Did you consider when the target version is a prerelease?
| // BackportPendingLabel returns the label set on an upstream PR whose backport to | ||
| // the given version has an open (pending) backport PR. | ||
| func BackportPendingLabel(version string) string { | ||
| return fmt.Sprintf("%s%s", backportPendingPrefix, MajorMinorErsion(version)) |
Member
There was a problem hiding this comment.
FYI (no change required): @sekhar-isovalent was talking about switching this over to the form backport-pending/vX.Y. This would make some other tooling stuff a bit simpler. We didn't take any steps towards that yet, but this is one more thing that could break if/when we go through with that.
This branch has not been deployed
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
2 participants
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.
A fix that lands in an older stable branch while still missing from a newer one is an upgrade regression, since upgrades move from an older to a newer minor and the user loses the fix on the way up. Nothing catches this today: a backporter opens the vX.15 backport PR, a bugfix then merges carrying needs-backport for both X.15 and X.14, and the subsequently-opened vX.14 backport picks it up. It ships in X.14 but not X.15, guarded only by the implicit "please stop merging backport PRs" message before a release.
This adds a check to the step-1 pre-check. Taking two stable branches A and B where B is newer, a violation is a merged upstream PR carrying backport-done for A together with either needs-backport or backport-pending for B. It runs relative to the branch being released and covers both directions, so releasing any branch surfaces both the branches it lags and those it is ahead of. Violations hard-block the release, matching the existing release-blocker check rather than the softer opened-backports prompt, with --force still overriding and the offending PRs printed with their search URLs. Only the --maintained-minors (default 3) most-recent branches are compared, plus the released one, so stale needs-backport labels on EOL branches are not reported as violations nobody intends to fix.
Releasing v1.15.5 with a fix that reached v1.13 and v1.14 but not v1.15 looks like this (PR numbers and titles are illustrative):
When the invariant holds it prints "✅ Backport ordering invariant satisfied across all active stable branches." instead, and with --force the violations are still listed but the release continues.
One caveat: detection relies on the lagging branch carrying an explicit needs-backport or backport-pending label, so a fix never labelled for the newer branch at all is invisible to a label-pair query. An explicit frozen backport-candidate list would still be complementary. The check also inherits the pre-release early return in CheckReleaseBlockers.Run, so it does not run for RCs, consistent with the checks around it.
This PR was prepared with AIL:3.