xds: prevent LDS flaps in mesh gateways due to unstable datacenter li…

…sts (#9651) 1.7.x backport of #9651
hashicorp · Feb 8, 2021 · 733d41a · 733d41a
1 parent 111b627
commit 733d41a
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 6 deletions.
diff --git a/.changelog/9651.txt b/.changelog/9651.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+xds: prevent LDS flaps in mesh gateways due to unstable datacenter lists; also prevent some flaps in terminating gateways as well
+```
diff --git a/agent/proxycfg/snapshot.go b/agent/proxycfg/snapshot.go
@@ -2,6 +2,7 @@ package proxycfg
 
 import (
 	"context"
+	"sort"
 
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/mitchellh/copystructure"
@@ -55,6 +56,20 @@ type configSnapshotMeshGateway struct {
 	GatewayGroups map[string]structs.CheckServiceNodes
 }
 
+func (c *configSnapshotMeshGateway) Datacenters() []string {
+	sz := len(c.GatewayGroups)
+
+	dcs := make([]string, 0, sz)
+	for dc := range c.GatewayGroups {
+		dcs = append(dcs, dc)
+	}
+
+	// Always sort the results to ensure we generate deterministic things over
+	// xDS, such as mesh-gateway listener filter chains.
+	sort.Strings(dcs)
+	return dcs
+}
+
 func (c *configSnapshotMeshGateway) IsEmpty() bool {
 	if c == nil {
 		return true

diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go
@@ -113,11 +113,13 @@ func makeExposeClusterName(destinationPort int) string {
 // for a mesh gateway. This will include 1 cluster per remote datacenter as well as
 // 1 cluster for each service subset.
 func (s *Server) clustersFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) {
+	datacenters := cfgSnap.MeshGateway.Datacenters()
+
 	// 1 cluster per remote dc + 1 cluster per local service (this is a lower bound - all subset specific clusters will be appended)
-	clusters := make([]proto.Message, 0, len(cfgSnap.MeshGateway.GatewayGroups)+len(cfgSnap.MeshGateway.ServiceGroups))
+	clusters := make([]proto.Message, 0, len(datacenters)+len(cfgSnap.MeshGateway.ServiceGroups))
 
 	// generate the remote dc clusters
-	for dc, _ := range cfgSnap.MeshGateway.GatewayGroups {
+	for _, dc := range datacenters {
 		clusterName := connect.DatacenterSNI(dc, cfgSnap.Roots.TrustDomain)
 
 		cluster, err := s.makeMeshGatewayCluster(clusterName, cfgSnap)

diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go
@@ -169,10 +169,17 @@ func (s *Server) filterSubsetEndpoints(subset *structs.ServiceResolverSubset, en
 }
 
 func (s *Server) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) {
-	resources := make([]proto.Message, 0, len(cfgSnap.MeshGateway.GatewayGroups)+len(cfgSnap.MeshGateway.ServiceGroups))
+	datacenters := cfgSnap.MeshGateway.Datacenters()
+	resources := make([]proto.Message, 0, len(datacenters)+len(cfgSnap.MeshGateway.ServiceGroups))
 
 	// generate the endpoints for the gateways in the remote datacenters
-	for dc, endpoints := range cfgSnap.MeshGateway.GatewayGroups {
+	for _, dc := range datacenters {
+		endpoints, ok := cfgSnap.MeshGateway.GatewayGroups[dc]
+		if !ok { // not possible
+			s.Logger.Error("skipping mesh gateway endpoints because no definition found", "datacenter", dc)
+			continue
+		}
+
 		clusterName := connect.DatacenterSNI(dc, cfgSnap.Roots.TrustDomain)
 		la := makeLoadAssignment(
 			clusterName,

diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go
@@ -549,7 +549,8 @@ func (s *Server) makeGatewayListener(name, addr string, port int, cfgSnap *proxy
 
 	// TODO (mesh-gateway) - Do we need to create clusters for all the old trust domains as well?
 	// We need 1 Filter Chain per datacenter
-	for dc := range cfgSnap.MeshGateway.GatewayGroups {
+	datacenters := cfgSnap.MeshGateway.Datacenters()
+	for _, dc := range datacenters {
 		clusterName := connect.DatacenterSNI(dc, cfgSnap.Roots.TrustDomain)
 		filterName := fmt.Sprintf("%s_%s", name, dc)
 		dcTCPProxy, err := makeTCPProxyFilter(filterName, clusterName, "mesh_gateway_remote_")

diff --git a/agent/xds/listeners_test.go b/agent/xds/listeners_test.go
@@ -298,11 +298,14 @@ func TestListenersFromSnapshot(t *testing.T) {
 						ProxyFeatures: sf,
 					}
 					listeners, err := s.listenersFromSnapshot(cInfo, snap)
+					require.NoError(err)
+
+					// The order of listeners returned via LDS isn't relevant, so it's safe
+					// to sort these for the purposes of test comparisons.
 					sort.Slice(listeners, func(i, j int) bool {
 						return listeners[i].(*envoy.Listener).Name < listeners[j].(*envoy.Listener).Name
 					})
 
-					require.NoError(err)
 					r, err := createResponse(ListenerType, "00000001", "00000001", listeners)
 					require.NoError(err)