From 90c8f9aef1a89062d7d60c00a1cc51ae88010495 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 13 Mar 2025 11:31:04 -0400 Subject: [PATCH] Don't write out dummy zone hints in clusters with no zones If you set `trafficDistribution: PreferClose` on a service in a cluster with no defined zones, then it would add hints: forZones: - name: "" to each endpoint. This ended up working anyway since kube-proxy would likewise end up looking for an endpoint for the "" zone, but it's unnecessary, since you'd get exactly the same behavior by just leaving all of the endpoints unhinted. (Of course there's no point in using PreferClose traffic distribution in this case, but this will make PreferSameNode cleaner.) --- .../endpointslice/trafficdist/trafficdist.go | 31 +++-- .../trafficdist/trafficdist_test.go | 125 ++++++++++++++++++ 2 files changed, 147 insertions(+), 9 deletions(-) diff --git a/staging/src/k8s.io/endpointslice/trafficdist/trafficdist.go b/staging/src/k8s.io/endpointslice/trafficdist/trafficdist.go index cf6c7b42c6a..c4250bb44bd 100644 --- a/staging/src/k8s.io/endpointslice/trafficdist/trafficdist.go +++ b/staging/src/k8s.io/endpointslice/trafficdist/trafficdist.go @@ -115,13 +115,19 @@ func (preferCloseHeuristic) needsUpdate(slice *discoveryv1.EndpointSlice) bool { if !endpointReady(endpoint) { continue } - var zone string - if endpoint.Zone != nil { - zone = *endpoint.Zone - } - if endpoint.Hints == nil || len(endpoint.Hints.ForZones) != 1 || endpoint.Hints.ForZones[0].Name != zone { - return true + if endpoint.Zone != nil { + // We want a zone hint. + if endpoint.Hints == nil || len(endpoint.Hints.ForZones) != 1 || endpoint.Hints.ForZones[0].Name != *endpoint.Zone { + // ...but either it's missing or it's incorrect + return true + } + } else { + // We don't want a zone hint. + if endpoint.Hints != nil && len(endpoint.Hints.ForZones) > 0 { + // ...but we have a stale hint. + return true + } } } return false @@ -134,10 +140,17 @@ func (preferCloseHeuristic) update(slice *discoveryv1.EndpointSlice) { continue } - var zone string + var forZones []discoveryv1.ForZone if endpoint.Zone != nil { - zone = *endpoint.Zone + forZones = []discoveryv1.ForZone{{Name: *endpoint.Zone}} + } + + if forZones != nil { + slice.Endpoints[i].Hints = &discoveryv1.EndpointHints{ + ForZones: forZones, + } + } else { + slice.Endpoints[i].Hints = nil } - slice.Endpoints[i].Hints = &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: zone}}} } } diff --git a/staging/src/k8s.io/endpointslice/trafficdist/trafficdist_test.go b/staging/src/k8s.io/endpointslice/trafficdist/trafficdist_test.go index 32d1d3c644b..5ad5c4a45ab 100644 --- a/staging/src/k8s.io/endpointslice/trafficdist/trafficdist_test.go +++ b/staging/src/k8s.io/endpointslice/trafficdist/trafficdist_test.go @@ -264,6 +264,131 @@ func TestReconcileHints(t *testing.T) { }, }, }, + { + name: "should not create zone hints if there are no zones", + + trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferClose), + slicesToCreate: []*discoveryv1.EndpointSlice{ + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.1"}, + NodeName: ptr.To("node-1"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.2"}, + NodeName: ptr.To("node-2"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.3"}, + NodeName: ptr.To("node-3"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.4"}, + NodeName: ptr.To("node-4"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + }, + slicesToUpdate: []*discoveryv1.EndpointSlice{ + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.5"}, + NodeName: ptr.To("node-5"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.6"}, + NodeName: ptr.To("node-6"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.7"}, + NodeName: ptr.To("node-7"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.8"}, + NodeName: ptr.To("node-8"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + }, + wantSlicesToCreate: []*discoveryv1.EndpointSlice{ + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.1"}, + NodeName: ptr.To("node-1"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.2"}, + NodeName: ptr.To("node-2"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.3"}, + NodeName: ptr.To("node-3"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.4"}, + NodeName: ptr.To("node-4"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + }, + wantSlicesToUpdate: []*discoveryv1.EndpointSlice{ + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.5"}, + NodeName: ptr.To("node-5"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.6"}, + NodeName: ptr.To("node-6"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.7"}, + NodeName: ptr.To("node-7"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.8"}, + NodeName: ptr.To("node-8"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + }, + }, { name: "unready endpoints should not trigger updates",