Skip to content

Commit 19952a2

Browse files
danwinshipdanwinship
committed
Implement the EndpointSlice controller side of PreferSameZone/PreferSameNode
1 parent 90c8f9a commit 19952a2

File tree

6 files changed

+605
-20
lines changed

6 files changed

+605
-20
lines changed

‎pkg/controller/endpointslice/endpointslice_controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ func NewController(ctx context.Context, podInformer coreinformers.PodInformer,
186186
c.topologyCache,
187187
c.eventRecorder,
188188
ControllerName,
189-
endpointslicerec.WithTrafficDistributionEnabled(utilfeature.DefaultFeatureGate.Enabled(features.ServiceTrafficDistribution)),
189+
endpointslicerec.WithTrafficDistributionEnabled(utilfeature.DefaultFeatureGate.Enabled(features.ServiceTrafficDistribution), utilfeature.DefaultFeatureGate.Enabled(features.PreferSameTrafficDistribution)),
190190
)
191191

192192
return c

‎staging/src/k8s.io/endpointslice/metrics/metrics.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ var (
104104
},
105105
[]string{
106106
"topology", // either "Auto" or "Disabled"
107-
"traffic_distribution", // "PreferClose" or <empty>
107+
"traffic_distribution", // a trafficDistribution value or <empty>
108108
},
109109
)
110110

