Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optionally fetch Marathon endpoint from ZK #183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions configuration/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ func FromFile(filePath string) (Configuration, error) {
conf := &Configuration{}
err := conf.FromFile(filePath)
setValueFromEnv(&conf.Marathon.Endpoint, "MARATHON_ENDPOINT")
setValueFromEnv(&conf.Marathon.Zookeeper.Host, "MARATHON_ZK_HOST")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate ZK_HOST here allows for bamboo and marathon to use different ZK_hosts if necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this affect backwards compatibility?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is backwards compatible. To use the feature you need to set MARATHON_ZK_HOST, MARATHON_ZK_PATH, and MARATHON_USE_ZK. Otherwise old behavior is used. MARATHON_ENDPOINT is ignored when MARATHON_USE_ZK is set.

setValueFromEnv(&conf.Marathon.Zookeeper.Path, "MARATHON_ZK_PATH")
setBoolValueFromEnv(&conf.Marathon.UseZookeeper, "MARATHON_USE_ZK")
setValueFromEnv(&conf.Marathon.User, "MARATHON_USER")
setValueFromEnv(&conf.Marathon.Password, "MARATHON_PASSWORD")
setBoolValueFromEnv(&conf.Marathon.UseEventStream, "MARATHON_USE_EVENT_STREAM")
Expand Down
53 changes: 49 additions & 4 deletions configuration/marathon.go
Original file line number Diff line number Diff line change
@@ -1,21 +1,66 @@
package configuration

import (
"github.com/QubitProducts/bamboo/Godeps/_workspace/src/github.com/samuel/go-zookeeper/zk"
"strings"
"time"
)

/*
Mesos Marathon configuration
*/
type Marathon struct {
// comma separated marathon http endpoints including port number
Endpoint string
User string
Password string

Endpoint string
UseZookeeper bool
Zookeeper Zookeeper
User string
Password string
UseEventStream bool
}

func (m Marathon) Endpoints() []string {
if m.UseZookeeper {
endpoints, err := zkEndpoints(m.Zookeeper)
if err != nil {
return nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can't return an error, let's log it. This is a fairly important error case.

}
return endpoints
}
return strings.Split(m.Endpoint, ",")
}

func zkEndpoints(zkConf Zookeeper) ([]string, error) {
// Only tested with marathon 0.11.1, assumes http:// for marathon
const scheme = "http://"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could try just pinging it with and without https. Hmm.

const leaderNode = "leader"

conn, _, err := zk.Connect(zkConf.ConnectionString(), time.Second*10)

if err != nil {
return nil, err
}

defer conn.Close()

var leaderPath = zkConf.Path + "/" + leaderNode

keys, _, err := conn.Children(leaderPath)

if err != nil {
return nil, err
}

endpoints := make([]string, 0, len(keys))

for _, childPath := range keys {
data, _, err := conn.Get(leaderPath + "/" + childPath)
if err != nil {
return nil, err
}
// TODO configurable http://??
endpoints = append(endpoints, scheme+string(data))
}

return endpoints, nil
}
14 changes: 7 additions & 7 deletions main/bamboo/bamboo.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,11 @@ func listenToMarathonEventStream(conf *configuration.Configuration, sub api.Even
client := &http.Client{}
client.Timeout = 0 * time.Second

for _, marathon := range conf.Marathon.Endpoints() {
ticker := time.NewTicker(1 * time.Second)
eventsURL := marathon + "/v2/events"
go func() {
for _ = range ticker.C {
ticker := time.NewTicker(1 * time.Second)
go func() {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?)

for _ = range ticker.C {
for _, marathon := range conf.Marathon.Endpoints() {
eventsURL := marathon + "/v2/events"
req, err := http.NewRequest("GET", eventsURL, nil)
req.Header.Set("Accept", "text/event-stream")
if len(conf.Marathon.User) > 0 && len(conf.Marathon.Password) > 0 {
Expand Down Expand Up @@ -236,8 +236,8 @@ func listenToMarathonEventStream(conf *configuration.Configuration, sub api.Even

log.Println("Event stream connection was closed. Re-opening...")
}
}()
}
}
}()
}

func configureLog() {
Expand Down