search

fabric8io / kubeflix

Kubernetes integration with Netflix OSS
266 stars 69 forks source link

specifying the hystrix stream url #71

Closed raffaelespazzoli closed 8 years ago

raffaelespazzoli commented 8 years ago

hi,

there should be a way to specify where the hystrix stream can be found for the observed pods. Not all webapps will have the hystrix stream at /hystrix.stream For example many webapps will have a context root before that.

I started a similar project to what you're doing here and stopped when I saw that you guys where going to provide "official" support. I had solved this problem, you may want to take a look: https://github.com/raffaelespazzoli/hystrix-kuberneters-instance-discovery

thanks Raffaele

iocanel commented 8 years ago

Unfortunately, turbine's discovery mechanism stops to host and port and doesn't include path or something like this. So this pretty much rules out supporting pods with different paths.

So we are forced to use a path that will be global. But yes we need this path to be configurable.

I noticed that you are using a config.properties to specify that prefix, which is fine, but I think that we can further improve that.

Possible alternatives:

i) using configmap. ii) using env vars.

The configmap / archaius integration have been proposed by @christian-posta for other uses cases too and I think its an amazing idea. Here could be a really nice fit.

So we could start by implementing (ii) and when we have the integration ready move to (i).

@raffaelespazzoli: its great to see that "great minds think alike". If you want to take a stub at either (i) or (ii) please do. If you don't have the cycles we'll try to add it asap.

raffaelespazzoli commented 8 years ago

@iocanel I actually overrode also the turbine initializer so to read the path from an environment variable that is passed to the turbine pod (actually implementing you second suggestion). I realized I hadn't pushed that piece, you can find the code here

here is how it would work in the turbine pod:

        env:
        - name: turbineInstanceUrlSuffix
          value: :8080/mbl/hystrix.stream
iocanel commented 8 years ago

@raffaelespazzoli: that's cool do u want to create a PR?

raffaelespazzoli commented 8 years ago

I'm working on it.

iocanel commented 8 years ago

@raffaelespazzoli: Took things one step further and added configmap support too.

Also, migrated the whole turbine server to use spring boot and spring cloud, so we now get for free env var mapping for all turbine properties.

Thanks again for the contribution