Merge pull request #125225 from aojea/ipmode

fix loadbalancer status comparison

pkg/apis/core/v1/helper/helpers.go

@@ -140,35 +140,6 @@ func IsServiceIPSet(service *v1.Service) bool {
 	return service.Spec.ClusterIP != v1.ClusterIPNone && service.Spec.ClusterIP != ""
 }
 
-// LoadBalancerStatusEqual evaluates the given load balancers' ingress IP addresses
-// and hostnames and returns true if equal or false if otherwise
-// TODO: make method on LoadBalancerStatus?
-func LoadBalancerStatusEqual(l, r *v1.LoadBalancerStatus) bool {
-	return ingressSliceEqual(l.Ingress, r.Ingress)
-}
-
-func ingressSliceEqual(lhs, rhs []v1.LoadBalancerIngress) bool {
-	if len(lhs) != len(rhs) {
-		return false
-	}
-	for i := range lhs {
-		if !ingressEqual(&lhs[i], &rhs[i]) {
-			return false
-		}
-	}
-	return true
-}
-
-func ingressEqual(lhs, rhs *v1.LoadBalancerIngress) bool {
-	if lhs.IP != rhs.IP {
-		return false
-	}
-	if lhs.Hostname != rhs.Hostname {
-		return false
-	}
-	return true
-}
-
 // GetAccessModesAsString returns a string representation of an array of access modes.
 // modes, when present, are always in the same order: RWO,ROX,RWX,RWOP.
 func GetAccessModesAsString(modes []v1.PersistentVolumeAccessMode) string {

pkg/apis/core/v1/helper/helpers_test.go

@@ -723,86 +723,3 @@ func TestHugePageUnitSizeFromByteSize(t *testing.T) {
 		}
 	}
 }
-
-func TestLoadBalancerStatusEqual(t *testing.T) {
-
-	testCases := []struct {
-		left      *v1.LoadBalancerStatus
-		right     *v1.LoadBalancerStatus
-		name      string
-		expectVal bool
-	}{{
-		name: "left equals right",
-		left: &v1.LoadBalancerStatus{
-			Ingress: []v1.LoadBalancerIngress{{
-				IP:       "1.1.1.1",
-				Hostname: "host1",
-			}},
-		},
-		right: &v1.LoadBalancerStatus{
-			Ingress: []v1.LoadBalancerIngress{{
-				IP:       "1.1.1.1",
-				Hostname: "host1",
-			}},
-		},
-		expectVal: true,
-	}, {
-		name: "length of LoadBalancerIngress slice is not equal",
-		left: &v1.LoadBalancerStatus{
-			Ingress: []v1.LoadBalancerIngress{{
-				IP:       "1.1.1.1",
-				Hostname: "host1",
-			}, {
-				IP:       "1.1.1.2",
-				Hostname: "host1",
-			}},
-		},
-		right: &v1.LoadBalancerStatus{
-			Ingress: []v1.LoadBalancerIngress{{
-				IP:       "1.1.1.1",
-				Hostname: "host1",
-			}},
-		},
-		expectVal: false,
-	}, {
-		name: "LoadBalancerIngress ip is not equal",
-		left: &v1.LoadBalancerStatus{
-			Ingress: []v1.LoadBalancerIngress{{
-				IP:       "1.1.1.2",
-				Hostname: "host1",
-			}},
-		},
-		right: &v1.LoadBalancerStatus{
-			Ingress: []v1.LoadBalancerIngress{{
-				IP:       "1.1.1.1",
-				Hostname: "host1",
-			}},
-		},
-		expectVal: false,
-	}, {
-		name: "LoadBalancerIngress hostname is not equal",
-		left: &v1.LoadBalancerStatus{
-			Ingress: []v1.LoadBalancerIngress{{
-				IP:       "1.1.1.1",
-				Hostname: "host2",
-			}},
-		},
-		right: &v1.LoadBalancerStatus{
-			Ingress: []v1.LoadBalancerIngress{{
-				IP:       "1.1.1.1",
-				Hostname: "host1",
-			}},
-		},
-		expectVal: false,
-	}}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			v := LoadBalancerStatusEqual(tc.left, tc.right)
-			if v != tc.expectVal {
-				t.Errorf("test %s failed. left input=%v, right input=%v, Got %v but expected %v",
-					tc.name, tc.left, tc.right, v, tc.expectVal)
-			}
-		})
-	}
-}

