Only pass one hostname via EDS and prefer healthy ones (#8084)

Co-authored-by: Matt Keeler <mkeeler@users.noreply.github.com>

Currently when passing hostname clusters to Envoy, we set each service instance registered with Consul as an LbEndpoint for the cluster.

However, Envoy can only handle one per cluster:
[2020-06-04 18:32:34.094][1][warning][config] [source/common/config/grpc_subscription_impl.cc:87] gRPC config for type.googleapis.com/envoy.api.v2.Cluster rejected: Error adding/updating cluster(s) dc2.internal.ddd90499-9b47-91c5-4616-c0cbf0fc358a.consul: LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint, server.dc2.consul: LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint

Envoy is currently handling this gracefully by only picking one of the endpoints. However, we should avoid passing multiple to avoid these warning logs.

This PR:

* Ensures we only pass one endpoint, which is tied to one service instance.
* We prefer sending an endpoint which is marked as Healthy by Consul.
* If no endpoints are healthy we emit a warning and skip the cluster.
* If multiple unique hostnames are spread across service instances we emit a warning and let the user know which will be resolved.

agent/proxycfg/state.go

@@ -1568,7 +1568,7 @@ func (s *state) hostnameEndpoints(loggerName string, localDC string, nodes struc
 		sid := nodes[0].Service.CompoundServiceName()
 
 		s.logger.Named(loggerName).
-			Warn("service contains instances with mix of hostnames and IP addresses; only hostnames will be passed to Envoy.",
+			Warn("service contains instances with mix of hostnames and IP addresses; only hostnames will be passed to Envoy",
 				"dc", dc, "service", sid.String())
 	}
 	return resp

agent/xds/clusters.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"github.com/hashicorp/consul/logging"
 	"time"
 
 	envoy "github.com/envoyproxy/go-control-plane/envoy/api/v2"
@@ -582,6 +583,7 @@ type gatewayClusterOpts struct {
 	hostnameEndpoints structs.CheckServiceNodes
 }
 
+// makeGatewayCluster creates an Envoy cluster for a mesh or terminating gateway
 func (s *Server) makeGatewayCluster(snap *proxycfg.ConfigSnapshot, opts gatewayClusterOpts) *envoy.Cluster {
 	cfg, err := ParseGatewayConfig(snap.Proxy.Config)
 	if err != nil {
@@ -631,11 +633,52 @@ func (s *Server) makeGatewayCluster(snap *proxycfg.ConfigSnapshot, opts gatewayC
 	}
 	cluster.ClusterDiscoveryType = &discoveryType
 
-	endpoints := make([]envoyendpoint.LbEndpoint, 0, len(opts.hostnameEndpoints))
+	endpoints := make([]envoyendpoint.LbEndpoint, 0, 1)
+	uniqueHostnames := make(map[string]bool)
 
-	for _, e := range opts.hostnameEndpoints {
+	var (
-		endpoints = append(endpoints, makeLbEndpoint(e, opts.isRemote, opts.onlyPassing))
+		hostname string
+		idx      int
+	)
+	for i, e := range opts.hostnameEndpoints {
+		addr, port := e.BestAddress(opts.isRemote)
+		uniqueHostnames[addr] = true
+
+		health, weight := calculateEndpointHealthAndWeight(e, opts.onlyPassing)
+		if health == envoycore.HealthStatus_UNHEALTHY {
+			continue
 		}
+
+		if len(endpoints) == 0 {
+			endpoints = append(endpoints, makeLbEndpoint(addr, port, health, weight))
+
+			hostname = addr
+			idx = i
+			break
+		}
+	}
+
+	dc := opts.hostnameEndpoints[idx].Node.Datacenter
+	service := opts.hostnameEndpoints[idx].Service.CompoundServiceName()
+
+	loggerName := logging.TerminatingGateway
+	if snap.Kind == structs.ServiceKindMeshGateway {
+		loggerName = logging.MeshGateway
+	}
+
+	if len(endpoints) == 0 {
+		s.Logger.Named(loggerName).
+			Warn("service does not contain any healthy instances, skipping Envoy cluster creation",
+				"dc", dc, "service", service.String())
+
+		return nil
+	}
+	if len(uniqueHostnames) > 1 {
+		s.Logger.Named(loggerName).
+			Warn(fmt.Sprintf("service contains instances with more than one unique hostname; only %q be resolved by Envoy", hostname),
+				"dc", dc, "service", service.String())
+	}
+
 	cluster.LoadAssignment = &envoy.ClusterLoadAssignment{
 		ClusterName: cluster.Name,
 		Endpoints: []envoyendpoint.LocalityLbEndpoints{
@@ -683,10 +726,7 @@ func makeThresholdsIfNeeded(limits UpstreamLimits) []*envoycluster.CircuitBreake
 	return []*envoycluster.CircuitBreakers_Thresholds{threshold}
 }
 
-func makeLbEndpoint(csn structs.CheckServiceNode, isRemote, onlyPassing bool) envoyendpoint.LbEndpoint {
+func makeLbEndpoint(addr string, port int, health envoycore.HealthStatus, weight int) envoyendpoint.LbEndpoint {
-	health, weight := calculateEndpointHealthAndWeight(csn, onlyPassing)
-	addr, port := csn.BestAddress(isRemote)
-
 	return envoyendpoint.LbEndpoint{
 		HostIdentifier: &envoyendpoint.LbEndpoint_Endpoint{
 			Endpoint: &envoyendpoint.Endpoint{

agent/xds/testdata/clusters/mesh-gateway-ignore-extra-resolvers.golden

@@ -54,18 +54,6 @@
                 },
                 "healthStatus": "HEALTHY",
                 "loadBalancingWeight": 1
-              },
-              {
-                "endpoint": {
-                  "address": {
-                    "socketAddress": {
-                      "address": "456.us-west-2.elb.notaws.com",
-                      "portValue": 443
-                    }
-                  }
-                },
-                "healthStatus": "HEALTHY",
-                "loadBalancingWeight": 1
               }
             ]
           }

agent/xds/testdata/clusters/mesh-gateway-service-subsets.golden

@@ -54,18 +54,6 @@
                 },
                 "healthStatus": "HEALTHY",
                 "loadBalancingWeight": 1
-              },
-              {
-                "endpoint": {
-                  "address": {
-                    "socketAddress": {
-                      "address": "456.us-west-2.elb.notaws.com",
-                      "portValue": 443
-                    }
-                  }
-                },
-                "healthStatus": "HEALTHY",
-                "loadBalancingWeight": 1
               }
             ]
           }

agent/xds/testdata/clusters/mesh-gateway-service-timeouts.golden

@@ -54,18 +54,6 @@
                 },
                 "healthStatus": "HEALTHY",
                 "loadBalancingWeight": 1
-              },
-              {
-                "endpoint": {
-                  "address": {
-                    "socketAddress": {
-                      "address": "456.us-west-2.elb.notaws.com",
-                      "portValue": 443
-                    }
-                  }
-                },
-                "healthStatus": "HEALTHY",
-                "loadBalancingWeight": 1
               }
             ]
           }

