Skip to content

Commit 90c8f9a

Browse files
committed
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.)
1 parent 413af83 commit 90c8f9a

File tree

2 files changed

+147
-9
lines changed

2 files changed

+147
-9
lines changed

staging/src/k8s.io/endpointslice/trafficdist/trafficdist.go

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -115,13 +115,19 @@ func (preferCloseHeuristic) needsUpdate(slice *discoveryv1.EndpointSlice) bool {
115115
if !endpointReady(endpoint) {
116116
continue
117117
}
118-
var zone string
119-
if endpoint.Zone != nil {
120-
zone = *endpoint.Zone
121-
}
122118

123-
if endpoint.Hints == nil || len(endpoint.Hints.ForZones) != 1 || endpoint.Hints.ForZones[0].Name != zone {
124-
return true
119+
if endpoint.Zone != nil {
120+
// We want a zone hint.
121+
if endpoint.Hints == nil || len(endpoint.Hints.ForZones) != 1 || endpoint.Hints.ForZones[0].Name != *endpoint.Zone {
122+
// ...but either it's missing or it's incorrect
123+
return true
124+
}
125+
} else {
126+
// We don't want a zone hint.
127+
if endpoint.Hints != nil && len(endpoint.Hints.ForZones) > 0 {
128+
// ...but we have a stale hint.
129+
return true
130+
}
125131
}
126132
}
127133
return false
@@ -134,10 +140,17 @@ func (preferCloseHeuristic) update(slice *discoveryv1.EndpointSlice) {
134140
continue
135141
}
136142

137-
var zone string
143+
var forZones []discoveryv1.ForZone
138144
if endpoint.Zone != nil {
139-
zone = *endpoint.Zone
145+
forZones = []discoveryv1.ForZone{{Name: *endpoint.Zone}}
146+
}
147+
148+
if forZones != nil {
149+
slice.Endpoints[i].Hints = &discoveryv1.EndpointHints{
150+
ForZones: forZones,
151+
}
152+
} else {
153+
slice.Endpoints[i].Hints = nil
140154
}
141-
slice.Endpoints[i].Hints = &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: zone}}}
142155
}
143156
}

staging/src/k8s.io/endpointslice/trafficdist/trafficdist_test.go

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,131 @@ func TestReconcileHints(t *testing.T) {
264264
},
265265
},
266266
},
267+
{
268+
name: "should not create zone hints if there are no zones",
269+
270+
trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferClose),
271+
slicesToCreate: []*discoveryv1.EndpointSlice{
272+
{
273+
Endpoints: []discoveryv1.Endpoint{
274+
{
275+
Addresses: []string{"10.0.0.1"},
276+
NodeName: ptr.To("node-1"),
277+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
278+
},
279+
{
280+
Addresses: []string{"10.0.0.2"},
281+
NodeName: ptr.To("node-2"),
282+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
283+
},
284+
},
285+
},
286+
{
287+
Endpoints: []discoveryv1.Endpoint{
288+
{
289+
Addresses: []string{"10.0.0.3"},
290+
NodeName: ptr.To("node-3"),
291+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
292+
},
293+
{
294+
Addresses: []string{"10.0.0.4"},
295+
NodeName: ptr.To("node-4"),
296+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
297+
},
298+
},
299+
},
300+
},
301+
slicesToUpdate: []*discoveryv1.EndpointSlice{
302+
{
303+
Endpoints: []discoveryv1.Endpoint{
304+
{
305+
Addresses: []string{"10.0.0.5"},
306+
NodeName: ptr.To("node-5"),
307+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
308+
},
309+
{
310+
Addresses: []string{"10.0.0.6"},
311+
NodeName: ptr.To("node-6"),
312+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
313+
},
314+
},
315+
},
316+
{
317+
Endpoints: []discoveryv1.Endpoint{
318+
{
319+
Addresses: []string{"10.0.0.7"},
320+
NodeName: ptr.To("node-7"),
321+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
322+
},
323+
{
324+
Addresses: []string{"10.0.0.8"},
325+
NodeName: ptr.To("node-8"),
326+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
327+
},
328+
},
329+
},
330+
},
331+
wantSlicesToCreate: []*discoveryv1.EndpointSlice{
332+
{
333+
Endpoints: []discoveryv1.Endpoint{
334+
{
335+
Addresses: []string{"10.0.0.1"},
336+
NodeName: ptr.To("node-1"),
337+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
338+
},
339+
{
340+
Addresses: []string{"10.0.0.2"},
341+
NodeName: ptr.To("node-2"),
342+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
343+
},
344+
},
345+
},
346+
{
347+
Endpoints: []discoveryv1.Endpoint{
348+
{
349+
Addresses: []string{"10.0.0.3"},
350+
NodeName: ptr.To("node-3"),
351+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
352+
},
353+
{
354+
Addresses: []string{"10.0.0.4"},
355+
NodeName: ptr.To("node-4"),
356+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
357+
},
358+
},
359+
},
360+
},
361+
wantSlicesToUpdate: []*discoveryv1.EndpointSlice{
362+
{
363+
Endpoints: []discoveryv1.Endpoint{
364+
{
365+
Addresses: []string{"10.0.0.5"},
366+
NodeName: ptr.To("node-5"),
367+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
368+
},
369+
{
370+
Addresses: []string{"10.0.0.6"},
371+
NodeName: ptr.To("node-6"),
372+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
373+
},
374+
},
375+
},
376+
{
377+
Endpoints: []discoveryv1.Endpoint{
378+
{
379+
Addresses: []string{"10.0.0.7"},
380+
NodeName: ptr.To("node-7"),
381+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
382+
},
383+
{
384+
Addresses: []string{"10.0.0.8"},
385+
NodeName: ptr.To("node-8"),
386+
Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)},
387+
},
388+
},
389+
},
390+
},
391+
},
267392
{
268393
name: "unready endpoints should not trigger updates",
269394

0 commit comments

Comments
 (0)