Add service.spec.AllocateLoadBalancerNodePorts

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

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

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

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 {

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:

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
 		}

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()

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.

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.