Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor catalogsource controller and entitysource to be implementations of generic interfaces #173

Closed
anik120 opened this issue Apr 18, 2023 · 4 comments
Assignees
Labels
kind/design Categorizes issue or PR as related to design.

Comments

@anik120
Copy link
Contributor

anik120 commented Apr 18, 2023

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.
@joelanford
Copy link
Member

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?

@everettraven
Copy link
Contributor

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

@anik120
Copy link
Contributor Author

anik120 commented Apr 19, 2023

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 anik120 added the kind/design Categorizes issue or PR as related to design. label Apr 19, 2023
@anik120 anik120 self-assigned this Apr 19, 2023
@m1kola
Copy link
Member

m1kola commented Oct 12, 2023

@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?

@ankitathomas ankitathomas closed this as not planned Won't fix, can't repro, duplicate, stale Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/design Categorizes issue or PR as related to design.
Projects
Status: Implementing
Development

Successfully merging a pull request may close this issue.

5 participants