Skip to content

Decoupling Event Sources and Dependent Resource and Related Issues #1379

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

Closed
2 tasks
csviri opened this issue Jul 29, 2022 · 0 comments · Fixed by #1378
Closed
2 tasks

Decoupling Event Sources and Dependent Resource and Related Issues #1379

csviri opened this issue Jul 29, 2022 · 0 comments · Fixed by #1378
Assignees
Labels
architecture kind/feature Categorizes issue or PR as related to a new feature.
Milestone

Comments

@csviri
Copy link
Collaborator

csviri commented Jul 29, 2022

There are couple of issues that needs to be solved that are related to each other. This is an umbrella issue that tries to explain the problems, relationship between them, and set the goals and solution.

1. Decoupling Caches from Event Sources in API

Event Sources propagate the caches (both for informers and external resources), this is essential in a controller. Accessing the caches is now done through context.getSecondaryResource. However that we actually have a cache not on the event source and not some special additional own implementation of caching (what might be a case in the future), this API should not expose these internal. So we should remove the overloaded version, where the event source is referenced: #1240

2. Event Sources and Dependent Resources decoupling

Dependent resources now read the resource from the event source directly. This again makes an unnecessary coupling of the functionality provided by dependent resources with with event source. Would be much cleaner to access the cache just with the "standard" way, so with context.getSecondaryResource.

This would also help with the mocking and unit testing: #1355
Since users will be able to mock the context and do unit testing with dependent resources much easier.

Unfortunately there is one place where the dependent resource uses event source (so needs a direct reference to it now), is when filtering event propagation, so on our own changes we don't trigger reconciliation:

Might be bette to abstract away this in the future:
https://github.com/java-operator-sdk/java-operator-sdk/blob/9028970f278316c3236efd70819dae2dcddce576/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java#L230-L230

3. Handling Multiple Dependent Resources

see also: #1175

Currently without explicit labeling dependent resource are not able to handle multiple resources of the same type.
For this in the related PR, but also partially to nicer access caches (point 1.) the related PR introduces ResourceDiscriminator,
basically to filter target resources.

https://github.com/java-operator-sdk/java-operator-sdk/blob/9028970f278316c3236efd70819dae2dcddce576/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java#L9-L14

Note that the goal here is that using this construct both int the Context and through it also the dependent resources, the users are able to select elegantly resources from caches. With this having multiple resources for the same resource type.
See samples:
https://github.com/java-operator-sdk/java-operator-sdk/blob/9028970f278316c3236efd70819dae2dcddce576/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java

However now it provides access to low level API's for effective access of indexes, in event source, see sample:

https://github.com/java-operator-sdk/java-operator-sdk/blob/9028970f278316c3236efd70819dae2dcddce576/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/indexdiscriminator/IndexDiscriminator.java

TODO / Discuss

  • have a nicer api for discriminator to access only indexes? So a generic indexing API ?
  • now the configuration of standalone and managed event sources it little messy, discuss design that
@csviri csviri self-assigned this Jul 29, 2022
@csviri csviri added kind/feature Categorizes issue or PR as related to a new feature. architecture labels Jul 29, 2022
@csviri csviri linked a pull request Jul 29, 2022 that will close this issue
@csviri csviri added this to the 3.2 milestone Jul 29, 2022
@csviri csviri modified the milestones: 3.2, 3.3 Aug 30, 2022
@csviri csviri closed this as completed Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant