✨ Add ReleaseVersionPriority feature gate for re-release upgrade support#2543
✨ Add ReleaseVersionPriority feature gate for re-release upgrade support#2543rashmigottipati wants to merge 4 commits intooperator-framework:mainfrom
Conversation
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR introduces an alpha feature gate to allow OLMv1 upgrade resolution to treat “re-released” bundles (same semver, higher release/build value like 2.0.0+1 -> 2.0.0+2) as valid successors when explicitly enabled.
Changes:
- Added
ReleaseVersionPriorityfeature gate (Alpha, default disabled). - Added
SameVersionHigherRelease()predicate and integrated it intoSuccessorsOf()behind the feature gate. - Added unit tests for
SameVersionHigherRelease()and forSuccessorsOf()with the gate disabled.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/operator-controller/features/features.go | Defines the new ReleaseVersionPriority feature gate and its spec. |
| internal/operator-controller/catalogmetadata/filter/successors.go | Conditionally expands successor matching to include same-version/higher-release bundles when gated on. |
| internal/operator-controller/catalogmetadata/filter/bundle_predicates.go | Adds the SameVersionHigherRelease() predicate. |
| internal/operator-controller/catalogmetadata/filter/bundle_predicates_test.go | Adds predicate unit tests including edge cases. |
| internal/operator-controller/catalogmetadata/filter/successors_test.go | Adds a regression test to ensure default (gate-off) behavior does not accept higher-release bundles. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/operator-controller/catalogmetadata/filter/successors_test.go
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2543 +/- ##
=======================================
Coverage 68.62% 68.62%
=======================================
Files 131 131
Lines 9288 9298 +10
=======================================
+ Hits 6374 6381 +7
- Misses 2429 2432 +3
Partials 485 485
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/operator-controller/catalogmetadata/filter/successors_test.go
Outdated
Show resolved
Hide resolved
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
|
If I understand this correctly, it looks like this introduces the new behavior that an explicit upgrade edge doesn't actually have to exist in the catalog to upgrade to a bundle that has the same version and a higher release. Is that the intent? (If so let's make that clear in the PR description). Is it also the intent that we'll inherit the upgrade edges of the first release of the version? |
| const testPackageName = "test-package" | ||
|
|
||
| // Expected bundle version 2.0.0+1 | ||
| expect, err := bundle.NewLegacyRegistryV1VersionRelease("2.0.0+1") |
There was a problem hiding this comment.
I'd suggest putting the expect value in the test case, and adding cases that start from different values. For example: from a version without a release.
Also related: do we need to add code that knows how to parse the new Release field of the olm.package property (around here:
There was a problem hiding this comment.
iirc, @grokspawn's design called for a specific precedence when handling both release and build metadata that we will need to account for.
| // If release version priority is enabled, also consider bundles | ||
| // with the same semantic version but higher release as valid successors. | ||
| // This allows upgrades to re-released bundles (e.g., 2.0.0+1 -> 2.0.0+2). |
There was a problem hiding this comment.
What if the re-release version is already in the explicit upgrade graph? In that case, should we ignore it and let its explicit channel entry take precedence? That would preserve existing behavior.
Description
Adds the
ReleaseVersionPriorityfeature gate (Alpha, disabled by default) to enable upgrades between re-released bundles with the same semantic version but different release values (e.g.,2.0.0+1to2.0.0+2).Changes in the PR:
ReleaseVersionPriority(Alpha, disabled by default)SameVersionHigherRelease()to identify bundles with same version but higher releaseSuccessorsOf()to conditionally include higher-release bundles as valid successors when the feature gate is enabledSameVersionHigherRelease()predicate with edge casesSuccessorsOf()integration with feature gate disabled (testing the default behavior)Reviewer Checklist
Link to Github Issue: #2495
Epic: #2479