Skip to content

Commit

Permalink
Identify services using both the ID and the Node
Browse files Browse the repository at this point in the history
This aligns Fabio with the Consul docs[0] which state that
a serviceID has to be unique per agent but not cluster wide.

Fixes fabiolb#383

[0] https://www.consul.io/api/agent/service.html#id
  • Loading branch information
alvaroaleman committed Dec 16, 2017
1 parent 693296b commit e41126e
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 3 deletions.
2 changes: 1 addition & 1 deletion registry/consul/passing.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func passingServices(checks []*api.HealthCheck, status []string) []*api.HealthCh
log.Printf("[DEBUG] consul: Skipping service %q since node %q is in maintenance mode: %s", svc.ServiceID, svc.Node, c.Output)
goto skip
}
if c.CheckID == "_service_maintenance:"+svc.ServiceID && c.Status == "critical" {
if c.CheckID == "_service_maintenance:"+svc.ServiceID && c.Node == svc.Node && c.Status == "critical" {
log.Printf("[DEBUG] consul: Skipping service %q since it is in maintenance mode: %s", svc.ServiceID, c.Output)
goto skip
}
Expand Down
2 changes: 2 additions & 0 deletions registry/consul/passing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ func TestPassingServices(t *testing.T) {
serfPass = &api.HealthCheck{Node: "node", CheckID: "serfHealth", Status: "passing"}
serfFail = &api.HealthCheck{Node: "node", CheckID: "serfHealth", Status: "critical"}
svc1Pass = &api.HealthCheck{Node: "node", CheckID: "service:abc", Status: "passing", ServiceName: "abc", ServiceID: "abc-1"}
svc1N2Pass = &api.HealthCheck{Node: "node2", CheckID: "service:abc", Status: "passing", ServiceName: "abc", ServiceID: "abc-1"}
svc1Warn = &api.HealthCheck{Node: "node", CheckID: "service:abc", Status: "warning", ServiceName: "abc", ServiceID: "abc-2"}
svc1Crit = &api.HealthCheck{Node: "node", CheckID: "service:abc", Status: "critical", ServiceName: "abc", ServiceID: "abc-3"}
svc2Pass = &api.HealthCheck{Node: "node", CheckID: "my-check-id", Status: "passing", ServiceName: "def", ServiceID: "def-1"}
Expand All @@ -36,6 +37,7 @@ func TestPassingServices(t *testing.T) {
{[]string{"passing"}, []*api.HealthCheck{serfFail, nodeMaint, svc1Maint, svc1Pass}, nil},
{[]string{"passing"}, []*api.HealthCheck{svc1ID2Maint, svc1Pass}, []*api.HealthCheck{svc1Pass}},
{[]string{"passing"}, []*api.HealthCheck{svc1Maint, svc1Pass, svc2Pass}, []*api.HealthCheck{svc2Pass}},
{[]string{"passing"}, []*api.HealthCheck{svc1Maint, svc1N2Pass}, []*api.HealthCheck{svc1N2Pass}},
{[]string{"passing", "warning"}, []*api.HealthCheck{serfPass, svc1Pass, svc1Crit}, []*api.HealthCheck{svc1Pass}},
{[]string{"passing", "warning"}, []*api.HealthCheck{serfPass, svc1Warn, svc1Crit}, []*api.HealthCheck{svc1Warn}},
{[]string{"passing", "warning"}, []*api.HealthCheck{serfPass, svc1Pass, svc1Warn}, []*api.HealthCheck{svc1Pass, svc1Warn}},
Expand Down
7 changes: 5 additions & 2 deletions registry/consul/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ func servicesConfig(client *api.Client, checks []*api.HealthCheck, tagPrefix str
// map service name to list of service passing for which the health check is ok
m := map[string]map[string]bool{}
for _, check := range checks {
name, id := check.ServiceName, check.ServiceID
// Make the node part of the id, because according to the Consul docs
// the ServiceID is unique per agent but not cluster wide
// https://www.consul.io/api/agent/service.html#id
name, id := check.ServiceName, check.ServiceID+check.Node

if _, ok := m[name]; !ok {
m[name] = map[string]bool{}
Expand Down Expand Up @@ -85,7 +88,7 @@ func serviceConfig(client *api.Client, name string, passing map[string]bool, tag
for _, svc := range svcs {
// check if the instance is in the list of instances
// which passed the health check
if _, ok := passing[svc.ServiceID]; !ok {
if _, ok := passing[svc.ServiceID+svc.Node]; !ok {
continue
}

Expand Down

0 comments on commit e41126e

Please sign in to comment.