staging/src/k8s.io/cloud-provider/service/helpers/helper.go

@@ -23,6 +23,7 @@ import (
 	"strings"
 
 	v1 "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"
@@ -124,7 +125,7 @@ func HasLBFinalizer(service *v1.Service) bool {
 
 // LoadBalancerStatusEqual checks if load balancer status are equal
 func LoadBalancerStatusEqual(l, r *v1.LoadBalancerStatus) bool {
-	return ingressSliceEqual(l.Ingress, r.Ingress)
+	return apiequality.Semantic.DeepEqual(l.Ingress, r.Ingress)
 }
 
 // PatchService patches the given service's Status or ObjectMeta based on the original and
@@ -160,28 +161,3 @@ func getPatchBytes(oldSvc, newSvc *v1.Service) ([]byte, error) {
 	return patchBytes, nil
 
 }
-
-func ingressSliceEqual(lhs, rhs []v1.LoadBalancerIngress) bool {
-	if len(lhs) != len(rhs) {
-		return false
-	}
-	for i := range lhs {
-		if !ingressEqual(&lhs[i], &rhs[i]) {
-			return false
-		}
-	}
-	return true
-}
-
-func ingressEqual(lhs, rhs *v1.LoadBalancerIngress) bool {
-	if lhs.IP != rhs.IP {
-		return false
-	}
-	if lhs.Hostname != rhs.Hostname {
-		return false
-	}
-	if lhs.IPMode != rhs.IPMode {
-		return false
-	}
-	return true
-}

staging/src/k8s.io/cloud-provider/service/helpers/helper_test.go

@@ -26,6 +26,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes/fake"
 	utilnet "k8s.io/utils/net"
+	"k8s.io/utils/ptr"
 )
 
 /*
@@ -347,3 +348,152 @@ func Test_getPatchBytes(t *testing.T) {
 func addAnnotations(svc *v1.Service) {
 	svc.Annotations["foo"] = "bar"
 }
+
+func TestLoadBalancerStatusEqual(t *testing.T) {
+	tests := []struct {
+		name string
+		l    *v1.LoadBalancerStatus
+		r    *v1.LoadBalancerStatus
+		want bool
+	}{
+		{
+			name: "empty",
+			l:    &v1.LoadBalancerStatus{},
+			r:    &v1.LoadBalancerStatus{},
+			want: true,
+		},
+		{
+			name: "same ip",
+			l: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1"},
+				},
+			},
+			r: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1"},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "different ip",
+			l: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.2"},
+				},
+			},
+			r: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1"},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "same ipmode",
+			l: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1", IPMode: ptr.To(v1.LoadBalancerIPModeVIP)},
+				},
+			},
+			r: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1", IPMode: ptr.To(v1.LoadBalancerIPModeVIP)},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "only one ipmode set",
+			l: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1"},
+				},
+			},
+			r: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1", IPMode: ptr.To(v1.LoadBalancerIPModeVIP)},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "different ipmode",
+			l: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1", IPMode: ptr.To(v1.LoadBalancerIPModeProxy)},
+				},
+			},
+			r: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1", IPMode: ptr.To(v1.LoadBalancerIPModeVIP)},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "same ports",
+			l: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1", Ports: []v1.PortStatus{{Port: 80, Protocol: v1.ProtocolTCP}}},
+				},
+			},
+			r: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1", Ports: []v1.PortStatus{{Port: 80, Protocol: v1.ProtocolTCP}}},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "same ports different protocol",
+			l: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1", Ports: []v1.PortStatus{{Port: 80, Protocol: v1.ProtocolTCP}}},
+				},
+			},
+			r: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1", Ports: []v1.PortStatus{{Port: 80, Protocol: v1.ProtocolUDP}}},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "only one port",
+			l: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1", Ports: []v1.PortStatus{{Port: 80, Protocol: v1.ProtocolTCP}}},
+				},
+			},
+			r: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1", Ports: []v1.PortStatus{}},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "only one port",
+			l: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1", Ports: []v1.PortStatus{{Port: 80, Protocol: v1.ProtocolTCP}}},
+				},
+			},
+			r: &v1.LoadBalancerStatus{
+				Ingress: []v1.LoadBalancerIngress{
+					{IP: "192.168.0.1", Ports: []v1.PortStatus{}},
+				},
+			},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := LoadBalancerStatusEqual(tt.l, tt.r); got != tt.want {
+				t.Errorf("LoadBalancerStatusEqual() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}