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

Blacklisted Healthcheck types #120

Merged
merged 1 commit into from
Sep 13, 2016
Merged
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ config-file | | Path to a JSON file to read conf
consul-auth | `false` | Use Consul with authentication
consul-auth-password | | The basic authentication password
consul-auth-username | | The basic authentication username
consul-ignored-healthchecks | | A comma separated blacklist of Marathon health check types that will not be migrated to Consul, e.g. command,tcp
consul-name-separator | `.` | Separator used to create default service name for Consul
consul-get-services-retry | `3` | Number of retries on failure when performing requests to Consul. Each retry uses different cached agent
consul-max-agent-failures | `3` | Max number of consecutive request failures for agent before removal from cache
Expand Down
1 change: 1 addition & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func (config *Config) parseFlags() {
flag.Uint32Var(&config.Consul.AgentFailuresTolerance, "consul-max-agent-failures", 3, "Max number of consecutive request failures for agent before removal from cache")
flag.Uint32Var(&config.Consul.RequestRetries, "consul-get-services-retry", 3, "Number of retries on failure when performing requests to Consul. Each retry uses different cached agent")
flag.StringVar(&config.Consul.ConsulNameSeparator, "consul-name-separator", ".", "Separator used to create default service name for Consul")
flag.StringVar(&config.Consul.IgnoredHealthChecks, "consul-ignored-healthchecks", "", "A comma separated blacklist of Marathon health check types that will not be migrated to Consul, e.g. command,tcp")

// Web
flag.StringVar(&config.Web.Listen, "listen", ":4000", "Accept connections at this address")
Expand Down
1 change: 1 addition & 0 deletions consul/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type Config struct {
RequestRetries uint32
AgentFailuresTolerance uint32
ConsulNameSeparator string
IgnoredHealthChecks string
}

type Auth struct {
Expand Down
17 changes: 17 additions & 0 deletions consul/consul.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,13 @@ func (c *Consul) serviceID(task *apps.Task, name string, port int) string {
func (c *Consul) marathonToConsulChecks(task *apps.Task, healthChecks []apps.HealthCheck, serviceAddress string) consulapi.AgentServiceChecks {
var checks = make(consulapi.AgentServiceChecks, 0, len(healthChecks))

ignoredHealthCheckTypes := c.getIgnoredHealthCheckTypes()
for _, check := range healthChecks {
if contains(ignoredHealthCheckTypes, check.Protocol) {
log.WithField("Id", task.AppID.String()).WithField("Address", serviceAddress).
Info(fmt.Sprintf("Ignoring health check of type %s", check.Protocol))
continue
}
var port int
if check.Port != 0 {
port = check.Port
Expand Down Expand Up @@ -349,6 +355,17 @@ func (c *Consul) marathonToConsulChecks(task *apps.Task, healthChecks []apps.Hea
return checks
}

func (c *Consul) getIgnoredHealthCheckTypes() []string {
ignoredTypes := make([]string, 0)
Copy link
Contributor

@janisz janisz Sep 12, 2016

Choose a reason for hiding this comment

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

How about something like this:

ignoredTypes := strings.Split(strings.ToUpper(c.config.IgnoredHealthChecks), ","))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I've fixed it so ToUpper is called once, but still I'd like to check if the input is valid (trim, remove empty values):

ignoredTypes := make([]string, 0)
    for _, ignoredType := range strings.Split(strings.ToUpper(c.config.IgnoredHealthChecks), ",") {
        var ignoredType = strings.TrimSpace(ignoredType)
        if ignoredType != "" {
            ignoredTypes = append(ignoredTypes, ignoredType)
        }
    }
    return ignoredTypes

By default it returns empty splice.

for _, ignoredType := range strings.Split(strings.ToUpper(c.config.IgnoredHealthChecks), ",") {
var ignoredType = strings.TrimSpace(ignoredType)
if ignoredType != "" {
ignoredTypes = append(ignoredTypes, ignoredType)
}
}
return ignoredTypes
}

func (c *Consul) AddAgentsFromApps(apps []*apps.App) {
for _, app := range apps {
if !app.IsConsulApp() {
Expand Down
120 changes: 120 additions & 0 deletions consul/consul_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,31 @@ func TestAddAgentsFromApp(t *testing.T) {
assert.NoError(t, err)
}

func TestGetIgnoredHealthCheckTypes(t *testing.T) {
t.Parallel()

// given
var ignoredTypes = []struct {
config string
parsed []string
}{
{",command ", []string{"COMMAND"}},
{"tcp,http", []string{"TCP", "HTTP"}},
{"tcp, command,", []string{"TCP", "COMMAND"}},
{"HTTP ", []string{"HTTP"}},
{"", []string{}},
{" ,", []string{}},
}

for _, types := range ignoredTypes {
// when
consul := New(Config{IgnoredHealthChecks: types.config})

// then
assert.Equal(t, types.parsed, consul.getIgnoredHealthCheckTypes())
}
}

func TestMarathonTaskToConsulServiceMapping(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -788,6 +813,101 @@ func TestMarathonTaskToConsulServiceMapping(t *testing.T) {
}, service.Checks)
}

func TestMarathonTaskToConsulServiceMapping_IgnoredHealthcheckTypes(t *testing.T) {
t.Parallel()

// given
consul := New(Config{Tag: "marathon", IgnoredHealthChecks: "command,tcp"})
app := &apps.App{
ID: "someApp",
HealthChecks: []apps.HealthCheck{
{
Path: "/api/health?with=query",
Protocol: "HTTP",
PortIndex: 0,
IntervalSeconds: 60,
TimeoutSeconds: 20,
MaxConsecutiveFailures: 3,
},
{
Path: "",
Protocol: "HTTP",
PortIndex: 0,
IntervalSeconds: 60,
TimeoutSeconds: 20,
MaxConsecutiveFailures: 3,
},
{
Path: "/api/health?with=query",
Protocol: "INVALID_PROTOCOL",
PortIndex: 0,
IntervalSeconds: 60,
TimeoutSeconds: 20,
MaxConsecutiveFailures: 3,
},
{
Path: "/secure/health?with=query",
Protocol: "HTTPS",
PortIndex: 0,
IntervalSeconds: 50,
TimeoutSeconds: 20,
MaxConsecutiveFailures: 3,
},
{
Protocol: "TCP",
PortIndex: 1,
IntervalSeconds: 40,
TimeoutSeconds: 20,
MaxConsecutiveFailures: 3,
},
{
Protocol: "COMMAND",
Command: struct {
Value string `json:"value"`
}{Value: "echo 1"},
IntervalSeconds: 30,
TimeoutSeconds: 20,
MaxConsecutiveFailures: 3,
},
},
Labels: map[string]string{
"consul": "true",
"public": "tag",
},
}
task := &apps.Task{
ID: "someTask",
AppID: app.ID,
Host: "127.0.0.6",
Ports: []int{8090, 8443},
}

// when
services, err := consul.marathonTaskToConsulServices(task, app)
service := services[0]

// then
assert.NoError(t, err)
assert.Equal(t, "127.0.0.6", service.Address)
assert.Equal(t, []string{"marathon", "public", "marathon-task:someTask"}, service.Tags)
assert.Equal(t, 8090, service.Port)
assert.Nil(t, service.Check)
assert.Equal(t, 2, len(service.Checks))

assert.Equal(t, consulapi.AgentServiceChecks{
{
HTTP: "http://127.0.0.6:8090/api/health?with=query",
Interval: "60s",
Timeout: "20s",
},
{
HTTP: "https://127.0.0.6:8090/secure/health?with=query",
Interval: "50s",
Timeout: "20s",
},
}, service.Checks)
}

func TestMarathonTaskToConsulServiceMapping_NotResolvableTaskHost(t *testing.T) {
t.Parallel()

Expand Down
3 changes: 2 additions & 1 deletion debian/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
"Tag": "marathon",
"Timeout": 3000000000,
"AgentFailuresTolerance": 3,
"RequestRetries": 5
"RequestRetries": 5,
"IgnoredHealthChecks": ""
},
"Web": {
"Listen": ":4000",
Expand Down