fixup! add peer locality to discovery chains

hashicorp · Mar 10, 2023 · 4331622 · 4331622
1 parent 045e0ab
commit 4331622
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 20 deletions.
diff --git a/agent/configentry/discoverychain.go b/agent/configentry/discoverychain.go
@@ -114,6 +114,16 @@ func (e *DiscoveryChainSet) AddProxyDefaults(entries ...*structs.ProxyConfigEntr
 	}
 }
 
+// AddPeers adds cluster peers. Convenience function for testing.
+func (e *DiscoveryChainSet) AddPeers(entries ...*pbpeering.Peering) {
+	if e.Peers == nil {
+		e.Peers = make(map[string]*pbpeering.Peering)
+	}
+	for _, entry := range entries {
+		e.Peers[entry.Name] = entry
+	}
+}
+
 // AddEntries adds generic configs. Convenience function for testing. Panics on
 // operator error.
 func (e *DiscoveryChainSet) AddEntries(entries ...structs.ConfigEntry) {

diff --git a/agent/consul/discoverychain/compile.go b/agent/consul/discoverychain/compile.go
@@ -737,8 +737,9 @@ func (c *compiler) newTarget(opts structs.DiscoveryTargetOpts) *structs.Discover
 		// Use the same representation for the name. This will NOT be overridden
 		// later.
 		t.Name = t.SNI
+	} else {
 		peer := c.entries.Peers[opts.Peer]
-		if peer != nil && peer.Remote != nil && peer.Remote.Locality != nil {
+		if peer != nil && peer.Remote != nil {
 			t.Locality = pbpeering.LocalityToStructs(peer.Remote.Locality)
 		}
 	}

diff --git a/agent/consul/discoverychain/compile_test.go b/agent/consul/discoverychain/compile_test.go
@@ -9,6 +9,8 @@ import (
 	"github.com/hashicorp/consul/agent/configentry"
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
+	"github.com/hashicorp/consul/proto/private/pbcommon"
+	"github.com/hashicorp/consul/proto/private/pbpeering"
 )
 
 type compileTestCase struct {
@@ -1578,12 +1580,25 @@ func testcase_Failover_Targets() compileTestCase {
 						{Datacenter: "dc3"},
 						{Service: "new-main"},
 						{Peer: "cluster-01"},
+						{Peer: "cluster-02"},
 					},
 				},
 			},
 		},
 	)
 
+	entries.AddPeers(
+		&pbpeering.Peering{
+			Name: "cluster-01",
+			Remote: &pbpeering.RemoteInfo{
+				Locality: &pbcommon.Locality{
+					Region: "us-west-1",
+					Zone:   "us-west-1a",
+				},
+			},
+		},
+	)
+
 	expect := &structs.CompiledDiscoveryChain{
 		Protocol:  "tcp",
 		StartNode: "resolver:main.default.default.dc1",
@@ -1599,6 +1614,7 @@ func testcase_Failover_Targets() compileTestCase {
 							"main.default.default.dc3",
 							"new-main.default.default.dc1",
 							"main.default.default.external.cluster-01",
+							"main.default.default.external.cluster-02",
 						},
 					},
 				},
@@ -1626,6 +1642,21 @@ func testcase_Failover_Targets() compileTestCase {
 			"main.default.default.external.cluster-01": newTarget(structs.DiscoveryTargetOpts{
 				Service: "main",
 				Peer:    "cluster-01",
+			}, func(t *structs.DiscoveryTarget) {
+				t.SNI = ""
+				t.Name = ""
+				t.Datacenter = ""
+				t.MeshGateway = structs.MeshGatewayConfig{
+					Mode: structs.MeshGatewayModeRemote,
+				}
+				t.Locality = &structs.Locality{
+					Region: "us-west-1",
+					Zone:   "us-west-1a",
+				}
+			}),
+			"main.default.default.external.cluster-02": newTarget(structs.DiscoveryTargetOpts{
+				Service: "main",
+				Peer:    "cluster-02",
 			}, func(t *structs.DiscoveryTarget) {
 				t.SNI = ""
 				t.Name = ""

diff --git a/agent/consul/state/config_entry.go b/agent/consul/state/config_entry.go
@@ -3,7 +3,6 @@ package state
 import (
 	"errors"
 	"fmt"
-	"strings"
 
 	memdb "github.com/hashicorp/go-memdb"
 
@@ -1444,7 +1443,7 @@ func readDiscoveryChainConfigEntriesTxn(
 	peerEntMeta := structs.DefaultEnterpriseMetaInPartition(entMeta.PartitionOrDefault())
 	for peerName := range todoPeers {
 		q := Query{
-			Value:          strings.ToLower(peerName),
+			Value:          peerName,
 			EnterpriseMeta: *peerEntMeta,
 		}
 		idx, entry, err := peeringReadTxn(tx, ws, q)
@@ -1455,11 +1454,6 @@ func readDiscoveryChainConfigEntriesTxn(
 			maxIdx = idx
 		}
 
-		if entry == nil {
-			res.Peers[peerName] = nil
-			continue
-		}
-
 		res.Peers[peerName] = entry
 	}
 

diff --git a/agent/consul/state/config_entry_test.go b/agent/consul/state/config_entry_test.go
@@ -2067,7 +2067,7 @@ func TestStore_ReadDiscoveryChainConfigEntries_SubsetSplit(t *testing.T) {
 	require.Len(t, entrySet.Services, 1)
 }
 
-func TestStore_ReadDiscoveryChainConfigEntries_PeerLocality(t *testing.T) {
+func TestStore_ReadDiscoveryChainConfigEntries_FetchPeers(t *testing.T) {
 	s := testConfigStateStore(t)
 
 	entries := []structs.ConfigEntry{

diff --git a/agent/structs/config_entry_discoverychain.go b/agent/structs/config_entry_discoverychain.go
@@ -18,6 +18,7 @@ import (
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/cache"
 	"github.com/hashicorp/consul/lib"
+	"github.com/hashicorp/consul/lib/maps"
 )
 
 const (
@@ -888,17 +889,7 @@ func (e *ServiceResolverConfigEntry) RelatedPeers() []string {
 		}
 	}
 
-	if len(peers) == 0 {
-		return nil
-	}
-
-	out := make([]string, 0, len(peers))
-	for svc := range peers {
-		out = append(out, svc)
-	}
-	sort.Strings(out)
-
-	return out
+	return maps.SliceOfKeys(peers)
 }
 
 func (e *ServiceResolverConfigEntry) MarshalJSON() ([]byte, error) {