From 0d0b81b9375c9c6850d78270b8a7504af6602ff5 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Sun, 9 Mar 2025 14:35:03 -0700 Subject: [PATCH 1/5] Update trafficDistribution API spec docs for GA graduation --- pkg/apis/core/types.go | 24 +++++++++++------------- staging/src/k8s.io/api/core/v1/types.go | 25 +++++++++++-------------- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index c4ea9e27329..537f783fa17 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -4550,13 +4550,11 @@ const ( // These are valid values for the TrafficDistribution field of a Service. const ( - // Indicates a preference for routing traffic to endpoints that are - // topologically proximate to the client. The interpretation of "topologically - // proximate" may vary across implementations and could encompass endpoints - // within the same node, rack, zone, or even region. Setting this value gives - // implementations permission to make different tradeoffs, e.g. optimizing for - // proximity rather than equal distribution of load. Users should not set this - // value if such tradeoffs are not acceptable. + // Indicates a preference for routing traffic to endpoints that are in the + // same zone as the client. Setting this value gives implementations + // permission to make different tradeoffs, e.g. optimizing for proximity + // rather than equal distribution of load. Users should not set this value + // if such tradeoffs are not acceptable. ServiceTrafficDistributionPreferClose = "PreferClose" ) @@ -4824,12 +4822,12 @@ type ServiceSpec struct { // +optional InternalTrafficPolicy *ServiceInternalTrafficPolicy - // TrafficDistribution offers a way to express preferences for how traffic is - // distributed to Service endpoints. Implementations can use this field as a - // hint, but are not required to guarantee strict adherence. If the field is - // not set, the implementation will apply its default routing strategy. If set - // to "PreferClose", implementations should prioritize endpoints that are - // topologically close (e.g., same zone). + // TrafficDistribution offers a way to express preferences for how traffic + // is distributed to Service endpoints. Implementations can use this field + // as a hint, but are not required to guarantee strict adherence. If the + // field is not set, the implementation will apply its default routing + // strategy. If set to "PreferClose", implementations should prioritize + // endpoints that are in the same zone. // +optional TrafficDistribution *string } diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 5cc5f28000e..793794710a9 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -5344,13 +5344,11 @@ const ( // These are valid values for the TrafficDistribution field of a Service. const ( - // Indicates a preference for routing traffic to endpoints that are - // topologically proximate to the client. The interpretation of "topologically - // proximate" may vary across implementations and could encompass endpoints - // within the same node, rack, zone, or even region. Setting this value gives - // implementations permission to make different tradeoffs, e.g. optimizing for - // proximity rather than equal distribution of load. Users should not set this - // value if such tradeoffs are not acceptable. + // Indicates a preference for routing traffic to endpoints that are in the + // same zone as the client. Setting this value gives implementations + // permission to make different tradeoffs, e.g. optimizing for proximity + // rather than equal distribution of load. Users should not set this value + // if such tradeoffs are not acceptable. ServiceTrafficDistributionPreferClose = "PreferClose" ) @@ -5699,13 +5697,12 @@ type ServiceSpec struct { // +optional InternalTrafficPolicy *ServiceInternalTrafficPolicy `json:"internalTrafficPolicy,omitempty" protobuf:"bytes,22,opt,name=internalTrafficPolicy"` - // TrafficDistribution offers a way to express preferences for how traffic is - // distributed to Service endpoints. Implementations can use this field as a - // hint, but are not required to guarantee strict adherence. If the field is - // not set, the implementation will apply its default routing strategy. If set - // to "PreferClose", implementations should prioritize endpoints that are - // topologically close (e.g., same zone). - // This is a beta field and requires enabling ServiceTrafficDistribution feature. + // TrafficDistribution offers a way to express preferences for how traffic + // is distributed to Service endpoints. Implementations can use this field + // as a hint, but are not required to guarantee strict adherence. If the + // field is not set, the implementation will apply its default routing + // strategy. If set to "PreferClose", implementations should prioritize + // endpoints that are in the same zone. // +featureGate=ServiceTrafficDistribution // +optional TrafficDistribution *string `json:"trafficDistribution,omitempty" protobuf:"bytes,23,opt,name=trafficDistribution"` From ca43bb17190a0717664945b6b9481a636bd0b097 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Sun, 9 Mar 2025 14:54:36 -0700 Subject: [PATCH 2/5] Run 'make update' --- api/openapi-spec/swagger.json | 2 +- api/openapi-spec/v3/api__v1_openapi.json | 2 +- pkg/generated/openapi/zz_generated.openapi.go | 2 +- staging/src/k8s.io/api/core/v1/generated.proto | 13 ++++++------- .../api/core/v1/types_swagger_doc_generated.go | 2 +- 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index b6315f4d5e4..e5c7257912a 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -11430,7 +11430,7 @@ "description": "sessionAffinityConfig contains the configurations of session affinity." }, "trafficDistribution": { - "description": "TrafficDistribution offers a way to express preferences for how traffic is distributed to Service endpoints. Implementations can use this field as a hint, but are not required to guarantee strict adherence. If the field is not set, the implementation will apply its default routing strategy. If set to \"PreferClose\", implementations should prioritize endpoints that are topologically close (e.g., same zone). This is a beta field and requires enabling ServiceTrafficDistribution feature.", + "description": "TrafficDistribution offers a way to express preferences for how traffic is distributed to Service endpoints. Implementations can use this field as a hint, but are not required to guarantee strict adherence. If the field is not set, the implementation will apply its default routing strategy. If set to \"PreferClose\", implementations should prioritize endpoints that are in the same zone.", "type": "string" }, "type": { diff --git a/api/openapi-spec/v3/api__v1_openapi.json b/api/openapi-spec/v3/api__v1_openapi.json index d0280f54459..e5e0a1bb314 100644 --- a/api/openapi-spec/v3/api__v1_openapi.json +++ b/api/openapi-spec/v3/api__v1_openapi.json @@ -7769,7 +7769,7 @@ "description": "sessionAffinityConfig contains the configurations of session affinity." }, "trafficDistribution": { - "description": "TrafficDistribution offers a way to express preferences for how traffic is distributed to Service endpoints. Implementations can use this field as a hint, but are not required to guarantee strict adherence. If the field is not set, the implementation will apply its default routing strategy. If set to \"PreferClose\", implementations should prioritize endpoints that are topologically close (e.g., same zone). This is a beta field and requires enabling ServiceTrafficDistribution feature.", + "description": "TrafficDistribution offers a way to express preferences for how traffic is distributed to Service endpoints. Implementations can use this field as a hint, but are not required to guarantee strict adherence. If the field is not set, the implementation will apply its default routing strategy. If set to \"PreferClose\", implementations should prioritize endpoints that are in the same zone.", "type": "string" }, "type": { diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index 765246a719f..a78884dc30c 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/pkg/generated/openapi/zz_generated.openapi.go @@ -31768,7 +31768,7 @@ func schema_k8sio_api_core_v1_ServiceSpec(ref common.ReferenceCallback) common.O }, "trafficDistribution": { SchemaProps: spec.SchemaProps{ - Description: "TrafficDistribution offers a way to express preferences for how traffic is distributed to Service endpoints. Implementations can use this field as a hint, but are not required to guarantee strict adherence. If the field is not set, the implementation will apply its default routing strategy. If set to \"PreferClose\", implementations should prioritize endpoints that are topologically close (e.g., same zone). This is a beta field and requires enabling ServiceTrafficDistribution feature.", + Description: "TrafficDistribution offers a way to express preferences for how traffic is distributed to Service endpoints. Implementations can use this field as a hint, but are not required to guarantee strict adherence. If the field is not set, the implementation will apply its default routing strategy. If set to \"PreferClose\", implementations should prioritize endpoints that are in the same zone.", Type: []string{"string"}, Format: "", }, diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index 44e5d861fec..11ee434ee8a 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -6132,13 +6132,12 @@ message ServiceSpec { // +optional optional string internalTrafficPolicy = 22; - // TrafficDistribution offers a way to express preferences for how traffic is - // distributed to Service endpoints. Implementations can use this field as a - // hint, but are not required to guarantee strict adherence. If the field is - // not set, the implementation will apply its default routing strategy. If set - // to "PreferClose", implementations should prioritize endpoints that are - // topologically close (e.g., same zone). - // This is a beta field and requires enabling ServiceTrafficDistribution feature. + // TrafficDistribution offers a way to express preferences for how traffic + // is distributed to Service endpoints. Implementations can use this field + // as a hint, but are not required to guarantee strict adherence. If the + // field is not set, the implementation will apply its default routing + // strategy. If set to "PreferClose", implementations should prioritize + // endpoints that are in the same zone. // +featureGate=ServiceTrafficDistribution // +optional optional string trafficDistribution = 23; diff --git a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go index ee9f102678c..2e089fc975a 100644 --- a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go @@ -2489,7 +2489,7 @@ var map_ServiceSpec = map[string]string{ "allocateLoadBalancerNodePorts": "allocateLoadBalancerNodePorts defines if NodePorts will be automatically allocated for services with type LoadBalancer. Default is \"true\". It may be set to \"false\" if the cluster load-balancer does not rely on NodePorts. If the caller requests specific NodePorts (by specifying a value), those requests will be respected, regardless of this field. This field may only be set for services with type LoadBalancer and will be cleared if the type is changed to any other type.", "loadBalancerClass": "loadBalancerClass is the class of the load balancer implementation this Service belongs to. If specified, the value of this field must be a label-style identifier, with an optional prefix, e.g. \"internal-vip\" or \"example.com/internal-vip\". Unprefixed names are reserved for end-users. This field can only be set when the Service type is 'LoadBalancer'. If not set, the default load balancer implementation is used, today this is typically done through the cloud provider integration, but should apply for any default implementation. If set, it is assumed that a load balancer implementation is watching for Services with a matching class. Any default load balancer implementation (e.g. cloud providers) should ignore Services that set this field. This field can only be set when creating or updating a Service to type 'LoadBalancer'. Once set, it can not be changed. This field will be wiped when a service is updated to a non 'LoadBalancer' type.", "internalTrafficPolicy": "InternalTrafficPolicy describes how nodes distribute service traffic they receive on the ClusterIP. If set to \"Local\", the proxy will assume that pods only want to talk to endpoints of the service on the same node as the pod, dropping the traffic if there are no local endpoints. The default value, \"Cluster\", uses the standard behavior of routing to all endpoints evenly (possibly modified by topology and other features).", - "trafficDistribution": "TrafficDistribution offers a way to express preferences for how traffic is distributed to Service endpoints. Implementations can use this field as a hint, but are not required to guarantee strict adherence. If the field is not set, the implementation will apply its default routing strategy. If set to \"PreferClose\", implementations should prioritize endpoints that are topologically close (e.g., same zone). This is a beta field and requires enabling ServiceTrafficDistribution feature.", + "trafficDistribution": "TrafficDistribution offers a way to express preferences for how traffic is distributed to Service endpoints. Implementations can use this field as a hint, but are not required to guarantee strict adherence. If the field is not set, the implementation will apply its default routing strategy. If set to \"PreferClose\", implementations should prioritize endpoints that are in the same zone.", } func (ServiceSpec) SwaggerDoc() map[string]string { From 2492eddd20b02c1a57976171bd70e5ce4ed0157d Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Sun, 9 Mar 2025 15:16:56 -0700 Subject: [PATCH 3/5] Bump ServiceTrafficDistribution feature-gate to GA in 1.33 and set LockToDefault=true --- pkg/features/versioned_kube_features.go | 1 + .../featuregates_linter/test_data/versioned_feature_list.yaml | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/pkg/features/versioned_kube_features.go b/pkg/features/versioned_kube_features.go index 89e650c1c46..854f3cf18dc 100644 --- a/pkg/features/versioned_kube_features.go +++ b/pkg/features/versioned_kube_features.go @@ -725,6 +725,7 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate ServiceTrafficDistribution: { {Version: version.MustParse("1.30"), Default: false, PreRelease: featuregate.Alpha}, {Version: version.MustParse("1.31"), Default: true, PreRelease: featuregate.Beta}, + {Version: version.MustParse("1.33"), Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // GA and LockToDefault in 1.33, remove in 1.36 }, SidecarContainers: { diff --git a/test/featuregates_linter/test_data/versioned_feature_list.yaml b/test/featuregates_linter/test_data/versioned_feature_list.yaml index a32fecff12a..e07f1c50a70 100644 --- a/test/featuregates_linter/test_data/versioned_feature_list.yaml +++ b/test/featuregates_linter/test_data/versioned_feature_list.yaml @@ -1284,6 +1284,10 @@ lockToDefault: false preRelease: Beta version: "1.31" + - default: true + lockToDefault: true + preRelease: GA + version: "1.33" - name: SidecarContainers versionedSpecs: - default: false From 4e317265c77d534329922fa1bf4fa2d665cd8bc3 Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Sun, 9 Mar 2025 16:34:39 -0700 Subject: [PATCH 4/5] 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."_ --- pkg/proxy/topology.go | 26 +------- pkg/proxy/topology_test.go | 118 ++++++++++--------------------------- 2 files changed, 33 insertions(+), 111 deletions(-) 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) From dc8061881773827aefc90d9f3d5ca7381b681d7a Mon Sep 17 00:00:00 2001 From: Gaurav Ghildiyal Date: Sun, 9 Mar 2025 16:58:05 -0700 Subject: [PATCH 5/5] Use SetFeatureGateEmulationVersionDuringTest() for testing ServiceTrafficDistribution feature-flag disablement in control plane components Ref. https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/feature-gates.md#disablement-tests --- test/integration/service/service_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/integration/service/service_test.go b/test/integration/service/service_test.go index 11859c0421f..296a52daf61 100644 --- a/test/integration/service/service_test.go +++ b/test/integration/service/service_test.go @@ -33,6 +33,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" utilrand "k8s.io/apimachinery/pkg/util/rand" + "k8s.io/apimachinery/pkg/util/version" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -631,6 +632,7 @@ func Test_TrafficDistribution_FeatureGateEnableDisable(t *testing.T) { //////////////////////////////////////////////////////////////////////////// server1.TearDownFn() + featuregatetesting.SetFeatureGateEmulationVersionDuringTest(t, utilfeature.DefaultFeatureGate, version.MustParse("1.32")) featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceTrafficDistribution, false) server2 := kubeapiservertesting.StartTestServerOrDie(t, nil, framework.DefaultTestServerFlags(), sharedEtcd) defer server2.TearDownFn()