From eae4a19bd35ee4042234fbf6739b450961edd9bb Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 4 Jul 2021 14:49:44 -0700 Subject: [PATCH 1/2] Fix small bug with AllocateLoadBalancerNodePorts If the user specified a port, DO reserve it, even if they asked you not to allocate new ports. --- api/openapi-spec/swagger.json | 2 +- pkg/apis/core/types.go | 12 ++--- pkg/registry/core/service/storage/rest.go | 28 ++++++++---- .../core/service/storage/rest_test.go | 45 ++++++++++++++----- .../src/k8s.io/api/core/v1/generated.proto | 13 +++--- staging/src/k8s.io/api/core/v1/types.go | 13 +++--- .../core/v1/types_swagger_doc_generated.go | 2 +- 7 files changed, 79 insertions(+), 36 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index a255566ecbb..512444ce699 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -9296,7 +9296,7 @@ "description": "ServiceSpec describes the attributes that a user creates on a service.", "properties": { "allocateLoadBalancerNodePorts": { - "description": "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.", + "description": "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. This field is beta-level and is only honored by servers that enable the ServiceLBNodePortControl feature.", "type": "boolean" }, "clusterIP": { diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 57f31639f1c..532bc634f28 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -3731,11 +3731,13 @@ type ServiceSpec struct { PublishNotReadyAddresses bool // 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. + // 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. + // This field is beta-level and is only honored by servers that enable the ServiceLBNodePortControl feature. // +optional AllocateLoadBalancerNodePorts *bool diff --git a/pkg/registry/core/service/storage/rest.go b/pkg/registry/core/service/storage/rest.go index 5d6102d6295..be029edc557 100644 --- a/pkg/registry/core/service/storage/rest.go +++ b/pkg/registry/core/service/storage/rest.go @@ -226,10 +226,7 @@ func (rs *REST) Create(ctx context.Context, obj runtime.Object, createValidation nodePortOp := portallocator.StartOperation(rs.serviceNodePorts, dryrun.IsDryRun(options.DryRun)) defer nodePortOp.Finish() - // 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 service.Spec.Type == api.ServiceTypeNodePort || service.Spec.Type == api.ServiceTypeLoadBalancer { if err := initNodePorts(service, nodePortOp); err != nil { return nil, err } @@ -334,10 +331,16 @@ func (rs *REST) releaseAllocatedResources(svc *api.Service) { } func shouldAllocateNodePorts(service *api.Service) bool { - if utilfeature.DefaultFeatureGate.Enabled(features.ServiceLBNodePortControl) { - return *service.Spec.AllocateLoadBalancerNodePorts + if service.Spec.Type == api.ServiceTypeNodePort { + return true } - return true + if service.Spec.Type == api.ServiceTypeLoadBalancer { + if utilfeature.DefaultFeatureGate.Enabled(features.ServiceLBNodePortControl) { + return *service.Spec.AllocateLoadBalancerNodePorts + } + return true + } + return false } // externalTrafficPolicyUpdate adjusts ExternalTrafficPolicy during service update if needed. @@ -477,8 +480,7 @@ 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 && shouldAllocateNodePorts(service)) { + if service.Spec.Type == api.ServiceTypeNodePort || service.Spec.Type == api.ServiceTypeLoadBalancer { if err := updateNodePorts(oldService, service, nodePortOp); err != nil { return nil, false, err } @@ -1172,6 +1174,10 @@ func initNodePorts(service *api.Service, nodePortOp *portallocator.PortAllocatio svcPortToNodePort := map[int]int{} for i := range service.Spec.Ports { servicePort := &service.Spec.Ports[i] + if servicePort.NodePort == 0 && !shouldAllocateNodePorts(service) { + // Don't allocate new ports, but do respect specific requests. + continue + } allocatedNodePort := svcPortToNodePort[int(servicePort.Port)] if allocatedNodePort == 0 { // This will only scan forward in the service.Spec.Ports list because any matches @@ -1224,6 +1230,10 @@ func updateNodePorts(oldService, newService *api.Service, nodePortOp *portalloca for i := range newService.Spec.Ports { servicePort := &newService.Spec.Ports[i] + if servicePort.NodePort == 0 && !shouldAllocateNodePorts(newService) { + // Don't allocate new ports, but do respect specific requests. + continue + } nodePort := ServiceNodePort{Protocol: servicePort.Protocol, NodePort: servicePort.NodePort} if nodePort.NodePort != 0 { if !containsNumber(oldNodePortsNumbers, int(nodePort.NodePort)) && !portAllocated[int(nodePort.NodePort)] { diff --git a/pkg/registry/core/service/storage/rest_test.go b/pkg/registry/core/service/storage/rest_test.go index c472ee8c8a4..a4a0260d123 100644 --- a/pkg/registry/core/service/storage/rest_test.go +++ b/pkg/registry/core/service/storage/rest_test.go @@ -762,28 +762,53 @@ func TestAllocateLoadBalancerNodePorts(t *testing.T) { allocateNodePortGate bool expectError bool }{{ - name: "allocate false, gate on", - svc: svctest.MakeService("alloc-false", svctest.SetTypeLoadBalancer, svctest.SetAllocateLBNodePortFalse), + name: "allocate false, gate on, not specified", + svc: svctest.MakeService("alloc-false", + svctest.SetTypeLoadBalancer, + svctest.SetAllocateLBNodePortFalse), expectNodePorts: false, allocateNodePortGate: true, }, { - name: "allocate true, gate on", - svc: svctest.MakeService("alloc-true", svctest.SetTypeLoadBalancer, svctest.SetAllocateLBNodePortTrue), + name: "allocate true, gate on, not specified", + svc: svctest.MakeService("alloc-true", + svctest.SetTypeLoadBalancer, + svctest.SetAllocateLBNodePortTrue), expectNodePorts: true, allocateNodePortGate: true, }, { - name: "allocate nil, gate off", - svc: svctest.MakeService("alloc-nil", svctest.SetTypeLoadBalancer), + name: "allocate false, gate on, port specified", + svc: svctest.MakeService("alloc-false-specific", + svctest.SetTypeLoadBalancer, + svctest.SetNodePorts(30000), + svctest.SetAllocateLBNodePortFalse), + expectNodePorts: true, + allocateNodePortGate: true, + }, { + name: "allocate true, gate on, port specified", + svc: svctest.MakeService("alloc-true-specific", + svctest.SetTypeLoadBalancer, + svctest.SetNodePorts(30000), + svctest.SetAllocateLBNodePortTrue), + expectNodePorts: true, + allocateNodePortGate: true, + }, { + name: "allocate nil, gate off", + svc: svctest.MakeService("alloc-nil", + svctest.SetTypeLoadBalancer), expectNodePorts: true, allocateNodePortGate: false, }, { - name: "allocate false, gate off", - svc: svctest.MakeService("alloc-false", svctest.SetTypeLoadBalancer, svctest.SetAllocateLBNodePortFalse), + name: "allocate false, gate off", + svc: svctest.MakeService("alloc-false", + svctest.SetTypeLoadBalancer, + svctest.SetAllocateLBNodePortFalse), expectNodePorts: true, allocateNodePortGate: false, }, { - name: "allocate true, gate off", - svc: svctest.MakeService("alloc-true", svctest.SetTypeLoadBalancer, svctest.SetAllocateLBNodePortTrue), + name: "allocate true, gate off", + svc: svctest.MakeService("alloc-true", + svctest.SetTypeLoadBalancer, + svctest.SetAllocateLBNodePortTrue), expectNodePorts: true, allocateNodePortGate: false, }} diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index c2f66dd2196..b8bae92c432 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -5018,11 +5018,14 @@ message ServiceSpec { optional string ipFamilyPolicy = 17; // 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. + // 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. + // This field is beta-level and is only honored by servers that enable the ServiceLBNodePortControl feature. + // +featureGate=ServiceLBNodePortControl // +optional optional bool allocateLoadBalancerNodePorts = 20; diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 3f6a6211b21..2b38051ba16 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -4276,11 +4276,14 @@ type ServiceSpec struct { 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. + // 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. + // This field is beta-level and is only honored by servers that enable the ServiceLBNodePortControl feature. + // +featureGate=ServiceLBNodePortControl // +optional AllocateLoadBalancerNodePorts *bool `json:"allocateLoadBalancerNodePorts,omitempty" protobuf:"bytes,20,opt,name=allocateLoadBalancerNodePorts"` 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 f29297985f8..29bba3f3c63 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 @@ -2246,7 +2246,7 @@ var map_ServiceSpec = map[string]string{ "sessionAffinityConfig": "sessionAffinityConfig contains the configurations of session affinity.", "ipFamilies": "IPFamilies is a list of IP families (e.g. IPv4, IPv6) assigned to this service, and is gated by the \"IPv6DualStack\" feature gate. This field is usually assigned automatically based on cluster configuration and the ipFamilyPolicy field. If this field is specified manually, the requested family is available in the cluster, and ipFamilyPolicy allows it, it will be used; otherwise creation of the service will fail. This field is conditionally mutable: it allows for adding or removing a secondary IP family, but it does not allow changing the primary IP family of the Service. Valid values are \"IPv4\" and \"IPv6\". This field only applies to Services of types ClusterIP, NodePort, and LoadBalancer, and does apply to \"headless\" services. This field will be wiped when updating a Service to type ExternalName.\n\nThis field may hold a maximum of two entries (dual-stack families, in either order). These families must correspond to the values of the clusterIPs field, if specified. Both clusterIPs and ipFamilies are governed by the ipFamilyPolicy field.", "ipFamilyPolicy": "IPFamilyPolicy represents the dual-stack-ness requested or required by this Service, and is gated by the \"IPv6DualStack\" feature gate. If there is no value provided, then this field will be set to SingleStack. Services can be \"SingleStack\" (a single IP family), \"PreferDualStack\" (two IP families on dual-stack configured clusters or a single IP family on single-stack clusters), or \"RequireDualStack\" (two IP families on dual-stack configured clusters, otherwise fail). The ipFamilies and clusterIPs fields depend on the value of this field. This field will be wiped when updating a service to type ExternalName.", - "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. 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.", + "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. This field is beta-level and is only honored by servers that enable the ServiceLBNodePortControl feature.", "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 specifies if the cluster internal traffic should be routed to all endpoints or node-local endpoints only. \"Cluster\" routes internal traffic to a Service to all endpoints. \"Local\" routes traffic to node-local endpoints only, traffic is dropped if no node-local endpoints are ready. The default value is \"Cluster\".", } From 5b787aa18431fdce683563aa41f3a298191fc2fc Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 4 Jul 2021 15:08:43 -0700 Subject: [PATCH 2/2] Clean up testing of AllocateLoadBalancerNodePorts We only need one "tweak" function, and it should be set automatically in most cases. --- pkg/api/service/testing/make.go | 21 +++++++------- .../core/service/storage/rest_test.go | 28 +++++++++---------- 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/pkg/api/service/testing/make.go b/pkg/api/service/testing/make.go index 15ab04aeb11..45bb1ef8db4 100644 --- a/pkg/api/service/testing/make.go +++ b/pkg/api/service/testing/make.go @@ -66,6 +66,7 @@ func SetTypeClusterIP(svc *api.Service) { } svc.Spec.ExternalName = "" svc.Spec.ExternalTrafficPolicy = "" + svc.Spec.AllocateLoadBalancerNodePorts = nil } // SetTypeNodePort sets the service type to NodePort and clears other fields. @@ -73,12 +74,14 @@ func SetTypeNodePort(svc *api.Service) { svc.Spec.Type = api.ServiceTypeNodePort svc.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeCluster svc.Spec.ExternalName = "" + svc.Spec.AllocateLoadBalancerNodePorts = nil } // SetTypeLoadBalancer sets the service type to LoadBalancer and clears other fields. func SetTypeLoadBalancer(svc *api.Service) { svc.Spec.Type = api.ServiceTypeLoadBalancer svc.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeCluster + svc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) svc.Spec.ExternalName = "" } @@ -89,16 +92,7 @@ func SetTypeExternalName(svc *api.Service) { svc.Spec.ExternalTrafficPolicy = "" svc.Spec.ClusterIP = "" svc.Spec.ClusterIPs = nil -} - -// SetTypeExternalNameTrue sets the allocate LB node port to true. -func SetAllocateLBNodePortTrue(svc *api.Service) { - svc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true) -} - -// SetTypeExternalNameFalse sets the allocate LB node port to false. -func SetAllocateLBNodePortFalse(svc *api.Service) { - svc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(false) + svc.Spec.AllocateLoadBalancerNodePorts = nil } // SetPorts sets the service ports list. @@ -160,3 +154,10 @@ func SetInternalTrafficPolicy(policy api.ServiceInternalTrafficPolicyType) Tweak svc.Spec.InternalTrafficPolicy = &policy } } + +// SetAllocateLoadBalancerNodePorts sets the allocate LB node port field. +func SetAllocateLoadBalancerNodePorts(val bool) Tweak { + return func(svc *api.Service) { + svc.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(val) + } +} diff --git a/pkg/registry/core/service/storage/rest_test.go b/pkg/registry/core/service/storage/rest_test.go index a4a0260d123..9679026325e 100644 --- a/pkg/registry/core/service/storage/rest_test.go +++ b/pkg/registry/core/service/storage/rest_test.go @@ -736,7 +736,7 @@ func TestServiceRegistryLoadBalancerService(t *testing.T) { ctx := genericapirequest.NewDefaultContext() storage, server := NewTestREST(t, []api.IPFamily{api.IPv4Protocol}) defer server.Terminate(t) - svc := svctest.MakeService("foo", svctest.SetTypeLoadBalancer, svctest.SetAllocateLBNodePortTrue) + svc := svctest.MakeService("foo", svctest.SetTypeLoadBalancer) _, err := storage.Create(ctx, svc, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) if err != nil { t.Errorf("Failed to create service: %#v", err) @@ -765,14 +765,14 @@ func TestAllocateLoadBalancerNodePorts(t *testing.T) { name: "allocate false, gate on, not specified", svc: svctest.MakeService("alloc-false", svctest.SetTypeLoadBalancer, - svctest.SetAllocateLBNodePortFalse), + svctest.SetAllocateLoadBalancerNodePorts(false)), expectNodePorts: false, allocateNodePortGate: true, }, { name: "allocate true, gate on, not specified", svc: svctest.MakeService("alloc-true", svctest.SetTypeLoadBalancer, - svctest.SetAllocateLBNodePortTrue), + svctest.SetAllocateLoadBalancerNodePorts(true)), expectNodePorts: true, allocateNodePortGate: true, }, { @@ -780,7 +780,7 @@ func TestAllocateLoadBalancerNodePorts(t *testing.T) { svc: svctest.MakeService("alloc-false-specific", svctest.SetTypeLoadBalancer, svctest.SetNodePorts(30000), - svctest.SetAllocateLBNodePortFalse), + svctest.SetAllocateLoadBalancerNodePorts(false)), expectNodePorts: true, allocateNodePortGate: true, }, { @@ -788,27 +788,30 @@ func TestAllocateLoadBalancerNodePorts(t *testing.T) { svc: svctest.MakeService("alloc-true-specific", svctest.SetTypeLoadBalancer, svctest.SetNodePorts(30000), - svctest.SetAllocateLBNodePortTrue), + svctest.SetAllocateLoadBalancerNodePorts(true)), expectNodePorts: true, allocateNodePortGate: true, }, { name: "allocate nil, gate off", svc: svctest.MakeService("alloc-nil", - svctest.SetTypeLoadBalancer), + svctest.SetTypeLoadBalancer, + func(s *api.Service) { + s.Spec.AllocateLoadBalancerNodePorts = nil + }), expectNodePorts: true, allocateNodePortGate: false, }, { name: "allocate false, gate off", svc: svctest.MakeService("alloc-false", svctest.SetTypeLoadBalancer, - svctest.SetAllocateLBNodePortFalse), + svctest.SetAllocateLoadBalancerNodePorts(false)), expectNodePorts: true, allocateNodePortGate: false, }, { name: "allocate true, gate off", svc: svctest.MakeService("alloc-true", svctest.SetTypeLoadBalancer, - svctest.SetAllocateLBNodePortTrue), + svctest.SetAllocateLoadBalancerNodePorts(true)), expectNodePorts: true, allocateNodePortGate: false, }} @@ -990,9 +993,7 @@ func TestServiceRegistryUpdateMultiPortLoadBalancerService(t *testing.T) { svctest.SetTypeLoadBalancer, svctest.SetPorts( svctest.MakeServicePort("p", 6502, intstr.FromInt(6502), api.ProtocolTCP), - svctest.MakeServicePort("q", 8086, intstr.FromInt(8086), api.ProtocolTCP)), - svctest.SetAllocateLBNodePortTrue, - ) + svctest.MakeServicePort("q", 8086, intstr.FromInt(8086), api.ProtocolTCP))) obj, err := storage.Create(ctx, svc1, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) if err != nil { t.Fatalf("Unexpected error: %v", err) @@ -1346,7 +1347,7 @@ func TestServiceRegistryIPLoadBalancer(t *testing.T) { storage, server := NewTestREST(t, []api.IPFamily{api.IPv4Protocol}) defer server.Terminate(t) - svc := svctest.MakeService("foo", svctest.SetTypeLoadBalancer, svctest.SetAllocateLBNodePortTrue) + svc := svctest.MakeService("foo", svctest.SetTypeLoadBalancer) ctx := genericapirequest.NewDefaultContext() createdSvc, err := storage.Create(ctx, svc, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) if createdSvc == nil || err != nil { @@ -1377,7 +1378,6 @@ func TestServiceRegistryExternalTrafficHealthCheckNodePortAllocation(t *testing. defer server.Terminate(t) svc := svctest.MakeService("external-lb-esipp", svctest.SetTypeLoadBalancer, - svctest.SetAllocateLBNodePortTrue, func(s *api.Service) { s.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeLocal }, @@ -1405,7 +1405,6 @@ func TestServiceRegistryExternalTrafficHealthCheckNodePortUserAllocation(t *test defer server.Terminate(t) svc := svctest.MakeService("external-lb-esipp", svctest.SetTypeLoadBalancer, - svctest.SetAllocateLBNodePortTrue, func(s *api.Service) { // hard-code NodePort to make sure it doesn't conflict with the healthport. // TODO: remove this once http://issue.k8s.io/93922 fixes auto-allocation conflicting with user-specified health check ports @@ -1455,7 +1454,6 @@ func TestServiceRegistryExternalTrafficGlobal(t *testing.T) { defer server.Terminate(t) svc := svctest.MakeService("external-lb-esipp", svctest.SetTypeLoadBalancer, - svctest.SetAllocateLBNodePortTrue, func(s *api.Service) { s.Spec.ExternalTrafficPolicy = api.ServiceExternalTrafficPolicyTypeCluster },