Catalogd integration foundations #175
Conversation
…ic interfaces This commit: * Replaces the RegistryClient interface with a EntitySourceConnector interface * Renames the grpcGrpcClient to grpcClientConnector, so that it can be indicated that the grpcClientConnector is an implentation of EntitySourceConnector(i.e lays the ground work for future connector implemenation) * Renames the catalogSource_controller to entitysource_reconciler, to lay the ground work for entities being built from sources other than olm v0 catalogsources. tldr of what components looks like after changes: entitysource_reconciler uses EntitySourceConnector to connect to an entity source, and build a cache. Signed-off-by: Anik <[email protected]>
…uilder This commit: * Pulls the cache into it's own package, that the entitysource_reconciler(entity_cache_builder) then imports as an attribute (previously, the EntitySourceReconciler was implementing the input.Entity interface that requires a Get implementation, which was overriding the Get method of the controller manager. This was an additional indication that the cache build and maintainace should be in it's own package) * Renames the entitysource_reconciler to be entity_cache_builder tldr of what componenets look like after changes: entity_cache_builder uses an EntitySourceConnector to build and maintain a cache of entities Signed-off-by: Anik <[email protected]>
anik120
force-pushed
the
catalogd-integration-foundations
branch
from
a38275d to
baf71dc
Compare
| } | ||
|
|
||
| // TODO: find better way to identify catalogSources unmanaged by olm | ||
| // func isManagedCatalogSource(catalogSource v1alpha1.CatalogSource) bool { |
There was a problem hiding this comment.
I need more information about what this is for. Since an entity_cache_builder shouldn't make any assumptions about what the nature of the entity is (catalogsource or otherwise), I've commented it out now, but once I've learned the purpose of this I can find a better place for this (possibly the catalogsource entitySourceConnector).
There was a problem hiding this comment.
I think this was a remnant of the idea that operator-controller would:
- not need OLMv0's catalog-operator to actually reconcile
CatalogSourceobjects - (therefore) the only catalog sources that would be usable in this context are ones where the spec tells us the address to connect to.
In the end, we decided to just run OLMv0 so that we could use any catalog source.
| ) | ||
|
|
||
| type EntitySourceConnector interface { | ||
| ListEntities(ctx context.Context, catsrc *v1alpha1.CatalogSource) ([]*input.Entity, error) |
There was a problem hiding this comment.
I think I'm missing something here. This still seems specific to OLMv0 catalog sources. Based on the PR description, it seemed like the goal was to remove the assumptions about the underlying API type.
There was a problem hiding this comment.
My understanding of this PR is that it is trying to generalize the old OLMv0 CatalogSource controller into one that can be used for a different entity source as long as that entity source meets the EntitySourceConnector interface.
After reading #173 (comment) and looking through the code a bit more I think this isn't necessary and is probably introducing too much complexity in the beginning. As mentioned in the comment, it looks like the Operator controller expects to have a resolver that satisfies the deppy input.EntitySource interface so I think all we need for integrating catalogd is:
- Create a new entity source for catalogd
- For now lets not worry about caching anything. Lets try to keep it barebones and just make a request for the resources we need every time.
- Replace:
Line 108 in ca22669
with something like:
Resolver: resolution.NewOperatorResolver(mgr.GetClient(), catalogdEntitySrc)
|
@anik120 should this be closed? |
openshift-merge-robot
added
the
needs-rebase
|
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/test-infra repository. |
|
Pretty sure this is obsolete now, so closing. |
This PR:
EntitySourceConnectorinterface and refactors the current catalogsource entitysource to be an implementation ofEntitySourceConnectorcatalogsource_controllerto be aentity_cache_builder, and refactors it such that it connects to an entity source using anEntitySourceConnector, and builds and maintains the cache using information fetch from the entity source.closes #173