From 2ea3d320b04c9ed792edcf23f6de4ec65a4d98c1 Mon Sep 17 00:00:00 2001
From: Antonio Ojea <aojea@redhat.com>
Date: Thu, 18 Nov 2021 09:25:52 +0100
Subject: [PATCH] integration tests service node port control

When a service is created with AllocateLoadBalancerNodePorts to false
it should not allocate node ports.
If the same service is updated to set AllocateLoadBalancerNodePorts
to true, it should allocate node ports.

When a service is updated from ClusterIP type to LoadBalancer type,
and AllocateLoadBalancerNodePorts is set to false, it should not
allocate node ports.
---
 test/integration/service/loadbalancer_test.go | 108 ++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/test/integration/service/loadbalancer_test.go b/test/integration/service/loadbalancer_test.go
index 480d9f43594..18ad2a16567 100644
--- a/test/integration/service/loadbalancer_test.go
+++ b/test/integration/service/loadbalancer_test.go
@@ -78,6 +78,60 @@ func Test_ServiceLoadBalancerDisableAllocateNodePorts(t *testing.T) {
 	}
 }
 
+// Test_ServiceUpdateLoadBalancerAllocateNodePorts tests that a Service that is updated from ClusterIP to LoadBalancer
+// with spec.allocateLoadBalancerNodePorts=false does not allocate node ports for the Service
+func Test_ServiceUpdateLoadBalancerDisableAllocateNodePorts(t *testing.T) {
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceLBNodePortControl, true)()
+
+	controlPlaneConfig := framework.NewIntegrationTestControlPlaneConfig()
+	_, server, closeFn := framework.RunAnAPIServer(controlPlaneConfig)
+	defer closeFn()
+
+	config := restclient.Config{Host: server.URL}
+	client, err := clientset.NewForConfig(&config)
+	if err != nil {
+		t.Fatalf("Error creating clientset: %v", err)
+	}
+
+	ns := framework.CreateTestingNamespace("test-service-allocate-node-ports", server, t)
+	defer framework.DeleteTestingNamespace(ns, server, t)
+
+	service := &corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-123",
+		},
+		Spec: corev1.ServiceSpec{
+			Type: corev1.ServiceTypeClusterIP,
+			Ports: []corev1.ServicePort{{
+				Port: int32(80),
+			}},
+			Selector: map[string]string{
+				"foo": "bar",
+			},
+		},
+	}
+
+	service, err = client.CoreV1().Services(ns.Name).Create(context.TODO(), service, metav1.CreateOptions{})
+	if err != nil {
+		t.Fatalf("Error creating test service: %v", err)
+	}
+
+	if serviceHasNodePorts(service) {
+		t.Error("found node ports when none was expected")
+	}
+
+	service.Spec.Type = corev1.ServiceTypeLoadBalancer
+	service.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(false)
+	service, err = client.CoreV1().Services(ns.Name).Update(context.TODO(), service, metav1.UpdateOptions{})
+	if err != nil {
+		t.Fatalf("Error updating test service: %v", err)
+	}
+
+	if serviceHasNodePorts(service) {
+		t.Error("found node ports when none was expected")
+	}
+}
+
 // Test_ServiceLoadBalancerSwitchToDeallocatedNodePorts test that switching a Service
 // to spec.allocateLoadBalancerNodePorts=false, does not de-allocate existing node ports.
 func Test_ServiceLoadBalancerEnableThenDisableAllocatedNodePorts(t *testing.T) {
@@ -132,6 +186,60 @@ func Test_ServiceLoadBalancerEnableThenDisableAllocatedNodePorts(t *testing.T) {
 	}
 }
 
+// Test_ServiceLoadBalancerDisableThenEnableAllocatedNodePorts test that switching a Service
+// to spec.allocateLoadBalancerNodePorts=true from false, allocate new node ports.
+func Test_ServiceLoadBalancerDisableThenEnableAllocatedNodePorts(t *testing.T) {
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceLBNodePortControl, true)()
+
+	controlPlaneConfig := framework.NewIntegrationTestControlPlaneConfig()
+	_, server, closeFn := framework.RunAnAPIServer(controlPlaneConfig)
+	defer closeFn()
+
+	config := restclient.Config{Host: server.URL}
+	client, err := clientset.NewForConfig(&config)
+	if err != nil {
+		t.Fatalf("Error creating clientset: %v", err)
+	}
+
+	ns := framework.CreateTestingNamespace("test-service-reallocate-node-ports", server, t)
+	defer framework.DeleteTestingNamespace(ns, server, t)
+
+	service := &corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-123",
+		},
+		Spec: corev1.ServiceSpec{
+			Type:                          corev1.ServiceTypeLoadBalancer,
+			AllocateLoadBalancerNodePorts: utilpointer.BoolPtr(false),
+			Ports: []corev1.ServicePort{{
+				Port: int32(80),
+			}},
+			Selector: map[string]string{
+				"foo": "bar",
+			},
+		},
+	}
+
+	service, err = client.CoreV1().Services(ns.Name).Create(context.TODO(), service, metav1.CreateOptions{})
+	if err != nil {
+		t.Fatalf("Error creating test service: %v", err)
+	}
+
+	if serviceHasNodePorts(service) {
+		t.Error("not expected node ports but found one")
+	}
+
+	service.Spec.AllocateLoadBalancerNodePorts = utilpointer.BoolPtr(true)
+	service, err = client.CoreV1().Services(ns.Name).Update(context.TODO(), service, metav1.UpdateOptions{})
+	if err != nil {
+		t.Fatalf("Error updating test service: %v", err)
+	}
+
+	if !serviceHasNodePorts(service) {
+		t.Error("expected node ports but found none")
+	}
+}
+
 func serviceHasNodePorts(svc *corev1.Service) bool {
 	for _, port := range svc.Spec.Ports {
 		if port.NodePort > 0 {