Skip to content

Possibility to create DependentResources via custom Factory #1855

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
nsoubelet opened this issue Apr 12, 2023 · 10 comments · Fixed by #2175
Closed

Possibility to create DependentResources via custom Factory #1855

nsoubelet opened this issue Apr 12, 2023 · 10 comments · Fixed by #2175
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@nsoubelet
Copy link

nsoubelet commented Apr 12, 2023

Hello,
I have a pretty specific use case for which I would like to have a KubernetesDependentResource with a constructor over which I need to pass certain parameters.
I noticed there is a DependentResourceFactory interface but seems to be used internally and not meant to be specified by implementors (Controllers).

This is what I need to achieve:

public class MyDeleterBulkPods
        extends
        KubernetesDependentResource<Pod, MyResource>
        implements
        Creator<Pod, MyResource>,
        Updater<Pod, MyResource>,
        Deleter<MyResource>,
        BulkDependentResource<Pod, MyResource> {

     public MyDeleterBulkPods(param1, param2....) {
        super(Pod.class);
        ... internal setup
    }

}

public class CRUDPodsBulkDependentResource extends MyDeleterBulkPods
    implements GarbageCollected<MyResource> {

    public CRUDPodsBulkDependentResource(param1, param2....) {
        super(param1, param2....);
        ... internal setup
    }

}

>> Controller has...
@ControllerConfiguration(
        dependents = @Dependent(type = CRUDPodsBulkDependentResource.class))

I think that implementation should not be difficult, a possible flexible way, which may be the base for bigger features like injection in Spring or any other Jakarta-CDI-based framework like Quarkus, is by recognizing dynamically whether a @Dependent type is an actual DependentResource or a delegate (factory).
In that approach, we could decide in method instantiateAndConfigureIfNeeded whether create the instance locally or delegate it.

Could be something like:

@ControllerConfiguration(        
        dependents = @Dependent( type = DR.class, strategy = Strategies.DELEGATE ), @Dependent( type = DR2.class, strategy = Strategies.DELEGATE ))
public class MyController
        implements
        DependentResourcesFactory,
        Reconciler<MyCustomResource>,
        Cleaner<MyCustomResource>,
        ResourceEventAware<MyCustomResource> {

...

	DependentResource createFrom(DependentResourceSpec spec, C configuration) {
                ... logic for determining what type we need to create
                if (spec.getDependentResourceClass() ...);
		  return new DR(param1, param2,....)
               ... etc
	}

}

Hope I made myself clear, if not please comment!

Thanks in advanced

@csviri
Copy link
Collaborator

csviri commented Apr 13, 2023

Hi @nsoubelet ,

after our initial discussion, was thinking that, this should be actually possible with custom configurations, this is not a very good documented feature, will add docs and sample integration test. But it should be possible as shown here:

Define a custom configuration for the dependent resource:

https://github.com/java-operator-sdk/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java#L412-L412

You can override the config values as shown here:

https://github.com/java-operator-sdk/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java#L412-L412

Is this good enough for you?

@csviri csviri self-assigned this Apr 13, 2023
@nsoubelet
Copy link
Author

Hi @csviri
Thanks for your comments. I saw that option when exploring the code, but did not get entirely the idea of how it works to be honest, however, as you suggested, I'll give a try.

In the meantime, what I did is what you suggested yesterday which is use the standalone approach and call DR's reconcile during controller's reconciliation, as shown in StandaloneBulkDependentReconciler sample.

What I suggested in the ticket is having different strategies for DR instantiation, which could be the base for having delegates that look up within a context, for example. I guess that could be easier to read and follow the code and to adapt to different frameworks.

Btw, nice lib guys! I love it, great work :)

@csviri
Copy link
Collaborator

csviri commented Apr 13, 2023

What I suggested in the ticket is having different strategies for DR instantiation, which could be the base for having delegates that look up within a context, for example. I guess that could be easier to read and follow the code and to adapt to different frameworks.

Yes this is definetelly we should look into it.

thank you!

cc @metacosm

@csviri csviri removed their assignment Apr 19, 2023
@metacosm
Copy link
Collaborator

metacosm commented Apr 21, 2023

I'm not sure about all that was discussed but the Quarkus extension already makes use of the DependentResourceFactory interface by providing a tailored implementation that actually looks up the dependent resource implementations among available CDI beans (in the Quarkus extension, all dependent resources are CDI beans). As you justly mentioned, this interface is not really meant for external consumption and replacing the implementation provided by the ConfigurationService that the framework flavour is using might cause issues: for example, using a different factory in Quarkus would probably break the integration.

@nsoubelet
Copy link
Author

Hi @metacosm, I was not aware.
In this particular case I'm using SpringBoot, but I will take a look to see how it is implemented for Quarkus, concept and strategy should be the same.

@metacosm
Copy link
Collaborator

That said, it might be beneficial to maybe clean this up a little bit and make it part of the documented API since this should indeed help for framework integration.

@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Jun 21, 2023
@openshift-ci-robot openshift-ci-robot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed stale labels Jun 28, 2023
@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Aug 28, 2023
@openshift-ci openshift-ci bot removed the stale label Aug 30, 2023
@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Oct 29, 2023
@openshift-ci openshift-ci bot removed the stale label Nov 8, 2023
Copy link

github-actions bot commented Jan 8, 2024

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
4 participants