From 5441c83e8bdf835e7d6a386936ca57f2c8e5eb9e Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Fri, 28 Sep 2018 08:46:41 +0200 Subject: [PATCH 01/12] [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. --- agent/consul/state/catalog.go | 35 +++++++++++---- agent/consul/state/catalog_test.go | 70 +++++++++++++++++++++--------- 2 files changed, 76 insertions(+), 29 deletions(-) diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index e4685bc20b1d..0acf8a4146d3 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -1236,8 +1236,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 +1258,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 +1274,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 +1315,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 bb42eb562b46..c29ae9ed1de0 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] @@ -2177,32 +2177,60 @@ func TestStateStore_EnsureCheck(t *testing.T) { t.Fatalf("bad: %#v", checks[0]) } - // Modify the health check - check.Output = "bbb" - if err := s.EnsureCheck(4, check); err != nil { - t.Fatalf("err: %s", err) - } + 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) + } - // Check that we successfully updated - idx, checks, err = s.NodeChecks(nil, "node1") - if err != nil { - t.Fatalf("err: %s", err) + 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) + } } - if idx != 4 { - t.Fatalf("bad index: %d", idx) + // 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 len(checks) != 1 { - t.Fatalf("wrong number of checks: %d", len(checks)) + if err := s.EnsureCheck(4, check); err != nil { + t.Fatalf("err: %s", err) } - if checks[0].Output != "bbb" { - t.Fatalf("wrong check output: %#v", checks[0]) + testCheckOutput(4, 3, check.Output) + + // 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 checks[0].CreateIndex != 3 || checks[0].ModifyIndex != 4 { - t.Fatalf("bad index: %#v", checks[0]) + if err := s.EnsureCheck(5, check); err != nil { + t.Fatalf("err: %s", err) } + 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) } } From 6515fb59c5e363f1d6f7bf5f0edd09bd6061e016 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Fri, 28 Sep 2018 13:08:33 +0200 Subject: [PATCH 02/12] [Performance for large clusters] Only updates index of service if service is really modified --- agent/consul/state/catalog.go | 23 ++++++++++++++++------- agent/consul/state/catalog_test.go | 12 ++++++++++-- agent/consul/state/state_store_test.go | 18 +++++++++++++++++- agent/structs/structs.go | 24 ++++++++++++++++++++++++ agent/structs/structs_test.go | 11 +++++++++++ 5 files changed, 78 insertions(+), 10 deletions(-) diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index 0acf8a4146d3..4cd6a68b324c 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -687,9 +687,16 @@ 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) + modified := true if existing != nil { - entry.CreateIndex = existing.(*structs.ServiceNode).CreateIndex - entry.ModifyIndex = idx + serviceNode := existing.(*structs.ServiceNode) + entry.CreateIndex = serviceNode.CreateIndex + entry.ModifyIndex = serviceNode.ModifyIndex + if entry.IsSame(serviceNode) { + modified = false + } else { + entry.ModifyIndex = idx + } } else { entry.CreateIndex = idx entry.ModifyIndex = idx @@ -708,11 +715,13 @@ func (s *Store) ensureServiceTxn(tx *memdb.Txn, idx uint64, node string, svc *st if err := tx.Insert("services", entry); err != nil { return fmt.Errorf("failed inserting service: %s", err) } - if err := tx.Insert("index", &IndexEntry{"services", idx}); err != nil { - return fmt.Errorf("failed updating index: %s", err) - } - if err := tx.Insert("index", &IndexEntry{serviceIndexName(svc.Service), idx}); err != nil { - return fmt.Errorf("failed updating index: %s", err) + if modified { + if err := tx.Insert("index", &IndexEntry{"services", idx}); err != nil { + return fmt.Errorf("failed updating index: %s", err) + } + if err := tx.Insert("index", &IndexEntry{serviceIndexName(svc.Service), idx}); err != nil { + return fmt.Errorf("failed updating index: %s", err) + } } return nil diff --git a/agent/consul/state/catalog_test.go b/agent/consul/state/catalog_test.go index c29ae9ed1de0..b2d05b653555 100644 --- a/agent/consul/state/catalog_test.go +++ b/agent/consul/state/catalog_test.go @@ -1,6 +1,7 @@ package state import ( + "encoding/json" "fmt" "reflect" "sort" @@ -2933,7 +2934,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") } @@ -3289,6 +3291,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{ @@ -3300,6 +3303,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{ @@ -3341,6 +3345,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, @@ -3352,6 +3357,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, @@ -3372,7 +3378,9 @@ 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) + dump0, _ := json.Marshal(dump[0]) + expect0, _ := json.Marshal(expect[0]) + t.Fatalf("len(dump):= %v bad:\n%#v\nVS\n%#v", len(dump), string(dump0), string(expect0)) } // 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 c875d40d9a13..da5a297414d6 100644 --- a/agent/consul/state/state_store_test.go +++ b/agent/consul/state/state_store_test.go @@ -57,12 +57,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 +89,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 e684647ec056..89580bb40191 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -629,6 +629,30 @@ func (s *NodeService) IsSame(other *NodeService) bool { return true } +// IsSame checks if one ServiceNode is the same as another, without looking +// at the Raft 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. +func (s *ServiceNode) IsSame(other *ServiceNode) bool { + 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 || + 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 928989e2a7cc..fe713bce9196 100644 --- a/agent/structs/structs_test.go +++ b/agent/structs/structs_test.go @@ -291,6 +291,7 @@ func TestStructs_NodeService_IsSame(t *testing.T) { Port: 1234, EnableTagOverride: true, ProxyDestination: "db", + Weights: &Weights{Passing: 1, Warning: 1}, } if !ns.IsSame(ns) { t.Fatalf("should be equal to itself") @@ -309,6 +310,7 @@ func TestStructs_NodeService_IsSame(t *testing.T) { "meta1": "value1", }, ProxyDestination: "db", + Weights: &Weights{Passing: 1, Warning: 1}, RaftIndex: RaftIndex{ CreateIndex: 1, ModifyIndex: 2, @@ -345,6 +347,15 @@ func TestStructs_NodeService_IsSame(t *testing.T) { check(func() { other.Kind = ServiceKindConnectProxy }, func() { other.Kind = "" }) check(func() { other.ProxyDestination = "" }, func() { other.ProxyDestination = "db" }) 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.IsSame(otherServiceNodeCopy2) { + t.Fatalf("copy should be the same, but was\n %#v\nVS\n %#v", otherServiceNode, otherServiceNodeCopy2) + } } func TestStructs_HealthCheck_IsSame(t *testing.T) { From 11ce5a224d1b047623993c98ccfcb360806545f8 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Fri, 28 Sep 2018 14:31:47 +0200 Subject: [PATCH 03/12] [Performance for large clusters] Only updates index of nodes if node is really modified --- agent/consul/state/catalog.go | 5 ++ agent/consul/state/catalog_test.go | 43 +++++++++++---- agent/consul/state/state_store_test.go | 7 +++ agent/structs/structs.go | 11 ++++ agent/structs/structs_test.go | 76 ++++++++++++++++++++++++++ 5 files changed, 131 insertions(+), 11 deletions(-) diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index 4cd6a68b324c..31065144c168 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 diff --git a/agent/consul/state/catalog_test.go b/agent/consul/state/catalog_test.go index b2d05b653555..f780b8b1c9b5 100644 --- a/agent/consul/state/catalog_test.go +++ b/agent/consul/state/catalog_test.go @@ -1,7 +1,6 @@ package state import ( - "encoding/json" "fmt" "reflect" "sort" @@ -509,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) @@ -518,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) } } @@ -727,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) } @@ -746,15 +752,32 @@ func TestStateStore_EnsureNode(t *testing.T) { t.Fatalf("bad index: %d", idx) } + // 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 != 2 || out.Address != "1.1.1.2" { + t.Fatalf("node was modified: %#v", out) + } + // Node upsert preserves the create index - if err := s.EnsureNode(3, in); err != nil { + 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.2" { + if out.CreateIndex != 1 || out.ModifyIndex != 3 || out.Address != "1.1.1.3" { t.Fatalf("node was modified: %#v", out) } if idx != 3 { @@ -2919,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") } @@ -3378,9 +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]) { - dump0, _ := json.Marshal(dump[0]) - expect0, _ := json.Marshal(expect[0]) - t.Fatalf("len(dump):= %v bad:\n%#v\nVS\n%#v", len(dump), string(dump0), string(expect0)) + 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 da5a297414d6..368f460bc03e 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 { diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 89580bb40191..e36ea532a67c 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -364,6 +364,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 { diff --git a/agent/structs/structs_test.go b/agent/structs/structs_test.go index fe713bce9196..a2aebdb7dea4 100644 --- a/agent/structs/structs_test.go +++ b/agent/structs/structs_test.go @@ -152,6 +152,82 @@ func testServiceNode() *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, + }, + } + if !n.IsSame(other) { + t.Fatalf("should be equal, was %#v VS %#v", n, other) + } + + other.ID = types.NodeID("") + if n.IsSame(other) { + t.Fatalf("should be different, was %#v VS %#v", n, other) + } + other.ID = id + + other.Node = "other" + if n.IsSame(other) { + t.Fatalf("should be different, was %#v VS %#v", n, other) + } + other.Node = node + + other.Datacenter = "dcX" + if n.IsSame(other) { + t.Fatalf("should be different, was %#v VS %#v", n, other) + } + other.Datacenter = datacenter + + other.Address = "127.0.0.1" + if n.IsSame(other) { + t.Fatalf("should be different, was %#v VS %#v", n, other) + } + other.Address = address + + other.TaggedAddresses = map[string]string{ + "my": "address", + } + if n.IsSame(other) { + t.Fatalf("should be different, was %#v VS %#v", n, other) + } + other.TaggedAddresses = map[string]string{} + other.Meta = map[string]string{ + "my": "meta", + } + if n.IsSame(other) { + t.Fatalf("should be different, was %#v VS %#v", n, other) + } + other.Meta = map[string]string{} + if !n.IsSame(other) { + t.Fatalf("should be equal, was %#v VS %#v", n, other) + } +} + func TestStructs_ServiceNode_PartialClone(t *testing.T) { sn := testServiceNode() From eb95545b9abd7b729b7bb37baafb367ea2f5485d Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Fri, 5 Oct 2018 21:34:30 +0200 Subject: [PATCH 04/12] Added comments / ensure IsSame() has clear semantics --- agent/consul/state/catalog.go | 4 ++++ agent/structs/structs.go | 3 +++ 2 files changed, 7 insertions(+) diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index 31065144c168..dfd4927c5ac7 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -697,6 +697,10 @@ func (s *Store) ensureServiceTxn(tx *memdb.Txn, idx uint64, node string, svc *st 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.IsSame(serviceNode) { modified = false } else { diff --git a/agent/structs/structs.go b/agent/structs/structs.go index e36ea532a67c..4a4a7a9049f4 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -653,8 +653,11 @@ func (s *ServiceNode) IsSame(other *ServiceNode) bool { !reflect.DeepEqual(s.ServiceTags, other.ServiceTags) || s.ServiceAddress != other.ServiceAddress || s.ServicePort != other.ServicePort || + s.Datacenter != other.Datacenter || + !reflect.DeepEqual(s.NodeMeta, other.NodeMeta) || !reflect.DeepEqual(s.ServiceMeta, other.ServiceMeta) || !reflect.DeepEqual(s.ServiceWeights, other.ServiceWeights) || + !reflect.DeepEqual(s.TaggedAddresses, other.TaggedAddresses) || s.ServiceEnableTagOverride != other.ServiceEnableTagOverride || s.ServiceProxyDestination != other.ServiceProxyDestination || s.ServiceConnect != other.ServiceConnect { From 82a903e0ed715a0bc1b7af0785ce19cb414a3898 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Fri, 5 Oct 2018 22:07:40 +0200 Subject: [PATCH 05/12] Avoid having modified boolean, return nil directly if stutures are Same --- agent/consul/state/catalog.go | 36 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index dfd4927c5ac7..816e482819af 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -692,7 +692,14 @@ 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) - modified := true + // Get the node + n, err := tx.First("nodes", "id", node) + if err != nil { + return fmt.Errorf("failed node lookup: %s", err) + } + if n == nil { + return ErrMissingNode + } if existing != nil { serviceNode := existing.(*structs.ServiceNode) entry.CreateIndex = serviceNode.CreateIndex @@ -702,35 +709,22 @@ func (s *Store) ensureServiceTxn(tx *memdb.Txn, idx uint64, node string, svc *st // 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.IsSame(serviceNode) { - modified = false - } else { - entry.ModifyIndex = idx + return nil } } else { entry.CreateIndex = idx - entry.ModifyIndex = idx - } - - // Get the node - n, err := tx.First("nodes", "id", node) - if err != nil { - return fmt.Errorf("failed node lookup: %s", err) - } - if n == nil { - return ErrMissingNode } + entry.ModifyIndex = idx // Insert the service and update the index if err := tx.Insert("services", entry); err != nil { return fmt.Errorf("failed inserting service: %s", err) } - if modified { - if err := tx.Insert("index", &IndexEntry{"services", idx}); err != nil { - return fmt.Errorf("failed updating index: %s", err) - } - if err := tx.Insert("index", &IndexEntry{serviceIndexName(svc.Service), idx}); err != nil { - return fmt.Errorf("failed updating index: %s", err) - } + if err := tx.Insert("index", &IndexEntry{"services", idx}); err != nil { + return fmt.Errorf("failed updating index: %s", err) + } + if err := tx.Insert("index", &IndexEntry{serviceIndexName(svc.Service), idx}); err != nil { + return fmt.Errorf("failed updating index: %s", err) } return nil From efce3f168d07aba1d847099f7c928a3200071638 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Fri, 5 Oct 2018 23:01:25 +0200 Subject: [PATCH 06/12] Fixed unstable unit tests TestLeader_ChangeServerID --- agent/consul/leader_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/agent/consul/leader_test.go b/agent/consul/leader_test.go index ca42a5404a01..35a4321caaa0 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 From 4b1f466f666c7d42449ad37ff8d4885d040d1da8 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Tue, 9 Oct 2018 16:16:36 +0200 Subject: [PATCH 07/12] Rewrite TestNode_IsSame() for better readability as suggested by @banks --- agent/structs/structs_test.go | 56 +++++++++++------------------------ 1 file changed, 18 insertions(+), 38 deletions(-) diff --git a/agent/structs/structs_test.go b/agent/structs/structs_test.go index a2aebdb7dea4..c1b127640fa8 100644 --- a/agent/structs/structs_test.go +++ b/agent/structs/structs_test.go @@ -181,48 +181,28 @@ func TestNode_IsSame(t *testing.T) { ModifyIndex: 3, }, } - if !n.IsSame(other) { - t.Fatalf("should be equal, was %#v VS %#v", n, other) - } - - other.ID = types.NodeID("") - if n.IsSame(other) { - t.Fatalf("should be different, was %#v VS %#v", n, other) - } - other.ID = id - - other.Node = "other" - if n.IsSame(other) { - t.Fatalf("should be different, was %#v VS %#v", n, other) - } - other.Node = node + check := func(twiddle, restore func()) { + if !n.IsSame(other) || !other.IsSame(n) { + t.Fatalf("should be the same") + } - other.Datacenter = "dcX" - if n.IsSame(other) { - t.Fatalf("should be different, was %#v VS %#v", n, other) - } - other.Datacenter = datacenter + twiddle() + if n.IsSame(other) || other.IsSame(n) { + t.Fatalf("should be different, was %#v VS %#v", n, other) + } - other.Address = "127.0.0.1" - if n.IsSame(other) { - 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") + } } - other.Address = address + 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{} }) - other.TaggedAddresses = map[string]string{ - "my": "address", - } - if n.IsSame(other) { - t.Fatalf("should be different, was %#v VS %#v", n, other) - } - other.TaggedAddresses = map[string]string{} - other.Meta = map[string]string{ - "my": "meta", - } - if n.IsSame(other) { - t.Fatalf("should be different, was %#v VS %#v", n, other) - } - other.Meta = map[string]string{} if !n.IsSame(other) { t.Fatalf("should be equal, was %#v VS %#v", n, other) } From aab2d16bea5141f5d5d3ab340a6b97bdf59bceef Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Tue, 9 Oct 2018 18:07:36 +0200 Subject: [PATCH 08/12] Rename ServiceNode.IsSame() into IsSameService() + added unit tests --- agent/consul/state/catalog.go | 2 +- agent/structs/structs.go | 14 +++++--- agent/structs/structs_test.go | 68 ++++++++++++++++++++++++++++++++++- 3 files changed, 77 insertions(+), 7 deletions(-) diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index 816e482819af..2c453a4a663a 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -708,7 +708,7 @@ func (s *Store) ensureServiceTxn(tx *memdb.Txn, idx uint64, node string, svc *st // 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.IsSame(serviceNode) { + if entry.IsSameService(serviceNode) { return nil } } else { diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 4a4a7a9049f4..213549773e74 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -640,11 +640,18 @@ func (s *NodeService) IsSame(other *NodeService) bool { return true } -// IsSame checks if one ServiceNode is the same as another, without looking +// IsSameService checks if one ServiceNode is the same as another, without looking // at the Raft 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. -func (s *ServiceNode) IsSame(other *ServiceNode) bool { +// 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 || @@ -653,11 +660,8 @@ func (s *ServiceNode) IsSame(other *ServiceNode) bool { !reflect.DeepEqual(s.ServiceTags, other.ServiceTags) || s.ServiceAddress != other.ServiceAddress || s.ServicePort != other.ServicePort || - s.Datacenter != other.Datacenter || - !reflect.DeepEqual(s.NodeMeta, other.NodeMeta) || !reflect.DeepEqual(s.ServiceMeta, other.ServiceMeta) || !reflect.DeepEqual(s.ServiceWeights, other.ServiceWeights) || - !reflect.DeepEqual(s.TaggedAddresses, other.TaggedAddresses) || s.ServiceEnableTagOverride != other.ServiceEnableTagOverride || s.ServiceProxyDestination != other.ServiceProxyDestination || s.ServiceConnect != other.ServiceConnect { diff --git a/agent/structs/structs_test.go b/agent/structs/structs_test.go index c1b127640fa8..273cec819990 100644 --- a/agent/structs/structs_test.go +++ b/agent/structs/structs_test.go @@ -182,6 +182,7 @@ func TestNode_IsSame(t *testing.T) { }, } check := func(twiddle, restore func()) { + t.Helper() if !n.IsSame(other) || !other.IsSame(n) { t.Fatalf("should be the same") } @@ -208,6 +209,54 @@ func TestNode_IsSame(t *testing.T) { } } +func TestStructs_ServiceNode_IsSameService(t *testing.T) { + sn := testServiceNode() + 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 + + 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.ServiceTags = []string{"new", "tags"} }, func() { other.ServiceTags = serviceTags }) + check(func() { other.ServiceWeights = Weights{Passing: 42, Warning: 41} }, func() { other.ServiceWeights = serviceWeights }) +} + func TestStructs_ServiceNode_PartialClone(t *testing.T) { sn := testServiceNode() @@ -256,6 +305,23 @@ func TestStructs_ServiceNode_PartialClone(t *testing.T) { } } +func TestStructs_ServiceNode_IsSame(t *testing.T) { + sn := testServiceNode() + + sn2 := sn.ToNodeService().ToServiceNode("node1") + // These two fields get lost in the conversion, so we have to zero-value + // them out before we do the compare. + sn.ID = "" + sn.Address = "" + sn.Datacenter = "" + sn.TaggedAddresses = nil + sn.NodeMeta = nil + sn.ServiceWeights = Weights{Passing: 1, Warning: 1} + if !reflect.DeepEqual(sn, sn2) { + t.Fatalf("bad: %#v, but expected %#v", sn2, sn) + } +} + func TestStructs_ServiceNode_Conversions(t *testing.T) { sn := testServiceNode() @@ -409,7 +475,7 @@ func TestStructs_NodeService_IsSame(t *testing.T) { t.Fatalf("copy should be the same, but was\n %#v\nVS\n %#v", copyNodeService, other) } otherServiceNodeCopy2 := copyNodeService.ToServiceNode("node1") - if !otherServiceNode.IsSame(otherServiceNodeCopy2) { + if !otherServiceNode.IsSameService(otherServiceNodeCopy2) { t.Fatalf("copy should be the same, but was\n %#v\nVS\n %#v", otherServiceNode, otherServiceNodeCopy2) } } From 8c535cd9391913b181f6a8f48d053e37d2b7898c Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Wed, 10 Oct 2018 19:06:45 +0200 Subject: [PATCH 09/12] Do not duplicate TestStructs_ServiceNode_Conversions() and increase test coverage of IsSameService --- agent/structs/structs_test.go | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/agent/structs/structs_test.go b/agent/structs/structs_test.go index 3704c2ae08a4..bebcf9a1c8cf 100644 --- a/agent/structs/structs_test.go +++ b/agent/structs/structs_test.go @@ -313,23 +313,6 @@ func TestStructs_ServiceNode_PartialClone(t *testing.T) { } } -func TestStructs_ServiceNode_IsSame(t *testing.T) { - sn := testServiceNode(t) - - sn2 := sn.ToNodeService().ToServiceNode("node1") - // These two fields get lost in the conversion, so we have to zero-value - // them out before we do the compare. - sn.ID = "" - sn.Address = "" - sn.Datacenter = "" - sn.TaggedAddresses = nil - sn.NodeMeta = nil - sn.ServiceWeights = Weights{Passing: 1, Warning: 1} - if !reflect.DeepEqual(sn, sn2) { - t.Fatalf("bad: %#v, but expected %#v", sn2, sn) - } -} - func TestStructs_ServiceNode_Conversions(t *testing.T) { sn := testServiceNode(t) @@ -344,6 +327,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) { From b29fcd5ba61143becf7c3e112080dbf54630f696 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Wed, 10 Oct 2018 19:12:32 +0200 Subject: [PATCH 10/12] Clearer documentation in IsSameService --- agent/structs/structs.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 64762eb6b870..8134421eee2f 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -791,9 +791,10 @@ func (s *NodeService) IsSame(other *NodeService) bool { return true } -// IsSameService checks if one ServiceNode is the same as another, without looking -// at the Raft 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 +// 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. From f8bf4c8585f5a58680481ebdf570a39ef8dbd136 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Wed, 10 Oct 2018 23:28:07 +0200 Subject: [PATCH 11/12] Take into account ServiceProxy into ServiceNode.IsSameService() --- agent/structs/structs.go | 1 + 1 file changed, 1 insertion(+) diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 8134421eee2f..6035496632a4 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -816,6 +816,7 @@ func (s *ServiceNode) IsSameService(other *ServiceNode) bool { !reflect.DeepEqual(s.ServiceWeights, other.ServiceWeights) || s.ServiceEnableTagOverride != other.ServiceEnableTagOverride || s.ServiceProxyDestination != other.ServiceProxyDestination || + !reflect.DeepEqual(s.ServiceProxy, other.ServiceProxy) || s.ServiceConnect != other.ServiceConnect { return false } From 9c1ffd6313c0640ec8550e9d9c5c04a82c7414bf Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Thu, 11 Oct 2018 01:19:54 +0200 Subject: [PATCH 12/12] Fixed IsSameService() with all new structures --- agent/structs/structs.go | 2 +- agent/structs/structs_test.go | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/agent/structs/structs.go b/agent/structs/structs.go index 6035496632a4..0cd02eab23fa 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -817,7 +817,7 @@ func (s *ServiceNode) IsSameService(other *ServiceNode) bool { s.ServiceEnableTagOverride != other.ServiceEnableTagOverride || s.ServiceProxyDestination != other.ServiceProxyDestination || !reflect.DeepEqual(s.ServiceProxy, other.ServiceProxy) || - s.ServiceConnect != other.ServiceConnect { + !reflect.DeepEqual(s.ServiceConnect, other.ServiceConnect) { return false } diff --git a/agent/structs/structs_test.go b/agent/structs/structs_test.go index bebcf9a1c8cf..1404662b65ea 100644 --- a/agent/structs/structs_test.go +++ b/agent/structs/structs_test.go @@ -234,6 +234,9 @@ func TestStructs_ServiceNode_IsSameService(t *testing.T) { 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) @@ -263,8 +266,11 @@ func TestStructs_ServiceNode_IsSameService(t *testing.T) { 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) {