agent/xds/testdata/clusters/mesh-gateway-using-federation-states.golden

@@ -54,18 +54,6 @@
                 },
                 "healthStatus": "HEALTHY",
                 "loadBalancingWeight": 1
-              },
-              {
-                "endpoint": {
-                  "address": {
-                    "socketAddress": {
-                      "address": "456.us-west-2.elb.notaws.com",
-                      "portValue": 443
-                    }
-                  }
-                },
-                "healthStatus": "HEALTHY",
-                "loadBalancingWeight": 1
               }
             ]
           }

agent/xds/testdata/clusters/mesh-gateway.golden

@@ -54,18 +54,6 @@
                 },
                 "healthStatus": "HEALTHY",
                 "loadBalancingWeight": 1
-              },
-              {
-                "endpoint": {
-                  "address": {
-                    "socketAddress": {
-                      "address": "456.us-west-2.elb.notaws.com",
-                      "portValue": 443
-                    }
-                  }
-                },
-                "healthStatus": "HEALTHY",
-                "loadBalancingWeight": 1
               }
             ]
           }

agent/xds/testdata/clusters/terminating-gateway-hostname-service-subsets.golden

@@ -65,18 +65,6 @@
         "endpoints": [
           {
             "lbEndpoints": [
-              {
-                "endpoint": {
-                  "address": {
-                    "socketAddress": {
-                      "address": "api.mydomain",
-                      "portValue": 8081
-                    }
-                  }
-                },
-                "healthStatus": "UNHEALTHY",
-                "loadBalancingWeight": 1
-              },
               {
                 "endpoint": {
                   "address": {

agent/xds/testdata/clusters/terminating-gateway-ignore-extra-resolvers.golden

@@ -11,18 +11,6 @@
         "endpoints": [
           {
             "lbEndpoints": [
-              {
-                "endpoint": {
-                  "address": {
-                    "socketAddress": {
-                      "address": "api.mydomain",
-                      "portValue": 8081
-                    }
-                  }
-                },
-                "healthStatus": "UNHEALTHY",
-                "loadBalancingWeight": 1
-              },
               {
                 "endpoint": {
                   "address": {

agent/xds/testdata/clusters/terminating-gateway-service-subsets.golden

@@ -11,18 +11,6 @@
         "endpoints": [
           {
             "lbEndpoints": [
-              {
-                "endpoint": {
-                  "address": {
-                    "socketAddress": {
-                      "address": "api.mydomain",
-                      "portValue": 8081
-                    }
-                  }
-                },
-                "healthStatus": "UNHEALTHY",
-                "loadBalancingWeight": 1
-              },
               {
                 "endpoint": {
                   "address": {

agent/xds/testdata/clusters/terminating-gateway.golden

@@ -11,18 +11,6 @@
         "endpoints": [
           {
             "lbEndpoints": [
-              {
-                "endpoint": {
-                  "address": {
-                    "socketAddress": {
-                      "address": "api.mydomain",
-                      "portValue": 8081
-                    }
-                  }
-                },
-                "healthStatus": "UNHEALTHY",
-                "loadBalancingWeight": 1
-              },
               {
                 "endpoint": {
                   "address": {