Optionally fetch Marathon endpoint from ZK #183
Conversation
| go func() { | ||
| for _ = range ticker.C { | ||
| ticker := time.NewTicker(1 * time.Second) | ||
| go func() { |
There was a problem hiding this comment.
Since the endpoints may change, I needed to allow this loop to call conf.Marathon.Endpoints() again after all endpoints have had failures. The prior code also would connect to all valid supplied endpoints. It is my understanding that this is not necessary-- with my change only one endpoint is connected at a time.
There was a problem hiding this comment.
LGTM. Though I'd be fine with making a larger change to handle errors explicitly (if conf.Marathon.Endpoints() does return nil here, I think it panics?)
| if m.UseZookeeper { | ||
| endpoints, err := zkEndpoints(m.Zookeeper) | ||
| if err != nil { | ||
| return nil |
There was a problem hiding this comment.
If we can't return an error, let's log it. This is a fairly important error case.
|
Thinking about this more, I'd really prefer to have more explicit error handling around the |
|
Any update on this? I'd love to see this in the latest bamboo Docker image :) |
|
Cleaning up old PRs, feel free to reopen if still relevant. |
This gives new configuration options to allow fetching marathon endpoints as registered in zookeeper rather than hardcoded endpoints. I've used this patch for some time in our production deploy with marathon 0.11.1.
I have not tested this with https only marathon. We keep http open. I also have not tested this in conjunction with marathon user/password options.