From bb5c2e802b78a7a7c515b63cfe29e1e114ce1981 Mon Sep 17 00:00:00 2001
From: "R.B. Boyer" <rb@hashicorp.com>
Date: Fri, 5 Feb 2021 16:28:07 -0600
Subject: [PATCH] xds: deduplicate mesh gateway listeners in a stable way
 (#9650)

In a situation where the mesh gateway is configured to bind to multiple
network interfaces, we use a feature called 'tagged addresses'.
Sometimes an address is duplicated across multiple tags such as 'lan'
and 'lan_ipv4'.

There is code to deduplicate these things when creating envoy listeners,
but that code doesn't ensure that the same tag wins every time. If the
winning tag flaps between xDS discovery requests it will cause the
listener to be drained and replaced.
---
 .changelog/9650.txt         |  3 +++
 agent/xds/listeners.go      | 38 +++++++++++++++++++++----------------
 agent/xds/listeners_test.go | 10 +++++-----
 3 files changed, 30 insertions(+), 21 deletions(-)
 create mode 100644 .changelog/9650.txt

diff --git a/.changelog/9650.txt b/.changelog/9650.txt
new file mode 100644
index 0000000000..f0d404a68b
--- /dev/null
+++ b/.changelog/9650.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+xds: deduplicate mesh gateway listeners by address in a stable way to prevent some LDS churn
+```
diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go
index e13193ad6c..b928ef1385 100644
--- a/agent/xds/listeners.go
+++ b/agent/xds/listeners.go
@@ -7,6 +7,7 @@ import (
 	"net"
 	"net/url"
 	"regexp"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -198,13 +199,11 @@ func (s *Server) listenersFromSnapshotGateway(cInfo connectionInfo, cfgSnap *pro
 		s.Logger.Warn("failed to parse Connect.Proxy.Config", "error", err)
 	}
 
-	// Prevent invalid configurations of binding to the same port/addr twice
-	// including with the any addresses
+	// We'll collect all of the desired listeners first, and deduplicate them later.
 	type namedAddress struct {
 		name string
 		structs.ServiceAddress
 	}
-	seen := make(map[structs.ServiceAddress]bool)
 	addrs := make([]namedAddress, 0)
 
 	var resources []proto.Message
@@ -218,10 +217,7 @@ func (s *Server) listenersFromSnapshotGateway(cInfo connectionInfo, cfgSnap *pro
 			Address: addr,
 			Port:    cfgSnap.Port,
 		}
-		if !seen[a] {
-			addrs = append(addrs, namedAddress{name: "default", ServiceAddress: a})
-			seen[a] = true
-		}
+		addrs = append(addrs, namedAddress{name: "default", ServiceAddress: a})
 	}
 
 	if cfg.BindTaggedAddresses {
@@ -230,10 +226,7 @@ func (s *Server) listenersFromSnapshotGateway(cInfo connectionInfo, cfgSnap *pro
 				Address: addrCfg.Address,
 				Port:    addrCfg.Port,
 			}
-			if !seen[a] {
-				addrs = append(addrs, namedAddress{name: name, ServiceAddress: a})
-				seen[a] = true
-			}
+			addrs = append(addrs, namedAddress{name: name, ServiceAddress: a})
 		}
 	}
 
@@ -242,14 +235,27 @@ func (s *Server) listenersFromSnapshotGateway(cInfo connectionInfo, cfgSnap *pro
 			Address: addrCfg.Address,
 			Port:    addrCfg.Port,
 		}
-		if !seen[a] {
-			addrs = append(addrs, namedAddress{name: name, ServiceAddress: a})
-			seen[a] = true
-		}
+		addrs = append(addrs, namedAddress{name: name, ServiceAddress: a})
 	}
 
-	// Make listeners once deduplicated
+	// Prevent invalid configurations of binding to the same port/addr twice
+	// including with the any addresses
+	//
+	// Sort the list and then if two items share a service address, take the
+	// first one to ensure we generate one listener per address and it's
+	// stable.
+	sort.Slice(addrs, func(i, j int) bool {
+		return addrs[i].name < addrs[j].name
+	})
+
+	// Make listeners and deduplicate on the fly.
+	seen := make(map[structs.ServiceAddress]bool)
 	for _, a := range addrs {
+		if seen[a.ServiceAddress] {
+			continue
+		}
+		seen[a.ServiceAddress] = true
+
 		var l *envoy.Listener
 
 		switch cfgSnap.Kind {
diff --git a/agent/xds/listeners_test.go b/agent/xds/listeners_test.go
index 3f200368f9..28becd6eb6 100644
--- a/agent/xds/listeners_test.go
+++ b/agent/xds/listeners_test.go
@@ -345,15 +345,15 @@ func TestListenersFromSnapshot(t *testing.T) {
 					"envoy_gateway_no_default_bind":       true,
 					"envoy_gateway_bind_tagged_addresses": true,
 					"envoy_gateway_bind_addresses": map[string]structs.ServiceAddress{
+						// This bind address should not get a listener due to deduplication and it sorts to the end
+						"z-duplicate-of-tagged-wan-addr": {
+							Address: "198.18.0.1",
+							Port:    443,
+						},
 						"foo": {
 							Address: "198.17.2.3",
 							Port:    8080,
 						},
-						// This bind address should not get a listener due to deduplication
-						"duplicate-of-tagged-wan-addr": {
-							Address: "198.18.0.1",
-							Port:    443,
-						},
 					},
 				}
 			},