[WiP] Adds traffic route rules appliers #4
Conversation
|
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. |
|
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
<#4 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABcmYfcq7o9xwsg4UwcOxL_aLAOhu3hSks5tJQ5pgaJpZM4RZXPY>
.
|
|
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. |
|
Tests are fixed, I had a logic error in them. |
|
@lordofthejars Did you get a chance to look at the new version? |
|
I am waiting for the change you mentioned me in twitter, but I couldn't see any new commit to master. |
|
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… |
|
On Monday I'll work on that to have the new elements on place, and then we can merge the PR. |
|
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 |
|
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! |
|
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. |
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
Adapterinterface. 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.