From 1ac5f661ed8dd3bdca8536d4fab7efa3e8511d32 Mon Sep 17 00:00:00 2001 From: Miles Bryant Date: Wed, 2 Jul 2025 13:36:18 +0100 Subject: [PATCH 1/2] Don't log irrelevant zone hints message on no endpoints Update pkg/proxy/topology.go Co-authored-by: Dan Winship Add unit test case --- pkg/proxy/topology.go | 6 ++++++ pkg/proxy/topology_test.go | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/pkg/proxy/topology.go b/pkg/proxy/topology.go index c91961d7cec..c76ea30c91d 100644 --- a/pkg/proxy/topology.go +++ b/pkg/proxy/topology.go @@ -153,6 +153,12 @@ func CategorizeEndpoints(endpoints []Endpoint, svcInfo ServicePort, nodeName str // hinted for this node's zone, then it returns "PreferSameZone". // - Otherwise it returns "" (meaning, no topology / default traffic distribution). func topologyModeFromHints(svcInfo ServicePort, endpoints []Endpoint, nodeName, zone string) string { + if len(endpoints) == 0 { + // The code below assumes at least 1 endpoint; if there are no endpoints, + // there are no hints. + return "" + } + hasEndpointForNode := false allEndpointsHaveNodeHints := true hasEndpointForZone := false diff --git a/pkg/proxy/topology_test.go b/pkg/proxy/topology_test.go index 807dc513d95..c5ef39dbbc6 100644 --- a/pkg/proxy/topology_test.go +++ b/pkg/proxy/topology_test.go @@ -390,6 +390,13 @@ func TestCategorizeEndpoints(t *testing.T) { clusterEndpoints: nil, localEndpoints: sets.New[string]("10.0.0.1:80"), allEndpoints: sets.New[string]("10.0.0.1:80"), + }, { + name: "empty cluster endpoints when no service endpoints exist", + serviceInfo: &BaseServicePortInfo{}, + endpoints: nil, + clusterEndpoints: sets.New[string](), + localEndpoints: nil, + allEndpoints: nil, }} for _, tc := range testCases { From 1cec0acebf27234d26bdd9c040a61a1265df13b0 Mon Sep 17 00:00:00 2001 From: Miles Bryant Date: Fri, 4 Jul 2025 11:13:58 +0100 Subject: [PATCH 2/2] Short circuit CategorizeEndpoints when there are no endpoints PR feedback --- pkg/proxy/topology.go | 5 +++++ pkg/proxy/topology_test.go | 12 ++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/pkg/proxy/topology.go b/pkg/proxy/topology.go index c76ea30c91d..e30b3d5233e 100644 --- a/pkg/proxy/topology.go +++ b/pkg/proxy/topology.go @@ -42,6 +42,11 @@ import ( // Serving-Terminating endpoints (independently for Cluster and Local) if no Ready // endpoints are available. func CategorizeEndpoints(endpoints []Endpoint, svcInfo ServicePort, nodeName string, nodeLabels map[string]string) (clusterEndpoints, localEndpoints, allReachableEndpoints []Endpoint, hasAnyEndpoints bool) { + if len(endpoints) == 0 { + // If there are no endpoints, we have nothing to categorize + return + } + var topologyMode string var useServingTerminatingEndpoints bool diff --git a/pkg/proxy/topology_test.go b/pkg/proxy/topology_test.go index c5ef39dbbc6..ad523c23306 100644 --- a/pkg/proxy/topology_test.go +++ b/pkg/proxy/topology_test.go @@ -391,10 +391,10 @@ func TestCategorizeEndpoints(t *testing.T) { localEndpoints: sets.New[string]("10.0.0.1:80"), allEndpoints: sets.New[string]("10.0.0.1:80"), }, { - name: "empty cluster endpoints when no service endpoints exist", + name: "empty endpoints when no service endpoints exist", serviceInfo: &BaseServicePortInfo{}, endpoints: nil, - clusterEndpoints: sets.New[string](), + clusterEndpoints: nil, localEndpoints: nil, allEndpoints: nil, }} @@ -405,7 +405,7 @@ func TestCategorizeEndpoints(t *testing.T) { clusterEndpoints, localEndpoints, allEndpoints, hasAnyEndpoints := CategorizeEndpoints(tc.endpoints, tc.serviceInfo, tc.nodeName, tc.nodeLabels) - if tc.clusterEndpoints == nil && clusterEndpoints != nil { + if len(tc.clusterEndpoints) == 0 && len(clusterEndpoints) != 0 { t.Errorf("expected no cluster endpoints but got %v", clusterEndpoints) } else { err := checkExpectedEndpoints(tc.clusterEndpoints, clusterEndpoints) @@ -414,7 +414,7 @@ func TestCategorizeEndpoints(t *testing.T) { } } - if tc.localEndpoints == nil && localEndpoints != nil { + if len(tc.localEndpoints) == 0 && len(localEndpoints) != 0 { t.Errorf("expected no local endpoints but got %v", localEndpoints) } else { err := checkExpectedEndpoints(tc.localEndpoints, localEndpoints) @@ -424,9 +424,9 @@ func TestCategorizeEndpoints(t *testing.T) { } var expectedAllEndpoints sets.Set[string] - if tc.clusterEndpoints != nil && tc.localEndpoints == nil { + if len(tc.clusterEndpoints) != 0 && len(tc.localEndpoints) == 0 { expectedAllEndpoints = tc.clusterEndpoints - } else if tc.localEndpoints != nil && tc.clusterEndpoints == nil { + } else if len(tc.localEndpoints) != 0 && len(tc.clusterEndpoints) == 0 { expectedAllEndpoints = tc.localEndpoints } else { expectedAllEndpoints = tc.allEndpoints