From 1ee11bea4a95c67b0c9d2aed2ea425e169b6b378 Mon Sep 17 00:00:00 2001 From: freddygv Date: Tue, 9 Jun 2020 19:20:51 -0600 Subject: [PATCH 1/4] Only pass one hostname via EDS and prefer healthy ones --- agent/xds/clusters.go | 55 ++++++++++++++++--- ...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 ---- 10 files changed, 48 insertions(+), 115 deletions(-) diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 99c15a34a69..b1799412958 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,53 @@ 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) + + var ( + hostname string + idx int + ) + for i, e := range opts.hostnameEndpoints { + addr, port := e.BestAddress(opts.isRemote) + if !uniqueHostnames[addr] { + 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 + } + } - for _, e := range opts.hostnameEndpoints { - endpoints = append(endpoints, makeLbEndpoint(e, opts.isRemote, opts.onlyPassing)) + 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) > 0 { + 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 +727,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 95e66aad533..b3401212b19 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 95e66aad533..b3401212b19 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 7941d1510b1..56b4721ea09 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 a2b1ade8e21..68093d0bcf4 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 a2b1ade8e21..68093d0bcf4 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 1ff54b0bea5..81df077841a 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 e9261c21c96..9d6568baff9 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 e9261c21c96..9d6568baff9 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 a4c47330552..1dde72869a5 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": { From 9568945d56ac082080c792ee00e84776ba6f9904 Mon Sep 17 00:00:00 2001 From: Freddy Date: Thu, 11 Jun 2020 10:59:03 -0600 Subject: [PATCH 2/4] Update agent/xds/clusters.go Co-authored-by: Matt Keeler --- agent/xds/clusters.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index b1799412958..0c8835416ac 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -642,9 +642,7 @@ func (s *Server) makeGatewayCluster(snap *proxycfg.ConfigSnapshot, opts gatewayC ) for i, e := range opts.hostnameEndpoints { addr, port := e.BestAddress(opts.isRemote) - if !uniqueHostnames[addr] { - uniqueHostnames[addr] = true - } + uniqueHostnames[addr] = true health, weight := calculateEndpointHealthAndWeight(e, opts.onlyPassing) if health == envoycore.HealthStatus_UNHEALTHY { From fab08337f6bdd207981759e49dd8bcb57815d14f Mon Sep 17 00:00:00 2001 From: Freddy Date: Thu, 11 Jun 2020 11:01:16 -0600 Subject: [PATCH 3/4] Update agent/xds/clusters.go Co-authored-by: Matt Keeler --- agent/xds/clusters.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 0c8835416ac..113b5d741b1 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -672,7 +672,7 @@ func (s *Server) makeGatewayCluster(snap *proxycfg.ConfigSnapshot, opts gatewayC return nil } - if len(uniqueHostnames) > 0 { + 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()) From e9ab735d686d086032b34718c81adf29dafcd0a4 Mon Sep 17 00:00:00 2001 From: freddygv Date: Fri, 12 Jun 2020 09:41:27 -0600 Subject: [PATCH 4/4] PR comments --- agent/proxycfg/state.go | 2 +- agent/xds/clusters.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/agent/proxycfg/state.go b/agent/proxycfg/state.go index 97801a84e38..7f46177ea12 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 113b5d741b1..c081a7bdf3a 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -654,6 +654,7 @@ func (s *Server) makeGatewayCluster(snap *proxycfg.ConfigSnapshot, opts gatewayC hostname = addr idx = i + break } } @@ -674,7 +675,7 @@ func (s *Server) makeGatewayCluster(snap *proxycfg.ConfigSnapshot, opts gatewayC } 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), + Warn(fmt.Sprintf("service contains instances with more than one unique hostname; only %q be resolved by Envoy", hostname), "dc", dc, "service", service.String()) }