re #149 support event streaming

[bsideup]

I'm happy to contribute this small, but very helpful improvement - event stream listener.

Starting from Marathon 0.9.0 it provides /v2/events endpoint, which is implementation of Server-sent Events. It's much better compared to event callbacks since there is no need to register Bamboo as callback on Marathon.

Implementation is backward compatible with 0.7.0 because this mode is disabled by default. It could be enabled in config like that:

{
  "Marathon": {
    "Endpoint": "...",
    "UseEventStream": true
  }
}

NB: if "UseEventStream" is enabled then callbacks are not auto-registered.

When Bamboo will drop support for Marathon < 0.9.0 then this property could be true by default.

Implementation was tested with Marathon 0.9.0, covered cases:

1. Listening for events
2. Re-connection when Marathon is not responding
3. Bad data from Marathon
4. Many events at a time

[lclarkmichalek]

Looks good for the most part, I've added a couple of comments. On a broader note, I'd love to see some of this broken out into smaller functions; a function to parse a single message and return an event would be great, as it'd allow for very easy testing. At the moment, this is a rather had function to test.

[bsideup]

@bluepeppers I've made some changes, including:

1. Reuse code from EventSubscriptionAPI for parsing and dispatching events
2. Use Ticker for retrying
3. Log every error

Currently listenToEventStream contains only SSE handling logic. Should I split it more?

[lclarkmichalek]

I like this. I'd like to split it up more, and break the whole thing out behind an interface, but this is a move in the right direction. Seeing as this is opt in, I'd be happy merging it now, but will probably wait till Monday, and confer with @activars about the best way to test this mode with some decent traffic.

[bsideup]

@bluepeppers ok, lets wait what @activars will say :)

[j1n6]

Looks good :+1:
We will test this branch on our dev cluster.

[j1n6]

don't worry about the abstraction too much, we will refactor some common features in next release.

[drewrobb]

docker build with this was broken for me-- use of the http.Client timeout requires go 1.4, the dockerfile uses go 1.2

[drewrobb]

You also may want to wait until the fix for https://github.com/mesosphere/marathon/issues/1926 is included in a marathon release (probably 0.11). Otherwise there could in some cases be a delay for bamboo to receive events.

[lclarkmichalek]

@bsideup hey, sorry we haven't done anything on this. I've finally automated some bamboo integration testing stuff, and so I'll give this a shot on the staging cluster in the next week.

[bsideup]

Hi @bluepeppers ! Sounds great :) Ping me if anything will not work for you :)

[lclarkmichalek]

One minor thing; we have a username and password for marathon configurable: https://github.com/QubitProducts/bamboo/blob/master/configuration/marathon.go#L10-L15. The event stream request should probably support HTTP basic auth also

[bsideup]

@bluepeppers it was introduced after this PR :) But I can support it if you want

[bsideup]

@drewrobb good catch. Looks like ubuntu base image comes with Go 1.2 by default. Any idea how to fix it?

[bsideup]

@bluepeppers rebased on top of last master

[bsideup]

@bluepeppers implemented basic auth

[lclarkmichalek]

The go vet message at https://travis-ci.org/QubitProducts/bamboo/builds/80229954 was very useful. Without that change, the goroutine was listening to the same (last) marathon always, which causes the problem @drewrobb linked with marathon to be much more of an issue. I was having problems getting event streaming to work with this patch, but I think that go vet may have found the problem.

[bsideup]

@bluepeppers yes, already fixed :)

[bsideup]

@bluepeppers any news?

[drewrobb]

+1 to getting this in, I have been using it for a while now successfully

[lavcraft]

+1

[j1n6]

Appologize we haven't got chance to test this out because there are a lot of other tasks we need to work on internally. If anyone has time to test it out & sign-off on the status (with 3 thumbs up), we can speed up the release process.

We will deploy this on an internal test cluster to test it out in November.

[bsideup]

Since this is an optional mode I think it's pretty safe to merge, I know at least 2 production deployments running my fork so it's battle tested:) On Tue, 27 Oct 2015 at 6:59 PM Jing Dong notifications@github.com wrote:

Appologize we haven't got chance to test this out because there are a lot of other tasks we need to work on internally. If anyone has time to test it out & sign-off on the status (with 3 thumbs up), we can speed up the release process.

We will deploy this on an internal test cluster to test it out in November.

— Reply to this email directly or view it on GitHub https://github.com/QubitProducts/bamboo/pull/151#issuecomment-151569035.

[lclarkmichalek]

I'll merge this today (absent any issues). Sorry it took so long

[bsideup]

Awesome, thanks! On Thu, 12 Nov 2015 at 8:58 PM Laurie Clark-Michalek < notifications@github.com> wrote:

Merged #151 https://github.com/QubitProducts/bamboo/pull/151.

— Reply to this email directly or view it on GitHub https://github.com/QubitProducts/bamboo/pull/151#event-462793867.

[lavcraft]

Yeah, thanks guys!

[bsideup]

@bluepeppers was it released btw?

[lclarkmichalek]

No. The next release will probably be 0.3, with this and #180 being the features I want to test the most. I'm running master + #180 in dev atm, though there's still some extra work to be done around the UI for #180.

[bsideup]

@bluepeppers ok, keep me in touch then :) Thanks!