Repository: allegro/marathon-consul (go)

Issue:
Disabling command healthchecks registration
Currently marathon-consul migrates all healthchecks from marathon/mesos to consul. I would like to have a possibility to disable migration for healthchecks type of command.

Pull request:
Blacklisted Healthcheck types
Introducing `consul-ignored-healthchecks` configuration property which allows one to ignore specified types of Marathon health checks during service registration in Consul.

Fixes #118 

Patch:
diff --git a/README.md b/README.md
index 7eeb56e..dd33021 100644
--- a/README.md
+++ b/README.md
@@ -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
diff --git a/config/config.go b/config/config.go
index 56f3beb..996004c 100644
--- a/config/config.go
+++ b/config/config.go
@@ -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")
diff --git a/consul/config.go b/consul/config.go
index 715e410..1d4cac1 100644
--- a/consul/config.go
+++ b/consul/config.go
@@ -15,6 +15,7 @@ type Config struct {
 	RequestRetries         uint32
 	AgentFailuresTolerance uint32
 	ConsulNameSeparator    string
+	IgnoredHealthChecks    string
 }
 
 type Auth struct {
diff --git a/consul/consul.go b/consul/consul.go
index 31e2f47..cf27750 100644
--- a/consul/consul.go
+++ b/consul/consul.go
@@ -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
@@ -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)
+	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() {
diff --git a/debian/config.json b/debian/config.json
index de19080..04c2b18 100644
--- a/debian/config.json
+++ b/debian/config.json
@@ -15,7 +15,8 @@
     "Tag": "marathon",
     "Timeout": 3000000000,
     "AgentFailuresTolerance": 3,
-    "RequestRetries": 5
+    "RequestRetries": 5,
+    "IgnoredHealthChecks": ""
   },
   "Web": {
     "Listen": ":4000",


Test patch:
diff --git a/consul/consul_test.go b/consul/consul_test.go
index f6ce778..358304d 100644
--- a/consul/consul_test.go
+++ b/consul/consul_test.go
@@ -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()
 
@@ -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()