Turbine stream not working for clusters discovered by KubernetesInstanceDiscovery

[mlamina]

It's actually two issues, but they are related:

1. If I configure breakerbox to use KubernetesInstanceDiscovery, the YamlInstanceDiscovery still seems to be active. Is there a reason for that? This seems confusing and can lead to errors in the configuration if users just leave out the 'turbine' field in their yaml file.
2. The metrics dashboard only works for those clusters that are specified via yaml. Clusters disovered by KubernetesInstanceDiscovery lead to the following:

"GET /turbine.stream?cluster=staging-auth-facade&delay=1000 HTTP/1.1" 404 (Cluster not found). Am I missing something here?

[chrisgray]

1) https://github.com/yammer/breakerbox/commit/22944efea20d113823a5642246b8ebe2ac6efbe0 2) That is fixed in this commit, I'll cut a release so it's available in 0.4.4 (https://github.com/yammer/breakerbox/commit/044e65cbd3df37506c3447dc8ef5a758e04949f7)

[chrisgray]

https://github.com/yammer/breakerbox/releases/tag/breakerbox-parent-0.4.4

[mlamina]

Awesome! Thank you :)

[mlamina]

So I've tried the new 0.4.4 release and I still get the same error. A curl to /turbine.stream\?cluster\={MY-CLUSTER}\&delay\=1000 gives me this 404 response:

Problem accessing /turbine.stream. Reason: Cluster not found

The cluster shows up in the frontend and configures my services correctly

[chrisgray]

I forgot that we'll need to make a change to KubernetesInstanceDiscovery so that you also register your new clusters!

`TurbineInstanceDiscovery.registerClusters(Collection clusterNames, String pathToStream)

Here you can see an example: https://github.com/yammer/breakerbox/blob/master/breakerbox-turbine/src/main/java/com/yammer/breakerbox/turbine/YamlInstanceDiscovery.java#L42-L43

and also https://github.com/yammer/breakerbox/blob/master/breakerbox-turbine/src/main/java/com/yammer/breakerbox/turbine/TurbineInstanceDiscovery.java

For the pathToStream you can use this value: https://github.com/yammer/breakerbox/blob/master/breakerbox-turbine/src/main/java/com/yammer/breakerbox/turbine/YamlInstanceConfiguration.java#L25

This can probably be refactored a bit so it's just the default

[chrisgray]

Another thing I've been thinking is since Turbines InstanceDiscovery doesn't offer a way to register new clusters in between calls. I might just have a wrapper so that we parse the clusterNames from all the Instance objects that you've just created and returned. That way not every single InstanceDiscovery instance needs to make sure to register new cluster names and potentially forget to.

[mlamina]

I went for the option to register the clusters directly in KubernetesInstanceDiscovery, since I don't feel like I have enough insights into the project to implement a universal cluster registration. If I should make a guess, I believe this would be a good place to do it:

https://github.com/yammer/breakerbox/blob/master/breakerbox-turbine/src/main/java/com/yammer/breakerbox/turbine/ConcatenatingInstanceDiscovery.java

However I am unsure how exactly this class fits into the bigger picture.

[chrisgray]

That sounds good. We can merge it in for now, then I'll take another pass and have InstanceDiscovery always register new cluster names so that you don't have to manually call it in every InstanceDiscovery implementation.