diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 1b5aedab25e..75828a30501 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -3692,6 +3692,15 @@ type ServiceSpec struct { // This field is alpha-level and is only honored by servers that enable the ServiceTopology feature. // +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. + // allocateLoadBalancerNodePorts may only be set for services with type LoadBalancer + // and will be cleared if the type is changed to any other type. + // This field is alpha-level and is only honored by servers that enable the ServiceLBNodePortControl feature. + // +optional + AllocateLoadBalancerNodePorts *bool } // ServicePort represents the port on which the service is exposed diff --git a/pkg/apis/core/v1/defaults.go b/pkg/apis/core/v1/defaults.go index 560c8ea0a97..d1b81bae73b 100644 --- a/pkg/apis/core/v1/defaults.go +++ b/pkg/apis/core/v1/defaults.go @@ -166,6 +166,14 @@ func SetDefaults_Service(obj *v1.Service) { // further IPFamilies, IPFamilyPolicy defaulting is in ClusterIP alloc/reserve logic // note: conversion logic handles cases where ClusterIPs is used (but not ClusterIP). } + + if utilfeature.DefaultFeatureGate.Enabled(features.ServiceLBNodePortControl) { + if obj.Spec.Type == v1.ServiceTypeLoadBalancer { + if obj.Spec.AllocateLoadBalancerNodePorts == nil { + obj.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) + } + } + } } func SetDefaults_Pod(obj *v1.Pod) { // If limits are specified, but requests are not, default requests to limits diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index e1951ae0d90..ba2bb22793d 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -4357,6 +4357,16 @@ func ValidateService(service *core.Service) field.ErrorList { } } + if service.Spec.AllocateLoadBalancerNodePorts != nil && service.Spec.Type != core.ServiceTypeLoadBalancer { + allErrs = append(allErrs, field.Forbidden(specPath.Child("allocateLoadBalancerNodePorts"), "may only be used when `type` is 'LoadBalancer'")) + } + + if utilfeature.DefaultFeatureGate.Enabled(features.ServiceLBNodePortControl) { + if service.Spec.Type == core.ServiceTypeLoadBalancer && service.Spec.AllocateLoadBalancerNodePorts == nil { + allErrs = append(allErrs, field.Required(field.NewPath("allocateLoadBalancerNodePorts"), "")) + } + } + // external traffic fields allErrs = append(allErrs, validateServiceExternalTrafficFieldsValue(service)...) return allErrs diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index cedf0080237..d6c05b8e5f8 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -11169,6 +11169,13 @@ func TestValidateServiceCreate(t *testing.T) { }, numErrs: 1, }, + { + name: "Use AllocateLoadBalancerNodePorts when type is not LoadBalancer", + tweakSvc: func(s *core.Service) { + s.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) + }, + numErrs: 1, + }, } for _, tc := range testCases { @@ -13539,6 +13546,13 @@ func TestValidateServiceUpdate(t *testing.T) { }, numErrs: 1, }, + { + name: "Set AllocateLoadBalancerNodePorts when type is not LoadBalancer", + tweakSvc: func(oldSvc, newSvc *core.Service) { + newSvc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) + }, + numErrs: 1, + }, } for _, tc := range testCases { diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 41bf65ddbc4..4cf0e4df646 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -708,6 +708,12 @@ const ( // alpha: v1.20 // Adds support for kubelet to detect node shutdown and gracefully terminate pods prior to the node being shutdown. GracefulNodeShutdown featuregate.Feature = "GracefulNodeShutdown" + + // owner: @andrewsykim @uablrek + // alpha: v1.20 + // + // Allows control if NodePorts shall be created for services with "type: LoadBalancer" by defining the spec.AllocateLoadBalancerNodePorts field (bool) + ServiceLBNodePortControl featuregate.Feature = "ServiceLBNodePortControl" ) func init() { @@ -814,6 +820,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS ExecProbeTimeout: {Default: true, PreRelease: featuregate.GA}, // lock to default in v1.21 and remove in v1.22 KubeletCredentialProviders: {Default: false, PreRelease: featuregate.Alpha}, GracefulNodeShutdown: {Default: false, PreRelease: featuregate.Alpha}, + ServiceLBNodePortControl: {Default: false, PreRelease: featuregate.Alpha}, // inherited features from generic apiserver, relisted here to get a conflict if it is changed // unintentionally on either side: diff --git a/pkg/registry/core/service/storage/rest.go b/pkg/registry/core/service/storage/rest.go index af759b5dcc3..694120a2302 100644 --- a/pkg/registry/core/service/storage/rest.go +++ b/pkg/registry/core/service/storage/rest.go @@ -231,7 +231,10 @@ func (rs *REST) Create(ctx context.Context, obj runtime.Object, createValidation nodePortOp := portallocator.StartOperation(rs.serviceNodePorts, dryrun.IsDryRun(options.DryRun)) defer nodePortOp.Finish() - if service.Spec.Type == api.ServiceTypeNodePort || service.Spec.Type == api.ServiceTypeLoadBalancer { + // TODO: This creates nodePorts if needed. In the future nodePorts may be cleared if *never* used. + // But for now we stick to the KEP "don't allocate new node ports but do not deallocate existing node ports if set" + if service.Spec.Type == api.ServiceTypeNodePort || + (service.Spec.Type == api.ServiceTypeLoadBalancer && shouldAllocateNodePorts(service)) { if err := initNodePorts(service, nodePortOp); err != nil { return nil, err } @@ -335,6 +338,13 @@ func (rs *REST) releaseAllocatedResources(svc *api.Service) { } } +func shouldAllocateNodePorts(service *api.Service) bool { + if utilfeature.DefaultFeatureGate.Enabled(features.ServiceLBNodePortControl) { + return *service.Spec.AllocateLoadBalancerNodePorts + } + return true +} + // externalTrafficPolicyUpdate adjusts ExternalTrafficPolicy during service update if needed. // It is necessary because we default ExternalTrafficPolicy field to different values. // (NodePort / LoadBalancer: default is Global; Other types: default is empty.) @@ -472,7 +482,8 @@ func (rs *REST) Update(ctx context.Context, name string, objInfo rest.UpdatedObj releaseNodePorts(oldService, nodePortOp) } // Update service from any type to NodePort or LoadBalancer, should update NodePort. - if service.Spec.Type == api.ServiceTypeNodePort || service.Spec.Type == api.ServiceTypeLoadBalancer { + if service.Spec.Type == api.ServiceTypeNodePort || + (service.Spec.Type == api.ServiceTypeLoadBalancer && shouldAllocateNodePorts(service)) { if err := updateNodePorts(oldService, service, nodePortOp); err != nil { return nil, false, err } diff --git a/pkg/registry/core/service/storage/rest_test.go b/pkg/registry/core/service/storage/rest_test.go index a33f3d43d14..c54b4efa777 100644 --- a/pkg/registry/core/service/storage/rest_test.go +++ b/pkg/registry/core/service/storage/rest_test.go @@ -50,6 +50,7 @@ import ( "k8s.io/kubernetes/pkg/features" netutil "k8s.io/utils/net" + utilpointer "k8s.io/utils/pointer" ) var ( @@ -1157,6 +1158,165 @@ func TestServiceRegistryExternalService(t *testing.T) { storage.serviceNodePorts.Release(nodePort) } } +func TestAllocateLoadBalancerNodePorts(t *testing.T) { + testcases := []struct { + name string + svc *api.Service + expectNodePorts bool + allocateNodePortGate bool + expectError bool + }{ + { + name: "allocate nil, gate on", + svc: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "alloc-nil"}, + Spec: api.ServiceSpec{ + AllocateLoadBalancerNodePorts: nil, + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeLoadBalancer, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: intstr.FromInt(6502), + }}, + }, + }, + expectNodePorts: true, + allocateNodePortGate: true, + expectError: true, + }, + { + name: "allocate false, gate on", + svc: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "alloc-false"}, + Spec: api.ServiceSpec{ + AllocateLoadBalancerNodePorts: utilpointer.BoolPtr(false), + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeLoadBalancer, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: intstr.FromInt(6502), + }}, + }, + }, + expectNodePorts: false, + allocateNodePortGate: true, + }, + { + name: "allocate true, gate on", + svc: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "alloc-true"}, + Spec: api.ServiceSpec{ + AllocateLoadBalancerNodePorts: utilpointer.BoolPtr(true), + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeLoadBalancer, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: intstr.FromInt(6502), + }}, + }, + }, + expectNodePorts: true, + allocateNodePortGate: true, + }, + { + name: "allocate nil, gate off", + svc: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "alloc-false"}, + Spec: api.ServiceSpec{ + AllocateLoadBalancerNodePorts: nil, + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeLoadBalancer, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: intstr.FromInt(6502), + }}, + }, + }, + expectNodePorts: true, + allocateNodePortGate: false, + }, + { + name: "allocate false, gate off", + svc: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "alloc-false"}, + Spec: api.ServiceSpec{ + AllocateLoadBalancerNodePorts: utilpointer.BoolPtr(false), + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeLoadBalancer, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: intstr.FromInt(6502), + }}, + }, + }, + expectNodePorts: true, + allocateNodePortGate: false, + }, + { + name: "allocate true, gate off", + svc: &api.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "alloc-true"}, + Spec: api.ServiceSpec{ + AllocateLoadBalancerNodePorts: utilpointer.BoolPtr(true), + Selector: map[string]string{"bar": "baz"}, + SessionAffinity: api.ServiceAffinityNone, + Type: api.ServiceTypeLoadBalancer, + Ports: []api.ServicePort{{ + Port: 6502, + Protocol: api.ProtocolTCP, + TargetPort: intstr.FromInt(6502), + }}, + }, + }, + expectNodePorts: true, + allocateNodePortGate: false, + }, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + ctx := genericapirequest.NewDefaultContext() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceLBNodePortControl, tc.allocateNodePortGate)() + + storage, registry, server := NewTestREST(t, nil, singleStackIPv4) + defer server.Terminate(t) + + _, err := storage.Create(ctx, tc.svc, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) + if err != nil { + if tc.expectError { + return + } + t.Errorf("%s; Failed to create service: %#v", tc.name, err) + } + srv, err := registry.GetService(ctx, tc.svc.Name, &metav1.GetOptions{}) + if err != nil { + t.Errorf("%s; Unexpected error: %v", tc.name, err) + } + if srv == nil { + t.Fatalf("%s; Failed to find service: %s", tc.name, tc.svc.Name) + } + serviceNodePorts := collectServiceNodePorts(srv) + if (len(serviceNodePorts) != 0) != tc.expectNodePorts { + t.Errorf("%s; Allocated NodePorts not as expected", tc.name) + } + + for i := range serviceNodePorts { + nodePort := serviceNodePorts[i] + // Release the node port at the end of the test case. + storage.serviceNodePorts.Release(nodePort) + } + }) + } +} func TestServiceRegistryDelete(t *testing.T) { ctx := genericapirequest.NewDefaultContext() diff --git a/pkg/registry/core/service/strategy.go b/pkg/registry/core/service/strategy.go index c84e0315ce9..1af90c0aa63 100644 --- a/pkg/registry/core/service/strategy.go +++ b/pkg/registry/core/service/strategy.go @@ -179,6 +179,11 @@ func dropServiceDisabledFields(newSvc *api.Service, oldSvc *api.Service) { if !utilfeature.DefaultFeatureGate.Enabled(features.ServiceTopology) && !topologyKeysInUse(oldSvc) { newSvc.Spec.TopologyKeys = nil } + + // Clear AllocateLoadBalancerNodePorts if ServiceLBNodePortControl if not enabled + if !utilfeature.DefaultFeatureGate.Enabled(features.ServiceLBNodePortControl) { + newSvc.Spec.AllocateLoadBalancerNodePorts = nil + } } // returns true if svc.Spec.ServiceIPFamily field is in use @@ -357,6 +362,11 @@ func dropTypeDependentFields(newSvc *api.Service, oldSvc *api.Service) { newSvc.Spec.HealthCheckNodePort = 0 } + // AllocateLoadBalancerNodePorts may only be set for type LoadBalancer + if newSvc.Spec.Type != api.ServiceTypeLoadBalancer { + newSvc.Spec.AllocateLoadBalancerNodePorts = nil + } + // NOTE: there are other fields like `selector` which we could wipe. // Historically we did not wipe them and they are not allocated from // finite pools, so we are (currently) choosing to leave them alone. diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 7a631587cf7..0f109be3c2c 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -4220,6 +4220,15 @@ type ServiceSpec struct { // wiped when updating a service to type ExternalName. // +optional IPFamilyPolicy *IPFamilyPolicyType `json:"ipFamilyPolicy,omitempty" protobuf:"bytes,17,opt,name=ipFamilyPolicy,casttype=IPFamilyPolicyType"` + + // 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. + // allocateLoadBalancerNodePorts may only be set for services with type LoadBalancer + // and will be cleared if the type is changed to any other type. + // This field is alpha-level and is only honored by servers that enable the ServiceLBNodePortControl feature. + // +optional + AllocateLoadBalancerNodePorts *bool `json:"allocateLoadBalancerNodePorts,omitempty" protobuf:"bytes,20,opt,name=allocateLoadBalancerNodePorts"` } // ServicePort contains information on service's port.