‎staging/src/k8s.io/endpointslice/reconciler.go‎

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,13 @@ type Reconciler struct {
5151
// topologyCache tracks the distribution of Nodes and endpoints across zones
5252
// to enable TopologyAwareHints.
5353
topologyCache *topologycache.TopologyCache
54-
// trafficDistributionEnabled determines if endpointDistribution field is to
54+
// trafficDistributionEnabled determines if trafficDistribution field is to
5555
// be considered when reconciling EndpointSlice hints.
5656
trafficDistributionEnabled bool
57+
// preferSameTrafficDistribution determines if the new (PreferSameZone /
58+
// PreferSameNode) trafficDistribution values should be considered when
59+
// reconciling EndpointSlice hints.
60+
preferSameTrafficDistribution bool
5761
// eventRecorder allows Reconciler to record and publish events.
5862
eventRecorder record.EventRecorder
5963
controllerName string
@@ -63,12 +67,32 @@ type ReconcilerOption func(*Reconciler)
6367

6468
// WithTrafficDistributionEnabled controls whether the Reconciler considers the
6569
// `trafficDistribution` field while reconciling EndpointSlices.
66-
func WithTrafficDistributionEnabled(enabled bool) ReconcilerOption {
70+
func WithTrafficDistributionEnabled(enabled, preferSame bool) ReconcilerOption {
6771
return func(r *Reconciler) {
6872
r.trafficDistributionEnabled = enabled
73+
r.preferSameTrafficDistribution = preferSame
6974
}
7075
}
7176

77+
// validTrafficDistribution determines whether TrafficDistribution is set and valid for
78+
// this cluster.
79+
func (r *Reconciler) validTrafficDistribution(trafficDistribution *string) bool {
80+
if trafficDistribution == nil || !r.trafficDistributionEnabled {
81+
return false
82+
}
83+
if *trafficDistribution == corev1.ServiceTrafficDistributionPreferClose {
84+
return true
85+
}
86+
if !r.preferSameTrafficDistribution {
87+
return false
88+
}
89+
if *trafficDistribution == corev1.ServiceTrafficDistributionPreferSameZone ||
90+
*trafficDistribution == corev1.ServiceTrafficDistributionPreferSameNode {
91+
return true
92+
}
93+
return false
94+
}
95+
7296
// endpointMeta includes the attributes we group slices on, this type helps with
7397
// that logic in Reconciler
7498
type endpointMeta struct {
@@ -275,7 +299,7 @@ func (r *Reconciler) reconcileByAddressType(logger klog.Logger, service *corev1.
275299
Unchanged: unchangedSlices(existingSlices, slicesToUpdate, slicesToDelete),
276300
}
277301

278-
canUseTrafficDistribution := r.trafficDistributionEnabled && !hintsEnabled(service.Annotations)
302+
canUseTrafficDistribution := r.validTrafficDistribution(service.Spec.TrafficDistribution) && !hintsEnabled(service.Annotations)
279303

280304
// Check if we need to add/remove hints based on the topology annotation.
281305
//
@@ -455,10 +479,8 @@ func (r *Reconciler) finalize(
455479
topologyLabel = "Auto"
456480
}
457481
var trafficDistribution string
458-
if r.trafficDistributionEnabled && !hintsEnabled(service.Annotations) {
459-
if service.Spec.TrafficDistribution != nil && *service.Spec.TrafficDistribution == corev1.ServiceTrafficDistributionPreferClose {
460-
trafficDistribution = *service.Spec.TrafficDistribution
461-
}
482+
if r.validTrafficDistribution(service.Spec.TrafficDistribution) && !hintsEnabled(service.Annotations) {
483+
trafficDistribution = *service.Spec.TrafficDistribution
462484
}
463485

464486
numSlicesChanged := len(slicesToCreate) + len(slicesToUpdate) + len(slicesToDelete)

‎staging/src/k8s.io/endpointslice/reconciler_test.go‎

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2017,11 +2017,13 @@ func TestReconcile_TrafficDistribution(t *testing.T) {
20172017
desc string
20182018

20192019
trafficDistributionFeatureGateEnabled bool
2020+
preferSameFeatureGateEnabled bool
20202021
trafficDistribution *string
20212022
topologyAnnotation string
20222023

2023-
// Defines how many hints belong to a particular zone.
2024+
// Defines how many hints belong to a particular zone/node
20242025
wantHintsDistributionByZone map[string]int
2026+
wantHintsDistributionByNode map[string]int
20252027
// Number of endpoints where the zone hints are different from the zone of
20262028
// the endpoint itself.
20272029
wantEndpointsWithCrossZoneHints int
@@ -2123,6 +2125,62 @@ func TestReconcile_TrafficDistribution(t *testing.T) {
21232125
slicesChangedPerSyncTrafficDist: 0, // 0 means trafficDistribution was not used.
21242126
},
21252127
},
2128+
{
2129+
name: "trafficDistribution=PreferSameNode, PSTD enabled",
2130+
desc: "When trafficDistribution is PreferSameNode and PreferSameTrafficDistribution is enabled, both zone and node hints should be filled out",
2131+
trafficDistributionFeatureGateEnabled: true,
2132+
preferSameFeatureGateEnabled: true,
2133+
trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferSameNode),
2134+
topologyAnnotation: "Disabled",
2135+
wantHintsDistributionByZone: map[string]int{
2136+
"zone-a": 1, // {pod-0}
2137+
"zone-b": 3, // {pod-1, pod-2, pod-3}
2138+
"zone-c": 2, // {pod-4, pod-5}
2139+
},
2140+
wantHintsDistributionByNode: map[string]int{
2141+
"node-0": 1, // {pod-0}
2142+
"node-1": 3, // {pod-1, pod-2, pod-3}
2143+
"node-2": 2, // {pod-4, pod-5}
2144+
},
2145+
wantMetrics: expectedMetrics{
2146+
desiredSlices: 1,
2147+
actualSlices: 1,
2148+
desiredEndpoints: 6,
2149+
addedPerSync: 6,
2150+
removedPerSync: 0,
2151+
numCreated: 1,
2152+
numUpdated: 0,
2153+
numDeleted: 0,
2154+
slicesChangedPerSync: 0, // 0 means either topologyAnnotation or trafficDistribution was used.
2155+
slicesChangedPerSyncTopology: 0, // 0 means topologyAnnotation was not used.
2156+
slicesChangedPerSyncTrafficDist: 1, // 1 EPS configured using trafficDistribution.
2157+
servicesCountByTrafficDistribution: map[string]int{
2158+
"PreferSameNode": 1,
2159+
},
2160+
},
2161+
},
2162+
{
2163+
name: "trafficDistribution=PreferSameZone, PSTD disabled",
2164+
desc: "When trafficDistribution is PreferSameZone and PreferSameTrafficDistribution is disabled, no hints should be set",
2165+
trafficDistributionFeatureGateEnabled: true,
2166+
preferSameFeatureGateEnabled: false,
2167+
trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferSameZone),
2168+
topologyAnnotation: "Disabled",
2169+
wantHintsDistributionByZone: map[string]int{"": 6}, // Equivalent to no hints.
2170+
wantMetrics: expectedMetrics{
2171+
desiredSlices: 1,
2172+
actualSlices: 1,
2173+
desiredEndpoints: 6,
2174+
addedPerSync: 6,
2175+
removedPerSync: 0,
2176+
numCreated: 1,
2177+
numUpdated: 0,
2178+
numDeleted: 0,
2179+
slicesChangedPerSync: 1, // 1 means both topologyAnnotation and trafficDistribution were not used.
2180+
slicesChangedPerSyncTopology: 0, // 0 means topologyAnnotation was not used.
2181+
slicesChangedPerSyncTrafficDist: 0, // 0 means trafficDistribution was not used.
2182+
},
2183+
},
21262184
}
21272185

21282186
// Make assertions.
@@ -2135,6 +2193,7 @@ func TestReconcile_TrafficDistribution(t *testing.T) {
21352193

21362194
r := newReconciler(client, nodes, defaultMaxEndpointsPerSlice)
21372195
r.trafficDistributionEnabled = tc.trafficDistributionFeatureGateEnabled
2196+
r.preferSameTrafficDistribution = tc.preferSameFeatureGateEnabled
21382197
r.topologyCache = topologycache.NewTopologyCache()
21392198
r.topologyCache.SetNodes(logger, nodes)
21402199

@@ -2397,8 +2456,13 @@ func expectMetrics(t *testing.T, em expectedMetrics) {
23972456
t.Errorf("Expected slicesChangedPerSyncTopology to be %d, got %v", em.slicesChangedPerSyncTopology, actualSlicesChangedPerSyncTopology)
23982457
}
23992458

2400-
actualSlicesChangedPerSyncTrafficDist, err := testutil.GetHistogramMetricValue(metrics.EndpointSlicesChangedPerSync.WithLabelValues("Disabled", "PreferClose"))
2401-
handleErr(t, err, "slicesChangedPerSyncTrafficDist")
2459+
actualSlicesChangedPreferClose, err := testutil.GetHistogramMetricValue(metrics.EndpointSlicesChangedPerSync.WithLabelValues("Disabled", "PreferClose"))
2460+
handleErr(t, err, "slicesChangedPreferClose")
2461+
actualSlicesChangedPreferSameZone, err := testutil.GetHistogramMetricValue(metrics.EndpointSlicesChangedPerSync.WithLabelValues("Disabled", "PreferSameZone"))
2462+
handleErr(t, err, "slicesChangedPreferSameZone")
2463+
actualSlicesChangedPreferSameNode, err := testutil.GetHistogramMetricValue(metrics.EndpointSlicesChangedPerSync.WithLabelValues("Disabled", "PreferSameNode"))
2464+
handleErr(t, err, "slicesChangedPreferSameNode")
2465+
actualSlicesChangedPerSyncTrafficDist := actualSlicesChangedPreferClose + actualSlicesChangedPreferSameZone + actualSlicesChangedPreferSameNode
24022466
if actualSlicesChangedPerSyncTrafficDist != float64(em.slicesChangedPerSyncTrafficDist) {
24032467
t.Errorf("Expected slicesChangedPerSyncTrafficDist to be %d, got %v", em.slicesChangedPerSyncTrafficDist, actualSlicesChangedPerSyncTopology)
24042468
}

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

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,14 @@ package trafficdist
2020
import (
2121
corev1 "k8s.io/api/core/v1"
2222
discoveryv1 "k8s.io/api/discovery/v1"
23+
"k8s.io/apimachinery/pkg/util/sets"
24+
)
25+
26+
// TrafficDistribution values supported by preferCloseHeuristic
27+
var closeTrafficDistribution = sets.New(
28+
corev1.ServiceTrafficDistributionPreferClose,
29+
corev1.ServiceTrafficDistributionPreferSameZone,
30+
corev1.ServiceTrafficDistributionPreferSameNode,
2331
)
2432

2533
// ReconcileHints will reconcile hints for the given EndpointSlices.
@@ -28,12 +36,12 @@ import (
2836
func ReconcileHints(trafficDistribution *string, slicesToCreate, slicesToUpdate, slicesUnchanged []*discoveryv1.EndpointSlice) ([]*discoveryv1.EndpointSlice, []*discoveryv1.EndpointSlice, []*discoveryv1.EndpointSlice) {
2937
var h heuristic = &defaultHeuristic{}
3038

31-
if trafficDistribution != nil && *trafficDistribution == corev1.ServiceTrafficDistributionPreferClose {
32-
h = &preferCloseHeuristic{}
39+
if trafficDistribution != nil && closeTrafficDistribution.Has(*trafficDistribution) {
40+
h = &preferCloseHeuristic{*trafficDistribution == corev1.ServiceTrafficDistributionPreferSameNode}
3341
}
3442

3543
// Identify the Unchanged slices that need an update because of missing or
36-
// incorrect zone hint.
44+
// incorrect hints.
3745
//
3846
// Uses filtering in place to remove any endpoints that are no longer
3947
// unchanged and need to be moved to slicesToUpdate
@@ -101,13 +109,14 @@ func (defaultHeuristic) update(slice *discoveryv1.EndpointSlice) {
101109
}
102110
}
103111

104-
// preferCloseHeuristic adds
112+
// preferCloseHeuristic implements PreferSameZone/PreferClose and PreferSameNode
105113
type preferCloseHeuristic struct {
114+
generateNodeHints bool
106115
}
107116

108117
// needsUpdate returns true if any ready endpoint in the slice has a
109118
// missing or incorrect hint.
110-
func (preferCloseHeuristic) needsUpdate(slice *discoveryv1.EndpointSlice) bool {
119+
func (h preferCloseHeuristic) needsUpdate(slice *discoveryv1.EndpointSlice) bool {
111120
if slice == nil {
112121
return false
113122
}
@@ -129,25 +138,44 @@ func (preferCloseHeuristic) needsUpdate(slice *discoveryv1.EndpointSlice) bool {
129138
return true
130139
}
131140
}
141+
142+
if endpoint.NodeName != nil && h.generateNodeHints {
143+
// We want a node hint.
144+
if endpoint.Hints == nil || len(endpoint.Hints.ForNodes) != 1 || endpoint.Hints.ForNodes[0].Name != *endpoint.NodeName {
145+
// ...but it's either missing or incorrect
146+
return true
147+
}
148+
} else {
149+
// We don't want a node hint.
150+
if endpoint.Hints != nil && len(endpoint.Hints.ForNodes) > 0 {
151+
// ... but we have a stale hint.
152+
return true
153+
}
154+
}
132155
}
133156
return false
134157
}
135158

136159
// update adds a same zone topology hint for all ready endpoints
137-
func (preferCloseHeuristic) update(slice *discoveryv1.EndpointSlice) {
160+
func (h preferCloseHeuristic) update(slice *discoveryv1.EndpointSlice) {
138161
for i, endpoint := range slice.Endpoints {
139162
if !endpointReady(endpoint) {
140163
continue
141164
}
142165

143166
var forZones []discoveryv1.ForZone
167+
var forNodes []discoveryv1.ForNode
144168
if endpoint.Zone != nil {
145169
forZones = []discoveryv1.ForZone{{Name: *endpoint.Zone}}
146170
}
171+
if endpoint.NodeName != nil && h.generateNodeHints {
172+
forNodes = []discoveryv1.ForNode{{Name: *endpoint.NodeName}}
173+
}
147174

148-
if forZones != nil {
175+
if forZones != nil || forNodes != nil {
149176
slice.Endpoints[i].Hints = &discoveryv1.EndpointHints{
150177
ForZones: forZones,
178+
ForNodes: forNodes,
151179
}
152180
} else {
153181
slice.Endpoints[i].Hints = nil

0 commit comments

Comments
 (0)