operator-controller icon indicating copy to clipboard operation
operator-controller copied to clipboard

Refactor catalogsource controller and entitysource to be implementations of generic interfaces

Open anik120 opened this issue 2 years ago • 4 comments

Motivation:

Operator Catalog content from index images are converted into Entities (and variables) for deppy. However, right now the information needed for creation of Entities is fetched from a single, hardcoded source (OLM v0 grpc pods provided by v0 CatalogSources). The entitysource catalogsource is an implementation of an interface named GrpcClient, which makes the assumption that an entity source can only be a grpc client.

Also, there is a controller called catalogsource_controller, whose purpose is to build and maintain a cache of Entities for feeding to deppy, but it too makes the assumption that a CatalogSource will always be the sole provided of information needed to build Entities.

Goals:

Refactor to pave the way for different entity sources (eg OLM v1 catalogd, I.e pave the way for #161)

  • Since the catalogsource_controller is essentially a cache builder (and not a controller for catalogsource CRs), the catalogsource_controller should be refactored to be an entity_cache_builder
  • EntitySources should be refactored such that entitySourceConnectors are implementations of a EntitySourceConnector, so that the entity_cache_builder can use any of the EntitySourceConnector to connect to an entity source and fetch information from, in order to build the cache.

anik120 avatar Apr 18 '23 21:04 anik120

Hmm. Looking at main.go, I see us creating a catalog source reconciler, and then using that as an input.EntitySource when creating a new resolver.

And input.EntitySource looks like this: https://github.com/operator-framework/deppy/blob/dc02e928470f892984255923c1cc449fe4a574e8/pkg/deppy/input/entity_source.go#L26-L31

As a first pass, I think we should focus on implementing the EntitySource functions backed by a client that fetches the Package and BundleMetadata APIs. I'm not sure we need all of the controller structure around it at this point. WDYT?

joelanford avatar Apr 18 '23 21:04 joelanford

Hmm. Looking at main.go, I see us creating a catalog source reconciler, and then using that as an input.EntitySource when creating a new resolver.

And input.EntitySource looks like this: https://github.com/operator-framework/deppy/blob/dc02e928470f892984255923c1cc449fe4a574e8/pkg/deppy/input/entity_source.go#L26-L31

Thanks for the additional info @joelanford ! Helped me when looking through this understand a bit more about how the current OLMv0 integration is working.

So now my understanding is that to get catalogd integrated, we need to:

In the future we can take out all the OLMv0 integration and leave only the catalogd integration

everettraven avatar Apr 19 '23 10:04 everettraven

As a first pass, I think we should focus on implementing the EntitySource functions backed by a client that fetches the Package and BundleMetadata APIs. I'm not sure we need all of the controller structure around it at this point. WDYT?

That'll most certainly be the easiest thing to do. Keeping all the of the controller structure etc, ie having to maintain both v0 and v1 integration does introduce additional complexity.

Eg I didn't even get into details like v0 CatalogSources are namespace scoped but catalogd CatalogSources are cluster scoped, so what does that mean for the higher level interface EntitySourceConnector? etc

Last we synced, we were still assessing the business value of keeping the v0 integration, and being cautious about making a decision about the v0 integration one way or the other. That was also 2 milestones ago. Do we feel like we're in a better spot today to decide about the v0 integration, keeping in mind the overall OLM v1 goals, and how it fits into those goals? cc: @ankitathomas @varshaprasad96

tldr of where I stand : from engineering's perspective, it'll be much cleaner and easier to drive towards OLM v1 goals with one integration, than maintaining two integrations. However, if business needs dictates that we need to maintain both integrations, then it's not going to be impossible (as shown by the PR that lays the foundation work for that)

anik120 avatar Apr 19 '23 13:04 anik120

@anik120 do you think this one is still required? Most of the stuff referenced in this issue doesn't seem to exist anymore.

Can we close this?

m1kola avatar Oct 12 '23 15:10 m1kola