Merge pull request #102412 from andrewsykim/kill-service-topology

Remove deprecated alpha Service topologyKeys field

api/api-rules/violation_exceptions.list

@@ -160,7 +160,6 @@ API rule violation: list_type_missing,k8s.io/api/core/v1,ServiceAccount,ImagePul
 API rule violation: list_type_missing,k8s.io/api/core/v1,ServiceAccount,Secrets
 API rule violation: list_type_missing,k8s.io/api/core/v1,ServiceSpec,ExternalIPs
 API rule violation: list_type_missing,k8s.io/api/core/v1,ServiceSpec,LoadBalancerSourceRanges
-API rule violation: list_type_missing,k8s.io/api/core/v1,ServiceSpec,TopologyKeys
 API rule violation: list_type_missing,k8s.io/api/core/v1,TopologySelectorLabelRequirement,Values
 API rule violation: list_type_missing,k8s.io/api/core/v1,TopologySelectorTerm,MatchLabelExpressions
 API rule violation: list_type_missing,k8s.io/api/extensions/v1beta1,DaemonSetStatus,Conditions

cmd/kube-proxy/app/server.go

@@ -750,7 +750,7 @@ func (s *ProxyServer) Run() error {
 	// functions must configure their shared informer event handlers first.
 	informerFactory.Start(wait.NeverStop)
 
-	if utilfeature.DefaultFeatureGate.Enabled(features.ServiceTopology) || utilfeature.DefaultFeatureGate.Enabled(features.TopologyAwareHints) {
+	if utilfeature.DefaultFeatureGate.Enabled(features.TopologyAwareHints) {
 		// Make an informer that selects for our nodename.
 		currentNodeInformerFactory := informers.NewSharedInformerFactoryWithOptions(s.Client, s.ConfigSyncPeriod,
 			informers.WithTweakListOptions(func(options *metav1.ListOptions) {

pkg/apis/core/types.go

@@ -3560,11 +3560,6 @@ type LoadBalancerIngress struct {
 	Ports []PortStatus
 }
 
-const (
-	// MaxServiceTopologyKeys is the largest number of topology keys allowed on a service
-	MaxServiceTopologyKeys = 16
-)
-
 // IPFamily represents the IP Family (IPv4 or IPv6). This type is used
 // to express the family of an IP expressed by a type (e.g. service.spec.ipFamilies).
 type IPFamily string
@@ -3732,23 +3727,6 @@ type ServiceSpec struct {
 	// +optional
 	PublishNotReadyAddresses bool
 
-	// topologyKeys is a preference-order list of topology keys which
-	// implementations of services should use to preferentially sort endpoints
-	// when accessing this Service, it can not be used at the same time as
-	// externalTrafficPolicy=Local.
-	// Topology keys must be valid label keys and at most 16 keys may be specified.
-	// Endpoints are chosen based on the first topology key with available backends.
-	// If this field is specified and all entries have no backends that match
-	// the topology of the client, the service has no backends for that client
-	// and connections should fail.
-	// The special value "*" may be used to mean "any topology". This catch-all
-	// value, if used, only makes sense as the last value in the list.
-	// If this is not specified or empty, no topology constraints will be applied.
-	// This field is alpha-level and is only honored by servers that enable the ServiceTopology feature.
-	// This field is deprecated and will be removed in a future version.
-	// +optional
-	TopologyKeys []string
-
 	// 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.

pkg/apis/core/validation/validation.go

@@ -4327,35 +4327,6 @@ func ValidateService(service *core.Service) field.ErrorList {
 		ports[key] = true
 	}
 
-	// Validate TopologyKeys
-	if len(service.Spec.TopologyKeys) > 0 {
-		topoPath := specPath.Child("topologyKeys")
-		// topologyKeys is mutually exclusive with 'externalTrafficPolicy=Local'
-		if service.Spec.ExternalTrafficPolicy == core.ServiceExternalTrafficPolicyTypeLocal {
-			allErrs = append(allErrs, field.Forbidden(topoPath, "may not be specified when `externalTrafficPolicy=Local`"))
-		}
-		if len(service.Spec.TopologyKeys) > core.MaxServiceTopologyKeys {
-			allErrs = append(allErrs, field.TooMany(topoPath, len(service.Spec.TopologyKeys), core.MaxServiceTopologyKeys))
-		}
-		topoKeys := sets.NewString()
-		for i, key := range service.Spec.TopologyKeys {
-			keyPath := topoPath.Index(i)
-			if topoKeys.Has(key) {
-				allErrs = append(allErrs, field.Duplicate(keyPath, key))
-			}
-			topoKeys.Insert(key)
-			// "Any" must be the last value specified
-			if key == v1.TopologyKeyAny && i != len(service.Spec.TopologyKeys)-1 {
-				allErrs = append(allErrs, field.Invalid(keyPath, key, `"*" must be the last value specified`))
-			}
-			if key != v1.TopologyKeyAny {
-				for _, msg := range validation.IsQualifiedName(key) {
-					allErrs = append(allErrs, field.Invalid(keyPath, service.Spec.TopologyKeys, msg))
-				}
-			}
-		}
-	}
-
 	// Validate SourceRange field and annotation
 	_, ok := service.Annotations[core.AnnotationLoadBalancerSourceRangesKey]
 	if len(service.Spec.LoadBalancerSourceRanges) > 0 || ok {

pkg/apis/core/validation/validation_test.go

@@ -18,7 +18,6 @@ package validation
 
 import (
 	"bytes"
-	"fmt"
 	"math"
 	"reflect"
 	"strings"
@@ -10253,8 +10252,6 @@ func TestValidatePodEphemeralContainersUpdate(t *testing.T) {
 }
 
 func TestValidateServiceCreate(t *testing.T) {
-	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceTopology, true)()
-
 	requireDualStack := core.IPFamilyPolicyRequireDualStack
 	singleStack := core.IPFamilyPolicySingleStack
 	preferDualStack := core.IPFamilyPolicyPreferDualStack
@@ -11315,67 +11312,6 @@ func TestValidateServiceCreate(t *testing.T) {
 			numErrs: 0,
 		},
 
-		/* toplogy keys */
-		{
-			name: "valid topology keys",
-			tweakSvc: func(s *core.Service) {
-				s.Spec.TopologyKeys = []string{
-					"kubernetes.io/hostname",
-					"topology.kubernetes.io/zone",
-					"topology.kubernetes.io/region",
-					v1.TopologyKeyAny,
-				}
-			},
-			numErrs: 0,
-		},
-		{
-			name: "invalid topology key",
-			tweakSvc: func(s *core.Service) {
-				s.Spec.TopologyKeys = []string{"NoUppercaseOrSpecialCharsLike=Equals"}
-			},
-			numErrs: 1,
-		},
-		{
-			name: "too many topology keys",
-			tweakSvc: func(s *core.Service) {
-				for i := 0; i < core.MaxServiceTopologyKeys+1; i++ {
-					s.Spec.TopologyKeys = append(s.Spec.TopologyKeys, fmt.Sprintf("topologykey-%d", i))
-				}
-			},
-			numErrs: 1,
-		},
-		{
-			name: `"Any" was not the last key`,
-			tweakSvc: func(s *core.Service) {
-				s.Spec.TopologyKeys = []string{
-					"kubernetes.io/hostname",
-					v1.TopologyKeyAny,
-					"topology.kubernetes.io/zone",
-				}
-			},
-			numErrs: 1,
-		},
-		{
-			name: `duplicate topology key`,
-			tweakSvc: func(s *core.Service) {
-				s.Spec.TopologyKeys = []string{
-					"kubernetes.io/hostname",
-					"kubernetes.io/hostname",
-					"topology.kubernetes.io/zone",
-				}
-			},
-			numErrs: 1,
-		},
-		{
-			name: `use topology keys with externalTrafficPolicy=Local`,
-			tweakSvc: func(s *core.Service) {
-				s.Spec.ExternalTrafficPolicy = "Local"
-				s.Spec.TopologyKeys = []string{
-					"kubernetes.io/hostname",
-				}
-			},
-			numErrs: 1,
-		},
 		{
 			name: `valid appProtocol`,
 			tweakSvc: func(s *core.Service) {

pkg/features/kube_features.go

@@ -513,12 +513,6 @@ const (
 	// DaemonSets allow workloads to maintain availability during update per node
 	DaemonSetUpdateSurge featuregate.Feature = "DaemonSetUpdateSurge"
 
-	// owner: @m1093782566
-	// alpha: v1.17
-	//
-	// Enables topology aware service routing
-	ServiceTopology featuregate.Feature = "ServiceTopology"
-
 	// owner: @robscott
 	// kep: http://kep.k8s.io/1507
 	// alpha: v1.18
@@ -841,7 +835,6 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	PodDisruptionBudget:                            {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.25
 	CronJobControllerV2:                            {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.23
 	DaemonSetUpdateSurge:                           {Default: true, PreRelease: featuregate.Beta},                    // on by default in 1.22
-	ServiceTopology:                                {Default: false, PreRelease: featuregate.Alpha},
 	ServiceAppProtocol:                             {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.22
 	ImmutableEphemeralVolumes:                      {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.24
 	HugePageStorageMediumSize:                      {Default: true, PreRelease: featuregate.Beta},

pkg/proxy/service.go

@@ -54,7 +54,6 @@ type BaseServiceInfo struct {
 	nodeLocalExternal        bool
 	nodeLocalInternal        bool
 	internalTrafficPolicy    *v1.ServiceInternalTrafficPolicyType
-	topologyKeys             []string
 	hintsAnnotation          string
 }
 
@@ -134,11 +133,6 @@ func (info *BaseServiceInfo) InternalTrafficPolicy() *v1.ServiceInternalTrafficP
 	return info.internalTrafficPolicy
 }
 
-// TopologyKeys is part of ServicePort interface.
-func (info *BaseServiceInfo) TopologyKeys() []string {
-	return info.topologyKeys
-}
-
 // HintsAnnotation is part of ServicePort interface.
 func (info *BaseServiceInfo) HintsAnnotation() string {
 	return info.hintsAnnotation
@@ -170,7 +164,6 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic
 		nodeLocalExternal:     nodeLocalExternal,
 		nodeLocalInternal:     nodeLocalInternal,
 		internalTrafficPolicy: service.Spec.InternalTrafficPolicy,
-		topologyKeys:          service.Spec.TopologyKeys,
 		hintsAnnotation:       service.Annotations[v1.AnnotationTopologyAwareHints],
 	}
 

pkg/proxy/topology.go

@@ -31,10 +31,6 @@ func FilterEndpoints(endpoints []Endpoint, svcInfo ServicePort, nodeLabels map[s
 		return endpoints
 	}
 
-	if utilfeature.DefaultFeatureGate.Enabled(features.ServiceTopology) {
-		return deprecatedTopologyFilter(nodeLabels, svcInfo.TopologyKeys(), endpoints)
-	}
-
 	if utilfeature.DefaultFeatureGate.Enabled(features.ServiceInternalTrafficPolicy) && svcInfo.NodeLocalInternal() {
 		return filterEndpointsInternalTrafficPolicy(svcInfo.InternalTrafficPolicy(), endpoints)
 	}
@@ -89,67 +85,6 @@ func filterEndpointsWithHints(endpoints []Endpoint, hintsAnnotation string, node
 	return filteredEndpoints
 }
 
-// deprecatedTopologyFilter returns the appropriate endpoints based on the
-// cluster topology. This will be removed in an upcoming release along with the
-// ServiceTopology feature gate.
-//
-// This uses the current node's labels, which contain topology information, and
-// the required topologyKeys to find appropriate endpoints. If both the endpoint's
-// topology and the current node have matching values for topologyKeys[0], the
-// endpoint will be chosen.  If no endpoints are chosen, toplogyKeys[1] will be
-// considered, and so on.  If either the node or the endpoint do not have values
-// for a key, it is considered to not match.
-//
-// If topologyKeys is specified, but no endpoints are chosen for any key, the
-// service has no viable endpoints for clients on this node, and connections
-// should fail.
-//
-// The special key "*" may be used as the last entry in topologyKeys to indicate
-// "any endpoint" is acceptable.
-//
-// If topologyKeys is not specified or empty, no topology constraints will be
-// applied and this will return all endpoints.
-func deprecatedTopologyFilter(nodeLabels map[string]string, topologyKeys []string, endpoints []Endpoint) []Endpoint {
-	// Do not filter endpoints if service has no topology keys.
-	if len(topologyKeys) == 0 {
-		return endpoints
-	}
-
-	filteredEndpoints := []Endpoint{}
-
-	if len(nodeLabels) == 0 {
-		if topologyKeys[len(topologyKeys)-1] == v1.TopologyKeyAny {
-			// edge case: include all endpoints if topology key "Any" specified
-			// when we cannot determine current node's topology.
-			return endpoints
-		}
-		// edge case: do not include any endpoints if topology key "Any" is
-		// not specified when we cannot determine current node's topology.
-		return filteredEndpoints
-	}
-
-	for _, key := range topologyKeys {
-		if key == v1.TopologyKeyAny {
-			return endpoints
-		}
-		topologyValue, found := nodeLabels[key]
-		if !found {
-			continue
-		}
-
-		for _, ep := range endpoints {
-			topology := ep.GetTopology()
-			if value, found := topology[key]; found && value == topologyValue {
-				filteredEndpoints = append(filteredEndpoints, ep)
-			}
-		}
-		if len(filteredEndpoints) > 0 {
-			return filteredEndpoints
-		}
-	}
-	return filteredEndpoints
-}
-
 // filterEndpointsInternalTrafficPolicy returns the node local endpoints based
 // on configured InternalTrafficPolicy.
 //