✨ Add support for parsing and persisting explicit pkg.Release field#2543
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2543 +/- ##
==========================================
- Coverage 68.92% 68.03% -0.89%
==========================================
Files 141 144 +3
Lines 10005 10672 +667
==========================================
+ Hits 6896 7261 +365
- Misses 2594 2883 +289
- Partials 515 528 +13
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:
|
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.
|
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? |
|
#2273 makes the claim that future bundle types will drop this approach, not that registry+v1 bundles should. So that's my bad. |
Sadly, we have to rely on the presence of existing graph edges or we break assumptions users have with the registry+v1 bundle format, coming from v0. So even though we have the ability to prefer version+release over version, since replaces/skips use named bundles instead of bundle versions we have to support the older behavior. |
That would be a regression of a bug fix that #2273 made though, right?
Yeah, agreed. I reached the same conclusion after thinking about this more. So if I understand correctly now:
So the change we need now is "look for |
rashmigottipati
force-pushed
the
oprun-4277-implementation
branch
from
5681488 to
8dcab49
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Adjusted my comments. I missed the part of the #2273 description when it said that we wouldn't divert from semver ordering for any new bundle formats. |
rashmigottipati
force-pushed
the
oprun-4277-implementation
branch
from
a2e5062 to
1e9cded
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rashmigottipati
force-pushed
the
oprun-4277-implementation
branch
from
3af1ee4 to
8644489
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rashmigottipati
force-pushed
the
oprun-4277-implementation
branch
from
d7d5091 to
f1985dc
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rashmigottipati
force-pushed
the
oprun-4277-implementation
branch
2 times, most recently
from
caab602 to
fd1dee7
Compare
rashmigottipati
force-pushed
the
oprun-4277-implementation
branch
from
fd1dee7 to
8f24e45
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
internal/operator-controller/catalogmetadata/filter/successors_test.go:1
- This test file now uses
require.*(e.g.,require.Error,require.Contains) but does not importgithub.com/stretchr/testify/require, which will fail compilation. Add the missing import to match the new assertions.
package filter
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: grokspawn The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
Summary
Adds support for parsing the explicit
pkg.Releasefield from bundle metadata and persisting it through cluster storage (roundtripping). This PR is gated behind theBundleReleaseSupportfeature gate, which is disabled by default. The feature gate is explicitly enabled in experimental manifests to facilitate testing of experimental features.Changes
BundleReleaseSupportalpha feature gate (disabled by default)Releasefield toBundleMetadataCRD typeBundleReleaseKeylabel constant for annotation storagebundleutil.parseVersionRelease()to: Parse explicitpkg.Releasewhen feature gate enabled and field present. Fall back to legacy registry+v1 behavior (release in build metadata) otherwisebundleutil.MetadataFor()to serialize release value fromVersionReleaseFeature Gate Behavior
When enabled:
pkg.Releasefield: release parsed separately, build metadata preserved for proper semver purposeWhen feature gate disabled:
pkg.Releasefield ignoredReviewer Checklist
Link to Github Issue: #2495
Epic: #2479