From 4d38d21880a6f9c988fe5a1408ed71725f8084d8 Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Fri, 28 May 2021 12:19:30 -0400 Subject: [PATCH] apis: remove Service topologyKeys Signed-off-by: Andrew Sy Kim --- pkg/apis/core/types.go | 22 ------- pkg/apis/core/validation/validation.go | 29 ---------- pkg/apis/core/validation/validation_test.go | 64 --------------------- pkg/registry/core/service/strategy.go | 13 ----- staging/src/k8s.io/api/core/v1/types.go | 25 +------- 5 files changed, 2 insertions(+), 151 deletions(-) diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index fa67cfdeca7..98f8e0379a3 100644 --- a/pkg/apis/core/types.go +++ b/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. diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 02234bd4d47..c2e05ed61f0 100644 --- a/pkg/apis/core/validation/validation.go +++ b/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 { diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 72e16c5c0f6..d444b68b564 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/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) { diff --git a/pkg/registry/core/service/strategy.go b/pkg/registry/core/service/strategy.go index 1a6da3b27bf..0291f2ce35a 100644 --- a/pkg/registry/core/service/strategy.go +++ b/pkg/registry/core/service/strategy.go @@ -173,11 +173,6 @@ func dropServiceDisabledFields(newSvc *api.Service, oldSvc *api.Service) { } } - // Drop TopologyKeys if ServiceTopology is not enabled - if !utilfeature.DefaultFeatureGate.Enabled(features.ServiceTopology) && !topologyKeysInUse(oldSvc) { - newSvc.Spec.TopologyKeys = nil - } - // Clear AllocateLoadBalancerNodePorts if ServiceLBNodePortControl is not enabled if !utilfeature.DefaultFeatureGate.Enabled(features.ServiceLBNodePortControl) { if !allocateLoadBalancerNodePortsInUse(oldSvc) { @@ -232,14 +227,6 @@ func serviceDualStackFieldsInUse(svc *api.Service) bool { return ipFamilyPolicyInUse || ipFamiliesInUse || ClusterIPsInUse } -// returns true if svc.Spec.TopologyKeys field is in use -func topologyKeysInUse(svc *api.Service) bool { - if svc == nil { - return false - } - return len(svc.Spec.TopologyKeys) > 0 -} - // returns true when the svc.Status.Conditions field is in use. func serviceConditionsInUse(svc *api.Service) bool { if svc == nil { diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index db2a45269c0..d1dbeda8f36 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -30,8 +30,6 @@ const ( NamespaceAll string = "" // NamespaceNodeLease is the namespace where we place node lease objects (used for node heartbeats) NamespaceNodeLease string = "kube-node-lease" - // TopologyKeyAny is the service topology key that matches any node - TopologyKeyAny string = "*" ) // Volume represents a named volume in a pod that may be accessed by any container in the pod. @@ -4039,11 +4037,6 @@ type LoadBalancerIngress struct { Ports []PortStatus `json:"ports,omitempty" protobuf:"bytes,4,rep,name=ports"` } -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 @@ -4240,22 +4233,8 @@ type ServiceSpec struct { // +optional SessionAffinityConfig *SessionAffinityConfig `json:"sessionAffinityConfig,omitempty" protobuf:"bytes,14,opt,name=sessionAffinityConfig"` - // 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 `json:"topologyKeys,omitempty" protobuf:"bytes,16,opt,name=topologyKeys"` + // TopologyKeys is tombstoned to show why 16 is reserved protobuf tag. + //TopologyKeys []string `json:"topologyKeys,omitempty" protobuf:"bytes,16,opt,name=topologyKeys"` // IPFamily is tombstoned to show why 15 is a reserved protobuf tag. // IPFamily *IPFamily `json:"ipFamily,omitempty" protobuf:"bytes,15,opt,name=ipFamily,Configcasttype=IPFamily"`