From 19952a2b7bd575ccb22c510976f5676530dad49a Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 11 Feb 2025 14:33:55 -0500 Subject: [PATCH] Implement the EndpointSlice controller side of PreferSameZone/PreferSameNode --- .../endpointslice/endpointslice_controller.go | 2 +- .../k8s.io/endpointslice/metrics/metrics.go | 2 +- .../src/k8s.io/endpointslice/reconciler.go | 36 +- .../k8s.io/endpointslice/reconciler_test.go | 70 ++- .../endpointslice/trafficdist/trafficdist.go | 42 +- .../trafficdist/trafficdist_test.go | 473 +++++++++++++++++- 6 files changed, 605 insertions(+), 20 deletions(-) diff --git a/pkg/controller/endpointslice/endpointslice_controller.go b/pkg/controller/endpointslice/endpointslice_controller.go index 1617367ebdc..5e01fd334dc 100644 --- a/pkg/controller/endpointslice/endpointslice_controller.go +++ b/pkg/controller/endpointslice/endpointslice_controller.go @@ -186,7 +186,7 @@ func NewController(ctx context.Context, podInformer coreinformers.PodInformer, c.topologyCache, c.eventRecorder, ControllerName, - endpointslicerec.WithTrafficDistributionEnabled(utilfeature.DefaultFeatureGate.Enabled(features.ServiceTrafficDistribution)), + endpointslicerec.WithTrafficDistributionEnabled(utilfeature.DefaultFeatureGate.Enabled(features.ServiceTrafficDistribution), utilfeature.DefaultFeatureGate.Enabled(features.PreferSameTrafficDistribution)), ) return c diff --git a/staging/src/k8s.io/endpointslice/metrics/metrics.go b/staging/src/k8s.io/endpointslice/metrics/metrics.go index b7228a1810a..5323a5dbc7c 100644 --- a/staging/src/k8s.io/endpointslice/metrics/metrics.go +++ b/staging/src/k8s.io/endpointslice/metrics/metrics.go @@ -104,7 +104,7 @@ var ( }, []string{ "topology", // either "Auto" or "Disabled" - "traffic_distribution", // "PreferClose" or + "traffic_distribution", // a trafficDistribution value or }, ) diff --git a/staging/src/k8s.io/endpointslice/reconciler.go b/staging/src/k8s.io/endpointslice/reconciler.go index 9242a76c0bc..1c47793b4fa 100644 --- a/staging/src/k8s.io/endpointslice/reconciler.go +++ b/staging/src/k8s.io/endpointslice/reconciler.go @@ -51,9 +51,13 @@ type Reconciler struct { // topologyCache tracks the distribution of Nodes and endpoints across zones // to enable TopologyAwareHints. topologyCache *topologycache.TopologyCache - // trafficDistributionEnabled determines if endpointDistribution field is to + // trafficDistributionEnabled determines if trafficDistribution field is to // be considered when reconciling EndpointSlice hints. trafficDistributionEnabled bool + // preferSameTrafficDistribution determines if the new (PreferSameZone / + // PreferSameNode) trafficDistribution values should be considered when + // reconciling EndpointSlice hints. + preferSameTrafficDistribution bool // eventRecorder allows Reconciler to record and publish events. eventRecorder record.EventRecorder controllerName string @@ -63,12 +67,32 @@ type ReconcilerOption func(*Reconciler) // WithTrafficDistributionEnabled controls whether the Reconciler considers the // `trafficDistribution` field while reconciling EndpointSlices. -func WithTrafficDistributionEnabled(enabled bool) ReconcilerOption { +func WithTrafficDistributionEnabled(enabled, preferSame bool) ReconcilerOption { return func(r *Reconciler) { r.trafficDistributionEnabled = enabled + r.preferSameTrafficDistribution = preferSame } } +// validTrafficDistribution determines whether TrafficDistribution is set and valid for +// this cluster. +func (r *Reconciler) validTrafficDistribution(trafficDistribution *string) bool { + if trafficDistribution == nil || !r.trafficDistributionEnabled { + return false + } + if *trafficDistribution == corev1.ServiceTrafficDistributionPreferClose { + return true + } + if !r.preferSameTrafficDistribution { + return false + } + if *trafficDistribution == corev1.ServiceTrafficDistributionPreferSameZone || + *trafficDistribution == corev1.ServiceTrafficDistributionPreferSameNode { + return true + } + return false +} + // endpointMeta includes the attributes we group slices on, this type helps with // that logic in Reconciler type endpointMeta struct { @@ -275,7 +299,7 @@ func (r *Reconciler) reconcileByAddressType(logger klog.Logger, service *corev1. Unchanged: unchangedSlices(existingSlices, slicesToUpdate, slicesToDelete), } - canUseTrafficDistribution := r.trafficDistributionEnabled && !hintsEnabled(service.Annotations) + canUseTrafficDistribution := r.validTrafficDistribution(service.Spec.TrafficDistribution) && !hintsEnabled(service.Annotations) // Check if we need to add/remove hints based on the topology annotation. // @@ -455,10 +479,8 @@ func (r *Reconciler) finalize( topologyLabel = "Auto" } var trafficDistribution string - if r.trafficDistributionEnabled && !hintsEnabled(service.Annotations) { - if service.Spec.TrafficDistribution != nil && *service.Spec.TrafficDistribution == corev1.ServiceTrafficDistributionPreferClose { - trafficDistribution = *service.Spec.TrafficDistribution - } + if r.validTrafficDistribution(service.Spec.TrafficDistribution) && !hintsEnabled(service.Annotations) { + trafficDistribution = *service.Spec.TrafficDistribution } numSlicesChanged := len(slicesToCreate) + len(slicesToUpdate) + len(slicesToDelete) diff --git a/staging/src/k8s.io/endpointslice/reconciler_test.go b/staging/src/k8s.io/endpointslice/reconciler_test.go index f46fcfdabb5..d050f0c0712 100644 --- a/staging/src/k8s.io/endpointslice/reconciler_test.go +++ b/staging/src/k8s.io/endpointslice/reconciler_test.go @@ -2017,11 +2017,13 @@ func TestReconcile_TrafficDistribution(t *testing.T) { desc string trafficDistributionFeatureGateEnabled bool + preferSameFeatureGateEnabled bool trafficDistribution *string topologyAnnotation string - // Defines how many hints belong to a particular zone. + // Defines how many hints belong to a particular zone/node wantHintsDistributionByZone map[string]int + wantHintsDistributionByNode map[string]int // Number of endpoints where the zone hints are different from the zone of // the endpoint itself. wantEndpointsWithCrossZoneHints int @@ -2123,6 +2125,62 @@ func TestReconcile_TrafficDistribution(t *testing.T) { slicesChangedPerSyncTrafficDist: 0, // 0 means trafficDistribution was not used. }, }, + { + name: "trafficDistribution=PreferSameNode, PSTD enabled", + desc: "When trafficDistribution is PreferSameNode and PreferSameTrafficDistribution is enabled, both zone and node hints should be filled out", + trafficDistributionFeatureGateEnabled: true, + preferSameFeatureGateEnabled: true, + trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferSameNode), + topologyAnnotation: "Disabled", + wantHintsDistributionByZone: map[string]int{ + "zone-a": 1, // {pod-0} + "zone-b": 3, // {pod-1, pod-2, pod-3} + "zone-c": 2, // {pod-4, pod-5} + }, + wantHintsDistributionByNode: map[string]int{ + "node-0": 1, // {pod-0} + "node-1": 3, // {pod-1, pod-2, pod-3} + "node-2": 2, // {pod-4, pod-5} + }, + wantMetrics: expectedMetrics{ + desiredSlices: 1, + actualSlices: 1, + desiredEndpoints: 6, + addedPerSync: 6, + removedPerSync: 0, + numCreated: 1, + numUpdated: 0, + numDeleted: 0, + slicesChangedPerSync: 0, // 0 means either topologyAnnotation or trafficDistribution was used. + slicesChangedPerSyncTopology: 0, // 0 means topologyAnnotation was not used. + slicesChangedPerSyncTrafficDist: 1, // 1 EPS configured using trafficDistribution. + servicesCountByTrafficDistribution: map[string]int{ + "PreferSameNode": 1, + }, + }, + }, + { + name: "trafficDistribution=PreferSameZone, PSTD disabled", + desc: "When trafficDistribution is PreferSameZone and PreferSameTrafficDistribution is disabled, no hints should be set", + trafficDistributionFeatureGateEnabled: true, + preferSameFeatureGateEnabled: false, + trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferSameZone), + topologyAnnotation: "Disabled", + wantHintsDistributionByZone: map[string]int{"": 6}, // Equivalent to no hints. + wantMetrics: expectedMetrics{ + desiredSlices: 1, + actualSlices: 1, + desiredEndpoints: 6, + addedPerSync: 6, + removedPerSync: 0, + numCreated: 1, + numUpdated: 0, + numDeleted: 0, + slicesChangedPerSync: 1, // 1 means both topologyAnnotation and trafficDistribution were not used. + slicesChangedPerSyncTopology: 0, // 0 means topologyAnnotation was not used. + slicesChangedPerSyncTrafficDist: 0, // 0 means trafficDistribution was not used. + }, + }, } // Make assertions. @@ -2135,6 +2193,7 @@ func TestReconcile_TrafficDistribution(t *testing.T) { r := newReconciler(client, nodes, defaultMaxEndpointsPerSlice) r.trafficDistributionEnabled = tc.trafficDistributionFeatureGateEnabled + r.preferSameTrafficDistribution = tc.preferSameFeatureGateEnabled r.topologyCache = topologycache.NewTopologyCache() r.topologyCache.SetNodes(logger, nodes) @@ -2397,8 +2456,13 @@ func expectMetrics(t *testing.T, em expectedMetrics) { t.Errorf("Expected slicesChangedPerSyncTopology to be %d, got %v", em.slicesChangedPerSyncTopology, actualSlicesChangedPerSyncTopology) } - actualSlicesChangedPerSyncTrafficDist, err := testutil.GetHistogramMetricValue(metrics.EndpointSlicesChangedPerSync.WithLabelValues("Disabled", "PreferClose")) - handleErr(t, err, "slicesChangedPerSyncTrafficDist") + actualSlicesChangedPreferClose, err := testutil.GetHistogramMetricValue(metrics.EndpointSlicesChangedPerSync.WithLabelValues("Disabled", "PreferClose")) + handleErr(t, err, "slicesChangedPreferClose") + actualSlicesChangedPreferSameZone, err := testutil.GetHistogramMetricValue(metrics.EndpointSlicesChangedPerSync.WithLabelValues("Disabled", "PreferSameZone")) + handleErr(t, err, "slicesChangedPreferSameZone") + actualSlicesChangedPreferSameNode, err := testutil.GetHistogramMetricValue(metrics.EndpointSlicesChangedPerSync.WithLabelValues("Disabled", "PreferSameNode")) + handleErr(t, err, "slicesChangedPreferSameNode") + actualSlicesChangedPerSyncTrafficDist := actualSlicesChangedPreferClose + actualSlicesChangedPreferSameZone + actualSlicesChangedPreferSameNode if actualSlicesChangedPerSyncTrafficDist != float64(em.slicesChangedPerSyncTrafficDist) { t.Errorf("Expected slicesChangedPerSyncTrafficDist to be %d, got %v", em.slicesChangedPerSyncTrafficDist, actualSlicesChangedPerSyncTopology) } diff --git a/staging/src/k8s.io/endpointslice/trafficdist/trafficdist.go b/staging/src/k8s.io/endpointslice/trafficdist/trafficdist.go index c4250bb44bd..a95cd170393 100644 --- a/staging/src/k8s.io/endpointslice/trafficdist/trafficdist.go +++ b/staging/src/k8s.io/endpointslice/trafficdist/trafficdist.go @@ -20,6 +20,14 @@ package trafficdist import ( corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" + "k8s.io/apimachinery/pkg/util/sets" +) + +// TrafficDistribution values supported by preferCloseHeuristic +var closeTrafficDistribution = sets.New( + corev1.ServiceTrafficDistributionPreferClose, + corev1.ServiceTrafficDistributionPreferSameZone, + corev1.ServiceTrafficDistributionPreferSameNode, ) // ReconcileHints will reconcile hints for the given EndpointSlices. @@ -28,12 +36,12 @@ import ( func ReconcileHints(trafficDistribution *string, slicesToCreate, slicesToUpdate, slicesUnchanged []*discoveryv1.EndpointSlice) ([]*discoveryv1.EndpointSlice, []*discoveryv1.EndpointSlice, []*discoveryv1.EndpointSlice) { var h heuristic = &defaultHeuristic{} - if trafficDistribution != nil && *trafficDistribution == corev1.ServiceTrafficDistributionPreferClose { - h = &preferCloseHeuristic{} + if trafficDistribution != nil && closeTrafficDistribution.Has(*trafficDistribution) { + h = &preferCloseHeuristic{*trafficDistribution == corev1.ServiceTrafficDistributionPreferSameNode} } // Identify the Unchanged slices that need an update because of missing or - // incorrect zone hint. + // incorrect hints. // // Uses filtering in place to remove any endpoints that are no longer // unchanged and need to be moved to slicesToUpdate @@ -101,13 +109,14 @@ func (defaultHeuristic) update(slice *discoveryv1.EndpointSlice) { } } -// preferCloseHeuristic adds +// preferCloseHeuristic implements PreferSameZone/PreferClose and PreferSameNode type preferCloseHeuristic struct { + generateNodeHints bool } // needsUpdate returns true if any ready endpoint in the slice has a // missing or incorrect hint. -func (preferCloseHeuristic) needsUpdate(slice *discoveryv1.EndpointSlice) bool { +func (h preferCloseHeuristic) needsUpdate(slice *discoveryv1.EndpointSlice) bool { if slice == nil { return false } @@ -129,25 +138,44 @@ func (preferCloseHeuristic) needsUpdate(slice *discoveryv1.EndpointSlice) bool { return true } } + + if endpoint.NodeName != nil && h.generateNodeHints { + // We want a node hint. + if endpoint.Hints == nil || len(endpoint.Hints.ForNodes) != 1 || endpoint.Hints.ForNodes[0].Name != *endpoint.NodeName { + // ...but it's either missing or incorrect + return true + } + } else { + // We don't want a node hint. + if endpoint.Hints != nil && len(endpoint.Hints.ForNodes) > 0 { + // ... but we have a stale hint. + return true + } + } } return false } // update adds a same zone topology hint for all ready endpoints -func (preferCloseHeuristic) update(slice *discoveryv1.EndpointSlice) { +func (h preferCloseHeuristic) update(slice *discoveryv1.EndpointSlice) { for i, endpoint := range slice.Endpoints { if !endpointReady(endpoint) { continue } var forZones []discoveryv1.ForZone + var forNodes []discoveryv1.ForNode if endpoint.Zone != nil { forZones = []discoveryv1.ForZone{{Name: *endpoint.Zone}} } + if endpoint.NodeName != nil && h.generateNodeHints { + forNodes = []discoveryv1.ForNode{{Name: *endpoint.NodeName}} + } - if forZones != nil { + if forZones != nil || forNodes != nil { slice.Endpoints[i].Hints = &discoveryv1.EndpointHints{ ForZones: forZones, + ForNodes: forNodes, } } else { slice.Endpoints[i].Hints = nil diff --git a/staging/src/k8s.io/endpointslice/trafficdist/trafficdist_test.go b/staging/src/k8s.io/endpointslice/trafficdist/trafficdist_test.go index 5ad5c4a45ab..cc7b857ba93 100644 --- a/staging/src/k8s.io/endpointslice/trafficdist/trafficdist_test.go +++ b/staging/src/k8s.io/endpointslice/trafficdist/trafficdist_test.go @@ -188,6 +188,328 @@ func TestReconcileHints(t *testing.T) { }, }, }, + { + name: "should set zone hints with PreferSameZone", + + trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferSameZone), + slicesToCreate: []*discoveryv1.EndpointSlice{ + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.1"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-1"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.2"}, + Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-2"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.3"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-3"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.4"}, + Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-4"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + }, + slicesToUpdate: []*discoveryv1.EndpointSlice{ + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.5"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-5"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.6"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-6"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.7"}, + Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-7"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.8"}, + Zone: ptr.To("zone-c"), + NodeName: ptr.To("node-8"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + }, + wantSlicesToCreate: []*discoveryv1.EndpointSlice{ + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.1"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-1"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}}, + }, + { + Addresses: []string{"10.0.0.2"}, + Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-2"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-b"}}}, + }, + }, + }, + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.3"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-3"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}}, + }, + { + Addresses: []string{"10.0.0.4"}, + Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-4"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-b"}}}, + }, + }, + }, + }, + wantSlicesToUpdate: []*discoveryv1.EndpointSlice{ + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.5"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-5"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}}, + }, + { + Addresses: []string{"10.0.0.6"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-6"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}}, + }, + }, + }, + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.7"}, + Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-7"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-b"}}}, + }, + { + Addresses: []string{"10.0.0.8"}, + Zone: ptr.To("zone-c"), + NodeName: ptr.To("node-8"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ForZones: []discoveryv1.ForZone{{Name: "zone-c"}}}, + }, + }, + }, + }, + }, + { + name: "should set zone and node hints with PreferSameNode", + + trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferSameNode), + slicesToCreate: []*discoveryv1.EndpointSlice{ + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.1"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-1"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.2"}, + Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-2"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.3"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-3"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.4"}, + Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-4"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + }, + slicesToUpdate: []*discoveryv1.EndpointSlice{ + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.5"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-5"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.6"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-6"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.7"}, + Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-7"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + { + Addresses: []string{"10.0.0.8"}, + Zone: ptr.To("zone-c"), + NodeName: ptr.To("node-8"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + }, + }, + }, + }, + wantSlicesToCreate: []*discoveryv1.EndpointSlice{ + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.1"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-1"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}, + ForNodes: []discoveryv1.ForNode{{Name: "node-1"}}, + }, + }, + { + Addresses: []string{"10.0.0.2"}, + Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-2"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForZones: []discoveryv1.ForZone{{Name: "zone-b"}}, + ForNodes: []discoveryv1.ForNode{{Name: "node-2"}}, + }, + }, + }, + }, + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.3"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-3"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}, + ForNodes: []discoveryv1.ForNode{{Name: "node-3"}}, + }, + }, + { + Addresses: []string{"10.0.0.4"}, + Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-4"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForZones: []discoveryv1.ForZone{{Name: "zone-b"}}, + ForNodes: []discoveryv1.ForNode{{Name: "node-4"}}, + }, + }, + }, + }, + }, + wantSlicesToUpdate: []*discoveryv1.EndpointSlice{ + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.5"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-5"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}, + ForNodes: []discoveryv1.ForNode{{Name: "node-5"}}, + }, + }, + { + Addresses: []string{"10.0.0.6"}, + Zone: ptr.To("zone-a"), + NodeName: ptr.To("node-6"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForZones: []discoveryv1.ForZone{{Name: "zone-a"}}, + ForNodes: []discoveryv1.ForNode{{Name: "node-6"}}, + }, + }, + }, + }, + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.7"}, + Zone: ptr.To("zone-b"), + NodeName: ptr.To("node-7"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForZones: []discoveryv1.ForZone{{Name: "zone-b"}}, + ForNodes: []discoveryv1.ForNode{{Name: "node-7"}}, + }, + }, + { + Addresses: []string{"10.0.0.8"}, + Zone: ptr.To("zone-c"), + NodeName: ptr.To("node-8"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForZones: []discoveryv1.ForZone{{Name: "zone-c"}}, + ForNodes: []discoveryv1.ForNode{{Name: "node-8"}}, + }, + }, + }, + }, + }, + }, { name: "should correct incorrect hints with PreferClose", @@ -265,7 +587,7 @@ func TestReconcileHints(t *testing.T) { }, }, { - name: "should not create zone hints if there are no zones", + name: "should not create hints with PreferClose if there are no zones", trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferClose), slicesToCreate: []*discoveryv1.EndpointSlice{ @@ -389,6 +711,155 @@ func TestReconcileHints(t *testing.T) { }, }, }, + { + name: "should create only node hints with PreferSameNode if there are no zones", + + trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferSameNode), + 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)}, + Hints: &discoveryv1.EndpointHints{ + ForNodes: []discoveryv1.ForNode{{Name: "node-1"}}, + }, + }, + { + Addresses: []string{"10.0.0.2"}, + NodeName: ptr.To("node-2"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForNodes: []discoveryv1.ForNode{{Name: "node-2"}}, + }, + }, + }, + }, + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.3"}, + NodeName: ptr.To("node-3"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForNodes: []discoveryv1.ForNode{{Name: "node-3"}}, + }, + }, + { + Addresses: []string{"10.0.0.4"}, + NodeName: ptr.To("node-4"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForNodes: []discoveryv1.ForNode{{Name: "node-4"}}, + }, + }, + }, + }, + }, + wantSlicesToUpdate: []*discoveryv1.EndpointSlice{ + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.5"}, + NodeName: ptr.To("node-5"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForNodes: []discoveryv1.ForNode{{Name: "node-5"}}, + }, + }, + { + Addresses: []string{"10.0.0.6"}, + NodeName: ptr.To("node-6"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForNodes: []discoveryv1.ForNode{{Name: "node-6"}}, + }, + }, + }, + }, + { + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.7"}, + NodeName: ptr.To("node-7"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForNodes: []discoveryv1.ForNode{{Name: "node-7"}}, + }, + }, + { + Addresses: []string{"10.0.0.8"}, + NodeName: ptr.To("node-8"), + Conditions: discoveryv1.EndpointConditions{Ready: ptr.To(true)}, + Hints: &discoveryv1.EndpointHints{ + ForNodes: []discoveryv1.ForNode{{Name: "node-8"}}, + }, + }, + }, + }, + }, + }, { name: "unready endpoints should not trigger updates",