diff --git a/consul/state/state_store.go b/consul/state/state_store.go index 0f1d95fb90..7949d89f12 100644 --- a/consul/state/state_store.go +++ b/consul/state/state_store.go @@ -678,9 +678,11 @@ func (s *StateStore) ensureServiceTxn(tx *memdb.Txn, idx uint64, watches *DumbWa return fmt.Errorf("failed service lookup: %s", err) } - // Create the service node entry and populate the indexes. We leave the - // address blank and fill that in on the way out during queries. - entry := svc.ToServiceNode(node, "") + // Create the service node entry and populate the indexes. Note that + // conversion doesn't populate any of the node-specific information + // (Address and TaggedAddresses). 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 @@ -830,18 +832,23 @@ func (s *StateStore) parseServiceNodes(tx *memdb.Txn, services structs.ServiceNo var results structs.ServiceNodes for _, sn := range services { // Note that we have to clone here because we don't want to - // modify the address field on the object in the database, + // modify the node-related fields on the object in the database, // which is what we are referencing. - s := sn.Clone() + s := sn.PartialClone() - // Fill in the address of the node. + // Grab the corresponding node record. n, err := tx.First("nodes", "id", sn.Node) if err != nil { return nil, fmt.Errorf("failed node lookup: %s", err) } + + // Populate the node-related fields. The tagged addresses may be + // used by agents to perform address translation if they are + // configured to do that. node := n.(*structs.Node) s.Address = node.Address s.TaggedAddresses = node.TaggedAddresses + results = append(results, s) } return results, nil diff --git a/consul/structs/structs.go b/consul/structs/structs.go index 23c4c78eb4..f3f8ff9ea6 100644 --- a/consul/structs/structs.go +++ b/consul/structs/structs.go @@ -260,7 +260,11 @@ type Nodes []*Node // Maps service name to available tags type Services map[string][]string -// ServiceNode represents a node that is part of a service +// ServiceNode represents a node that is part of a service. Address and +// TaggedAddresses are node-related fields that are always empty in the state +// store and are filled in on the way out by parseServiceNodes(). This is also +// why PartialClone() skips them, because we know they are blank already so it +// would be a waste of time to copy them. type ServiceNode struct { Node string Address string @@ -275,14 +279,16 @@ type ServiceNode struct { RaftIndex } -// Clone returns a clone of the given service node. -func (s *ServiceNode) Clone() *ServiceNode { +// PartialClone() returns a clone of the given service node, minus the node- +// related fields that get filled in later, Address and TaggedAddresses. +func (s *ServiceNode) PartialClone() *ServiceNode { tags := make([]string, len(s.ServiceTags)) copy(tags, s.ServiceTags) return &ServiceNode{ - Node: s.Node, - Address: s.Address, + Node: s.Node, + // Skip Address, see above. + // Skip TaggedAddresses, see above. ServiceID: s.ServiceID, ServiceName: s.ServiceName, ServiceTags: tags, @@ -344,10 +350,11 @@ func (s *NodeService) IsSame(other *NodeService) bool { } // ToServiceNode converts the given node service to a service node. -func (s *NodeService) ToServiceNode(node, address string) *ServiceNode { +func (s *NodeService) ToServiceNode(node string) *ServiceNode { return &ServiceNode{ - Node: node, - Address: address, + Node: node, + // Skip Address, see ServiceNode definition. + // Skip TaggedAddresses, see ServiceNode definition. ServiceID: s.ID, ServiceName: s.Service, ServiceTags: s.Tags, diff --git a/consul/structs/structs_test.go b/consul/structs/structs_test.go index 8caa66ffe8..11c5b2aea3 100644 --- a/consul/structs/structs_test.go +++ b/consul/structs/structs_test.go @@ -108,8 +108,11 @@ func TestStructs_ACL_IsSame(t *testing.T) { // testServiceNode gives a fully filled out ServiceNode instance. func testServiceNode() *ServiceNode { return &ServiceNode{ - Node: "node1", - Address: "127.0.0.1", + Node: "node1", + Address: "127.0.0.1", + TaggedAddresses: map[string]string{ + "hello": "world", + }, ServiceID: "service1", ServiceName: "dogs", ServiceTags: []string{"prod", "v1"}, @@ -123,10 +126,20 @@ func testServiceNode() *ServiceNode { } } -func TestStructs_ServiceNode_Clone(t *testing.T) { +func TestStructs_ServiceNode_PartialClone(t *testing.T) { sn := testServiceNode() - clone := sn.Clone() + clone := sn.PartialClone() + + // Make sure the parts that weren't supposed to be cloned didn't get + // copied over, then zero-value them out so we can do a DeepEqual() on + // the rest of the contents. + if clone.Address != "" || len(clone.TaggedAddresses) != 0 { + t.Fatalf("bad: %v", clone) + } + + sn.Address = "" + sn.TaggedAddresses = nil if !reflect.DeepEqual(sn, clone) { t.Fatalf("bad: %v", clone) } @@ -140,7 +153,12 @@ func TestStructs_ServiceNode_Clone(t *testing.T) { func TestStructs_ServiceNode_Conversions(t *testing.T) { sn := testServiceNode() - sn2 := sn.ToNodeService().ToServiceNode("node1", "127.0.0.1") + 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.Address = "" + sn.TaggedAddresses = nil if !reflect.DeepEqual(sn, sn2) { t.Fatalf("bad: %v", sn2) }