Use Node Name for peering healthSnapshot instead of ID (#13773)

A Node ID is not a required field with Consul’s data model. Therefore we cannot reliably expect all uses to have it. However the node name is required and must be unique so its equally as good of a key for the internal healthSnapshot node tracking.

agent/grpc-external/services/peerstream/health_snapshot.go

@@ -8,7 +8,11 @@ import (
 // healthSnapshot represents a normalized view of a set of CheckServiceNodes
 // meant for easy comparison to aid in differential synchronization
 type healthSnapshot struct {
-	Nodes map[types.NodeID]*nodeSnapshot
+	// Nodes is a map of a node name to a nodeSnapshot. Ideally we would be able to use
+	// the types.NodeID and assume they are UUIDs for the map key but Consul doesn't
+	// require a NodeID. Therefore we must key off of the only bit of ID material
+	// that is required which is the node name.
+	Nodes map[string]*nodeSnapshot
 }
 
 type nodeSnapshot struct {
@@ -40,20 +44,20 @@ func newHealthSnapshot(all []structs.CheckServiceNode, partition, peerName strin
 	}
 
 	snap := &healthSnapshot{
-		Nodes: make(map[types.NodeID]*nodeSnapshot),
+		Nodes: make(map[string]*nodeSnapshot),
 	}
 
 	for _, instance := range all {
-		if instance.Node.ID == "" {
+		if instance.Node.Node == "" {
-			panic("TODO(peering): data should always have a node ID")
+			panic("TODO(peering): data should always have a node name")
 		}
-		nodeSnap, ok := snap.Nodes[instance.Node.ID]
+		nodeSnap, ok := snap.Nodes[instance.Node.Node]
 		if !ok {
 			nodeSnap = &nodeSnapshot{
 				Node:     instance.Node,
 				Services: make(map[structs.ServiceID]*serviceSnapshot),
 			}
-			snap.Nodes[instance.Node.ID] = nodeSnap
+			snap.Nodes[instance.Node.Node] = nodeSnap
 		}
 
 		if instance.Service.ID == "" {

agent/grpc-external/services/peerstream/health_snapshot_test.go

@@ -69,8 +69,8 @@ func TestHealthSnapshot(t *testing.T) {
 				},
 			},
 			expect: &healthSnapshot{
-				Nodes: map[types.NodeID]*nodeSnapshot{
+				Nodes: map[string]*nodeSnapshot{
-					"abc-123": {
+					"abc": {
 						Node: newNode("abc-123", "abc", "my-peer"),
 						Services: map[structs.ServiceID]*serviceSnapshot{
 							structs.NewServiceID("xyz-123", nil): {
@@ -88,14 +88,14 @@ func TestHealthSnapshot(t *testing.T) {
 			name: "multiple",
 			in: []structs.CheckServiceNode{
 				{
-					Node:    newNode("abc-123", "abc", ""),
+					Node:    newNode("", "abc", ""),
 					Service: newService("xyz-123", 8080, ""),
 					Checks: structs.HealthChecks{
 						newCheck("abc", "xyz-123", ""),
 					},
 				},
 				{
-					Node:    newNode("abc-123", "abc", ""),
+					Node:    newNode("", "abc", ""),
 					Service: newService("xyz-789", 8181, ""),
 					Checks: structs.HealthChecks{
 						newCheck("abc", "xyz-789", ""),
@@ -110,9 +110,9 @@ func TestHealthSnapshot(t *testing.T) {
 				},
 			},
 			expect: &healthSnapshot{
-				Nodes: map[types.NodeID]*nodeSnapshot{
+				Nodes: map[string]*nodeSnapshot{
-					"abc-123": {
+					"abc": {
-						Node: newNode("abc-123", "abc", "my-peer"),
+						Node: newNode("", "abc", "my-peer"),
 						Services: map[structs.ServiceID]*serviceSnapshot{
 							structs.NewServiceID("xyz-123", nil): {
 								Service: newService("xyz-123", 8080, "my-peer"),
@@ -128,7 +128,7 @@ func TestHealthSnapshot(t *testing.T) {
 							},
 						},
 					},
-					"def-456": {
+					"def": {
 						Node: newNode("def-456", "def", "my-peer"),
 						Services: map[structs.ServiceID]*serviceSnapshot{
 							structs.NewServiceID("xyz-456", nil): {

agent/grpc-external/services/peerstream/replication.go

@@ -290,8 +290,8 @@ func (s *Server) handleUpdateService(
 		deletedNodeChecks = make(map[nodeCheckTuple]struct{})
 	)
 	for _, csn := range storedInstances {
-		if _, ok := snap.Nodes[csn.Node.ID]; !ok {
+		if _, ok := snap.Nodes[csn.Node.Node]; !ok {
-			unusedNodes[string(csn.Node.ID)] = struct{}{}
+			unusedNodes[csn.Node.Node] = struct{}{}
 
 			// Since the node is not in the snapshot we can know the associated service
 			// instance is not in the snapshot either, since a service instance can't
@@ -316,7 +316,7 @@ func (s *Server) handleUpdateService(
 
 		// Delete the service instance if not in the snapshot.
 		sid := csn.Service.CompoundServiceID()
-		if _, ok := snap.Nodes[csn.Node.ID].Services[sid]; !ok {
+		if _, ok := snap.Nodes[csn.Node.Node].Services[sid]; !ok {
 			err := s.Backend.CatalogDeregister(&structs.DeregisterRequest{
 				Node:           csn.Node.Node,
 				ServiceID:      csn.Service.ID,
@@ -335,7 +335,7 @@ func (s *Server) handleUpdateService(
 
 		// Reconcile checks.
 		for _, chk := range csn.Checks {
-			if _, ok := snap.Nodes[csn.Node.ID].Services[sid].Checks[chk.CheckID]; !ok {
+			if _, ok := snap.Nodes[csn.Node.Node].Services[sid].Checks[chk.CheckID]; !ok {
 				// Checks without a ServiceID are node checks.
 				// If the node exists but the check does not then the check was deleted.
 				if chk.ServiceID == "" {