Skip to content

Commit 4e31726

Browse files
gauravkghildiyalgauravkghildiyal
committed
Remove usage of ServiceTrafficDistribution feature-gate from kube-proxy packages.
ServiceTrafficDistribution feature-gate is GA'd and enabled by default since 1.33. Since it is also locked-to-default, we can remove flag-usages in kube-proxy. NOTE that as per https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/feature-gates.md#disablement-tests: _"Disablement tests are only required to be preserved for components and libraries that support compatibility version. Tests for node and kubelet are unaffected by compatibility version."_
1 parent 2492edd commit 4e31726

File tree

2 files changed

+25
-103
lines changed

2 files changed

+25
-103
lines changed

‎pkg/proxy/topology.go‎

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@ package proxy
1818

1919
import (
2020
v1 "k8s.io/api/core/v1"
21-
utilfeature "k8s.io/apiserver/pkg/util/feature"
2221
"k8s.io/klog/v2"
23-
"k8s.io/kubernetes/pkg/features"
2422
)
2523

2624
// CategorizeEndpoints returns:
@@ -41,7 +39,7 @@ func CategorizeEndpoints(endpoints []Endpoint, svcInfo ServicePort, nodeLabels m
4139
var useTopology, useServingTerminatingEndpoints bool
4240

4341
if svcInfo.UsesClusterEndpoints() {
44-
useTopology = canUseTopology(endpoints, svcInfo, nodeLabels)
42+
useTopology = canUseTopology(endpoints, nodeLabels)
4543
clusterEndpoints = filterEndpoints(endpoints, func(ep Endpoint) bool {
4644
if !ep.IsReady() {
4745
return false
@@ -137,29 +135,11 @@ func CategorizeEndpoints(endpoints []Endpoint, svcInfo ServicePort, nodeLabels m
137135
return
138136
}
139137

140-
// canUseTopology returns true if topology aware routing is enabled and properly
141-
// configured in this cluster. That is, it checks that:
142-
// - The TopologyAwareHints or ServiceTrafficDistribution feature is enabled.
143-
// - If ServiceTrafficDistribution feature gate is not enabled, then the
144-
// hintsAnnotation should represent an enabled value.
138+
// canUseTopology returns true if all of the following is true:
145139
// - The node's labels include "topology.kubernetes.io/zone".
146140
// - All of the endpoints for this Service have a topology hint.
147141
// - At least one endpoint for this Service is hinted for this node's zone.
148-
func canUseTopology(endpoints []Endpoint, svcInfo ServicePort, nodeLabels map[string]string) bool {
149-
if !utilfeature.DefaultFeatureGate.Enabled(features.TopologyAwareHints) && !utilfeature.DefaultFeatureGate.Enabled(features.ServiceTrafficDistribution) {
150-
return false
151-
}
152-
153-
// Ignore value of hintsAnnotation if the ServiceTrafficDistribution feature
154-
// gate is enabled.
155-
if !utilfeature.DefaultFeatureGate.Enabled(features.ServiceTrafficDistribution) {
156-
// If the hintsAnnotation has a disabled value, we do not consider hints for route programming.
157-
hintsAnnotation := svcInfo.HintsAnnotation()
158-
if hintsAnnotation == "" || hintsAnnotation == "disabled" || hintsAnnotation == "Disabled" {
159-
return false
160-
}
161-
}
162-
142+
func canUseTopology(endpoints []Endpoint, nodeLabels map[string]string) bool {
163143
zone, foundZone := nodeLabels[v1.LabelTopologyZone]
164144
hasEndpointForZone := false
165145
for _, endpoint := range endpoints {

‎pkg/proxy/topology_test.go‎

Lines changed: 22 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,12 @@ func checkExpectedEndpoints(expected sets.Set[string], actual []Endpoint) error
4747

4848
func TestCategorizeEndpoints(t *testing.T) {
4949
testCases := []struct {
50-
name string
51-
hintsEnabled bool
52-
trafficDistFeatureEnabled bool
53-
pteEnabled bool
54-
nodeLabels map[string]string
55-
serviceInfo ServicePort
56-
endpoints []Endpoint
50+
name string
51+
hintsEnabled bool
52+
pteEnabled bool
53+
nodeLabels map[string]string
54+
serviceInfo ServicePort
55+
endpoints []Endpoint
5756

5857
// We distinguish `nil` ("service doesn't use this kind of endpoints") from
5958
// `sets.Set[string]()` ("service uses this kind of endpoints but has no endpoints").
@@ -79,7 +78,7 @@ func TestCategorizeEndpoints(t *testing.T) {
7978
clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.6:80"),
8079
localEndpoints: nil,
8180
}, {
82-
name: "hints, hints annotation == disabled, hints ignored",
81+
name: "hints, hints annotation == disabled, but endpointslice hints are not ignored since trafficDist feature-gate is always enabled by default",
8382
hintsEnabled: true,
8483
nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"},
8584
serviceInfo: &BaseServicePortInfo{hintsAnnotation: "disabled"},
@@ -89,20 +88,7 @@ func TestCategorizeEndpoints(t *testing.T) {
8988
&BaseEndpointInfo{endpoint: "10.1.2.5:80", zoneHints: sets.New[string]("zone-c"), ready: true},
9089
&BaseEndpointInfo{endpoint: "10.1.2.6:80", zoneHints: sets.New[string]("zone-a"), ready: true},
9190
},
92-
clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"),
93-
localEndpoints: nil,
94-
}, {
95-
name: "hints disabled, hints annotation == auto",
96-
hintsEnabled: false,
97-
nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"},
98-
serviceInfo: &BaseServicePortInfo{hintsAnnotation: "auto"},
99-
endpoints: []Endpoint{
100-
&BaseEndpointInfo{endpoint: "10.1.2.3:80", zoneHints: sets.New[string]("zone-a"), ready: true},
101-
&BaseEndpointInfo{endpoint: "10.1.2.4:80", zoneHints: sets.New[string]("zone-b"), ready: true},
102-
&BaseEndpointInfo{endpoint: "10.1.2.5:80", zoneHints: sets.New[string]("zone-c"), ready: true},
103-
&BaseEndpointInfo{endpoint: "10.1.2.6:80", zoneHints: sets.New[string]("zone-a"), ready: true},
104-
},
105-
clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"),
91+
clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.6:80"),
10692
localEndpoints: nil,
10793
}, {
10894

@@ -119,24 +105,10 @@ func TestCategorizeEndpoints(t *testing.T) {
119105
clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.6:80"),
120106
localEndpoints: nil,
121107
}, {
122-
name: "hints, hints annotation empty, hints ignored",
108+
name: "hints, hints annotation empty but endpointslice hints are not ignored since trafficDist feature-gate is always enabled by default",
123109
hintsEnabled: true,
124110
nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"},
125-
serviceInfo: &BaseServicePortInfo{},
126-
endpoints: []Endpoint{
127-
&BaseEndpointInfo{endpoint: "10.1.2.3:80", zoneHints: sets.New[string]("zone-a"), ready: true},
128-
&BaseEndpointInfo{endpoint: "10.1.2.4:80", zoneHints: sets.New[string]("zone-b"), ready: true},
129-
&BaseEndpointInfo{endpoint: "10.1.2.5:80", zoneHints: sets.New[string]("zone-c"), ready: true},
130-
&BaseEndpointInfo{endpoint: "10.1.2.6:80", zoneHints: sets.New[string]("zone-a"), ready: true},
131-
},
132-
clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"),
133-
localEndpoints: nil,
134-
}, {
135-
name: "hints, hints annotation empty but trafficDist feature gate enabled, hints are not ignored",
136-
hintsEnabled: true,
137-
trafficDistFeatureEnabled: true,
138-
nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"},
139-
serviceInfo: &BaseServicePortInfo{},
111+
serviceInfo: &BaseServicePortInfo{ /* hints annotation empty */ },
140112
endpoints: []Endpoint{
141113
&BaseEndpointInfo{endpoint: "10.1.2.3:80", zoneHints: sets.New[string]("zone-a"), ready: true},
142114
&BaseEndpointInfo{endpoint: "10.1.2.4:80", zoneHints: sets.New[string]("zone-b"), ready: true},
@@ -146,11 +118,10 @@ func TestCategorizeEndpoints(t *testing.T) {
146118
clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.6:80"),
147119
localEndpoints: nil,
148120
}, {
149-
name: "hints disabled, trafficDist feature gate enabled, hints are not ignored",
150-
hintsEnabled: false,
151-
trafficDistFeatureEnabled: true,
152-
nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"},
153-
serviceInfo: &BaseServicePortInfo{},
121+
name: "hints feature-gate disabled but endpointslice hints are not ignored since trafficDist feature-gate is always enabled by default",
122+
hintsEnabled: false,
123+
nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"},
124+
serviceInfo: &BaseServicePortInfo{},
154125
endpoints: []Endpoint{
155126
&BaseEndpointInfo{endpoint: "10.1.2.3:80", zoneHints: sets.New[string]("zone-a"), ready: true},
156127
&BaseEndpointInfo{endpoint: "10.1.2.4:80", zoneHints: sets.New[string]("zone-b"), ready: true},
@@ -160,11 +131,10 @@ func TestCategorizeEndpoints(t *testing.T) {
160131
clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.6:80"),
161132
localEndpoints: nil,
162133
}, {
163-
name: "externalTrafficPolicy: Local, topology ignored for Local endpoints",
164-
hintsEnabled: true,
165-
trafficDistFeatureEnabled: true,
166-
nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"},
167-
serviceInfo: &BaseServicePortInfo{externalPolicyLocal: true, nodePort: 8080, hintsAnnotation: "auto"},
134+
name: "externalTrafficPolicy: Local, topology ignored for Local endpoints",
135+
hintsEnabled: true,
136+
nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"},
137+
serviceInfo: &BaseServicePortInfo{externalPolicyLocal: true, nodePort: 8080, hintsAnnotation: "auto"},
168138
endpoints: []Endpoint{
169139
&BaseEndpointInfo{endpoint: "10.1.2.3:80", zoneHints: sets.New[string]("zone-a"), ready: true, isLocal: true},
170140
&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) {
175145
localEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.4:80"),
176146
allEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.4:80", "10.1.2.6:80"),
177147
}, {
178-
name: "internalTrafficPolicy: Local, topology ignored for Local endpoints",
179-
hintsEnabled: true,
180-
trafficDistFeatureEnabled: true,
181-
nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"},
182-
serviceInfo: &BaseServicePortInfo{internalPolicyLocal: true, hintsAnnotation: "auto", externalPolicyLocal: false, nodePort: 8080},
148+
name: "internalTrafficPolicy: Local, topology ignored for Local endpoints",
149+
hintsEnabled: true,
150+
nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"},
151+
serviceInfo: &BaseServicePortInfo{internalPolicyLocal: true, hintsAnnotation: "auto", externalPolicyLocal: false, nodePort: 8080},
183152
endpoints: []Endpoint{
184153
&BaseEndpointInfo{endpoint: "10.1.2.3:80", zoneHints: sets.New[string]("zone-a"), ready: true, isLocal: true},
185154
&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) {
271240
},
272241
clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.6:80"),
273242
localEndpoints: nil,
274-
}, {
275-
name: "hintsAnnotation empty, no filtering applied",
276-
hintsEnabled: true,
277-
nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"},
278-
serviceInfo: &BaseServicePortInfo{hintsAnnotation: ""},
279-
endpoints: []Endpoint{
280-
&BaseEndpointInfo{endpoint: "10.1.2.3:80", zoneHints: sets.New[string]("zone-a"), ready: true},
281-
&BaseEndpointInfo{endpoint: "10.1.2.4:80", zoneHints: sets.New[string]("zone-b"), ready: true},
282-
&BaseEndpointInfo{endpoint: "10.1.2.5:80", zoneHints: sets.New[string]("zone-c"), ready: true},
283-
&BaseEndpointInfo{endpoint: "10.1.2.6:80", zoneHints: sets.New[string]("zone-a"), ready: true},
284-
},
285-
clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"),
286-
localEndpoints: nil,
287-
}, {
288-
name: "hintsAnnotation disabled, no filtering applied",
289-
hintsEnabled: true,
290-
nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"},
291-
serviceInfo: &BaseServicePortInfo{hintsAnnotation: "disabled"},
292-
endpoints: []Endpoint{
293-
&BaseEndpointInfo{endpoint: "10.1.2.3:80", zoneHints: sets.New[string]("zone-a"), ready: true},
294-
&BaseEndpointInfo{endpoint: "10.1.2.4:80", zoneHints: sets.New[string]("zone-b"), ready: true},
295-
&BaseEndpointInfo{endpoint: "10.1.2.5:80", zoneHints: sets.New[string]("zone-c"), ready: true},
296-
&BaseEndpointInfo{endpoint: "10.1.2.6:80", zoneHints: sets.New[string]("zone-a"), ready: true},
297-
},
298-
clusterEndpoints: sets.New[string]("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"),
299-
localEndpoints: nil,
300243
}, {
301244
name: "missing hints, no filtering applied",
302245
hintsEnabled: true,
@@ -489,7 +432,6 @@ func TestCategorizeEndpoints(t *testing.T) {
489432
for _, tc := range testCases {
490433
t.Run(tc.name, func(t *testing.T) {
491434
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TopologyAwareHints, tc.hintsEnabled)
492-
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceTrafficDistribution, tc.trafficDistFeatureEnabled)
493435

494436
clusterEndpoints, localEndpoints, allEndpoints, hasAnyEndpoints := CategorizeEndpoints(tc.endpoints, tc.serviceInfo, tc.nodeLabels)
495437

0 commit comments

Comments
 (0)