fabric8io / kubernetes-client

Java client for Kubernetes & OpenShift
http://fabric8.io
Apache License 2.0
3.41k stars 1.46k forks source link

CustomResource should move to the kubernetes-model #2829

Open iocanel opened 3 years ago

iocanel commented 3 years ago

A user should be able to use CustomResource class without using the whole client. So, I think that the CustomResource should move in one of the model artifacts itself.

metacosm commented 3 years ago

This is something I've been thinking about as well, even started looking into it. This will be a major change, though and if we're going to do it, we should take the opportunity to maybe clean up CR handling as a whole, for example, removing the Namespaced interface, making CRs namespace-scoped by default and maybe using an annotation to signify scope instead of using a marker interface for that purpose?

iocanel commented 3 years ago

Agree! It is a major change.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

shawkins commented 2 years ago

For fabric8 6 CustomResource won't even be required in kubernetes-model - all of the entry points that dealt with it have been deprecated and can be removed.

metacosm commented 2 years ago

Yea but the idea here is to be able to use what's in CustomResource without having to import the full client dependencies. That said, it might make sense to completely remove the CustomResource class.

shawkins commented 2 years ago

@metacosm yes, I'd like to consider that more broadly #3645

manusa commented 2 years ago

This is now in the kubernetes-client-api module. Do you think this still needs to be moved to the kubernetes-model module? @shawkins @metacosm @iocanel

metacosm commented 2 years ago

As long as we don't need to import the full client library to use what's in CustomResource, I'm fine with it.

manusa commented 2 years ago

Well, now you'd need to import the kubernetes-client-api library, which I'm not sure fulfills your requirements or not.

The problem with moving it to the kubernetes-model-code module is that we need to move its package which might bother users (especially those of the OperatorSDK). So I think you folks should make the decision on this one.

manusa commented 2 years ago

As discussed during our triage meeting, CustomResource was already moved to the kuberentes-client-api module which satisfies one of the main concerns about pulling in the full client when all you need is a dependency to this class.

Since the CustomResource class seems to be completely redundant and (without further analysis) only brings in the spec and status fields (to enforce/encourage good practices), we should evaluate if we might want to completely remove this class.

Alternatives would be to add specific interfaces such as Specable, Statusable, and so on, that could be used to still encourage those good practices while dealing with custom resouces.

Steps would be:

rohanKanojia commented 2 years ago

@manusa : Why is this reopened?

manusa commented 2 years ago

https://github.com/fabric8io/kubernetes-client/issues/2829#issuecomment-1197676790 Needs to be addressed.

This issue is open in the interim while creating a proper issue to address what was agreed upon.

https://github.com/fabric8io/kubernetes-client/issues/3816

metacosm commented 7 months ago

Note that if the class is removed, it would still be useful to have a marker interface for easy discovery of Custom Resources (at least on first analysis, as the Quarkus extension for the Java Operator SDK relies on being able to identify Custom Resources for specific processing, such as CRD generation).

manusa commented 7 months ago

public interface CustomResource extends HasMetadata, HasSpec, HasStatus?

metacosm commented 7 months ago

public interface CustomResource extends HasMetadata, HasSpec, HasStatus?

Minimally, that interface should at least extend HasMetadata, for sure. Implementing the other two would take a stronger stance on best practices that we've been doing so far, i.e. require a custom resource to have a spec and status, imo.

csviri commented 7 months ago

We would need some adaptor/base class, the metadata is always going to be handled the same way since the contents of it never change.

I guess HasStatus and HasSpec need to be parameterized classes.