Log warning when locality is missing with failover

hashicorp · Aug 10, 2023 · 845b71e · 845b71e
1 parent facd5b0
commit 845b71e
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 5 deletions.
diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go
@@ -6,6 +6,7 @@ package xds
 import (
 	"errors"
 	"fmt"
+	"github.com/hashicorp/go-hclog"
 	"strconv"
 
 	envoy_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
@@ -136,6 +137,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg.
 		endpoints, ok := cfgSnap.ConnectProxy.PreparedQueryEndpoints[uid]
 		if ok {
 			la := makeLoadAssignment(
+				s.Logger,
 				cfgSnap,
 				clusterName,
 				nil,
@@ -161,6 +163,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg.
 			endpoints, ok := cfgSnap.ConnectProxy.DestinationGateways.Get(uid)
 			if ok {
 				la := makeLoadAssignment(
+					s.Logger,
 					cfgSnap,
 					name,
 					nil,
@@ -229,6 +232,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.C
 			clusterName := connect.GatewaySNI(key.Datacenter, key.Partition, cfgSnap.Roots.TrustDomain)
 
 			la := makeLoadAssignment(
+				s.Logger,
 				cfgSnap,
 				clusterName,
 				nil,
@@ -246,6 +250,7 @@ func (s *ResourceGenerator) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.C
 
 			clusterName := cfgSnap.ServerSNIFn(key.Datacenter, "")
 			la := makeLoadAssignment(
+				s.Logger,
 				cfgSnap,
 				clusterName,
 				nil,
@@ -418,6 +423,7 @@ func (s *ResourceGenerator) endpointsFromServicesAndResolvers(
 		for subsetName, groups := range clusterEndpoints {
 			clusterName := connect.ServiceSNI(svc.Name, subsetName, svc.NamespaceOrDefault(), svc.PartitionOrDefault(), cfgSnap.Datacenter, cfgSnap.Roots.TrustDomain)
 			la := makeLoadAssignment(
+				s.Logger,
 				cfgSnap,
 				clusterName,
 				nil,
@@ -455,6 +461,7 @@ func (s *ResourceGenerator) makeEndpointsForOutgoingPeeredServices(
 			groups := []loadAssignmentEndpointGroup{{Endpoints: serviceGroup.Nodes, OnlyPassing: false}}
 
 			la := makeLoadAssignment(
+				s.Logger,
 				cfgSnap,
 				clusterName,
 				nil,
@@ -619,6 +626,7 @@ func (s *ResourceGenerator) makeUpstreamLoadAssignmentForPeerService(
 			return la, nil
 		}
 		la = makeLoadAssignment(
+			s.Logger,
 			cfgSnap,
 			clusterName,
 			nil,
@@ -641,6 +649,7 @@ func (s *ResourceGenerator) makeUpstreamLoadAssignmentForPeerService(
 		return nil, nil
 	}
 	la = makeLoadAssignment(
+		s.Logger,
 		cfgSnap,
 		clusterName,
 		nil,
@@ -773,6 +782,7 @@ func (s *ResourceGenerator) endpointsFromDiscoveryChain(
 			}
 
 			la := makeLoadAssignment(
+				s.Logger,
 				cfgSnap,
 				clusterName,
 				ti.PrioritizeByLocality,
@@ -861,7 +871,7 @@ type loadAssignmentEndpointGroup struct {
 	OverrideHealth envoy_core_v3.HealthStatus
 }
 
-func makeLoadAssignment(cfgSnap *proxycfg.ConfigSnapshot, clusterName string, policy *structs.DiscoveryPrioritizeByLocality, endpointGroups []loadAssignmentEndpointGroup, localKey proxycfg.GatewayKey) *envoy_endpoint_v3.ClusterLoadAssignment {
+func makeLoadAssignment(logger hclog.Logger, cfgSnap *proxycfg.ConfigSnapshot, clusterName string, policy *structs.DiscoveryPrioritizeByLocality, endpointGroups []loadAssignmentEndpointGroup, localKey proxycfg.GatewayKey) *envoy_endpoint_v3.ClusterLoadAssignment {
 	cla := &envoy_endpoint_v3.ClusterLoadAssignment{
 		ClusterName: clusterName,
 		Endpoints:   make([]*envoy_endpoint_v3.LocalityLbEndpoints, 0, len(endpointGroups)),
@@ -878,7 +888,7 @@ func makeLoadAssignment(cfgSnap *proxycfg.ConfigSnapshot, clusterName string, po
 	var priority uint32
 
 	for _, endpointGroup := range endpointGroups {
-		endpointsByLocality, err := groupedEndpoints(cfgSnap.ServiceLocality, policy, endpointGroup.Endpoints)
+		endpointsByLocality, err := groupedEndpoints(logger, cfgSnap.ServiceLocality, policy, endpointGroup.Endpoints)
 
 		if err != nil {
 			continue

diff --git a/agent/xds/endpoints_test.go b/agent/xds/endpoints_test.go
@@ -4,6 +4,7 @@
 package xds
 
 import (
+	"github.com/hashicorp/go-hclog"
 	"path/filepath"
 	"sort"
 	"testing"
@@ -213,6 +214,7 @@ func Test_makeLoadAssignment(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got := makeLoadAssignment(
+				hclog.NewNullLogger(),
 				&proxycfg.ConfigSnapshot{ServiceLocality: tt.locality},
 				tt.clusterName,
 				nil,
@@ -223,6 +225,7 @@ func Test_makeLoadAssignment(t *testing.T) {
 
 			if tt.locality == nil {
 				got := makeLoadAssignment(
+					hclog.NewNullLogger(),
 					&proxycfg.ConfigSnapshot{ServiceLocality: &structs.Locality{Region: "us-west-1", Zone: "us-west-1a"}},
 					tt.clusterName,
 					nil,

diff --git a/agent/xds/locality_policy.go b/agent/xds/locality_policy.go
@@ -5,16 +5,18 @@ package xds
 
 import (
 	"fmt"
+	"github.com/hashicorp/go-hclog"
 
 	"github.com/hashicorp/consul/agent/structs"
 )
 
-func groupedEndpoints(locality *structs.Locality, policy *structs.DiscoveryPrioritizeByLocality, csns structs.CheckServiceNodes) ([]structs.CheckServiceNodes, error) {
+func groupedEndpoints(logger hclog.Logger, locality *structs.Locality, policy *structs.DiscoveryPrioritizeByLocality, csns structs.CheckServiceNodes) ([]structs.CheckServiceNodes, error) {
 	switch {
 	case policy == nil || policy.Mode == "" || policy.Mode == "none":
 		return []structs.CheckServiceNodes{csns}, nil
 	case policy.Mode == "failover":
-		return prioritizeByLocalityFailover(locality, csns), nil
+		log := logger.Named("locality")
+		return prioritizeByLocalityFailover(log, locality, csns), nil
 	default:
 		return nil, fmt.Errorf("unexpected priortize-by-locality mode %q", policy.Mode)
 	}

diff --git a/agent/xds/locality_policy_oss.go b/agent/xds/locality_policy_oss.go
@@ -8,8 +8,9 @@ package xds
 
 import (
 	"github.com/hashicorp/consul/agent/structs"
+	"github.com/hashicorp/go-hclog"
 )
 
-func prioritizeByLocalityFailover(locality *structs.Locality, csns structs.CheckServiceNodes) []structs.CheckServiceNodes {
+func prioritizeByLocalityFailover(_ hclog.Logger, _ *structs.Locality, _ structs.CheckServiceNodes) []structs.CheckServiceNodes {
 	return nil
 }