From b0fc91a1d24a98b47e25b45f6a9b16d5a5491443 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Thu, 11 Oct 2018 13:42:39 +0200 Subject: [PATCH] [Performance On Large clusters] Reduce updates on large services (#4720) * [Performance On Large clusters] Checks do update services/nodes only when really modified to avoid too many updates on very large clusters In a large cluster, when having a few thousands of nodes, the anti-entropy mechanism performs lots of changes (several per seconds) while there is no real change. This patch wants to improve this in order to increase Consul scalability when using many blocking requests on health for instance. * [Performance for large clusters] Only updates index of service if service is really modified * [Performance for large clusters] Only updates index of nodes if node is really modified * Added comments / ensure IsSame() has clear semantics * Avoid having modified boolean, return nil directly if stutures are Same * Fixed unstable unit tests TestLeader_ChangeServerID * Rewrite TestNode_IsSame() for better readability as suggested by @banks * Rename ServiceNode.IsSame() into IsSameService() + added unit tests * Do not duplicate TestStructs_ServiceNode_Conversions() and increase test coverage of IsSameService * Clearer documentation in IsSameService * Take into account ServiceProxy into ServiceNode.IsSameService() * Fixed IsSameService() with all new structures --- agent/consul/leader_test.go | 2 + agent/consul/state/catalog.go | 63 +++++++++--- agent/consul/state/catalog_test.go | 119 ++++++++++++++++------ agent/consul/state/state_store_test.go | 25 ++++- agent/structs/structs.go | 44 ++++++++ agent/structs/structs_test.go | 133 +++++++++++++++++++++++++ 6 files changed, 338 insertions(+), 48 deletions(-) diff --git a/agent/consul/leader_test.go b/agent/consul/leader_test.go index ca42a5404..35a4321ca 100644 --- a/agent/consul/leader_test.go +++ b/agent/consul/leader_test.go @@ -922,6 +922,8 @@ func TestLeader_ChangeServerID(t *testing.T) { defer os.RemoveAll(dir4) defer s4.Shutdown() joinLAN(t, s4, s1) + testrpc.WaitForTestAgent(t, s1.RPC, "dc1") + testrpc.WaitForTestAgent(t, s4.RPC, "dc1") servers[2] = s4 // While integrating #3327 it uncovered that this test was flaky. The diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index e4685bc20..2c453a4a6 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -430,6 +430,11 @@ func (s *Store) ensureNodeTxn(tx *memdb.Txn, idx uint64, node *structs.Node) err // Get the indexes. if n != nil { node.CreateIndex = n.CreateIndex + node.ModifyIndex = n.ModifyIndex + // We do not need to update anything + if node.IsSame(n) { + return nil + } node.ModifyIndex = idx } else { node.CreateIndex = idx @@ -687,14 +692,6 @@ func (s *Store) ensureServiceTxn(tx *memdb.Txn, idx uint64, node string, svc *st // conversion doesn't populate any of the node-specific information. // That's always populated when we read from the state store. entry := svc.ToServiceNode(node) - if existing != nil { - entry.CreateIndex = existing.(*structs.ServiceNode).CreateIndex - entry.ModifyIndex = idx - } else { - entry.CreateIndex = idx - entry.ModifyIndex = idx - } - // Get the node n, err := tx.First("nodes", "id", node) if err != nil { @@ -703,6 +700,21 @@ func (s *Store) ensureServiceTxn(tx *memdb.Txn, idx uint64, node string, svc *st if n == nil { return ErrMissingNode } + if existing != nil { + serviceNode := existing.(*structs.ServiceNode) + entry.CreateIndex = serviceNode.CreateIndex + entry.ModifyIndex = serviceNode.ModifyIndex + // We cannot return here because: we want to keep existing behaviour (ex: failed node lookup -> ErrMissingNode) + // It might be modified in future, but it requires changing many unit tests + // Enforcing saving the entry also ensures that if we add default values in .ToServiceNode() + // those values will be saved even if node is not really modified for a while. + if entry.IsSameService(serviceNode) { + return nil + } + } else { + entry.CreateIndex = idx + } + entry.ModifyIndex = idx // Insert the service and update the index if err := tx.Insert("services", entry); err != nil { @@ -1236,8 +1248,9 @@ func (s *Store) ensureCheckTxn(tx *memdb.Txn, idx uint64, hc *structs.HealthChec // Set the indexes if existing != nil { - hc.CreateIndex = existing.(*structs.HealthCheck).CreateIndex - hc.ModifyIndex = idx + existingCheck := existing.(*structs.HealthCheck) + hc.CreateIndex = existingCheck.CreateIndex + hc.ModifyIndex = existingCheck.ModifyIndex } else { hc.CreateIndex = idx hc.ModifyIndex = idx @@ -1257,6 +1270,7 @@ func (s *Store) ensureCheckTxn(tx *memdb.Txn, idx uint64, hc *structs.HealthChec return ErrMissingNode } + modified := true // If the check is associated with a service, check that we have // a registration for the service. if hc.ServiceID != "" { @@ -1272,14 +1286,24 @@ func (s *Store) ensureCheckTxn(tx *memdb.Txn, idx uint64, hc *structs.HealthChec svc := service.(*structs.ServiceNode) hc.ServiceName = svc.ServiceName hc.ServiceTags = svc.ServiceTags - if err = tx.Insert("index", &IndexEntry{serviceIndexName(svc.ServiceName), idx}); err != nil { - return fmt.Errorf("failed updating index: %s", err) + if existing != nil && existing.(*structs.HealthCheck).IsSame(hc) { + modified = false + } else { + // Check has been modified, we trigger a index service change + if err = tx.Insert("index", &IndexEntry{serviceIndexName(svc.ServiceName), idx}); err != nil { + return fmt.Errorf("failed updating index: %s", err) + } } } else { - // Update the status for all the services associated with this node - err = s.updateAllServiceIndexesOfNode(tx, idx, hc.Node) - if err != nil { - return err + if existing != nil && existing.(*structs.HealthCheck).IsSame(hc) { + modified = false + } else { + // Since the check has been modified, it impacts all services of node + // Update the status for all the services associated with this node + err = s.updateAllServiceIndexesOfNode(tx, idx, hc.Node) + if err != nil { + return err + } } } @@ -1303,6 +1327,13 @@ func (s *Store) ensureCheckTxn(tx *memdb.Txn, idx uint64, hc *structs.HealthChec } } } + if modified { + // We update the modify index, ONLY if something has changed, thus + // With constant output, no change is seen when watching a service + // With huge number of nodes where anti-entropy updates continuously + // the checks, but not the values within the check + hc.ModifyIndex = idx + } // Persist the check registration in the db. if err := tx.Insert("checks", hc); err != nil { diff --git a/agent/consul/state/catalog_test.go b/agent/consul/state/catalog_test.go index 61d8d55b7..33693ca83 100644 --- a/agent/consul/state/catalog_test.go +++ b/agent/consul/state/catalog_test.go @@ -294,7 +294,7 @@ func TestStateStore_EnsureRegistration(t *testing.T) { CheckID: "check1", Name: "check", Status: "critical", - RaftIndex: structs.RaftIndex{CreateIndex: 3, ModifyIndex: 4}, + RaftIndex: structs.RaftIndex{CreateIndex: 3, ModifyIndex: 3}, }, &structs.HealthCheck{ Node: "node1", @@ -491,8 +491,8 @@ func TestStateStore_EnsureRegistration_Restore(t *testing.T) { } c1 := out[0] if c1.Node != nodeName || c1.CheckID != "check1" || c1.Name != "check" || - c1.CreateIndex != 3 || c1.ModifyIndex != 4 { - t.Fatalf("bad check returned: %#v", c1) + c1.CreateIndex != 3 || c1.ModifyIndex != 3 { + t.Fatalf("bad check returned, should not be modified: %#v", c1) } c2 := out[1] @@ -508,6 +508,9 @@ func deprecatedEnsureNodeWithoutIDCanRegister(t *testing.T, s *Store, nodeName s in := &structs.Node{ Node: nodeName, Address: "1.1.1.9", + Meta: map[string]string{ + "version": string(txIdx), + }, } if err := s.EnsureNode(txIdx, in); err != nil { t.Fatalf("err: %s", err) @@ -517,10 +520,10 @@ func deprecatedEnsureNodeWithoutIDCanRegister(t *testing.T, s *Store, nodeName s t.Fatalf("err: %s", err) } if idx != txIdx { - t.Fatalf("index should be %q, was: %q", txIdx, idx) + t.Fatalf("index should be %v, was: %v", txIdx, idx) } if out.Node != nodeName { - t.Fatalf("unexpected result out = %q, nodeName supposed to be %s", out, nodeName) + t.Fatalf("unexpected result out = %v, nodeName supposed to be %s", out, nodeName) } } @@ -726,8 +729,12 @@ func TestStateStore_EnsureNode(t *testing.T) { } // Update the node registration - in.Address = "1.1.1.2" - if err := s.EnsureNode(2, in); err != nil { + in2 := &structs.Node{ + ID: in.ID, + Node: in.Node, + Address: "1.1.1.2", + } + if err := s.EnsureNode(2, in2); err != nil { t.Fatalf("err: %s", err) } @@ -745,15 +752,32 @@ func TestStateStore_EnsureNode(t *testing.T) { t.Fatalf("bad index: %d", idx) } - // Node upsert preserves the create index - if err := s.EnsureNode(3, in); err != nil { + // Re-inserting data should not modify ModifiedIndex + if err := s.EnsureNode(3, in2); err != nil { t.Fatalf("err: %s", err) } idx, out, err = s.GetNode("node1") if err != nil { t.Fatalf("err: %s", err) } - if out.CreateIndex != 1 || out.ModifyIndex != 3 || out.Address != "1.1.1.2" { + if out.CreateIndex != 1 || out.ModifyIndex != 2 || out.Address != "1.1.1.2" { + t.Fatalf("node was modified: %#v", out) + } + + // Node upsert preserves the create index + in3 := &structs.Node{ + ID: in.ID, + Node: in.Node, + Address: "1.1.1.3", + } + if err := s.EnsureNode(3, in3); err != nil { + t.Fatalf("err: %s", err) + } + idx, out, err = s.GetNode("node1") + if err != nil { + t.Fatalf("err: %s", err) + } + if out.CreateIndex != 1 || out.ModifyIndex != 3 || out.Address != "1.1.1.3" { t.Fatalf("node was modified: %#v", out) } if idx != 3 { @@ -2177,32 +2201,60 @@ func TestStateStore_EnsureCheck(t *testing.T) { t.Fatalf("bad: %#v", checks[0]) } - // Modify the health check - check.Output = "bbb" + testCheckOutput := func(expectedNodeIndex, expectedIndexForCheck uint64, outputTxt string) { + // Check that we successfully updated + idx, checks, err = s.NodeChecks(nil, "node1") + if err != nil { + t.Fatalf("err: %s", err) + } + if idx != expectedNodeIndex { + t.Fatalf("bad index: %d", idx) + } + + if len(checks) != 1 { + t.Fatalf("wrong number of checks: %d", len(checks)) + } + if checks[0].Output != outputTxt { + t.Fatalf("wrong check output: %#v", checks[0]) + } + if checks[0].CreateIndex != 3 || checks[0].ModifyIndex != expectedIndexForCheck { + t.Fatalf("bad index: %#v, expectedIndexForCheck:=%v ", checks[0], expectedIndexForCheck) + } + } + // Do not really modify the health check content the health check + check = &structs.HealthCheck{ + Node: "node1", + CheckID: "check1", + Name: "redis check", + Status: api.HealthPassing, + Notes: "test check", + Output: "aaa", + ServiceID: "service1", + ServiceName: "redis", + } if err := s.EnsureCheck(4, check); err != nil { t.Fatalf("err: %s", err) } + testCheckOutput(4, 3, check.Output) - // Check that we successfully updated - idx, checks, err = s.NodeChecks(nil, "node1") - if err != nil { + // Do modify the heathcheck + check = &structs.HealthCheck{ + Node: "node1", + CheckID: "check1", + Name: "redis check", + Status: api.HealthPassing, + Notes: "test check", + Output: "bbbmodified", + ServiceID: "service1", + ServiceName: "redis", + } + if err := s.EnsureCheck(5, check); err != nil { t.Fatalf("err: %s", err) } - if idx != 4 { - t.Fatalf("bad index: %d", idx) - } - if len(checks) != 1 { - t.Fatalf("wrong number of checks: %d", len(checks)) - } - if checks[0].Output != "bbb" { - t.Fatalf("wrong check output: %#v", checks[0]) - } - if checks[0].CreateIndex != 3 || checks[0].ModifyIndex != 4 { - t.Fatalf("bad index: %#v", checks[0]) - } + testCheckOutput(5, 5, "bbbmodified") // Index tables were updated - if idx := s.maxIndex("checks"); idx != 4 { + if idx := s.maxIndex("checks"); idx != 5 { t.Fatalf("bad index: %d", idx) } } @@ -2890,7 +2942,7 @@ func TestStateStore_CheckServiceNodes(t *testing.T) { } // Node updates alter the returned index and fire the watch. - testRegisterNode(t, s, 8, "node1") + testRegisterNodeWithChange(t, s, 8, "node1") if !watchFired(ws) { t.Fatalf("bad") } @@ -2905,7 +2957,8 @@ func TestStateStore_CheckServiceNodes(t *testing.T) { } // Service updates alter the returned index and fire the watch. - testRegisterService(t, s, 9, "node1", "service1") + + testRegisterServiceWithChange(t, s, 9, "node1", "service1", true) if !watchFired(ws) { t.Fatalf("bad") } @@ -3261,6 +3314,7 @@ func TestStateStore_NodeInfo_NodeDump(t *testing.T) { ID: "service1", Service: "service1", Address: "1.1.1.1", + Meta: make(map[string]string), Port: 1111, Weights: &structs.Weights{Passing: 1, Warning: 1}, RaftIndex: structs.RaftIndex{ @@ -3272,6 +3326,7 @@ func TestStateStore_NodeInfo_NodeDump(t *testing.T) { ID: "service2", Service: "service2", Address: "1.1.1.1", + Meta: make(map[string]string), Port: 1111, Weights: &structs.Weights{Passing: 1, Warning: 1}, RaftIndex: structs.RaftIndex{ @@ -3313,6 +3368,7 @@ func TestStateStore_NodeInfo_NodeDump(t *testing.T) { Service: "service1", Address: "1.1.1.1", Port: 1111, + Meta: make(map[string]string), Weights: &structs.Weights{Passing: 1, Warning: 1}, RaftIndex: structs.RaftIndex{ CreateIndex: 4, @@ -3324,6 +3380,7 @@ func TestStateStore_NodeInfo_NodeDump(t *testing.T) { Service: "service2", Address: "1.1.1.1", Port: 1111, + Meta: make(map[string]string), Weights: &structs.Weights{Passing: 1, Warning: 1}, RaftIndex: structs.RaftIndex{ CreateIndex: 5, @@ -3344,7 +3401,7 @@ func TestStateStore_NodeInfo_NodeDump(t *testing.T) { t.Fatalf("bad index: %d", idx) } if len(dump) != 1 || !reflect.DeepEqual(dump[0], expect[0]) { - t.Fatalf("bad: %#v", dump) + t.Fatalf("bad: len=%#v dump=%#v expect=%#v", len(dump), dump[0], expect[0]) } // Generate a dump of all the nodes diff --git a/agent/consul/state/state_store_test.go b/agent/consul/state/state_store_test.go index c875d40d9..368f460bc 100644 --- a/agent/consul/state/state_store_test.go +++ b/agent/consul/state/state_store_test.go @@ -40,6 +40,13 @@ func testRegisterNode(t *testing.T, s *Store, idx uint64, nodeID string) { testRegisterNodeWithMeta(t, s, idx, nodeID, nil) } +// testRegisterNodeWithChange registers a node and ensures it gets different from previous registration +func testRegisterNodeWithChange(t *testing.T, s *Store, idx uint64, nodeID string) { + testRegisterNodeWithMeta(t, s, idx, nodeID, map[string]string{ + "version": string(idx), + }) +} + func testRegisterNodeWithMeta(t *testing.T, s *Store, idx uint64, nodeID string, meta map[string]string) { node := &structs.Node{Node: nodeID, Meta: meta} if err := s.EnsureNode(idx, node); err != nil { @@ -57,12 +64,20 @@ func testRegisterNodeWithMeta(t *testing.T, s *Store, idx uint64, nodeID string, } } -func testRegisterService(t *testing.T, s *Store, idx uint64, nodeID, serviceID string) { +// testRegisterServiceWithChange registers a service and allow ensuring the consul index is updated +// even if service already exists if using `modifyAccordingIndex`. +// This is done by setting the transation ID in "version" meta so service will be updated if it already exists +func testRegisterServiceWithChange(t *testing.T, s *Store, idx uint64, nodeID, serviceID string, modifyAccordingIndex bool) { + meta := make(map[string]string) + if modifyAccordingIndex { + meta["version"] = string(idx) + } svc := &structs.NodeService{ ID: serviceID, Service: serviceID, Address: "1.1.1.1", Port: 1111, + Meta: meta, } if err := s.EnsureService(idx, nodeID, svc); err != nil { t.Fatalf("err: %s", err) @@ -81,6 +96,14 @@ func testRegisterService(t *testing.T, s *Store, idx uint64, nodeID, serviceID s } } +// testRegisterService register a service with given transation idx +// If the service already exists, transaction number might not be increased +// Use `testRegisterServiceWithChange()` if you want perform a registration that +// ensures the transaction is updated by setting idx in Meta of Service +func testRegisterService(t *testing.T, s *Store, idx uint64, nodeID, serviceID string) { + testRegisterServiceWithChange(t, s, idx, nodeID, serviceID, false) +} + func testRegisterCheck(t *testing.T, s *Store, idx uint64, nodeID string, serviceID string, checkID types.CheckID, state string) { chk := &structs.HealthCheck{ diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 00fb21047..0cd02eab2 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -437,6 +437,17 @@ type Node struct { } type Nodes []*Node +// IsSame return whether nodes are similar without taking into account +// RaftIndex fields. +func (n *Node) IsSame(other *Node) bool { + return n.ID == other.ID && + n.Node == other.Node && + n.Address == other.Address && + n.Datacenter == other.Datacenter && + reflect.DeepEqual(n.TaggedAddresses, other.TaggedAddresses) && + reflect.DeepEqual(n.Meta, other.Meta) +} + // ValidateMeta validates a set of key/value pairs from the agent config func ValidateMetadata(meta map[string]string, allowConsulPrefix bool) error { if len(meta) > metaMaxKeyPairs { @@ -780,6 +791,39 @@ func (s *NodeService) IsSame(other *NodeService) bool { return true } +// IsSameService checks if one Service of a ServiceNode is the same as another, +// without looking at the Raft information or Node information (that's why we +// didn't call it IsEqual). +// This is useful for seeing if an update would be idempotent for all the functional +// parts of the structure. +// In a similar fashion as ToNodeService(), fields related to Node are ignored +// see ServiceNode for more information. +func (s *ServiceNode) IsSameService(other *ServiceNode) bool { + // Skip the following fields, see ServiceNode definition + // Address string + // Datacenter string + // TaggedAddresses map[string]string + // NodeMeta map[string]string + if s.ID != other.ID || + s.Node != other.Node || + s.ServiceKind != other.ServiceKind || + s.ServiceID != other.ServiceID || + s.ServiceName != other.ServiceName || + !reflect.DeepEqual(s.ServiceTags, other.ServiceTags) || + s.ServiceAddress != other.ServiceAddress || + s.ServicePort != other.ServicePort || + !reflect.DeepEqual(s.ServiceMeta, other.ServiceMeta) || + !reflect.DeepEqual(s.ServiceWeights, other.ServiceWeights) || + s.ServiceEnableTagOverride != other.ServiceEnableTagOverride || + s.ServiceProxyDestination != other.ServiceProxyDestination || + !reflect.DeepEqual(s.ServiceProxy, other.ServiceProxy) || + !reflect.DeepEqual(s.ServiceConnect, other.ServiceConnect) { + return false + } + + return true +} + // ToServiceNode converts the given node service to a service node. func (s *NodeService) ToServiceNode(node string) *ServiceNode { theWeights := Weights{ diff --git a/agent/structs/structs_test.go b/agent/structs/structs_test.go index 903f9386e..1404662b6 100644 --- a/agent/structs/structs_test.go +++ b/agent/structs/structs_test.go @@ -162,6 +162,117 @@ func testServiceNode(t *testing.T) *ServiceNode { } } +func TestNode_IsSame(t *testing.T) { + id := types.NodeID("e62f3b31-9284-4e26-ab14-2a59dea85b55") + node := "mynode1" + address := "" + datacenter := "dc1" + n := &Node{ + ID: id, + Node: node, + Datacenter: datacenter, + Address: address, + TaggedAddresses: make(map[string]string), + Meta: make(map[string]string), + RaftIndex: RaftIndex{ + CreateIndex: 1, + ModifyIndex: 2, + }, + } + other := &Node{ + ID: id, + Node: node, + Datacenter: datacenter, + Address: address, + TaggedAddresses: make(map[string]string), + Meta: make(map[string]string), + RaftIndex: RaftIndex{ + CreateIndex: 1, + ModifyIndex: 3, + }, + } + check := func(twiddle, restore func()) { + t.Helper() + if !n.IsSame(other) || !other.IsSame(n) { + t.Fatalf("should be the same") + } + + twiddle() + if n.IsSame(other) || other.IsSame(n) { + t.Fatalf("should be different, was %#v VS %#v", n, other) + } + + restore() + if !n.IsSame(other) || !other.IsSame(n) { + t.Fatalf("should be the same") + } + } + check(func() { other.ID = types.NodeID("") }, func() { other.ID = id }) + check(func() { other.Node = "other" }, func() { other.Node = node }) + check(func() { other.Datacenter = "dcX" }, func() { other.Datacenter = datacenter }) + check(func() { other.Address = "127.0.0.1" }, func() { other.Address = address }) + check(func() { other.TaggedAddresses = map[string]string{"my": "address"} }, func() { other.TaggedAddresses = map[string]string{} }) + check(func() { other.Meta = map[string]string{"my": "meta"} }, func() { other.Meta = map[string]string{} }) + + if !n.IsSame(other) { + t.Fatalf("should be equal, was %#v VS %#v", n, other) + } +} + +func TestStructs_ServiceNode_IsSameService(t *testing.T) { + sn := testServiceNode(t) + node := "node1" + serviceID := sn.ServiceID + serviceAddress := sn.ServiceAddress + serviceEnableTagOverride := sn.ServiceEnableTagOverride + serviceMeta := make(map[string]string) + for k, v := range sn.ServiceMeta { + serviceMeta[k] = v + } + serviceName := sn.ServiceName + servicePort := sn.ServicePort + serviceTags := sn.ServiceTags + serviceWeights := Weights{Passing: 2, Warning: 1} + sn.ServiceWeights = serviceWeights + serviceProxyDestination := sn.ServiceProxyDestination + serviceProxy := sn.ServiceProxy + serviceConnect := sn.ServiceConnect + + n := sn.ToNodeService().ToServiceNode(node) + other := sn.ToNodeService().ToServiceNode(node) + + check := func(twiddle, restore func()) { + t.Helper() + if !n.IsSameService(other) || !other.IsSameService(n) { + t.Fatalf("should be the same") + } + + twiddle() + if n.IsSameService(other) || other.IsSameService(n) { + t.Fatalf("should be different, was %#v VS %#v", n, other) + } + + restore() + if !n.IsSameService(other) || !other.IsSameService(n) { + t.Fatalf("should be the same after restore, was:\n %#v VS\n %#v", n, other) + } + } + + check(func() { other.ServiceID = "66fb695a-c782-472f-8d36-4f3edd754b37" }, func() { other.ServiceID = serviceID }) + check(func() { other.Node = "other" }, func() { other.Node = node }) + check(func() { other.ServiceAddress = "1.2.3.4" }, func() { other.ServiceAddress = serviceAddress }) + check(func() { other.ServiceEnableTagOverride = !serviceEnableTagOverride }, func() { other.ServiceEnableTagOverride = serviceEnableTagOverride }) + check(func() { other.ServiceKind = "newKind" }, func() { other.ServiceKind = "" }) + check(func() { other.ServiceMeta = map[string]string{"my": "meta"} }, func() { other.ServiceMeta = serviceMeta }) + check(func() { other.ServiceName = "duck" }, func() { other.ServiceName = serviceName }) + check(func() { other.ServicePort = 65534 }, func() { other.ServicePort = servicePort }) + check(func() { other.ServiceProxyDestination = "duck" }, func() { other.ServiceProxyDestination = serviceProxyDestination }) + check(func() { other.ServiceTags = []string{"new", "tags"} }, func() { other.ServiceTags = serviceTags }) + check(func() { other.ServiceWeights = Weights{Passing: 42, Warning: 41} }, func() { other.ServiceWeights = serviceWeights }) + check(func() { other.ServiceProxy = ConnectProxyConfig{} }, func() { other.ServiceProxy = serviceProxy }) + check(func() { other.ServiceConnect = ServiceConnect{} }, func() { other.ServiceConnect = serviceConnect }) +} + func TestStructs_ServiceNode_PartialClone(t *testing.T) { sn := testServiceNode(t) @@ -222,6 +333,17 @@ func TestStructs_ServiceNode_Conversions(t *testing.T) { sn.NodeMeta = nil sn.ServiceWeights = Weights{Passing: 1, Warning: 1} require.Equal(t, sn, sn2) + if !sn.IsSameService(sn2) || !sn2.IsSameService(sn) { + t.Fatalf("bad: %#v, should be the same %#v", sn2, sn) + } + // Those fields are lost in conversion, so IsSameService() should not take them into account + sn.Address = "y" + sn.Datacenter = "z" + sn.TaggedAddresses = map[string]string{"one": "1", "two": "2"} + sn.NodeMeta = map[string]string{"meta": "data"} + if !sn.IsSameService(sn2) || !sn2.IsSameService(sn) { + t.Fatalf("bad: %#v, should be the same %#v", sn2, sn) + } } func TestStructs_NodeService_ValidateConnectProxy(t *testing.T) { @@ -358,6 +480,7 @@ func TestStructs_NodeService_IsSame(t *testing.T) { "foo": "bar", }, }, + Weights: &Weights{Passing: 1, Warning: 1}, } if !ns.IsSame(ns) { t.Fatalf("should be equal to itself") @@ -381,6 +504,7 @@ func TestStructs_NodeService_IsSame(t *testing.T) { "foo": "bar", }, }, + Weights: &Weights{Passing: 1, Warning: 1}, RaftIndex: RaftIndex{ CreateIndex: 1, ModifyIndex: 2, @@ -422,6 +546,15 @@ func TestStructs_NodeService_IsSame(t *testing.T) { check(func() { other.Proxy.LocalServicePort = 9999 }, func() { other.Proxy.LocalServicePort = 0 }) check(func() { other.Proxy.Config["baz"] = "XXX" }, func() { delete(other.Proxy.Config, "baz") }) check(func() { other.Connect.Native = true }, func() { other.Connect.Native = false }) + otherServiceNode := other.ToServiceNode("node1") + copyNodeService := otherServiceNode.ToNodeService() + if !copyNodeService.IsSame(other) { + t.Fatalf("copy should be the same, but was\n %#v\nVS\n %#v", copyNodeService, other) + } + otherServiceNodeCopy2 := copyNodeService.ToServiceNode("node1") + if !otherServiceNode.IsSameService(otherServiceNodeCopy2) { + t.Fatalf("copy should be the same, but was\n %#v\nVS\n %#v", otherServiceNode, otherServiceNodeCopy2) + } } func TestStructs_HealthCheck_IsSame(t *testing.T) {