From 7cf75dbdd8f20b5d960a0fdf9cfb89f5edd5abb4 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Mon, 9 Aug 2021 22:14:15 -0700 Subject: [PATCH] Svc REST: Beef up NodePort tests Remove old test from rest_test.go. --- .../core/service/storage/rest_test.go | 115 ----------------- .../core/service/storage/storage_test.go | 121 +++++++++++++++++- 2 files changed, 116 insertions(+), 120 deletions(-) diff --git a/pkg/registry/core/service/storage/rest_test.go b/pkg/registry/core/service/storage/rest_test.go index affc06dbff1..5ca2ebee3ad 100644 --- a/pkg/registry/core/service/storage/rest_test.go +++ b/pkg/registry/core/service/storage/rest_test.go @@ -930,121 +930,6 @@ func TestServiceRegistryInternalTrafficPolicyLocalThenCluster(t *testing.T) { } } -func TestUpdateNodePorts(t *testing.T) { - storage, server := NewTestREST(t, []api.IPFamily{api.IPv4Protocol}) - defer server.Terminate(t) - nodePortOp := portallocator.StartOperation(storage.alloc.serviceNodePorts, false) - - testCases := []struct { - name string - oldService *api.Service - newService *api.Service - expectSpecifiedNodePorts []int - }{{ - name: "Old service and new service have the same NodePort", - oldService: svctest.MakeService("foo", - svctest.SetTypeNodePort, - svctest.SetPorts( - svctest.MakeServicePort("", 6502, intstr.FromInt(6502), api.ProtocolTCP)), - svctest.SetNodePorts(30053)), - newService: svctest.MakeService("foo", - svctest.SetTypeNodePort, - svctest.SetPorts( - svctest.MakeServicePort("", 6502, intstr.FromInt(6502), api.ProtocolTCP)), - svctest.SetNodePorts(30053)), - expectSpecifiedNodePorts: []int{30053}, - }, { - name: "Old service has more NodePorts than new service has", - oldService: svctest.MakeService("foo", - svctest.SetTypeNodePort, - svctest.SetPorts( - svctest.MakeServicePort("port-tcp", 53, intstr.FromInt(6502), api.ProtocolTCP), - svctest.MakeServicePort("port-udp", 53, intstr.FromInt(6502), api.ProtocolUDP)), - svctest.SetNodePorts(30053, 30053)), - newService: svctest.MakeService("foo", - svctest.SetTypeNodePort, - svctest.SetPorts( - svctest.MakeServicePort("port-tcp", 53, intstr.FromInt(6502), api.ProtocolTCP)), - svctest.SetNodePorts(30053)), - expectSpecifiedNodePorts: []int{30053}, - }, { - name: "Change protocol of ServicePort without changing NodePort", - oldService: svctest.MakeService("foo", - svctest.SetTypeNodePort, - svctest.SetPorts( - svctest.MakeServicePort("port-tcp", 53, intstr.FromInt(6502), api.ProtocolTCP)), - svctest.SetNodePorts(30053)), - newService: svctest.MakeService("foo", - svctest.SetTypeNodePort, - svctest.SetPorts( - svctest.MakeServicePort("port-udp", 53, intstr.FromInt(6502), api.ProtocolUDP)), - svctest.SetNodePorts(30053)), - expectSpecifiedNodePorts: []int{30053}, - }, { - name: "Should allocate NodePort when changing service type to NodePort", - oldService: svctest.MakeService("foo", - svctest.SetTypeClusterIP, - svctest.SetPorts( - svctest.MakeServicePort("", 6502, intstr.FromInt(6502), api.ProtocolUDP))), - newService: svctest.MakeService("foo", - svctest.SetTypeNodePort, - svctest.SetPorts( - svctest.MakeServicePort("", 6502, intstr.FromInt(6502), api.ProtocolUDP))), - expectSpecifiedNodePorts: []int{}, - }, { - name: "Add new ServicePort with a different protocol without changing port numbers", - oldService: svctest.MakeService("foo", - svctest.SetTypeNodePort, - svctest.SetPorts( - svctest.MakeServicePort("port-tcp", 53, intstr.FromInt(6502), api.ProtocolTCP)), - svctest.SetNodePorts(30053)), - newService: svctest.MakeService("foo", - svctest.SetTypeNodePort, - svctest.SetPorts( - svctest.MakeServicePort("port-tcp", 53, intstr.FromInt(6502), api.ProtocolTCP), - svctest.MakeServicePort("port-udp", 53, intstr.FromInt(6502), api.ProtocolUDP)), - svctest.SetNodePorts(30053, 30053)), - expectSpecifiedNodePorts: []int{30053, 30053}, - }, { - name: "Change service type from ClusterIP to NodePort with same NodePort number but different protocols", - oldService: svctest.MakeService("foo", - svctest.SetTypeClusterIP, - svctest.SetPorts( - svctest.MakeServicePort("", 53, intstr.FromInt(6502), api.ProtocolTCP))), - newService: svctest.MakeService("foo", - svctest.SetTypeNodePort, - svctest.SetPorts( - svctest.MakeServicePort("port-tcp", 53, intstr.FromInt(6502), api.ProtocolTCP), - svctest.MakeServicePort("port-udp", 53, intstr.FromInt(6502), api.ProtocolUDP)), - svctest.SetNodePorts(30053, 30053)), - expectSpecifiedNodePorts: []int{30053, 30053}, - }} - - for _, test := range testCases { - err := updateNodePorts(test.oldService, test.newService, nodePortOp) - if err != nil { - t.Errorf("%q: unexpected error: %v", test.name, err) - continue - } - - serviceNodePorts := collectServiceNodePorts(test.newService) - if len(test.expectSpecifiedNodePorts) == 0 { - for _, nodePort := range serviceNodePorts { - if !storage.alloc.serviceNodePorts.Has(nodePort) { - t.Errorf("%q: unexpected NodePort %d, out of range", test.name, nodePort) - } - } - } else if !reflect.DeepEqual(serviceNodePorts, test.expectSpecifiedNodePorts) { - t.Errorf("%q: expected NodePorts %v, but got %v", test.name, test.expectSpecifiedNodePorts, serviceNodePorts) - } - for i := range serviceNodePorts { - nodePort := serviceNodePorts[i] - // Release the node port at the end of the test case. - storage.alloc.serviceNodePorts.Release(nodePort) - } - } -} - func TestServiceUpgrade(t *testing.T) { requireDualStack := api.IPFamilyPolicyRequireDualStack diff --git a/pkg/registry/core/service/storage/storage_test.go b/pkg/registry/core/service/storage/storage_test.go index 38bea012a67..b1f63dd20f2 100644 --- a/pkg/registry/core/service/storage/storage_test.go +++ b/pkg/registry/core/service/storage/storage_test.go @@ -6438,7 +6438,7 @@ func TestFeatureClusterIPs(t *testing.T) { func TestFeaturePorts(t *testing.T) { testCases := []cudTestCase{{ - name: "add", + name: "add_port", create: svcTestCase{ svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, svctest.SetPorts( @@ -6453,7 +6453,39 @@ func TestFeaturePorts(t *testing.T) { expectClusterIPs: true, }, }, { - name: "remove", + name: "add_port_ClusterIP-NodePort", + create: svcTestCase{ + svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, + svctest.SetPorts( + svctest.MakeServicePort("p", 80, intstr.FromInt(80), api.ProtocolTCP))), + expectClusterIPs: true, + }, + update: svcTestCase{ + svc: svctest.MakeService("foo", svctest.SetTypeNodePort, + svctest.SetPorts( + svctest.MakeServicePort("p", 80, intstr.FromInt(80), api.ProtocolTCP), + svctest.MakeServicePort("q", 443, intstr.FromInt(443), api.ProtocolTCP))), + expectClusterIPs: true, + expectNodePorts: true, + }, + }, { + name: "add_port_NodePort-ClusterIP", + create: svcTestCase{ + svc: svctest.MakeService("foo", svctest.SetTypeNodePort, + svctest.SetPorts( + svctest.MakeServicePort("p", 80, intstr.FromInt(80), api.ProtocolTCP))), + expectClusterIPs: true, + expectNodePorts: true, + }, + update: svcTestCase{ + svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, + svctest.SetPorts( + svctest.MakeServicePort("p", 80, intstr.FromInt(80), api.ProtocolTCP), + svctest.MakeServicePort("q", 443, intstr.FromInt(443), api.ProtocolTCP))), + expectClusterIPs: true, + }, + }, { + name: "remove_port", create: svcTestCase{ svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, svctest.SetPorts( @@ -6468,7 +6500,39 @@ func TestFeaturePorts(t *testing.T) { expectClusterIPs: true, }, }, { - name: "swap", + name: "remove_port_ClusterIP-NodePort", + create: svcTestCase{ + svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, + svctest.SetPorts( + svctest.MakeServicePort("p", 80, intstr.FromInt(80), api.ProtocolTCP), + svctest.MakeServicePort("q", 443, intstr.FromInt(443), api.ProtocolTCP))), + expectClusterIPs: true, + }, + update: svcTestCase{ + svc: svctest.MakeService("foo", svctest.SetTypeNodePort, + svctest.SetPorts( + svctest.MakeServicePort("p", 80, intstr.FromInt(80), api.ProtocolTCP))), + expectClusterIPs: true, + expectNodePorts: true, + }, + }, { + name: "remove_port_NodePort-ClusterIP", + create: svcTestCase{ + svc: svctest.MakeService("foo", svctest.SetTypeNodePort, + svctest.SetPorts( + svctest.MakeServicePort("p", 80, intstr.FromInt(80), api.ProtocolTCP), + svctest.MakeServicePort("q", 443, intstr.FromInt(443), api.ProtocolTCP))), + expectClusterIPs: true, + expectNodePorts: true, + }, + update: svcTestCase{ + svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, + svctest.SetPorts( + svctest.MakeServicePort("p", 80, intstr.FromInt(80), api.ProtocolTCP))), + expectClusterIPs: true, + }, + }, { + name: "swap_ports", create: svcTestCase{ svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, svctest.SetPorts( @@ -6484,7 +6548,39 @@ func TestFeaturePorts(t *testing.T) { expectClusterIPs: true, }, }, { - name: "modify", + name: "modify_ports", + create: svcTestCase{ + svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, + svctest.SetPorts( + svctest.MakeServicePort("p", 80, intstr.FromInt(80), api.ProtocolTCP), + svctest.MakeServicePort("q", 443, intstr.FromInt(443), api.ProtocolTCP))), + expectClusterIPs: true, + }, + update: svcTestCase{ + svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, + svctest.SetPorts( + svctest.MakeServicePort("p", 8080, intstr.FromInt(8080), api.ProtocolTCP), + svctest.MakeServicePort("q", 8443, intstr.FromInt(8443), api.ProtocolTCP))), + expectClusterIPs: true, + }, + }, { + name: "modify_protos", + create: svcTestCase{ + svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, + svctest.SetPorts( + svctest.MakeServicePort("p", 80, intstr.FromInt(80), api.ProtocolTCP), + svctest.MakeServicePort("q", 443, intstr.FromInt(443), api.ProtocolTCP))), + expectClusterIPs: true, + }, + update: svcTestCase{ + svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, + svctest.SetPorts( + svctest.MakeServicePort("p", 80, intstr.FromInt(80), api.ProtocolUDP), + svctest.MakeServicePort("q", 443, intstr.FromInt(443), api.ProtocolUDP))), + expectClusterIPs: true, + }, + }, { + name: "modify_ports_and_protos", create: svcTestCase{ svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, svctest.SetPorts( @@ -6500,7 +6596,22 @@ func TestFeaturePorts(t *testing.T) { expectClusterIPs: true, }, }, { - name: "wipe", + name: "add_alt_proto", + create: svcTestCase{ + svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, + svctest.SetPorts( + svctest.MakeServicePort("p", 53, intstr.FromInt(53), api.ProtocolTCP))), + expectClusterIPs: true, + }, + update: svcTestCase{ + svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, + svctest.SetPorts( + svctest.MakeServicePort("p", 53, intstr.FromInt(53), api.ProtocolTCP), + svctest.MakeServicePort("q", 53, intstr.FromInt(53), api.ProtocolUDP))), + expectClusterIPs: true, + }, + }, { + name: "wipe_all", create: svcTestCase{ svc: svctest.MakeService("foo", svctest.SetTypeClusterIP, svctest.SetPorts(