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

Kubernetes clients all clash with one another #14943

Closed
FroMage opened this issue Feb 9, 2021 · 11 comments · Fixed by #14946
Closed

Kubernetes clients all clash with one another #14943

FroMage opened this issue Feb 9, 2021 · 11 comments · Fixed by #14946
Labels
area/kubernetes kind/bug Something isn't working
Milestone

Comments

@FroMage
Copy link
Member

FroMage commented Feb 9, 2021

When importing the quarkus-openshift-client extension, we get an injectable OpenShiftClient which also extends KubernetesClient which is located in the transitive dependency quarkus-kubernetes-client, we get a CDI clash on any KubernetesClient injection points, because we now have two clients which implement it.

We could fix this by disabling the producer of KubernetesClient if the OpenShiftClient producer exists, but this won't scale for the other types of clients (there are 3 subtypes of KubernetesClient from 3 separate extensions that can all be imported).

So I think the best is to restrict the produced type of OpenShiftClient so that it does not clash, and users can inject either type of client separately.

@FroMage FroMage added the kind/bug Something isn't working label Feb 9, 2021
@ghost ghost added the area/kubernetes label Feb 9, 2021
@ghost
Copy link

ghost commented Feb 9, 2021

/cc @geoand

@geoand
Copy link
Contributor

geoand commented Feb 9, 2021

I kinda doubt anyone would want all types of clients around TBH.
I think we can just enforce that when the openshift-client extensions exists, the openshift-client should be used.

@FroMage
Copy link
Member Author

FroMage commented Feb 9, 2021

If you prefer, I can do it that way too, they should be interoperable and we can always switch behaviour later if required.

@geoand
Copy link
Contributor

geoand commented Feb 9, 2021

Yeah, I think that would be the best first step

@metacosm
Copy link
Contributor

metacosm commented Feb 9, 2021

I kinda doubt anyone would want all types of clients around TBH.
I think we can just enforce that when the openshift-client extensions exists, the openshift-client should be used.

That makes sense to me as well.

@metacosm
Copy link
Contributor

That said, I'm wondering if it would be possible to somehow select the appropriate extension at build time so that people who don't target OpenShift only get the added dependencies if they need them…
The use case here is the operator sdk extension which needs to get a client but won't know which flavor of the platform users will target. At the moment the most appropriate option would be to depend on the OpenShift extension but that means carrying the extra dependencies even for folks who don't need them…

@FroMage
Copy link
Member Author

FroMage commented Feb 10, 2021

Quarkus doesn't auto-select extensions for users, so we can't do that. But the operator-sdk will get the right client no matter what extension the user uses now.

@metacosm
Copy link
Contributor

The problem is that the sdk extension is dependent on the client extension. Which means that it needs to include the OpenShift extension and carry all the extra dependencies even when users don't target OpenShift.

@FroMage
Copy link
Member Author

FroMage commented Feb 10, 2021

Can't you leave it up to the sdk users to include that extension?

Oh, perhaps you need some actions to call only if it's an openshift client?

@FroMage
Copy link
Member Author

FroMage commented Feb 10, 2021

If that's the case, I think you can make the dependency optional in the SDK, and it will only realise if the user depends on it too.

@ghost ghost added this to the 1.12 - master milestone Feb 10, 2021
@metacosm
Copy link
Contributor

I need to look into it but the sdk extension expects to be getting a client, though maybe decoupling both might be a cleaner solution… Not sure at this point. The idea of depending on the kubernetes-client-extension was so that we could expose a different configuration interface to the client, one that would be specific to the sdk and not expose Quarkus-specific properties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kubernetes kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants