diff --git a/pkg/proxy/topology.go b/pkg/proxy/topology.go index 10887d9bc39..a79677964be 100644 --- a/pkg/proxy/topology.go +++ b/pkg/proxy/topology.go @@ -18,9 +18,7 @@ package proxy import ( v1 "k8s.io/api/core/v1" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" - "k8s.io/kubernetes/pkg/features" ) // CategorizeEndpoints returns: @@ -41,7 +39,7 @@ func CategorizeEndpoints(endpoints []Endpoint, svcInfo ServicePort, nodeLabels m var useTopology, useServingTerminatingEndpoints bool if svcInfo.UsesClusterEndpoints() { - useTopology = canUseTopology(endpoints, svcInfo, nodeLabels) + useTopology = canUseTopology(endpoints, nodeLabels) clusterEndpoints = filterEndpoints(endpoints, func(ep Endpoint) bool { if !ep.IsReady() { return false @@ -137,29 +135,11 @@ 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 or ServiceTrafficDistribution feature is enabled. -// - If ServiceTrafficDistribution feature gate is not enabled, then the -// hintsAnnotation should represent an enabled value. +// canUseTopology returns true if all of the following is true: // - 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) && !utilfeature.DefaultFeatureGate.Enabled(features.ServiceTrafficDistribution) { - 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 - } - } - +func canUseTopology(endpoints []Endpoint, nodeLabels map[string]string) bool { zone, foundZone := nodeLabels[v1.LabelTopologyZone] hasEndpointForZone := false for _, endpoint := range endpoints { diff --git a/pkg/proxy/topology_test.go b/pkg/proxy/topology_test.go index f0d5002b62e..684467dba0f 100644 --- a/pkg/proxy/topology_test.go +++ b/pkg/proxy/topology_test.go @@ -47,13 +47,12 @@ func checkExpectedEndpoints(expected sets.Set[string], actual []Endpoint) error func TestCategorizeEndpoints(t *testing.T) { testCases := []struct { - name string - hintsEnabled bool - trafficDistFeatureEnabled bool - pteEnabled bool - nodeLabels map[string]string - serviceInfo ServicePort - endpoints []Endpoint + name string + hintsEnabled 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"). @@ -79,7 +78,7 @@ func TestCategorizeEndpoints(t *testing.T) { clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.6:80"), localEndpoints: nil, }, { - name: "hints, hints annotation == disabled, hints ignored", + name: "hints, hints annotation == disabled, but endpointslice hints are not ignored since trafficDist feature-gate is always enabled by default", hintsEnabled: true, nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, serviceInfo: &BaseServicePortInfo{hintsAnnotation: "disabled"}, @@ -89,20 +88,7 @@ func TestCategorizeEndpoints(t *testing.T) { &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.4:80", "10.1.2.5:80", "10.1.2.6:80"), - localEndpoints: nil, - }, { - name: "hints disabled, hints annotation == auto", - hintsEnabled: false, - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, - serviceInfo: &BaseServicePortInfo{hintsAnnotation: "auto"}, - 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.4:80", "10.1.2.5:80", "10.1.2.6:80"), + clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.6:80"), localEndpoints: nil, }, { @@ -119,9 +105,22 @@ func TestCategorizeEndpoints(t *testing.T) { clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.6:80"), localEndpoints: nil, }, { - name: "hints, hints annotation empty, hints ignored", + name: "hints, hints annotation empty but endpointslice hints are not ignored since trafficDist feature-gate is always enabled by default", hintsEnabled: true, nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, + serviceInfo: &BaseServicePortInfo{ /* hints annotation empty */ }, + 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 feature-gate disabled but endpointslice hints are not ignored since trafficDist feature-gate is always enabled by default", + hintsEnabled: false, + 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}, @@ -129,42 +128,13 @@ func TestCategorizeEndpoints(t *testing.T) { &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.4:80", "10.1.2.5:80", "10.1.2.6:80"), - localEndpoints: nil, - }, { - 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"}, + 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"}, 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}, @@ -175,11 +145,10 @@ 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, - trafficDistFeatureEnabled: 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, + 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}, @@ -271,32 +240,6 @@ func TestCategorizeEndpoints(t *testing.T) { }, clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.6:80"), localEndpoints: nil, - }, { - name: "hintsAnnotation empty, no filtering applied", - hintsEnabled: true, - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, - serviceInfo: &BaseServicePortInfo{hintsAnnotation: ""}, - 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.4:80", "10.1.2.5:80", "10.1.2.6:80"), - localEndpoints: nil, - }, { - name: "hintsAnnotation disabled, no filtering applied", - hintsEnabled: true, - nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, - serviceInfo: &BaseServicePortInfo{hintsAnnotation: "disabled"}, - 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.4:80", "10.1.2.5:80", "10.1.2.6:80"), - localEndpoints: nil, }, { name: "missing hints, no filtering applied", hintsEnabled: true, @@ -489,7 +432,6 @@ func TestCategorizeEndpoints(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TopologyAwareHints, tc.hintsEnabled) - featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceTrafficDistribution, tc.trafficDistFeatureEnabled) clusterEndpoints, localEndpoints, allEndpoints, hasAnyEndpoints := CategorizeEndpoints(tc.endpoints, tc.serviceInfo, tc.nodeLabels)