From 166a8b2a58b5c643cdb1514a871bc5ff97add769 Mon Sep 17 00:00:00 2001 From: Freddy Date: Fri, 12 Jun 2020 13:46:17 -0600 Subject: [PATCH] Only pass one hostname via EDS and prefer healthy ones (#8084) Co-authored-by: Matt Keeler 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 | 2 +- agent/xds/clusters.go | 54 ++++++++++++++++--- ...mesh-gateway-ignore-extra-resolvers.golden | 12 ----- .../mesh-gateway-service-subsets.golden | 12 ----- .../mesh-gateway-service-timeouts.golden | 12 ----- ...esh-gateway-using-federation-states.golden | 12 ----- .../xds/testdata/clusters/mesh-gateway.golden | 12 ----- ...ng-gateway-hostname-service-subsets.golden | 12 ----- ...ting-gateway-ignore-extra-resolvers.golden | 12 ----- ...terminating-gateway-service-subsets.golden | 12 ----- .../clusters/terminating-gateway.golden | 12 ----- 11 files changed, 48 insertions(+), 116 deletions(-) diff --git a/agent/proxycfg/state.go b/agent/proxycfg/state.go index 97801a84e3..7f46177ea1 100644 --- a/agent/proxycfg/state.go +++ b/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 diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 99c15a34a6..c081a7bdf3 100644 --- a/agent/xds/clusters.go +++ b/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 { - endpoints = append(endpoints, makeLbEndpoint(e, opts.isRemote, opts.onlyPassing)) + var ( + 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 { - health, weight := calculateEndpointHealthAndWeight(csn, onlyPassing) - addr, port := csn.BestAddress(isRemote) - +func makeLbEndpoint(addr string, port int, health envoycore.HealthStatus, weight int) envoyendpoint.LbEndpoint { return envoyendpoint.LbEndpoint{ HostIdentifier: &envoyendpoint.LbEndpoint_Endpoint{ Endpoint: &envoyendpoint.Endpoint{ diff --git a/agent/xds/testdata/clusters/mesh-gateway-ignore-extra-resolvers.golden b/agent/xds/testdata/clusters/mesh-gateway-ignore-extra-resolvers.golden index 95e66aad53..b3401212b1 100644 --- a/agent/xds/testdata/clusters/mesh-gateway-ignore-extra-resolvers.golden +++ b/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 } ] } diff --git a/agent/xds/testdata/clusters/mesh-gateway-service-subsets.golden b/agent/xds/testdata/clusters/mesh-gateway-service-subsets.golden index 95e66aad53..b3401212b1 100644 --- a/agent/xds/testdata/clusters/mesh-gateway-service-subsets.golden +++ b/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 } ] } diff --git a/agent/xds/testdata/clusters/mesh-gateway-service-timeouts.golden b/agent/xds/testdata/clusters/mesh-gateway-service-timeouts.golden index 7941d1510b..56b4721ea0 100644 --- a/agent/xds/testdata/clusters/mesh-gateway-service-timeouts.golden +++ b/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 } ] } diff --git a/agent/xds/testdata/clusters/mesh-gateway-using-federation-states.golden b/agent/xds/testdata/clusters/mesh-gateway-using-federation-states.golden index a2b1ade8e2..68093d0bcf 100644 --- a/agent/xds/testdata/clusters/mesh-gateway-using-federation-states.golden +++ b/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 } ] } diff --git a/agent/xds/testdata/clusters/mesh-gateway.golden b/agent/xds/testdata/clusters/mesh-gateway.golden index a2b1ade8e2..68093d0bcf 100644 --- a/agent/xds/testdata/clusters/mesh-gateway.golden +++ b/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 } ] } diff --git a/agent/xds/testdata/clusters/terminating-gateway-hostname-service-subsets.golden b/agent/xds/testdata/clusters/terminating-gateway-hostname-service-subsets.golden index 1ff54b0bea..81df077841 100644 --- a/agent/xds/testdata/clusters/terminating-gateway-hostname-service-subsets.golden +++ b/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": { diff --git a/agent/xds/testdata/clusters/terminating-gateway-ignore-extra-resolvers.golden b/agent/xds/testdata/clusters/terminating-gateway-ignore-extra-resolvers.golden index e9261c21c9..9d6568baff 100644 --- a/agent/xds/testdata/clusters/terminating-gateway-ignore-extra-resolvers.golden +++ b/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": { diff --git a/agent/xds/testdata/clusters/terminating-gateway-service-subsets.golden b/agent/xds/testdata/clusters/terminating-gateway-service-subsets.golden index e9261c21c9..9d6568baff 100644 --- a/agent/xds/testdata/clusters/terminating-gateway-service-subsets.golden +++ b/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": { diff --git a/agent/xds/testdata/clusters/terminating-gateway.golden b/agent/xds/testdata/clusters/terminating-gateway.golden index a4c4733055..1dde72869a 100644 --- a/agent/xds/testdata/clusters/terminating-gateway.golden +++ b/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": {