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

@@ -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
-}
+	svc.Spec.AllocateLoadBalancerNodePorts = 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)
 }
 
 // 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)
+	}
+}

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.MakeServicePort("q", 8086, intstr.FromInt(8086), api.ProtocolTCP)))
-		svctest.SetAllocateLBNodePortTrue,
-	)
 	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
 		},