From 2bf668b6580c090dff8b7e3d46693126a0557a8d Mon Sep 17 00:00:00 2001 From: Frank Schroeder Date: Thu, 27 Apr 2017 16:03:05 -0700 Subject: [PATCH] api: Add ServiceTags to Health state endpoint (#153) This patch adds the ServiceTags to the /v1/health/state/ endpoint. Fixes #153 --- api/health.go | 1 + api/health_test.go | 24 +++- command/agent/local_test.go | 2 + consul/state/catalog.go | 15 +- consul/state/catalog_test.go | 184 ++++++++++++------------ consul/structs/structs.go | 4 +- consul/structs/structs_test.go | 2 + website/source/api/health.html.markdown | 23 +-- 8 files changed, 145 insertions(+), 110 deletions(-) diff --git a/api/health.go b/api/health.go index 8abe2393a..38c105fdb 100644 --- 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 index 877d9a5d5..f37edc871 100644 --- 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 index 8e5b3093d..7e41839b6 100644 --- 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 index f9959721b..59bfaf28c 100644 --- 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 index 05ea66f8f..0a7316dc5 100644 --- 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,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") - if err != nil { - t.Fatalf("err: %s", err) - } - if idx != 2 { - t.Fatalf("bad index: %d", idx) - } - if len(out.Services) != 1 { - 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) + 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}, + }, } - idx, r, err = s.NodeService("node1", "redis1") - if err != nil { - t.Fatalf("err: %s", err) + 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) } - if idx != 2 { - t.Fatalf("bad index: %d", idx) + if got, want := out.Services, svcmap; !verify.Values(t, "NodeServices", got, want) { + t.FailNow() } - 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") + if gotidx, wantidx := idx, uint64(2); err != nil || gotidx != wantidx { + 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") - if err != nil { - t.Fatalf("err: %s", err) - } - if idx != 3 { - t.Fatalf("bad index: %d", idx) - } - 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) + checks := structs.HealthChecks{ + &structs.HealthCheck{ + Node: "node1", + CheckID: "check1", + Name: "check", + Status: "critical", + RaftIndex: structs.RaftIndex{CreateIndex: 3, ModifyIndex: 3}, + }, } - idx, c, err = s.NodeCheck("node1", "check1") - if err != nil { - t.Fatalf("err: %s", err) + 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) } - if idx != 3 { - t.Fatalf("bad index: %d", idx) + if got, want := out, checks; !verify.Values(t, "NodeChecks", got, want) { + t.FailNow() } - 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") + 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", + 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 index 5cca0e4f2..12d88dda8 100644 --- 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 } diff --git a/consul/structs/structs_test.go b/consul/structs/structs_test.go index bc3b0e107..c8f657857 100644 --- a/consul/structs/structs_test.go +++ b/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, diff --git a/website/source/api/health.html.markdown b/website/source/api/health.html.markdown index cc75a25d2..c202bdd80 100644 --- a/website/source/api/health.html.markdown +++ b/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"] } ] ```