WIP: FBC data model migrations
Signed-off-by: Joe Lanford [email protected]<!--
Before making a PR, please read our contributing guidelines https://github.com/operator-framework/operator-lifecycle-manager/blob/master/CONTRIBUTING.md Note: Make sure your branch is rebased to the latest upstream master.
-->
WIP: This is an implementation with few tests. Before merge, we need to add some tests to ensure migrations work as expected and GRPC compatibility is maintained.
Description of the change:
- add olm.icon blob and migration
- fbc: promote bundle version from property to first-class field
- fbc: promote package-level metdata from olm.csv.metadata to olm.package blob
For all of these, the opm serve implementation is updated to correctly plumb the new fields into the GRPC API.
Motivation for the change:
- With the work in catalogd to allow querying a catalog by schema, package, and/or name, we can give catalogd clients more granular access to package metadata without the icon, which is beneficial for clients that want to list packages without also having to download all of the icons, which they may not care about (e.g. CLI clients or catalog resolvers like operator-controller)
- The current data model requires an
olm.packageproperty, where both the package name and version are required. The bundle schema already has the package name, and the bundle version is a key piece of information when dealing with bundles. Promoting the bundle version to a first class field and eliminating theolm.packageproperty from FBCs reduces duplication of package name and improves performance and ergonomics for clients that need to know the bundle version (which is virtually all clients). - Often, clients want to know package-level metadata like display name, a short description, its provider and maintainers, and lists of links and keywords. This metadata is essentially duplicated in all bundles unnecessarily and it is more useful when coloated with the package. Otherwise, clients need to also download all bundles and process them to find the "best" bundle from which to derive this metadata, this is wasteful of CPU and disk space.
Open questions:
- Is it okay to remove the
olm.package's.iconfield when we migrate toolm.icon? FBC clients that don't know about the newolm.iconwill think there is no icon (GRPC clients will still get the icon in the expected place). If we don't remove the.iconfield, the benefit of separating it out is completely eliminated, and we might as well not separate it because then we'd just duplicate the icon unnecessarily. - Is it okay to remove the
olm.bundle'solm.packageproperty when we promote the version to a first-class field? Same sort of problem as above, In this case, we could probably keep both the property and the version field, but then we would need extra validation to ensure they match and we'd be using slightly more space per bundle. - Do we agree that the following make sense as package-level fields?
- Short Description
- Display Name
- Provider
- Maintainers
- Links
- Keywords
- For all of the fields in (3) can we safely extract those from the bundle with the highest semver version in the package? Of those, I wonder most about links, which bundle authors may be setting to version-specific release notes or documentation. Maybe we leave links at the bundle level just in case? In the future, if there is a clearer need for package-level links we can always add a package-level links field and change the migration to promote any links that are present in all bundles?
Reviewer Checklist
- [ ] Implementation matches the proposed design, or proposal is updated to match implementation
- [ ] Sufficient unit test coverage
- [ ] Sufficient end-to-end test coverage
- [ ] Docs updated or added to
/docs - [ ] Commit messages sensible and descriptive
Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: joelanford
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [joelanford]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Codecov Report
:x: Patch coverage is 34.89933% with 194 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 54.88%. Comparing base (ef3bfdf) to head (dd26966).
:warning: Report is 120 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #1679 +/- ##
==========================================
- Coverage 55.17% 54.88% -0.30%
==========================================
Files 136 139 +3
Lines 15918 16161 +243
==========================================
+ Hits 8783 8870 +87
- Misses 5982 6118 +136
- Partials 1153 1173 +20
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
PRs go stale after 90 days of inactivity. If there is no further activity, the PR will be closed in another 30 days.