api: Add ServiceTags to Health state endpoint (#153)

This patch adds the ServiceTags to the /v1/health/state/<state>
endpoint.

Fixes #153

api/health.go

@@ -33,6 +33,7 @@ type HealthCheck struct {
 	Output      string
 	ServiceID   string
 	ServiceName string
+	ServiceTags []string
 }
 
 // HealthChecks is a collection of HealthCheck structs.

api/health_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/hashicorp/consul/testutil"
+	"github.com/pascaldekloe/goe/verify"
 )
 
 func TestHealth_Node(t *testing.T) {
@@ -173,7 +174,9 @@ func TestHealthChecks_AggregatedStatus(t *testing.T) {
 
 func TestHealth_Checks(t *testing.T) {
 	t.Parallel()
-	c, s := makeClient(t)
+	c, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) {
+		conf.NodeName = "node123"
+	})
 	defer s.Stop()
 
 	agent := c.Agent()
@@ -182,6 +185,7 @@ func TestHealth_Checks(t *testing.T) {
 	// Make a service with a check
 	reg := &AgentServiceRegistration{
 		Name: "foo",
+		Tags: []string{"bar"},
 		Check: &AgentServiceCheck{
 			TTL: "15s",
 		},
@@ -192,15 +196,27 @@ func TestHealth_Checks(t *testing.T) {
 	defer agent.ServiceDeregister("foo")
 
 	if err := testutil.WaitForResult(func() (bool, error) {
-		checks, meta, err := health.Checks("foo", nil)
+		checks := HealthChecks{
+			&HealthCheck{
+				Node:        "node123",
+				CheckID:     "service:foo",
+				Name:        "Service 'foo' check",
+				Status:      "critical",
+				ServiceID:   "foo",
+				ServiceName: "foo",
+				ServiceTags: []string{"bar"},
+			},
+		}
+
+		out, meta, err := health.Checks("foo", nil)
 		if err != nil {
 			return false, err
 		}
 		if meta.LastIndex == 0 {
 			return false, fmt.Errorf("bad: %v", meta)
 		}
-		if len(checks) == 0 {
+		if got, want := out, checks; !verify.Values(t, "checks", got, want) {
-			return false, fmt.Errorf("Bad: %v", checks)
+			return false, fmt.Errorf("health.Checks failed")
 		}
 		return true, nil
 	}); err != nil {

command/agent/local_test.go

@@ -978,6 +978,7 @@ func TestAgentAntiEntropy_Checks_ACLDeny(t *testing.T) {
 		Node:        agent.config.NodeName,
 		ServiceID:   "mysql",
 		ServiceName: "mysql",
+		ServiceTags: []string{"master"},
 		CheckID:     "mysql-check",
 		Name:        "mysql",
 		Status:      api.HealthPassing,
@@ -989,6 +990,7 @@ func TestAgentAntiEntropy_Checks_ACLDeny(t *testing.T) {
 		Node:        agent.config.NodeName,
 		ServiceID:   "api",
 		ServiceName: "api",
+		ServiceTags: []string{"foo"},
 		CheckID:     "api-check",
 		Name:        "api",
 		Status:      api.HealthPassing,

consul/state/catalog.go

@@ -908,8 +908,10 @@ func (s *Store) ensureCheckTxn(tx *memdb.Txn, idx uint64, hc *structs.HealthChec
 			return ErrMissingService
 		}
 
-		// Copy in the service name
+		// Copy in the service name and tags
-		hc.ServiceName = service.(*structs.ServiceNode).ServiceName
+		svc := service.(*structs.ServiceNode)
+		hc.ServiceName = svc.ServiceName
+		hc.ServiceTags = svc.ServiceTags
 	}
 
 	// Delete any sessions for this check if the health is critical.
@@ -1048,15 +1050,12 @@ func (s *Store) ChecksInState(ws memdb.WatchSet, state string) (uint64, structs.
 	var err error
 	if state == api.HealthAny {
 		iter, err = tx.Get("checks", "status")
-		if err != nil {
-			return 0, nil, fmt.Errorf("failed check lookup: %s", err)
-		}
 	} else {
 		iter, err = tx.Get("checks", "status", state)
+	}
 	if err != nil {
 		return 0, nil, fmt.Errorf("failed check lookup: %s", err)
 	}
-	}
 	ws.Add(iter.WatchCh())
 
 	var results structs.HealthChecks

consul/state/catalog_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/hashicorp/consul/types"
 	"github.com/hashicorp/go-memdb"
 	uuid "github.com/hashicorp/go-uuid"
+	"github.com/pascaldekloe/goe/verify"
 )
 
 func makeRandomNodeID(t *testing.T) types.NodeID {
@@ -27,42 +28,43 @@ func TestStateStore_EnsureRegistration(t *testing.T) {
 	s := testStateStore(t)
 
 	// Start with just a node.
+	nodeID := makeRandomNodeID(t)
 	req := &structs.RegisterRequest{
-		ID:      makeRandomNodeID(t),
+		ID:              nodeID,
 		Node:            "node1",
 		Address:         "1.2.3.4",
-		TaggedAddresses: map[string]string{
+		TaggedAddresses: map[string]string{"hello": "world"},
-			"hello": "world",
+		NodeMeta:        map[string]string{"somekey": "somevalue"},
-		},
-		NodeMeta: map[string]string{
-			"somekey": "somevalue",
-		},
 	}
-	nodeID := req.ID
 	if err := s.EnsureRegistration(1, req); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
 	// Retrieve the node and verify its contents.
 	verifyNode := func() {
+		node := &structs.Node{
+			ID:              nodeID,
+			Node:            "node1",
+			Address:         "1.2.3.4",
+			TaggedAddresses: map[string]string{"hello": "world"},
+			Meta:            map[string]string{"somekey": "somevalue"},
+			RaftIndex:       structs.RaftIndex{CreateIndex: 1, ModifyIndex: 1},
+		}
+
 		_, out, err := s.GetNode("node1")
 		if err != nil {
-			t.Fatalf("err: %s", err)
+			t.Fatalf("got err %s want nil", err)
 		}
-		if out.ID != nodeID ||
+		if got, want := out, node; !verify.Values(t, "GetNode", got, want) {
-			out.Node != "node1" || out.Address != "1.2.3.4" ||
+			t.FailNow()
-			len(out.TaggedAddresses) != 1 ||
-			out.TaggedAddresses["hello"] != "world" ||
-			out.Meta["somekey"] != "somevalue" ||
-			out.CreateIndex != 1 || out.ModifyIndex != 1 {
-			t.Fatalf("bad node returned: %#v", out)
 		}
+
 		_, out2, err := s.GetNodeID(nodeID)
 		if err != nil {
-			t.Fatalf("err: %s", err)
+			t.Fatalf("got err %s want nil", err)
 		}
-		if !reflect.DeepEqual(out, out2) {
+		if got, want := out, out2; !verify.Values(t, "GetNodeID", got, want) {
-			t.Fatalf("bad node returned: %#v -- %#v", out, out2)
+			t.FailNow()
 		}
 	}
 	verifyNode()
@@ -73,6 +75,7 @@ func TestStateStore_EnsureRegistration(t *testing.T) {
 		Service: "redis",
 		Address: "1.1.1.1",
 		Port:    8080,
+		Tags:    []string{"master"},
 	}
 	if err := s.EnsureRegistration(2, req); err != nil {
 		t.Fatalf("err: %s", err)
@@ -80,34 +83,31 @@ func TestStateStore_EnsureRegistration(t *testing.T) {
 
 	// Verify that the service got registered.
 	verifyService := func() {
-		idx, out, err := s.NodeServices(nil, "node1")
+		svcmap := map[string]*structs.NodeService{
-		if err != nil {
+			"redis1": &structs.NodeService{
-			t.Fatalf("err: %s", err)
+				ID:        "redis1",
-		}
+				Service:   "redis",
-		if idx != 2 {
+				Address:   "1.1.1.1",
-			t.Fatalf("bad index: %d", idx)
+				Port:      8080,
-		}
+				Tags:      []string{"master"},
-		if len(out.Services) != 1 {
+				RaftIndex: structs.RaftIndex{CreateIndex: 2, ModifyIndex: 2},
-			t.Fatalf("bad: %#v", out.Services)
+			},
-		}
-		r := out.Services["redis1"]
-		if r == nil || r.ID != "redis1" || r.Service != "redis" ||
-			r.Address != "1.1.1.1" || r.Port != 8080 ||
-			r.CreateIndex != 2 || r.ModifyIndex != 2 {
-			t.Fatalf("bad service returned: %#v", r)
 		}
 
-		idx, r, err = s.NodeService("node1", "redis1")
+		idx, out, err := s.NodeServices(nil, "node1")
-		if err != nil {
+		if gotidx, wantidx := idx, uint64(2); err != nil || gotidx != wantidx {
-			t.Fatalf("err: %s", err)
+			t.Fatalf("got err, idx: %s, %d want nil, %d", err, gotidx, wantidx)
 		}
-		if idx != 2 {
+		if got, want := out.Services, svcmap; !verify.Values(t, "NodeServices", got, want) {
-			t.Fatalf("bad index: %d", idx)
+			t.FailNow()
 		}
-		if r == nil || r.ID != "redis1" || r.Service != "redis" ||
+
-			r.Address != "1.1.1.1" || r.Port != 8080 ||
+		idx, r, err := s.NodeService("node1", "redis1")
-			r.CreateIndex != 2 || r.ModifyIndex != 2 {
+		if gotidx, wantidx := idx, uint64(2); err != nil || gotidx != wantidx {
-			t.Fatalf("bad service returned: %#v", r)
+			t.Fatalf("got err, idx: %s, %d want nil, %d", err, gotidx, wantidx)
+		}
+		if got, want := r, svcmap["redis1"]; !verify.Values(t, "NodeService", got, want) {
+			t.FailNow()
 		}
 	}
 	verifyNode()
@@ -125,44 +125,44 @@ func TestStateStore_EnsureRegistration(t *testing.T) {
 
 	// Verify that the check got registered.
 	verifyCheck := func() {
-		idx, out, err := s.NodeChecks(nil, "node1")
+		checks := structs.HealthChecks{
-		if err != nil {
+			&structs.HealthCheck{
-			t.Fatalf("err: %s", err)
+				Node:      "node1",
-		}
+				CheckID:   "check1",
-		if idx != 3 {
+				Name:      "check",
-			t.Fatalf("bad index: %d", idx)
+				Status:    "critical",
-		}
+				RaftIndex: structs.RaftIndex{CreateIndex: 3, ModifyIndex: 3},
-		if len(out) != 1 {
+			},
-			t.Fatalf("bad: %#v", out)
-		}
-		c := out[0]
-		if c.Node != "node1" || c.CheckID != "check1" || c.Name != "check" ||
-			c.CreateIndex != 3 || c.ModifyIndex != 3 {
-			t.Fatalf("bad check returned: %#v", c)
 		}
 
-		idx, c, err = s.NodeCheck("node1", "check1")
+		idx, out, err := s.NodeChecks(nil, "node1")
-		if err != nil {
+		if gotidx, wantidx := idx, uint64(3); err != nil || gotidx != wantidx {
-			t.Fatalf("err: %s", err)
+			t.Fatalf("got err, idx: %s, %d want nil, %d", err, gotidx, wantidx)
 		}
-		if idx != 3 {
+		if got, want := out, checks; !verify.Values(t, "NodeChecks", got, want) {
-			t.Fatalf("bad index: %d", idx)
+			t.FailNow()
 		}
-		if c.Node != "node1" || c.CheckID != "check1" || c.Name != "check" ||
+
-			c.CreateIndex != 3 || c.ModifyIndex != 3 {
+		idx, c, err := s.NodeCheck("node1", "check1")
-			t.Fatalf("bad check returned: %#v", c)
+		if gotidx, wantidx := idx, uint64(3); err != nil || gotidx != wantidx {
+			t.Fatalf("got err, idx: %s, %d want nil, %d", err, gotidx, wantidx)
+		}
+		if got, want := c, checks[0]; !verify.Values(t, "NodeCheck", got, want) {
+			t.FailNow()
 		}
 	}
 	verifyNode()
 	verifyService()
 	verifyCheck()
 
-	// Add in another check via the slice.
+	// Add a service check which should populate the ServiceName
+	// and ServiceTags fields in the response.
 	req.Checks = structs.HealthChecks{
 		&structs.HealthCheck{
 			Node:      "node1",
 			CheckID:   "check2",
 			Name:      "check",
+			ServiceID: "redis1",
 		},
 	}
 	if err := s.EnsureRegistration(4, req); err != nil {
@@ -173,26 +173,32 @@ func TestStateStore_EnsureRegistration(t *testing.T) {
 	verifyNode()
 	verifyService()
 	verifyChecks := func() {
-		idx, out, err := s.NodeChecks(nil, "node1")
+		checks := structs.HealthChecks{
-		if err != nil {
+			&structs.HealthCheck{
-			t.Fatalf("err: %s", err)
+				Node:      "node1",
-		}
+				CheckID:   "check1",
-		if idx != 4 {
+				Name:      "check",
-			t.Fatalf("bad index: %d", idx)
+				Status:    "critical",
-		}
+				RaftIndex: structs.RaftIndex{CreateIndex: 3, ModifyIndex: 4},
-		if len(out) != 2 {
+			},
-			t.Fatalf("bad: %#v", out)
+			&structs.HealthCheck{
-		}
+				Node:        "node1",
-		c1 := out[0]
+				CheckID:     "check2",
-		if c1.Node != "node1" || c1.CheckID != "check1" || c1.Name != "check" ||
+				Name:        "check",
-			c1.CreateIndex != 3 || c1.ModifyIndex != 4 {
+				Status:      "critical",
-			t.Fatalf("bad check returned: %#v", c1)
+				ServiceID:   "redis1",
+				ServiceName: "redis",
+				ServiceTags: []string{"master"},
+				RaftIndex:   structs.RaftIndex{CreateIndex: 4, ModifyIndex: 4},
+			},
 		}
 
-		c2 := out[1]
+		idx, out, err := s.NodeChecks(nil, "node1")
-		if c2.Node != "node1" || c2.CheckID != "check2" || c2.Name != "check" ||
+		if gotidx, wantidx := idx, uint64(4); err != nil || gotidx != wantidx {
-			c2.CreateIndex != 4 || c2.ModifyIndex != 4 {
+			t.Fatalf("got err, idx: %s, %d want nil, %d", err, gotidx, wantidx)
-			t.Fatalf("bad check returned: %#v", c2)
+		}
+		if got, want := out, checks; !verify.Values(t, "NodeChecks", got, want) {
+			t.FailNow()
 		}
 	}
 	verifyChecks()

consul/structs/structs.go

@@ -487,6 +487,7 @@ type HealthCheck struct {
 	Output      string        // Holds output of script runs
 	ServiceID   string        // optional associated service
 	ServiceName string        // optional service name
+	ServiceTags []string      // optional service tags
 
 	RaftIndex
 }
@@ -503,7 +504,8 @@ func (c *HealthCheck) IsSame(other *HealthCheck) bool {
 		c.Notes != other.Notes ||
 		c.Output != other.Output ||
 		c.ServiceID != other.ServiceID ||
-		c.ServiceName != other.ServiceName {
+		c.ServiceName != other.ServiceName ||
+		!reflect.DeepEqual(c.ServiceTags, other.ServiceTags) {
 		return false
 	}
 

consul/structs/structs_test.go

@@ -305,6 +305,7 @@ func TestStructs_HealthCheck_IsSame(t *testing.T) {
 		Output:      "lgtm",
 		ServiceID:   "service1",
 		ServiceName: "theservice",
+		ServiceTags: []string{"foo"},
 	}
 	if !hc.IsSame(hc) {
 		t.Fatalf("should be equal to itself")
@@ -319,6 +320,7 @@ func TestStructs_HealthCheck_IsSame(t *testing.T) {
 		Output:      "lgtm",
 		ServiceID:   "service1",
 		ServiceName: "theservice",
+		ServiceTags: []string{"foo"},
 		RaftIndex: RaftIndex{
 			CreateIndex: 1,
 			ModifyIndex: 2,

website/source/api/health.html.markdown

@@ -61,7 +61,8 @@ $ curl \
     "Notes": "",
     "Output": "",
     "ServiceID": "",
-    "ServiceName": ""
+    "ServiceName": "",
+    "ServiceTags": null
   },
   {
     "ID": "40e4a748-2192-161a-0510-9bf59fe950b5",
@@ -72,7 +73,8 @@ $ curl \
     "Notes": "",
     "Output": "",
     "ServiceID": "redis",
-    "ServiceName": "redis"
+    "ServiceName": "redis",
+    "ServiceTags": ["primary"] 
   }
 ]
 ```
@@ -133,7 +135,8 @@ $ curl \
     "Notes": "",
     "Output": "",
     "ServiceID": "redis",
-    "ServiceName": "redis"
+    "ServiceName": "redis",
+	"ServiceTags": ["primary"]
   }
 ]
 ```
@@ -211,7 +214,7 @@ $ curl \
     "Service": {
       "ID": "redis",
       "Service": "redis",
-      "Tags": null,
+      "Tags": ["primary"],
       "Address": "10.1.10.12",
       "Port": 8000
     },
@@ -224,7 +227,8 @@ $ curl \
         "Notes": "",
         "Output": "",
         "ServiceID": "redis",
-        "ServiceName": "redis"
+        "ServiceName": "redis",
+		"ServiceTags": ["primary"]
       },
       {
         "Node": "foobar",
@@ -234,7 +238,8 @@ $ curl \
         "Notes": "",
         "Output": "",
         "ServiceID": "",
-        "ServiceName": ""
+        "ServiceName": "",
+		"ServiceTags": null 
       }
     ]
   }
@@ -297,7 +302,8 @@ $ curl \
     "Notes": "",
     "Output": "",
     "ServiceID": "",
-    "ServiceName": ""
+    "ServiceName": "",
+	"ServiceTags": null
   },
   {
     "Node": "foobar",
@@ -307,7 +313,8 @@ $ curl \
     "Notes": "",
     "Output": "",
     "ServiceID": "redis",
-    "ServiceName": "redis"
+    "ServiceName": "redis",
+	"ServiceTags": ["primary"]
   }
 ]
 ```