diff --git a/pkg/proxy/topology.go b/pkg/proxy/topology.go index faa7ac2e1be..9ccd359534d 100644 --- a/pkg/proxy/topology.go +++ b/pkg/proxy/topology.go @@ -137,21 +137,27 @@ func CategorizeEndpoints(endpoints []Endpoint, svcInfo ServicePort, nodeLabels m return } -// canUseTopology returns true if topology aware routing is enabled and properly configured -// in this cluster. That is, it checks that: -// * The TopologyAwareHints feature is enabled -// * The "service.kubernetes.io/topology-aware-hints" annotation on this Service is set to "Auto" -// * The node's labels include "topology.kubernetes.io/zone" -// * All of the endpoints for this Service have a topology hint -// * At least one endpoint for this Service is hinted for this node's zone. +// canUseTopology returns true if topology aware routing is enabled and properly +// configured in this cluster. That is, it checks that: +// - The TopologyAwareHints or ServiceTrafficDistribution feature is enabled. +// - If ServiceTrafficDistribution feature gate is not enabled, then the +// hintsAnnotation should represent an enabled value. +// - The node's labels include "topology.kubernetes.io/zone". +// - All of the endpoints for this Service have a topology hint. +// - At least one endpoint for this Service is hinted for this node's zone. func canUseTopology(endpoints []Endpoint, svcInfo ServicePort, nodeLabels map[string]string) bool { - if !utilfeature.DefaultFeatureGate.Enabled(features.TopologyAwareHints) { + if !utilfeature.DefaultFeatureGate.Enabled(features.TopologyAwareHints) && !utilfeature.DefaultFeatureGate.Enabled(features.ServiceTrafficDistribution) { return false } - // Any non-empty and non-disabled values for the hints annotation are acceptable. - hintsAnnotation := svcInfo.HintsAnnotation() - if hintsAnnotation == "" || hintsAnnotation == "disabled" || hintsAnnotation == "Disabled" { - return false + + // Ignore value of hintsAnnotation if the ServiceTrafficDistribution feature + // gate is enabled. + if !utilfeature.DefaultFeatureGate.Enabled(features.ServiceTrafficDistribution) { + // If the hintsAnnotation has a disabled value, we do not consider hints for route programming. + hintsAnnotation := svcInfo.HintsAnnotation() + if hintsAnnotation == "" || hintsAnnotation == "disabled" || hintsAnnotation == "Disabled" { + return false + } } zone, ok := nodeLabels[v1.LabelTopologyZone] diff --git a/pkg/proxy/topology_test.go b/pkg/proxy/topology_test.go index e3865947ea7..a859c3892a4 100644 --- a/pkg/proxy/topology_test.go +++ b/pkg/proxy/topology_test.go @@ -47,12 +47,13 @@ func checkExpectedEndpoints(expected sets.Set[string], actual []Endpoint) error func TestCategorizeEndpoints(t *testing.T) { testCases := []struct { - name string - hintsEnabled bool - pteEnabled bool - nodeLabels map[string]string - serviceInfo ServicePort - endpoints []Endpoint + name string + hintsEnabled bool + trafficDistFeatureEnabled bool + pteEnabled bool + nodeLabels map[string]string + serviceInfo ServicePort + endpoints []Endpoint // We distinguish `nil` ("service doesn't use this kind of endpoints") from // `sets.Set[string]()` ("service uses this kind of endpoints but has no endpoints"). @@ -131,10 +132,39 @@ func TestCategorizeEndpoints(t *testing.T) { clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), localEndpoints: nil, }, { - name: "externalTrafficPolicy: Local, topology ignored for Local endpoints", - hintsEnabled: true, - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, - serviceInfo: &BaseServicePortInfo{externalPolicyLocal: true, nodePort: 8080, hintsAnnotation: "auto"}, + name: "hints, hints annotation empty but trafficDist feature gate enabled, hints are not ignored", + hintsEnabled: true, + trafficDistFeatureEnabled: true, + nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, + serviceInfo: &BaseServicePortInfo{}, + endpoints: []Endpoint{ + &BaseEndpointInfo{endpoint: "10.1.2.3:80", zoneHints: sets.New[string]("zone-a"), ready: true}, + &BaseEndpointInfo{endpoint: "10.1.2.4:80", zoneHints: sets.New[string]("zone-b"), ready: true}, + &BaseEndpointInfo{endpoint: "10.1.2.5:80", zoneHints: sets.New[string]("zone-c"), ready: true}, + &BaseEndpointInfo{endpoint: "10.1.2.6:80", zoneHints: sets.New[string]("zone-a"), ready: true}, + }, + clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.6:80"), + localEndpoints: nil, + }, { + name: "hints disabled, trafficDist feature gate enabled, hints are not ignored", + hintsEnabled: false, + trafficDistFeatureEnabled: true, + nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, + serviceInfo: &BaseServicePortInfo{}, + endpoints: []Endpoint{ + &BaseEndpointInfo{endpoint: "10.1.2.3:80", zoneHints: sets.New[string]("zone-a"), ready: true}, + &BaseEndpointInfo{endpoint: "10.1.2.4:80", zoneHints: sets.New[string]("zone-b"), ready: true}, + &BaseEndpointInfo{endpoint: "10.1.2.5:80", zoneHints: sets.New[string]("zone-c"), ready: true}, + &BaseEndpointInfo{endpoint: "10.1.2.6:80", zoneHints: sets.New[string]("zone-a"), ready: true}, + }, + clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.6:80"), + localEndpoints: nil, + }, { + name: "externalTrafficPolicy: Local, topology ignored for Local endpoints", + hintsEnabled: true, + trafficDistFeatureEnabled: true, + nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, + serviceInfo: &BaseServicePortInfo{externalPolicyLocal: true, nodePort: 8080, hintsAnnotation: "auto"}, endpoints: []Endpoint{ &BaseEndpointInfo{endpoint: "10.1.2.3:80", zoneHints: sets.New[string]("zone-a"), ready: true, isLocal: true}, &BaseEndpointInfo{endpoint: "10.1.2.4:80", zoneHints: sets.New[string]("zone-b"), ready: true, isLocal: true}, @@ -145,10 +175,11 @@ func TestCategorizeEndpoints(t *testing.T) { localEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.4:80"), allEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.4:80", "10.1.2.6:80"), }, { - name: "internalTrafficPolicy: Local, topology ignored for Local endpoints", - hintsEnabled: true, - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, - serviceInfo: &BaseServicePortInfo{internalPolicyLocal: true, hintsAnnotation: "auto", externalPolicyLocal: false, nodePort: 8080}, + name: "internalTrafficPolicy: Local, topology ignored for Local endpoints", + hintsEnabled: true, + trafficDistFeatureEnabled: true, + nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, + serviceInfo: &BaseServicePortInfo{internalPolicyLocal: true, hintsAnnotation: "auto", externalPolicyLocal: false, nodePort: 8080}, endpoints: []Endpoint{ &BaseEndpointInfo{endpoint: "10.1.2.3:80", zoneHints: sets.New[string]("zone-a"), ready: true, isLocal: true}, &BaseEndpointInfo{endpoint: "10.1.2.4:80", zoneHints: sets.New[string]("zone-b"), ready: true, isLocal: true}, @@ -458,6 +489,7 @@ func TestCategorizeEndpoints(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TopologyAwareHints, tc.hintsEnabled)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceTrafficDistribution, tc.trafficDistFeatureEnabled)() clusterEndpoints, localEndpoints, allEndpoints, hasAnyEndpoints := CategorizeEndpoints(tc.endpoints, tc.serviceInfo, tc.nodeLabels)