Translate legacy "bundle dependencies" to properties. #2254
Conversation
Operator dependencies are expressed as properties, but there was initially a notion of dependencies as distinct from properties. For that reason, catalog-operator must support existing catalog images that serve separate Bundle.dependencies and Bundle.properties fields by translating the three legacy dependency types (olm.label, olm.package, and olm.gvk) into their corresponding properties. Signed-off-by: Ben Luddy <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: benluddy The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
There was a problem hiding this comment.
couple of small comments, otherwise looks good.
This change makes me think we want to prioritize a story for "property-only" updates of operators. I think we would want to have existing CSVs get these annotations added to them without any other changes. (and there are other reasons we might want that too with declarative config)
The main blocker for that is just identity - but I think we could probably pick the the properties we use for identity today (package, name, version, channel) and use that as the identifier and potentially work on something a bit more solid in the future.
| Value: i, | ||
| } | ||
| } | ||
| var l, r []Property |
There was a problem hiding this comment.
I think some other places deduplicate properties (which would make this a map). Did we write down anything about multiple properties with the same exact content?
| if err != nil { | ||
| return nil, err | ||
| } | ||
| properties = append(properties, ps...) | ||
| } | ||
|
|
||
| // legacy support - if the grpc api doesn't contain required/provided apis, fallback to csv parsing |
There was a problem hiding this comment.
do we want to remove this now? I think this has the same backport range as the rest of the PR
| if err != nil { | ||
| return nil, err | ||
| } | ||
| namespacedCache := r.cache.Namespaced(namespaces...).WithExistingOperators(existingSnapshot) | ||
|
|
||
| _, existingInstallables, err := r.getBundleInstallables(registry.NewVirtualCatalogKey(namespaces[0]), existingSnapshot.Find(), namespacedCache, visited) |
There was a problem hiding this comment.
+1 this refactoring makes more sense
|
/lgtm |
|
/cherry-pick release-4.7 |
|
@kevinrizza: new pull request created: #2328 In response to this:
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/test-infra repository. |
Operator dependencies are expressed as properties, but there was
initially a notion of dependencies as distinct from properties. For
that reason, catalog-operator must support existing catalog images
that serve separate Bundle.dependencies and Bundle.properties fields
by translating the three legacy dependency types (olm.label,
olm.package, and olm.gvk) into their corresponding properties.