From 5b787aa18431fdce683563aa41f3a298191fc2fc Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 4 Jul 2021 15:08:43 -0700 Subject: [PATCH] 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 },