snowdrop / istio-java-api

A Java API to generate Istio descriptors, inspired by Fabric8's kubernetes-model.
Apache License 2.0
112 stars 33 forks source link

Cannot use Istio Client with versioned kubernetes client #7

Closed lordofthejars closed 6 years ago

lordofthejars commented 6 years ago

Currently Istio resources extends from HasMetadata interface. This interface comes from io.fabric8.kubernetes.api.model. The problem is that when you want to use the versioned uberjar kubernetes client, internally the client expects that any resource implements from io.fabric8.kubernetes.api.model.v3_1.HasMetadata so obviously I got some compilation errors.

So for now the only thing I see to fix this problem is to do as kubernetes-client is doing and release one istio-api depending on kubernetes-model and another one istio-api (classsifier versioned) that uses the uber versioned thingy.

Maybe @iocanel can come with another solution, but now this is what it comes to my mind.

iocanel commented 6 years ago

I think that the cleanest approach (if it works) would be to have istio optionally depend on the kubernetes-client and create a shaded jar of its own. The new shaded jar can depend on our f-k-c uberjar and use the same transformations.

I haven't tried something like this before but I think that it will just work.

On Mon, Jan 15, 2018 at 1:42 PM, Alex Soto notifications@github.com wrote:

Currently Istio resources extends from HasMetadata interface. This interface comes from io.fabric8.kubernetes.api.model. The problem is that when you want to use the versioned uberjar kubernetes client, internally the client expects that any resource implements from io.fabric8.kubernetes.api.model.v3_1.HasMetadata so obviously I got some compilation errors.

So for now the only thing I see to fix this problem is to do as kubernetes-client is doing and release one istio-api depending on kubernetes-model and another one istio-api (classsifier versioned) that uses the uber versioned thingy.

Maybe @iocanel https://github.com/iocanel can come with another solution, but now this is what it comes to my mind.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/snowdrop/istio-java-api/issues/7, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYiWNLQ3EBuGKuejyrPry8Lw2lGuN5hks5tKzmhgaJpZM4ReSsh .

-- Ioannis Canellos

Blog: http://iocanel.blogspot.com http://iocanel.blogspot.com/ Twitter: iocanel

metacosm commented 6 years ago

The question, though, is why does the client not rely on the kubernetes-model class and use a different one. The client should rely only on the published API. I think that would be the cleaner approach. What is the reason behind client using that particular class and not the one from the API?

iocanel commented 6 years ago

The client does rely on the model. It doesn't use something else. However, the client also provides an shaded uberjar, that contains versioned packaged (for all its modules and also for the model itself). This allows frameworks like arquillian-cube, use the versioned packages and stay out of the users way (letting the user use whatever version of the kubernetes-client fits their needs without conflicting with arquillian). I'll coin a new term for this approach. How about "soft modularity"? :-D

Joking aside, this does solve real problems that have been raised in the past from the RHOAR team about conflicts (e.g using arquillian-cube and spring-cloud-kubernetes etc), and its also what allows us today to use arquillian-cube to test the kubernetes-client itself.

On Mon, Jan 15, 2018 at 2:31 PM, Chris Laprun notifications@github.com wrote:

The question, though, is why does the client not rely on the kubernetes-model class and use a different one. The client should rely only on the published API. I think that would be the cleaner approach. What is the reason behind client using that particular class and not the one from the API?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/snowdrop/istio-java-api/issues/7#issuecomment-357670204, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYiWICfwPl9TG0RbXAtDgayCXKqWqwHks5tK0UjgaJpZM4ReSsh .

-- Ioannis Canellos

Blog: http://iocanel.blogspot.com http://iocanel.blogspot.com/ Twitter: iocanel

lordofthejars commented 6 years ago

Yeah @iocanel has explained very well, because of lack of classpath isolations we need to do this hacky thing.

metacosm commented 6 years ago

@lordofthejars how can I reproduce the issue you're seeing? Do you have a repo somewhere that I can clone to try it out?

lordofthejars commented 6 years ago

I have no repo but it is as easier as create an empty project, add this library:

<dependency>
  <groupId>io.fabric8</groupId>
  <artifactId>kubernetes-openshift-uberjar</artifactId>
  <version>3.1.8</version>
  <classifier>versioned</classifier>
</dependency>

And then try to call client.customResources(customResourceDefinition, IstioResource.class, KubernetesResourceList.class, DoneableIstioResource.class).create(resource);

were client is of type io.fabric8.kubernetes.clnt.v3_1.KubernetesClient

metacosm commented 6 years ago

Actually, using IstioClient won't even compile since the Adapter requires a "regular" KubernetesClient… so you'd like a completely shaded dependency, right?

lordofthejars commented 6 years ago

Exactly this is the problem. You can create your own Adapter with shaded client but you still have the problem that istio model depends on HasMetadata and not V3_1.HasMetadata

metacosm commented 6 years ago

Can't seem to get it to work… :(

lordofthejars commented 6 years ago

Yeah because what we need is an Istio model depending on versioned model.

metacosm commented 6 years ago

That was what I couldn't get to work but I think I cracked it (shaded jar newbie here) :)

metacosm commented 6 years ago

Deployed a 0.6-SNAPSHOT. Can you give it a try @lordofthejars? Would you mind reviewing @iocanel, please, when you get a chance?

lordofthejars commented 6 years ago

Downloading now :)

lordofthejars commented 6 years ago

Still the same. Look the imports are the next ones:

import io.fabric8.kubernetes.api.model.v3_1.KubernetesResourceList;
import io.fabric8.kubernetes.api.model.v3_1.apiextensions.CustomResourceDefinition;
import io.fabric8.kubernetes.clnt.v3_1.KubernetesClient;
import me.snowdrop.istio.api.model.DoneableIstioResource;
import me.snowdrop.istio.api.model.IstioResource;

Since IstioResource implements HasMetadata and not io.fabric8.kubernetes.api.model.v3_1.HasMetadata we still have the same compilation errors. It is not about creating a uber jar but create a jar that is creating the IstioResources implementing io.fabric8.kubernetes.api.model.v3_1.HasMetadata._

metacosm commented 6 years ago

Hmm, look at https://github.com/metacosm/istio-test-dsl/blob/with-uberjar/src/main/java/IstioYamlGenerator.java. It uses both uberjars and it successfully creates RouteRules. In istio-client-uberjar, IstioResource does implement v3_1.HasMetadata… Can you please double-check? By the way, istio-client-uberjar does not relocate the istio-model classes, it just makes use of the relocated kubernetes ones.

lordofthejars commented 6 years ago

humm maybe I got some cached thing, let me try again :)

lordofthejars commented 6 years ago

Great after deleting m2repo deps of istio-api, everything is back to normal hehehe and it works as expected. Thanks :)

metacosm commented 6 years ago

Good to hear! That's often an issue building against snapshots. Will release a new version now that this problem is addressed.