Merge pull request #125237 from aojea/ipmode_integration

improve loadbalancer IPMode testing

staging/src/k8s.io/cloud-provider/fake/fake.go

@@ -84,6 +84,7 @@ type Cloud struct {
 	ClusterList    []string
 	MasterName     string
 	ExternalIP     net.IP
+	BalancerIPMode *v1.LoadBalancerIPMode
 	Balancers      map[string]Balancer
 	updateCallLock sync.Mutex
 	UpdateCalls    []UpdateBalancerCall
@@ -224,7 +225,15 @@ func (f *Cloud) EnsureLoadBalancer(ctx context.Context, clusterName string, serv
 	f.Balancers[name] = Balancer{name, region, spec.LoadBalancerIP, spec.Ports, nodes}
 
 	status := &v1.LoadBalancerStatus{}
-	status.Ingress = []v1.LoadBalancerIngress{{IP: f.ExternalIP.String()}}
+	// process Ports
+	portStatus := []v1.PortStatus{}
+	for _, port := range spec.Ports {
+		portStatus = append(portStatus, v1.PortStatus{
+			Port:     port.Port,
+			Protocol: port.Protocol,
+		})
+	}
+	status.Ingress = []v1.LoadBalancerIngress{{IP: f.ExternalIP.String(), IPMode: f.BalancerIPMode, Ports: portStatus}}
 
 	return status, f.Err
 }

test/integration/service/loadbalancer_test.go

@@ -19,14 +19,15 @@ package service
 import (
 	"context"
 	"encoding/json"
-	"reflect"
 	"testing"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/strategicpatch"
+	"k8s.io/apimachinery/pkg/util/wait"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/client-go/informers"
 	clientset "k8s.io/client-go/kubernetes"
@@ -39,6 +40,7 @@ import (
 	"k8s.io/kubernetes/test/integration/framework"
 	"k8s.io/utils/net"
 	utilpointer "k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 )
 
 // Test_ServiceLoadBalancerAllocateNodePorts tests that a Service with spec.allocateLoadBalancerNodePorts=false
@@ -644,21 +646,62 @@ func newServiceController(t *testing.T, client *clientset.Clientset) (*serviceco
 
 // Test_ServiceLoadBalancerIPMode tests whether the cloud provider has correctly updated the ipMode field.
 func Test_ServiceLoadBalancerIPMode(t *testing.T) {
-	ipModeVIP := corev1.LoadBalancerIPModeVIP
+	baseService := &corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-update-load-balancer-ip-mode",
+		},
+		Spec: corev1.ServiceSpec{
+			Type: corev1.ServiceTypeLoadBalancer,
+			Ports: []corev1.ServicePort{{
+				Port: int32(80),
+			}},
+		},
+	}
+
 	testCases := []struct {
-		ipModeEnabled  bool
-		externalIP     string
-		expectedIPMode *corev1.LoadBalancerIPMode
+		ipModeEnabled   bool
+		setIPMode       *corev1.LoadBalancerIPMode
+		externalIP      string
+		expectedIngress corev1.LoadBalancerIngress
 	}{
 		{
-			ipModeEnabled:  false,
-			externalIP:     "1.2.3.4",
-			expectedIPMode: nil,
+			ipModeEnabled: false,
+			externalIP:    "1.2.3.4",
+			expectedIngress: corev1.LoadBalancerIngress{
+				IP:     "1.2.3.4",
+				IPMode: nil,
+				Ports:  []corev1.PortStatus{{Port: 80, Protocol: corev1.ProtocolTCP}},
+			},
 		},
 		{
-			ipModeEnabled:  true,
-			externalIP:     "1.2.3.5",
-			expectedIPMode: &ipModeVIP,
+			ipModeEnabled: true,
+			setIPMode:     nil,
+			externalIP:    "1.2.3.4",
+			expectedIngress: corev1.LoadBalancerIngress{
+				IP:     "1.2.3.4",
+				IPMode: ptr.To(corev1.LoadBalancerIPModeVIP),
+				Ports:  []corev1.PortStatus{{Port: 80, Protocol: corev1.ProtocolTCP}},
+			},
+		},
+		{
+			ipModeEnabled: true,
+			setIPMode:     ptr.To(corev1.LoadBalancerIPModeVIP),
+			externalIP:    "1.2.3.4",
+			expectedIngress: corev1.LoadBalancerIngress{
+				IP:     "1.2.3.4",
+				IPMode: ptr.To(corev1.LoadBalancerIPModeVIP),
+				Ports:  []corev1.PortStatus{{Port: 80, Protocol: corev1.ProtocolTCP}},
+			},
+		},
+		{
+			ipModeEnabled: true,
+			setIPMode:     ptr.To(corev1.LoadBalancerIPModeProxy),
+			externalIP:    "1.2.3.4",
+			expectedIngress: corev1.LoadBalancerIngress{
+				IP:     "1.2.3.4",
+				IPMode: ptr.To(corev1.LoadBalancerIPModeProxy),
+				Ports:  []corev1.PortStatus{{Port: 80, Protocol: corev1.ProtocolTCP}},
+			},
 		},
 	}
 
@@ -678,43 +721,68 @@ func Test_ServiceLoadBalancerIPMode(t *testing.T) {
 
 			controller, cloud, informer := newServiceController(t, client)
 			cloud.ExternalIP = net.ParseIPSloppy(tc.externalIP)
+			cloud.BalancerIPMode = tc.expectedIngress.IPMode
 
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 			informer.Start(ctx.Done())
 			go controller.Run(ctx, 1, controllersmetrics.NewControllerManagerMetrics("loadbalancer-test"))
 
-			service := &corev1.Service{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "test-update-load-balancer-ip-mode",
-				},
-				Spec: corev1.ServiceSpec{
-					Type: corev1.ServiceTypeLoadBalancer,
-					Ports: []corev1.ServicePort{{
-						Port: int32(80),
-					}},
-				},
-			}
-
-			service, err = client.CoreV1().Services(ns.Name).Create(ctx, service, metav1.CreateOptions{})
+			service, err := client.CoreV1().Services(ns.Name).Create(ctx, baseService, metav1.CreateOptions{})
 			if err != nil {
 				t.Fatalf("Error creating test service: %v", err)
 			}
 
-			time.Sleep(5 * time.Second) // sleep 5 second to wait for the service controller reconcile
-			service, err = client.CoreV1().Services(ns.Name).Get(ctx, service.Name, metav1.GetOptions{})
+			err = wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 10*time.Second, true, func(_ context.Context) (done bool, err error) {
+				service, err = client.CoreV1().Services(ns.Name).Get(ctx, service.Name, metav1.GetOptions{})
+				if err != nil {
+					t.Fatalf("Error getting test service: %v", err)
+				}
+				if len(service.Status.LoadBalancer.Ingress) != 1 {
+					return false, nil
+				}
+				return true, nil
+			})
 			if err != nil {
-				t.Fatalf("Error getting test service: %v", err)
-			}
-
-			if len(service.Status.LoadBalancer.Ingress) == 0 {
 				t.Fatalf("unexpected load balancer status")
 			}
 
-			gotIngress := service.Status.LoadBalancer.Ingress[0]
-			if gotIngress.IP != tc.externalIP || !reflect.DeepEqual(gotIngress.IPMode, tc.expectedIPMode) {
-				t.Errorf("unexpected load balancer ingress, got ingress %v, expected IP %v, expected ipMode %v",
-					gotIngress, tc.externalIP, tc.expectedIPMode)
+			ingress := service.Status.LoadBalancer.Ingress[0]
+			if !apiequality.Semantic.DeepEqual(&ingress, &tc.expectedIngress) {
+				t.Errorf("expected Ingress %v, got IP %v",
+					ingress, tc.expectedIngress)
+				if ingress.IPMode != nil && tc.expectedIngress.IPMode != nil {
+					t.Logf("IPMode %v expected %v", *ingress.IPMode, *tc.expectedIngress.IPMode)
+				}
+			}
+
+			// mutate the service and check the status is preserved
+			newService := service.DeepCopy()
+			newService.Spec.Ports[0].Port = 443
+			service, err = client.CoreV1().Services(ns.Name).Update(ctx, newService, metav1.UpdateOptions{})
+			if err != nil {
+				t.Fatalf("Error updating test service: %v", err)
+			}
+
+			expectedIngress := tc.expectedIngress
+			expectedIngress.Ports[0].Port = 443
+			err = wait.PollUntilContextTimeout(ctx, 500*time.Millisecond, 10*time.Second, true, func(_ context.Context) (done bool, err error) {
+				service, err = client.CoreV1().Services(ns.Name).Get(ctx, service.Name, metav1.GetOptions{})
+				if err != nil {
+					t.Fatalf("Error getting test service: %v", err)
+				}
+				if len(service.Status.LoadBalancer.Ingress) != 1 {
+					return false, nil
+				}
+				ingress = service.Status.LoadBalancer.Ingress[0]
+				if !apiequality.Semantic.DeepEqual(&ingress, &expectedIngress) {
+					t.Logf("Ingress %v Expected %v", ingress, expectedIngress)
+					return false, nil
+				}
+				return true, nil
+			})
+			if err != nil {
+				t.Fatalf("unexpected load balancer status")
 			}
 		})
 	}