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

This patch adds the ServiceTags to the /v1/health/state/<state> endpoint. Fixes hashicorp#153
yfouquet · Apr 28, 2017 · 9685bdc · 9685bdc
1 parent e4b02ac
commit 9685bdc
Show file tree

Hide file tree

Showing 8 changed files with 143 additions and 108 deletions.
diff --git a/api/health.go b/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.

diff --git a/api/health_test.go b/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 {
-			return false, fmt.Errorf("Bad: %v", checks)
+		if got, want := out, checks; !verify.Values(t, "checks", got, want) {
+			return false, fmt.Errorf("health.Checks failed")
 		}
 		return true, nil
 	}); err != nil {

diff --git a/command/agent/local_test.go b/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,

diff --git a/consul/state/catalog.go b/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
-		hc.ServiceName = service.(*structs.ServiceNode).ServiceName
+		// Copy in the service name and tags
+		svc := service.(*structs.ServiceNode)
+		hc.ServiceName = svc.ServiceName
+		hc.ServiceTags = svc.ServiceTags
 	}
 
 	// Delete any sessions for this check if the health is critical.
@@ -1048,14 +1050,11 @@ 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)
-		}
+	}
+	if err != nil {
+		return 0, nil, fmt.Errorf("failed check lookup: %s", err)
 	}
 	ws.Add(iter.WatchCh())
 

diff --git a/consul/state/catalog_test.go b/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),
-		Node:    "node1",
-		Address: "1.2.3.4",
-		TaggedAddresses: map[string]string{
-			"hello": "world",
-		},
-		NodeMeta: map[string]string{
-			"somekey": "somevalue",
-		},
+		ID:              nodeID,
+		Node:            "node1",
+		Address:         "1.2.3.4",
+		TaggedAddresses: map[string]string{"hello": "world"},
+		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 ||
-			out.Node != "node1" || out.Address != "1.2.3.4" ||
-			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)
+		if got, want := out, node; !verify.Values(t, "GetNode", got, want) {
+			t.FailNow()
 		}
+
 		_, 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) {
-			t.Fatalf("bad node returned: %#v -- %#v", out, out2)
+		if got, want := out, out2; !verify.Values(t, "GetNodeID", got, want) {
+			t.FailNow()
 		}
 	}
 	verifyNode()
@@ -73,41 +75,39 @@ 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)
 	}
 
 	// Verify that the service got registered.
 	verifyService := func() {
-		idx, out, err := s.NodeServices(nil, "node1")
-		if err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		if idx != 2 {
-			t.Fatalf("bad index: %d", idx)
+		svcmap := map[string]*structs.NodeService{
+			"redis1": &structs.NodeService{
+				ID:        "redis1",
+				Service:   "redis",
+				Address:   "1.1.1.1",
+				Port:      8080,
+				Tags:      []string{"master"},
+				RaftIndex: structs.RaftIndex{CreateIndex: 2, ModifyIndex: 2},
+			},
 		}
-		if len(out.Services) != 1 {
-			t.Fatalf("bad: %#v", out.Services)
+
+		idx, out, err := s.NodeServices(nil, "node1")
+		if gotidx, wantidx := idx, uint64(2); err != nil || gotidx != wantidx {
+			t.Fatalf("got err, idx: %s, %d want nil, %d", err, gotidx, wantidx)
 		}
-		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)
+		if got, want := out.Services, svcmap; !verify.Values(t, "NodeServices", got, want) {
+			t.FailNow()
 		}
 
-		idx, r, err = s.NodeService("node1", "redis1")
-		if err != nil {
-			t.Fatalf("err: %s", err)
+		idx, r, err := s.NodeService("node1", "redis1")
+		if gotidx, wantidx := idx, uint64(2); err != nil || gotidx != wantidx {
+			t.Fatalf("got err, idx: %s, %d want nil, %d", err, gotidx, wantidx)
 		}
-		if idx != 2 {
-			t.Fatalf("bad index: %d", idx)
-		}
-		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)
+		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")
-		if err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		if idx != 3 {
-			t.Fatalf("bad index: %d", idx)
+		checks := structs.HealthChecks{
+			&structs.HealthCheck{
+				Node:      "node1",
+				CheckID:   "check1",
+				Name:      "check",
+				Status:    "critical",
+				RaftIndex: structs.RaftIndex{CreateIndex: 3, ModifyIndex: 3},
+			},
 		}
-		if len(out) != 1 {
-			t.Fatalf("bad: %#v", out)
+
+		idx, out, err := s.NodeChecks(nil, "node1")
+		if gotidx, wantidx := idx, uint64(3); err != nil || gotidx != wantidx {
+			t.Fatalf("got err, idx: %s, %d want nil, %d", err, gotidx, wantidx)
 		}
-		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)
+		if got, want := out, checks; !verify.Values(t, "NodeChecks", got, want) {
+			t.FailNow()
 		}
 
-		idx, c, err = s.NodeCheck("node1", "check1")
-		if err != nil {
-			t.Fatalf("err: %s", err)
+		idx, c, err := s.NodeCheck("node1", "check1")
+		if gotidx, wantidx := idx, uint64(3); err != nil || gotidx != wantidx {
+			t.Fatalf("got err, idx: %s, %d want nil, %d", err, gotidx, wantidx)
 		}
-		if idx != 3 {
-			t.Fatalf("bad index: %d", idx)
-		}
-		if c.Node != "node1" || c.CheckID != "check1" || c.Name != "check" ||
-			c.CreateIndex != 3 || c.ModifyIndex != 3 {
-			t.Fatalf("bad check returned: %#v", c)
+		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",
+			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")
-		if err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		if idx != 4 {
-			t.Fatalf("bad index: %d", idx)
-		}
-		if len(out) != 2 {
-			t.Fatalf("bad: %#v", out)
-		}
-		c1 := out[0]
-		if c1.Node != "node1" || c1.CheckID != "check1" || c1.Name != "check" ||
-			c1.CreateIndex != 3 || c1.ModifyIndex != 4 {
-			t.Fatalf("bad check returned: %#v", c1)
+		checks := structs.HealthChecks{
+			&structs.HealthCheck{
+				Node:      "node1",
+				CheckID:   "check1",
+				Name:      "check",
+				Status:    "critical",
+				RaftIndex: structs.RaftIndex{CreateIndex: 3, ModifyIndex: 4},
+			},
+			&structs.HealthCheck{
+				Node:        "node1",
+				CheckID:     "check2",
+				Name:        "check",
+				Status:      "critical",
+				ServiceID:   "redis1",
+				ServiceName: "redis",
+				ServiceTags: []string{"master"},
+				RaftIndex:   structs.RaftIndex{CreateIndex: 4, ModifyIndex: 4},
+			},
 		}
 
-		c2 := out[1]
-		if c2.Node != "node1" || c2.CheckID != "check2" || c2.Name != "check" ||
-			c2.CreateIndex != 4 || c2.ModifyIndex != 4 {
-			t.Fatalf("bad check returned: %#v", c2)
+		idx, out, err := s.NodeChecks(nil, "node1")
+		if gotidx, wantidx := idx, uint64(4); err != nil || gotidx != wantidx {
+			t.Fatalf("got err, idx: %s, %d want nil, %d", err, gotidx, wantidx)
+		}
+		if got, want := out, checks; !verify.Values(t, "NodeChecks", got, want) {
+			t.FailNow()
 		}
 	}
 	verifyChecks()

diff --git a/consul/structs/structs.go b/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
 	}