From 34e551683478e4c454b923d43700335ce082401d Mon Sep 17 00:00:00 2001 From: Rebecca Zanzig Date: Thu, 11 Oct 2018 04:50:05 -0700 Subject: [PATCH] Support multiple tags for health and catalog http api endpoints (#4717) * Support multiple tags for health and catalog api endpoints Fixes #1781. Adds a `ServiceTags` field to the ServiceSpecificRequest to support multiple tags, updates the filter logic in the catalog store, and propagates these change through to the health and catalog endpoints. Note: Leaves `ServiceTag` in the struct, since it is being used as part of the DNS lookup, which in turn uses the health check. * Update the api package to support multiple tags Includes additional tests. * Update new tests to use the `require` library * Update HealthConnect check after a bad merge --- agent/catalog_endpoint.go | 2 +- agent/consul/catalog_endpoint.go | 10 +- agent/consul/catalog_endpoint_test.go | 42 ++++---- agent/consul/health_endpoint.go | 14 ++- agent/consul/health_endpoint_test.go | 63 ++++++++++++ agent/consul/state/catalog.go | 24 ++++- agent/consul/state/catalog_test.go | 95 +++++++----------- agent/health_endpoint.go | 4 +- agent/structs/structs.go | 1 + api/catalog.go | 30 +++++- api/catalog_test.go | 81 +++++++++++++++- api/health.go | 28 +++++- api/health_test.go | 132 ++++++++++++++++++++------ 13 files changed, 399 insertions(+), 127 deletions(-) diff --git a/agent/catalog_endpoint.go b/agent/catalog_endpoint.go index 10dc555e45ac..7381cdf6420c 100644 --- a/agent/catalog_endpoint.go +++ b/agent/catalog_endpoint.go @@ -188,7 +188,7 @@ func (s *HTTPServer) catalogServiceNodes(resp http.ResponseWriter, req *http.Req // Check for a tag params := req.URL.Query() if _, ok := params["tag"]; ok { - args.ServiceTag = params.Get("tag") + args.ServiceTags = params["tag"] args.TagFilter = true } diff --git a/agent/consul/catalog_endpoint.go b/agent/consul/catalog_endpoint.go index 6cebd4500578..47faeffb8638 100644 --- a/agent/consul/catalog_endpoint.go +++ b/agent/consul/catalog_endpoint.go @@ -273,7 +273,7 @@ func (c *Catalog) ServiceNodes(args *structs.ServiceSpecificRequest, reply *stru } if args.TagFilter { - return s.ServiceTagNodes(ws, args.ServiceName, args.ServiceTag) + return s.ServiceTagNodes(ws, args.ServiceName, args.ServiceTags) } return s.ServiceNodes(ws, args.ServiceName) @@ -334,6 +334,14 @@ func (c *Catalog) ServiceNodes(args *structs.ServiceSpecificRequest, reply *stru metrics.IncrCounterWithLabels([]string{"catalog", key, "query-tag"}, 1, []metrics.Label{{Name: "service", Value: args.ServiceName}, {Name: "tag", Value: args.ServiceTag}}) } + if len(args.ServiceTags) > 0 { + // Build metric labels + labels := []metrics.Label{{Name: "service", Value: args.ServiceName}} + for _, tag := range args.ServiceTags { + labels = append(labels, metrics.Label{Name: "tag", Value: tag}) + } + metrics.IncrCounterWithLabels([]string{"catalog", key, "query-tags"}, 1, labels) + } if len(reply.ServiceNodes) == 0 { metrics.IncrCounterWithLabels([]string{"catalog", key, "not-found"}, 1, []metrics.Label{{Name: "service", Value: args.ServiceName}}) diff --git a/agent/consul/catalog_endpoint_test.go b/agent/consul/catalog_endpoint_test.go index 79560ce808e0..d7569fa9961d 100644 --- a/agent/consul/catalog_endpoint_test.go +++ b/agent/consul/catalog_endpoint_test.go @@ -1589,7 +1589,7 @@ func TestCatalog_ListServiceNodes(t *testing.T) { args := structs.ServiceSpecificRequest{ Datacenter: "dc1", ServiceName: "db", - ServiceTag: "slave", + ServiceTags: []string{"slave"}, TagFilter: false, } var out structs.IndexedServiceNodes @@ -1647,16 +1647,16 @@ func TestCatalog_ListServiceNodes_NodeMetaFilter(t *testing.T) { if err := s1.fsm.State().EnsureNode(2, node2); err != nil { t.Fatalf("err: %v", err) } - if err := s1.fsm.State().EnsureService(3, "foo", &structs.NodeService{ID: "db", Service: "db", Tags: []string{"primary"}, Address: "127.0.0.1", Port: 5000}); err != nil { + if err := s1.fsm.State().EnsureService(3, "foo", &structs.NodeService{ID: "db", Service: "db", Tags: []string{"primary", "v2"}, Address: "127.0.0.1", Port: 5000}); err != nil { t.Fatalf("err: %v", err) } - if err := s1.fsm.State().EnsureService(4, "bar", &structs.NodeService{ID: "db2", Service: "db", Tags: []string{"secondary"}, Address: "127.0.0.2", Port: 5000}); err != nil { + if err := s1.fsm.State().EnsureService(4, "bar", &structs.NodeService{ID: "db2", Service: "db", Tags: []string{"secondary", "v2"}, Address: "127.0.0.2", Port: 5000}); err != nil { t.Fatalf("err: %v", err) } cases := []struct { filters map[string]string - tag string + tags []string services structs.ServiceNodes }{ // Basic meta filter @@ -1667,7 +1667,7 @@ func TestCatalog_ListServiceNodes_NodeMetaFilter(t *testing.T) { // Basic meta filter, tag { filters: map[string]string{"somekey": "somevalue"}, - tag: "primary", + tags: []string{"primary"}, services: structs.ServiceNodes{&structs.ServiceNode{Node: "foo", ServiceID: "db"}}, }, // Common meta filter @@ -1681,7 +1681,7 @@ func TestCatalog_ListServiceNodes_NodeMetaFilter(t *testing.T) { // Common meta filter, tag { filters: map[string]string{"common": "1"}, - tag: "secondary", + tags: []string{"secondary"}, services: structs.ServiceNodes{ &structs.ServiceNode{Node: "bar", ServiceID: "db2"}, }, @@ -1699,7 +1699,22 @@ func TestCatalog_ListServiceNodes_NodeMetaFilter(t *testing.T) { // Multiple filter values, tag { filters: map[string]string{"somekey": "somevalue", "common": "1"}, - tag: "primary", + tags: []string{"primary"}, + services: structs.ServiceNodes{&structs.ServiceNode{Node: "foo", ServiceID: "db"}}, + }, + // Common meta filter, single tag + { + filters: map[string]string{"common": "1"}, + tags: []string{"v2"}, + services: structs.ServiceNodes{ + &structs.ServiceNode{Node: "bar", ServiceID: "db2"}, + &structs.ServiceNode{Node: "foo", ServiceID: "db"}, + }, + }, + // Common meta filter, multiple tags + { + filters: map[string]string{"common": "1"}, + tags: []string{"v2", "primary"}, services: structs.ServiceNodes{&structs.ServiceNode{Node: "foo", ServiceID: "db"}}, }, } @@ -1709,17 +1724,12 @@ func TestCatalog_ListServiceNodes_NodeMetaFilter(t *testing.T) { Datacenter: "dc1", NodeMetaFilters: tc.filters, ServiceName: "db", - ServiceTag: tc.tag, - TagFilter: tc.tag != "", + ServiceTags: tc.tags, + TagFilter: len(tc.tags) > 0, } var out structs.IndexedServiceNodes - if err := msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &args, &out); err != nil { - t.Fatalf("err: %v", err) - } - - if len(out.ServiceNodes) != len(tc.services) { - t.Fatalf("bad: %v", out) - } + require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &args, &out)) + require.Len(t, out.ServiceNodes, len(tc.services)) for i, serviceNode := range out.ServiceNodes { if serviceNode.Node != tc.services[i].Node || serviceNode.ServiceID != tc.services[i].ServiceID { diff --git a/agent/consul/health_endpoint.go b/agent/consul/health_endpoint.go index 38b7a9c0a538..e82f3e8e2b20 100644 --- a/agent/consul/health_endpoint.go +++ b/agent/consul/health_endpoint.go @@ -170,6 +170,13 @@ func (h *Health) ServiceNodes(args *structs.ServiceSpecificRequest, reply *struc metrics.IncrCounterWithLabels([]string{"health", key, "query-tag"}, 1, []metrics.Label{{Name: "service", Value: args.ServiceName}, {Name: "tag", Value: args.ServiceTag}}) } + if len(args.ServiceTags) > 0 { + labels := []metrics.Label{{Name: "service", Value: args.ServiceName}} + for _, tag := range args.ServiceTags { + labels = append(labels, metrics.Label{Name: "tag", Value: tag}) + } + metrics.IncrCounterWithLabels([]string{"health", key, "query-tags"}, 1, labels) + } if len(reply.Nodes) == 0 { metrics.IncrCounterWithLabels([]string{"health", key, "not-found"}, 1, []metrics.Label{{Name: "service", Value: args.ServiceName}}) @@ -186,7 +193,12 @@ func (h *Health) serviceNodesConnect(ws memdb.WatchSet, s *state.Store, args *st } func (h *Health) serviceNodesTagFilter(ws memdb.WatchSet, s *state.Store, args *structs.ServiceSpecificRequest) (uint64, structs.CheckServiceNodes, error) { - return s.CheckServiceTagNodes(ws, args.ServiceName, args.ServiceTag) + // DNS service lookups populate the ServiceTag field. In this case, + // use ServiceTag instead of the ServiceTags field. + if args.ServiceTag != "" { + return s.CheckServiceTagNodes(ws, args.ServiceName, []string{args.ServiceTag}) + } + return s.CheckServiceTagNodes(ws, args.ServiceName, args.ServiceTags) } func (h *Health) serviceNodesDefault(ws memdb.WatchSet, s *state.Store, args *structs.ServiceSpecificRequest) (uint64, structs.CheckServiceNodes, error) { diff --git a/agent/consul/health_endpoint_test.go b/agent/consul/health_endpoint_test.go index 0524617c6f0d..cbe1141f2e90 100644 --- a/agent/consul/health_endpoint_test.go +++ b/agent/consul/health_endpoint_test.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/consul/testrpc" "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestHealth_ChecksInState(t *testing.T) { @@ -602,6 +603,68 @@ func TestHealth_ServiceNodes(t *testing.T) { } } +func TestHealth_ServiceNodes_MultipleServiceTags(t *testing.T) { + t.Parallel() + dir1, s1 := testServer(t) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + testrpc.WaitForLeader(t, s1.RPC, "dc1") + + arg := structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo", + Address: "127.0.0.1", + Service: &structs.NodeService{ + ID: "db", + Service: "db", + Tags: []string{"master", "v2"}, + }, + Check: &structs.HealthCheck{ + Name: "db connect", + Status: api.HealthPassing, + ServiceID: "db", + }, + } + var out struct{} + require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.Register", &arg, &out)) + + arg = structs.RegisterRequest{ + Datacenter: "dc1", + Node: "bar", + Address: "127.0.0.2", + Service: &structs.NodeService{ + ID: "db", + Service: "db", + Tags: []string{"slave", "v2"}, + }, + Check: &structs.HealthCheck{ + Name: "db connect", + Status: api.HealthWarning, + ServiceID: "db", + }, + } + require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.Register", &arg, &out)) + + var out2 structs.IndexedCheckServiceNodes + req := structs.ServiceSpecificRequest{ + Datacenter: "dc1", + ServiceName: "db", + ServiceTags: []string{"master", "v2"}, + TagFilter: true, + } + require.NoError(t, msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &req, &out2)) + + nodes := out2.Nodes + require.Len(t, nodes, 1) + require.Equal(t, nodes[0].Node.Node, "foo") + require.Contains(t, nodes[0].Service.Tags, "v2") + require.Contains(t, nodes[0].Service.Tags, "master") + require.Equal(t, nodes[0].Checks[0].Status, api.HealthPassing) +} + func TestHealth_ServiceNodes_NodeMetaFilter(t *testing.T) { t.Parallel() dir1, s1 := testServer(t) diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index 2c453a4a663a..162eed79cf66 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -909,8 +909,8 @@ func (s *Store) serviceNodes(ws memdb.WatchSet, serviceName string, connect bool } // ServiceTagNodes returns the nodes associated with a given service, filtering -// out services that don't contain the given tag. -func (s *Store) ServiceTagNodes(ws memdb.WatchSet, service string, tag string) (uint64, structs.ServiceNodes, error) { +// out services that don't contain the given tags. +func (s *Store) ServiceTagNodes(ws memdb.WatchSet, service string, tags []string) (uint64, structs.ServiceNodes, error) { tx := s.db.Txn(false) defer tx.Abort() @@ -928,7 +928,7 @@ func (s *Store) ServiceTagNodes(ws memdb.WatchSet, service string, tag string) ( var results structs.ServiceNodes for service := services.Next(); service != nil; service = services.Next() { svc := service.(*structs.ServiceNode) - if !serviceTagFilter(svc, tag) { + if !serviceTagsFilter(svc, tags) { results = append(results, svc) } } @@ -957,6 +957,20 @@ func serviceTagFilter(sn *structs.ServiceNode, tag string) bool { return true } +// serviceTagsFilter returns true (should filter) if the given service node +// doesn't contain the given set of tags. +func serviceTagsFilter(sn *structs.ServiceNode, tags []string) bool { + for _, tag := range tags { + if serviceTagFilter(sn, tag) { + // If any one of the expected tags was not found, filter the service + return true + } + } + + // If all tags were found, don't filter the service + return false +} + // ServiceAddressNodes returns the nodes associated with a given service, filtering // out services that don't match the given serviceAddress func (s *Store) ServiceAddressNodes(ws memdb.WatchSet, address string) (uint64, structs.ServiceNodes, error) { @@ -1645,7 +1659,7 @@ func (s *Store) checkServiceNodes(ws memdb.WatchSet, serviceName string, connect // CheckServiceTagNodes is used to query all nodes and checks for a given // service, filtering out services that don't contain the given tag. -func (s *Store) CheckServiceTagNodes(ws memdb.WatchSet, serviceName, tag string) (uint64, structs.CheckServiceNodes, error) { +func (s *Store) CheckServiceTagNodes(ws memdb.WatchSet, serviceName string, tags []string) (uint64, structs.CheckServiceNodes, error) { tx := s.db.Txn(false) defer tx.Abort() @@ -1663,7 +1677,7 @@ func (s *Store) CheckServiceTagNodes(ws memdb.WatchSet, serviceName, tag string) var results structs.ServiceNodes for service := iter.Next(); service != nil; service = iter.Next() { svc := service.(*structs.ServiceNode) - if !serviceTagFilter(svc, tag) { + if !serviceTagsFilter(svc, tags) { results = append(results, svc) } } diff --git a/agent/consul/state/catalog_test.go b/agent/consul/state/catalog_test.go index 33693ca83b86..3b1314f960da 100644 --- a/agent/consul/state/catalog_test.go +++ b/agent/consul/state/catalog_test.go @@ -15,6 +15,7 @@ import ( uuid "github.com/hashicorp/go-uuid" "github.com/pascaldekloe/goe/verify" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func makeRandomNodeID(t *testing.T) types.NodeID { @@ -1833,7 +1834,7 @@ func TestStateStore_ServiceTagNodes(t *testing.T) { // Listing with no results returns an empty list. ws := memdb.NewWatchSet() - idx, nodes, err := s.ServiceTagNodes(ws, "db", "master") + idx, nodes, err := s.ServiceTagNodes(ws, "db", []string{"master"}) if err != nil { t.Fatalf("err: %s", err) } @@ -1866,7 +1867,7 @@ func TestStateStore_ServiceTagNodes(t *testing.T) { // Read everything back. ws = memdb.NewWatchSet() - idx, nodes, err = s.ServiceTagNodes(ws, "db", "master") + idx, nodes, err = s.ServiceTagNodes(ws, "db", []string{"master"}) if err != nil { t.Fatalf("err: %s", err) } @@ -1927,62 +1928,38 @@ func TestStateStore_ServiceTagNodes_MultipleTags(t *testing.T) { t.Fatalf("err: %v", err) } - idx, nodes, err := s.ServiceTagNodes(nil, "db", "master") - if err != nil { - t.Fatalf("err: %s", err) - } - if idx != 19 { - t.Fatalf("bad: %v", idx) - } - if len(nodes) != 1 { - t.Fatalf("bad: %v", nodes) - } - if nodes[0].Node != "foo" { - t.Fatalf("bad: %v", nodes) - } - if nodes[0].Address != "127.0.0.1" { - t.Fatalf("bad: %v", nodes) - } - if !lib.StrContains(nodes[0].ServiceTags, "master") { - t.Fatalf("bad: %v", nodes) - } - if nodes[0].ServicePort != 8000 { - t.Fatalf("bad: %v", nodes) - } - - idx, nodes, err = s.ServiceTagNodes(nil, "db", "v2") - if err != nil { - t.Fatalf("err: %s", err) - } - if idx != 19 { - t.Fatalf("bad: %v", idx) - } - if len(nodes) != 3 { - t.Fatalf("bad: %v", nodes) - } - - idx, nodes, err = s.ServiceTagNodes(nil, "db", "dev") - if err != nil { - t.Fatalf("err: %s", err) - } - if idx != 19 { - t.Fatalf("bad: %v", idx) - } - if len(nodes) != 1 { - t.Fatalf("bad: %v", nodes) - } - if nodes[0].Node != "foo" { - t.Fatalf("bad: %v", nodes) - } - if nodes[0].Address != "127.0.0.1" { - t.Fatalf("bad: %v", nodes) - } - if !lib.StrContains(nodes[0].ServiceTags, "dev") { - t.Fatalf("bad: %v", nodes) - } - if nodes[0].ServicePort != 8001 { - t.Fatalf("bad: %v", nodes) - } + idx, nodes, err := s.ServiceTagNodes(nil, "db", []string{"master"}) + require.NoError(t, err) + require.Equal(t, int(idx), 19) + require.Len(t, nodes, 1) + require.Equal(t, nodes[0].Node, "foo") + require.Equal(t, nodes[0].Address, "127.0.0.1") + require.Contains(t, nodes[0].ServiceTags, "master") + require.Equal(t, nodes[0].ServicePort, 8000) + + idx, nodes, err = s.ServiceTagNodes(nil, "db", []string{"v2"}) + require.NoError(t, err) + require.Equal(t, int(idx), 19) + require.Len(t, nodes, 3) + + // Test filtering on multiple tags + idx, nodes, err = s.ServiceTagNodes(nil, "db", []string{"v2", "slave"}) + require.NoError(t, err) + require.Equal(t, int(idx), 19) + require.Len(t, nodes, 2) + require.Contains(t, nodes[0].ServiceTags, "v2") + require.Contains(t, nodes[0].ServiceTags, "slave") + require.Contains(t, nodes[1].ServiceTags, "v2") + require.Contains(t, nodes[1].ServiceTags, "slave") + + idx, nodes, err = s.ServiceTagNodes(nil, "db", []string{"dev"}) + require.NoError(t, err) + require.Equal(t, int(idx), 19) + require.Len(t, nodes, 1) + require.Equal(t, nodes[0].Node, "foo") + require.Equal(t, nodes[0].Address, "127.0.0.1") + require.Contains(t, nodes[0].ServiceTags, "dev") + require.Equal(t, nodes[0].ServicePort, 8001) } func TestStateStore_DeleteService(t *testing.T) { @@ -3141,7 +3118,7 @@ func TestStateStore_CheckServiceTagNodes(t *testing.T) { } ws := memdb.NewWatchSet() - idx, nodes, err := s.CheckServiceTagNodes(ws, "db", "master") + idx, nodes, err := s.CheckServiceTagNodes(ws, "db", []string{"master"}) if err != nil { t.Fatalf("err: %s", err) } diff --git a/agent/health_endpoint.go b/agent/health_endpoint.go index f87b89261efb..5348eac44f0b 100644 --- a/agent/health_endpoint.go +++ b/agent/health_endpoint.go @@ -161,10 +161,10 @@ func (s *HTTPServer) healthServiceNodes(resp http.ResponseWriter, req *http.Requ return nil, nil } - // Check for a tag + // Check for tags params := req.URL.Query() if _, ok := params["tag"]; ok { - args.ServiceTag = params.Get("tag") + args.ServiceTags = params["tag"] args.TagFilter = true } diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 0cd02eab23fa..cc9fa8047e18 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -347,6 +347,7 @@ type ServiceSpecificRequest struct { NodeMetaFilters map[string]string ServiceName string ServiceTag string + ServiceTags []string ServiceAddress string TagFilter bool // Controls tag filtering Source QuerySource diff --git a/api/catalog.go b/api/catalog.go index a311a59d4c3e..3ca89a472a76 100644 --- a/api/catalog.go +++ b/api/catalog.go @@ -165,23 +165,43 @@ func (c *Catalog) Services(q *QueryOptions) (map[string][]string, *QueryMeta, er // Service is used to query catalog entries for a given service func (c *Catalog) Service(service, tag string, q *QueryOptions) ([]*CatalogService, *QueryMeta, error) { - return c.service(service, tag, q, false) + var tags []string + if tag != "" { + tags = []string{tag} + } + return c.service(service, tags, q, false) +} + +// Supports multiple tags for filtering +func (c *Catalog) ServiceMultipleTags(service string, tags []string, q *QueryOptions) ([]*CatalogService, *QueryMeta, error) { + return c.service(service, tags, q, false) } // Connect is used to query catalog entries for a given Connect-enabled service func (c *Catalog) Connect(service, tag string, q *QueryOptions) ([]*CatalogService, *QueryMeta, error) { - return c.service(service, tag, q, true) + var tags []string + if tag != "" { + tags = []string{tag} + } + return c.service(service, tags, q, true) +} + +// Supports multiple tags for filtering +func (c *Catalog) ConnectMultipleTags(service string, tags []string, q *QueryOptions) ([]*CatalogService, *QueryMeta, error) { + return c.service(service, tags, q, true) } -func (c *Catalog) service(service, tag string, q *QueryOptions, connect bool) ([]*CatalogService, *QueryMeta, error) { +func (c *Catalog) service(service string, tags []string, q *QueryOptions, connect bool) ([]*CatalogService, *QueryMeta, error) { path := "/v1/catalog/service/" + service if connect { path = "/v1/catalog/connect/" + service } r := c.c.newRequest("GET", path) r.setQueryOptions(q) - if tag != "" { - r.params.Set("tag", tag) + if len(tags) > 0 { + for _, tag := range tags { + r.params.Add("tag", tag) + } } rtt, resp, err := requireOK(c.c.doRequest(r)) if err != nil { diff --git a/api/catalog_test.go b/api/catalog_test.go index 9f68786acf02..f583f9ec3954 100644 --- a/api/catalog_test.go +++ b/api/catalog_test.go @@ -5,11 +5,10 @@ import ( "testing" "time" - "github.com/stretchr/testify/require" - "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/testutil/retry" "github.com/pascaldekloe/goe/verify" + "github.com/stretchr/testify/require" ) func TestAPI_CatalogDatacenters(t *testing.T) { @@ -295,6 +294,84 @@ func TestAPI_CatalogServiceCached(t *testing.T) { require.Equal(time.Duration(0), meta.CacheAge) } +func TestAPI_CatalogService_SingleTag(t *testing.T) { + t.Parallel() + c, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { + conf.NodeName = "node123" + }) + defer s.Stop() + + agent := c.Agent() + catalog := c.Catalog() + + reg := &AgentServiceRegistration{ + Name: "foo", + ID: "foo1", + Tags: []string{"bar"}, + } + require.NoError(t, agent.ServiceRegister(reg)) + defer agent.ServiceDeregister("foo1") + + retry.Run(t, func(r *retry.R) { + services, meta, err := catalog.Service("foo", "bar", nil) + require.NoError(t, err) + require.NotEqual(t, meta.LastIndex, 0) + require.Len(t, services, 1) + require.Equal(t, services[0].ServiceID, "foo1") + }) +} + +func TestAPI_CatalogService_MultipleTags(t *testing.T) { + t.Parallel() + c, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { + conf.NodeName = "node123" + }) + defer s.Stop() + + agent := c.Agent() + catalog := c.Catalog() + + // Make two services with a check + reg := &AgentServiceRegistration{ + Name: "foo", + ID: "foo1", + Tags: []string{"bar"}, + } + require.NoError(t, agent.ServiceRegister(reg)) + defer agent.ServiceDeregister("foo1") + + reg2 := &AgentServiceRegistration{ + Name: "foo", + ID: "foo2", + Tags: []string{"bar", "v2"}, + } + require.NoError(t, agent.ServiceRegister(reg2)) + defer agent.ServiceDeregister("foo2") + + // Test searching with one tag (two results) + retry.Run(t, func(r *retry.R) { + services, meta, err := catalog.ServiceMultipleTags("foo", []string{"bar"}, nil) + + require.NoError(t, err) + require.NotEqual(t, meta.LastIndex, 0) + + // Should be 2 services with the `bar` tag + require.Len(t, services, 2) + }) + + // Test searching with two tags (one result) + retry.Run(t, func(r *retry.R) { + services, meta, err := catalog.ServiceMultipleTags("foo", []string{"bar", "v2"}, nil) + + require.NoError(t, err) + require.NotEqual(t, meta.LastIndex, 0) + + // Should be exactly 1 service, named "foo2" + require.Len(t, services, 1) + require.Equal(t, services[0].ServiceID, "foo2") + }) +} + func TestAPI_CatalogService_NodeMetaFilter(t *testing.T) { t.Parallel() meta := map[string]string{"somekey": "somevalue"} diff --git a/api/health.go b/api/health.go index 1835da559f08..eae6a01a8682 100644 --- a/api/health.go +++ b/api/health.go @@ -159,7 +159,15 @@ func (h *Health) Checks(service string, q *QueryOptions) (HealthChecks, *QueryMe // for a given service. It can optionally do server-side filtering on a tag // or nodes with passing health checks only. func (h *Health) Service(service, tag string, passingOnly bool, q *QueryOptions) ([]*ServiceEntry, *QueryMeta, error) { - return h.service(service, tag, passingOnly, q, false) + var tags []string + if tag != "" { + tags = []string{tag} + } + return h.service(service, tags, passingOnly, q, false) +} + +func (h *Health) ServiceMultipleTags(service string, tags []string, passingOnly bool, q *QueryOptions) ([]*ServiceEntry, *QueryMeta, error) { + return h.service(service, tags, passingOnly, q, false) } // Connect is equivalent to Service except that it will only return services @@ -168,18 +176,28 @@ func (h *Health) Service(service, tag string, passingOnly bool, q *QueryOptions) // passingOnly is true only instances where both the service and any proxy are // healthy will be returned. func (h *Health) Connect(service, tag string, passingOnly bool, q *QueryOptions) ([]*ServiceEntry, *QueryMeta, error) { - return h.service(service, tag, passingOnly, q, true) + var tags []string + if tag != "" { + tags = []string{tag} + } + return h.service(service, tags, passingOnly, q, true) +} + +func (h *Health) ConnectMultipleTags(service string, tags []string, passingOnly bool, q *QueryOptions) ([]*ServiceEntry, *QueryMeta, error) { + return h.service(service, tags, passingOnly, q, true) } -func (h *Health) service(service, tag string, passingOnly bool, q *QueryOptions, connect bool) ([]*ServiceEntry, *QueryMeta, error) { +func (h *Health) service(service string, tags []string, passingOnly bool, q *QueryOptions, connect bool) ([]*ServiceEntry, *QueryMeta, error) { path := "/v1/health/service/" + service if connect { path = "/v1/health/connect/" + service } r := h.c.newRequest("GET", path) r.setQueryOptions(q) - if tag != "" { - r.params.Set("tag", tag) + if len(tags) > 0 { + for _, tag := range tags { + r.params.Add("tag", tag) + } } if passingOnly { r.params.Set(HealthPassing, "1") diff --git a/api/health_test.go b/api/health_test.go index 18974acdc498..7a9147350e8e 100644 --- a/api/health_test.go +++ b/api/health_test.go @@ -283,6 +283,108 @@ func TestAPI_HealthService(t *testing.T) { }) } +func TestAPI_HealthService_SingleTag(t *testing.T) { + t.Parallel() + c, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { + conf.NodeName = "node123" + }) + defer s.Stop() + agent := c.Agent() + health := c.Health() + reg := &AgentServiceRegistration{ + Name: "foo", + ID: "foo1", + Tags: []string{"bar"}, + Check: &AgentServiceCheck{ + Status: HealthPassing, + TTL: "15s", + }, + } + require.NoError(t, agent.ServiceRegister(reg)) + defer agent.ServiceDeregister("foo1") + retry.Run(t, func(r *retry.R) { + services, meta, err := health.Service("foo", "bar", true, nil) + require.NoError(t, err) + require.NotEqual(t, meta.LastIndex, 0) + require.Len(t, services, 1) + require.Equal(t, services[0].Service.ID, "foo1") + }) +} +func TestAPI_HealthService_MultipleTags(t *testing.T) { + t.Parallel() + c, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { + conf.NodeName = "node123" + }) + defer s.Stop() + + agent := c.Agent() + health := c.Health() + + // Make two services with a check + reg := &AgentServiceRegistration{ + Name: "foo", + ID: "foo1", + Tags: []string{"bar"}, + Check: &AgentServiceCheck{ + Status: HealthPassing, + TTL: "15s", + }, + } + require.NoError(t, agent.ServiceRegister(reg)) + defer agent.ServiceDeregister("foo1") + + reg2 := &AgentServiceRegistration{ + Name: "foo", + ID: "foo2", + Tags: []string{"bar", "v2"}, + Check: &AgentServiceCheck{ + Status: HealthPassing, + TTL: "15s", + }, + } + require.NoError(t, agent.ServiceRegister(reg2)) + defer agent.ServiceDeregister("foo2") + + // Test searching with one tag (two results) + retry.Run(t, func(r *retry.R) { + services, meta, err := health.ServiceMultipleTags("foo", []string{"bar"}, true, nil) + + require.NoError(t, err) + require.NotEqual(t, meta.LastIndex, 0) + require.Len(t, services, 2) + }) + + // Test searching with two tags (one result) + retry.Run(t, func(r *retry.R) { + services, meta, err := health.ServiceMultipleTags("foo", []string{"bar", "v2"}, true, nil) + + require.NoError(t, err) + require.NotEqual(t, meta.LastIndex, 0) + require.Len(t, services, 1) + require.Equal(t, services[0].Service.ID, "foo2") + }) +} + +func TestAPI_HealthService_NodeMetaFilter(t *testing.T) { + t.Parallel() + meta := map[string]string{"somekey": "somevalue"} + c, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { + conf.NodeMeta = meta + }) + defer s.Stop() + + health := c.Health() + retry.Run(t, func(r *retry.R) { + // consul service should always exist... + checks, meta, err := health.Service("consul", "", true, &QueryOptions{NodeMeta: meta}) + require.NoError(t, err) + require.NotEqual(t, meta.LastIndex, 0) + require.NotEqual(t, len(checks), 0) + require.Equal(t, checks[0].Node.Datacenter, "dc1") + require.Contains(t, checks[0].Node.TaggedAddresses, "wan") + }) +} + func TestAPI_HealthConnect(t *testing.T) { t.Parallel() c, s := makeClient(t) @@ -335,36 +437,6 @@ func TestAPI_HealthConnect(t *testing.T) { }) } -func TestAPI_HealthService_NodeMetaFilter(t *testing.T) { - t.Parallel() - meta := map[string]string{"somekey": "somevalue"} - c, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { - conf.NodeMeta = meta - }) - defer s.Stop() - - health := c.Health() - retry.Run(t, func(r *retry.R) { - // consul service should always exist... - checks, meta, err := health.Service("consul", "", true, &QueryOptions{NodeMeta: meta}) - if err != nil { - r.Fatal(err) - } - if meta.LastIndex == 0 { - r.Fatalf("bad: %v", meta) - } - if len(checks) == 0 { - r.Fatalf("Bad: %v", checks) - } - if _, ok := checks[0].Node.TaggedAddresses["wan"]; !ok { - r.Fatalf("Bad: %v", checks[0].Node) - } - if checks[0].Node.Datacenter != "dc1" { - r.Fatalf("Bad datacenter: %v", checks[0].Node) - } - }) -} - func TestAPI_HealthState(t *testing.T) { t.Parallel() c, s := makeClient(t)