Skip to content

Commit

Permalink
feat: allow disabling pod locality in xDS (#10404)
Browse files Browse the repository at this point in the history
  • Loading branch information
yuval-k authored Nov 26, 2024
1 parent b937894 commit 7afa8dc
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 13 deletions.
5 changes: 5 additions & 0 deletions changelog/v1.18.0-rc3/pod-xds-disable.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
changelog:
- type: NON_USER_FACING
issueLink: https://github.com/solo-io/solo-projects/issues/7302
description: >-
Allow disabling pod locality considerations in xDS using undocumented env-var `DISABLE_POD_LOCALITY_XDS`.
38 changes: 30 additions & 8 deletions projects/gateway2/krtcollections/uniqueclients.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,20 @@ func labeledRole(role string, labels map[string]string) string {
return fmt.Sprintf("%s%s%d", role, xds.KeyDelimiter, utils.HashLabels(labels))
}

// note: if `ns“ is empty, we assume the user doesn't want to use pod locality info, so we won't modify the role.
func newUniqlyConnectedClient(node *envoy_config_core_v3.Node, ns string, labels map[string]string, locality PodLocality) UniqlyConnectedClient {
role := node.GetMetadata().GetFields()[xds.RoleKey].GetStringValue()
snapshotKey := labeledRole(role, labels)
resourceName := role
if ns != "" {
snapshotKey := labeledRole(role, labels)
resourceName = fmt.Sprintf("%s%s%s", snapshotKey, xds.KeyDelimiter, ns)
}
return UniqlyConnectedClient{
Role: role,
Namespace: ns,
Locality: locality,
Labels: labels,
resourceName: fmt.Sprintf("%s%s%s", snapshotKey, xds.KeyDelimiter, ns),
resourceName: resourceName,
}
}

Expand All @@ -91,6 +96,7 @@ type callbacks struct {
collection atomic.Pointer[callbacksCollection]
}

// If augmentedPods is nil, we won't use the pod locality info, and all pods for the same gateway will receive the same config.
type UniquelyConnectedClientsBulider func(ctx context.Context, handler *krt.DebugHandler, augmentedPods krt.Collection[LocalityPod]) krt.Collection[UniqlyConnectedClient]

// THIS IS THE SET OF THINGS WE RUN TRANSLATION FOR
Expand Down Expand Up @@ -173,7 +179,9 @@ func (x *callbacksCollection) del(sid int64) *UniqlyConnectedClient {
func (x *callbacksCollection) add(sid int64, r *envoy_service_discovery_v3.DiscoveryRequest) (string, bool, error) {

var pod *LocalityPod
if r.GetNode() != nil {
// see if user wants to use pod locality info
usePod := x.augmentedPods != nil
if usePod && r.GetNode() != nil {
podRef := getRef(r.GetNode())
k := krt.Key[LocalityPod](krt.Named{Name: podRef.Name, Namespace: podRef.Namespace}.ResourceName())
pod = x.augmentedPods.GetKey(k)
Expand All @@ -183,13 +191,22 @@ func (x *callbacksCollection) add(sid int64, r *envoy_service_discovery_v3.Disco
defer x.stateLock.Unlock()
c, ok := x.clients[sid]
if !ok {
if pod == nil {
// error if we can't get the pod
return "", false, fmt.Errorf("pod not found for node %v", r.GetNode())
var locality PodLocality
var ns string
var labels map[string]string
if usePod {
if pod == nil {
// we need to use the pod locality info, so it's an error if we can't get the pod
return "", false, fmt.Errorf("pod not found for node %v", r.GetNode())
} else {
locality = pod.Locality
ns = pod.Namespace
labels = pod.AugmentedLabels
}
}
x.logger.Debug("adding xds client", zap.Any("locality", pod.Locality), zap.String("ns", pod.Namespace), zap.Any("labels", pod.AugmentedLabels))
x.logger.Debug("adding xds client", zap.Any("locality", locality), zap.String("ns", ns), zap.Any("labels", labels))
// TODO: modify request to include the label that are relevant for the client?
ucc := newUniqlyConnectedClient(r.GetNode(), pod.Namespace, pod.AugmentedLabels, pod.Locality)
ucc := newUniqlyConnectedClient(r.GetNode(), ns, labels, locality)
c = newConnectedClient(ucc.resourceName)
x.clients[sid] = c
currentUnique := x.uniqClientsCount[ucc.resourceName]
Expand Down Expand Up @@ -277,6 +294,11 @@ func (x *callbacks) OnFetchRequest(ctx context.Context, r *envoy_service_discove
}

func (x *callbacksCollection) fetchRequest(_ context.Context, r *envoy_service_discovery_v3.DiscoveryRequest) error {
// nothing special to do in a fetch request, as we don't need to maintain state
if x.augmentedPods == nil {
return nil
}

var pod *LocalityPod
podRef := getRef(r.GetNode())
k := krt.Key[LocalityPod](krt.Named{Name: podRef.Name, Namespace: podRef.Namespace}.ResourceName())
Expand Down
29 changes: 25 additions & 4 deletions projects/gateway2/krtcollections/uniqueclients_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/solo-io/gloo/projects/gloo/pkg/xds"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/structpb"
"istio.io/istio/pkg/kube/krt"
"istio.io/istio/pkg/kube/krt/krttest"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -65,15 +66,35 @@ func TestUniqueClients(t *testing.T) {
},
result: sets.New(fmt.Sprintf("gloo-kube-gateway-api~best-proxy-role~%d~ns", utils.HashLabels(map[string]string{corev1.LabelTopologyRegion: "region", corev1.LabelTopologyZone: "zone", "a": "b"}))),
},
{
name: "no-pods",
inputs: nil,
requests: []*envoy_service_discovery_v3.DiscoveryRequest{
{
Node: &corev3.Node{
Id: "podname.ns",
Metadata: &structpb.Struct{
Fields: map[string]*structpb.Value{
xds.RoleKey: structpb.NewStringValue(glooutils.GatewayApiProxyValue + "~best-proxy-role"),
},
},
},
},
},
result: sets.New(fmt.Sprintf(glooutils.GatewayApiProxyValue + "~best-proxy-role")),
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
g := NewWithT(t)
mock := krttest.NewMock(t, tc.inputs)
nodes := NewNodeMetadataCollection(krttest.GetMockCollection[*corev1.Node](mock))
pods := NewLocalityPodsCollection(nodes, krttest.GetMockCollection[*corev1.Pod](mock), nil)
pods.Synced().WaitUntilSynced(context.Background().Done())
var pods krt.Collection[LocalityPod]
if tc.inputs != nil {
mock := krttest.NewMock(t, tc.inputs)
nodes := NewNodeMetadataCollection(krttest.GetMockCollection[*corev1.Node](mock))
pods = NewLocalityPodsCollection(nodes, krttest.GetMockCollection[*corev1.Pod](mock), nil)
pods.Synced().WaitUntilSynced(context.Background().Done())
}

cb, uccBuilder := NewUniquelyConnectedClients()
ucc := uccBuilder(context.Background(), nil, pods)
Expand Down
7 changes: 6 additions & 1 deletion projects/gateway2/setup/ggv2setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,12 @@ func StartGGv2WithConfig(ctx context.Context,
settingsGVR,
krt.WithName("GlooSettings"))

ucc := uccBuilder(ctx, setupOpts.KrtDebugger, augmentedPods)
augmentedPodsForUcc := augmentedPods
if envutils.IsEnvTruthy("DISABLE_POD_LOCALITY_XDS") {
augmentedPodsForUcc = nil
}

ucc := uccBuilder(ctx, setupOpts.KrtDebugger, augmentedPodsForUcc)

settingsSingle := krt.NewSingleton(func(ctx krt.HandlerContext) *glookubev1.Settings {
s := krt.FetchOne(ctx, setting,
Expand Down

0 comments on commit 7afa8dc

Please sign in to comment.