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

[WiP] Adds traffic route rules appliers #4

Closed lordofthejars closed 6 years ago

lordofthejars commented 6 years ago

I have created the first part of the PR which implies creating a class to apply istio resources to the cluster.

Currently, I have only implemented the RouteRoules but if we agree with everything I'll implement for the rest.

I'd like to discuss is the names of the classes (I am really bad naming classes) and the Adapter interface. I have created this interface for next reason: There are two different kubernetes client (the f8 kubernates client and the f8 kubernetes client uberjar versioned) so to give support to both possibilities I needed to create this abstraction. This also applies if someday we need to apply some specific operations for OpenShift client or other kubernetes client that are out there.

metacosm commented 6 years ago

I have taken your code and made some modifications which I think make it a little simpler (no need to create a new Applier implementation per resource). I pushed it at https://github.com/snowdrop/istio-java-api/tree/applier_module, let me know what you think. The tests are currently failing, it seems like the mock isn't triggered but I'm not sure why.

lordofthejars commented 6 years ago

I'll take a look but they were green before. I'll pull tomorrow and I will continue working.

Alex

El 10 ene. 2018 8:25 p. m., "Chris Laprun" notifications@github.com escribió:

I have taken your code and made some modifications which I think make it a little simpler (no need to create a new Applier implementation per resource). I pushed it at https://github.com/snowdrop/ istio-java-api/tree/applier_module, let me know what you think. The tests are currently failing, it seems like the mock isn't triggered but I'm not sure why.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/snowdrop/istio-java-api/pull/4#issuecomment-356709030, or mute the thread https://github.com/notifications/unsubscribe-auth/ABcmYfcq7o9xwsg4UwcOxL_aLAOhu3hSks5tJQ5pgaJpZM4RZXPY .

metacosm commented 6 years ago

I suspect it has something to do with the mocking set-up as stepping into the code shows the non-mocked methods behave as expected but the mock doesn't.

metacosm commented 6 years ago

Tests are fixed, I had a logic error in them.

metacosm commented 6 years ago

@lordofthejars Did you get a chance to look at the new version?

lordofthejars commented 6 years ago

I am waiting for the change you mentioned me in twitter, but I couldn't see any new commit to master.

metacosm commented 6 years ago

I have released istio-java-api 0.4 which enables you to create quota and such. It has a slight API change due to the issue of the missing spec element which should now be fixed. I'm leaning towards merging the applier_module branch soon…

lordofthejars commented 6 years ago

On Monday I'll work on that to have the new elements on place, and then we can merge the PR.

metacosm commented 6 years ago

You'll see that the new code doesn't really require much to add the new elements (and probably still can be improved): it should only be a matter of adding an entry in a map… see in particular: https://github.com/snowdrop/istio-java-api/blob/applier_module/istio-applier/src/main/java/me/snowdrop/istio/applier/IstioExecutor.java#L33

metacosm commented 6 years ago

Merged branch into master. I renamed the module to istio-client along with package name and IstioExecutor -> IstioClient since I think it's more explicit and other features might be added to interact with the Istio runtime at some point. Thank you again!

lordofthejars commented 6 years ago

Nice!!! Thank you very much. I see that it has been released, this means that I can start the integration with traffic rules in Arquillian Cube.