Skip to content

Commit 6938c29

Browse files
authored
Merge pull request kubernetes#125225 from aojea/ipmode
fix loadbalancer status comparison
2 parents e6e39db + 59adf3f commit 6938c29

File tree

4 files changed

+152
-138
lines changed

4 files changed

+152
-138
lines changed

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

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -140,35 +140,6 @@ func IsServiceIPSet(service *v1.Service) bool {
140140
return service.Spec.ClusterIP != v1.ClusterIPNone && service.Spec.ClusterIP != ""
141141
}
142142

143-
// LoadBalancerStatusEqual evaluates the given load balancers' ingress IP addresses
144-
// and hostnames and returns true if equal or false if otherwise
145-
// TODO: make method on LoadBalancerStatus?
146-
func LoadBalancerStatusEqual(l, r *v1.LoadBalancerStatus) bool {
147-
return ingressSliceEqual(l.Ingress, r.Ingress)
148-
}
149-
150-
func ingressSliceEqual(lhs, rhs []v1.LoadBalancerIngress) bool {
151-
if len(lhs) != len(rhs) {
152-
return false
153-
}
154-
for i := range lhs {
155-
if !ingressEqual(&lhs[i], &rhs[i]) {
156-
return false
157-
}
158-
}
159-
return true
160-
}
161-
162-
func ingressEqual(lhs, rhs *v1.LoadBalancerIngress) bool {
163-
if lhs.IP != rhs.IP {
164-
return false
165-
}
166-
if lhs.Hostname != rhs.Hostname {
167-
return false
168-
}
169-
return true
170-
}
171-
172143
// GetAccessModesAsString returns a string representation of an array of access modes.
173144
// modes, when present, are always in the same order: RWO,ROX,RWX,RWOP.
174145
func GetAccessModesAsString(modes []v1.PersistentVolumeAccessMode) string {

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

Lines changed: 0 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -723,86 +723,3 @@ func TestHugePageUnitSizeFromByteSize(t *testing.T) {
723723
}
724724
}
725725
}
726-
727-
func TestLoadBalancerStatusEqual(t *testing.T) {
728-
729-
testCases := []struct {
730-
left *v1.LoadBalancerStatus
731-
right *v1.LoadBalancerStatus
732-
name string
733-
expectVal bool
734-
}{{
735-
name: "left equals right",
736-
left: &v1.LoadBalancerStatus{
737-
Ingress: []v1.LoadBalancerIngress{{
738-
IP: "1.1.1.1",
739-
Hostname: "host1",
740-
}},
741-
},
742-
right: &v1.LoadBalancerStatus{
743-
Ingress: []v1.LoadBalancerIngress{{
744-
IP: "1.1.1.1",
745-
Hostname: "host1",
746-
}},
747-
},
748-
expectVal: true,
749-
}, {
750-
name: "length of LoadBalancerIngress slice is not equal",
751-
left: &v1.LoadBalancerStatus{
752-
Ingress: []v1.LoadBalancerIngress{{
753-
IP: "1.1.1.1",
754-
Hostname: "host1",
755-
}, {
756-
IP: "1.1.1.2",
757-
Hostname: "host1",
758-
}},
759-
},
760-
right: &v1.LoadBalancerStatus{
761-
Ingress: []v1.LoadBalancerIngress{{
762-
IP: "1.1.1.1",
763-
Hostname: "host1",
764-
}},
765-
},
766-
expectVal: false,
767-
}, {
768-
name: "LoadBalancerIngress ip is not equal",
769-
left: &v1.LoadBalancerStatus{
770-
Ingress: []v1.LoadBalancerIngress{{
771-
IP: "1.1.1.2",
772-
Hostname: "host1",
773-
}},
774-
},
775-
right: &v1.LoadBalancerStatus{
776-
Ingress: []v1.LoadBalancerIngress{{
777-
IP: "1.1.1.1",
778-
Hostname: "host1",
779-
}},
780-
},
781-
expectVal: false,
782-
}, {
783-
name: "LoadBalancerIngress hostname is not equal",
784-
left: &v1.LoadBalancerStatus{
785-
Ingress: []v1.LoadBalancerIngress{{
786-
IP: "1.1.1.1",
787-
Hostname: "host2",
788-
}},
789-
},
790-
right: &v1.LoadBalancerStatus{
791-
Ingress: []v1.LoadBalancerIngress{{
792-
IP: "1.1.1.1",
793-
Hostname: "host1",
794-
}},
795-
},
796-
expectVal: false,
797-
}}
798-
799-
for _, tc := range testCases {
800-
t.Run(tc.name, func(t *testing.T) {
801-
v := LoadBalancerStatusEqual(tc.left, tc.right)
802-
if v != tc.expectVal {
803-
t.Errorf("test %s failed. left input=%v, right input=%v, Got %v but expected %v",
804-
tc.name, tc.left, tc.right, v, tc.expectVal)
805-
}
806-
})
807-
}
808-
}

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

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"strings"
2424

2525
v1 "k8s.io/api/core/v1"
26+
apiequality "k8s.io/apimachinery/pkg/api/equality"
2627
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2728
"k8s.io/apimachinery/pkg/types"
2829
"k8s.io/apimachinery/pkg/util/strategicpatch"
@@ -124,7 +125,7 @@ func HasLBFinalizer(service *v1.Service) bool {
124125

125126
// LoadBalancerStatusEqual checks if load balancer status are equal
126127
func LoadBalancerStatusEqual(l, r *v1.LoadBalancerStatus) bool {
127-
return ingressSliceEqual(l.Ingress, r.Ingress)
128+
return apiequality.Semantic.DeepEqual(l.Ingress, r.Ingress)
128129
}
129130

130131
// 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) {
160161
return patchBytes, nil
161162

162163
}
163-
164-
func ingressSliceEqual(lhs, rhs []v1.LoadBalancerIngress) bool {
165-
if len(lhs) != len(rhs) {
166-
return false
167-
}
168-
for i := range lhs {
169-
if !ingressEqual(&lhs[i], &rhs[i]) {
170-
return false
171-
}
172-
}
173-
return true
174-
}
175-
176-
func ingressEqual(lhs, rhs *v1.LoadBalancerIngress) bool {
177-
if lhs.IP != rhs.IP {
178-
return false
179-
}
180-
if lhs.Hostname != rhs.Hostname {
181-
return false
182-
}
183-
if lhs.IPMode != rhs.IPMode {
184-
return false
185-
}
186-
return true
187-
}

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

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2727
"k8s.io/client-go/kubernetes/fake"
2828
utilnet "k8s.io/utils/net"
29+
"k8s.io/utils/ptr"
2930
)
3031

3132
/*
@@ -347,3 +348,152 @@ func Test_getPatchBytes(t *testing.T) {
347348
func addAnnotations(svc *v1.Service) {
348349
svc.Annotations["foo"] = "bar"
349350
}
351+
352+
func TestLoadBalancerStatusEqual(t *testing.T) {
353+
tests := []struct {
354+
name string
355+
l *v1.LoadBalancerStatus
356+
r *v1.LoadBalancerStatus
357+
want bool
358+
}{
359+
{
360+
name: "empty",
361+
l: &v1.LoadBalancerStatus{},
362+
r: &v1.LoadBalancerStatus{},
363+
want: true,
364+
},
365+
{
366+
name: "same ip",
367+
l: &v1.LoadBalancerStatus{
368+
Ingress: []v1.LoadBalancerIngress{
369+
{IP: "192.168.0.1"},
370+
},
371+
},
372+
r: &v1.LoadBalancerStatus{
373+
Ingress: []v1.LoadBalancerIngress{
374+
{IP: "192.168.0.1"},
375+
},
376+
},
377+
want: true,
378+
},
379+
{
380+
name: "different ip",
381+
l: &v1.LoadBalancerStatus{
382+
Ingress: []v1.LoadBalancerIngress{
383+
{IP: "192.168.0.2"},
384+
},
385+
},
386+
r: &v1.LoadBalancerStatus{
387+
Ingress: []v1.LoadBalancerIngress{
388+
{IP: "192.168.0.1"},
389+
},
390+
},
391+
want: false,
392+
},
393+
{
394+
name: "same ipmode",
395+
l: &v1.LoadBalancerStatus{
396+
Ingress: []v1.LoadBalancerIngress{
397+
{IP: "192.168.0.1", IPMode: ptr.To(v1.LoadBalancerIPModeVIP)},
398+
},
399+
},
400+
r: &v1.LoadBalancerStatus{
401+
Ingress: []v1.LoadBalancerIngress{
402+
{IP: "192.168.0.1", IPMode: ptr.To(v1.LoadBalancerIPModeVIP)},
403+
},
404+
},
405+
want: true,
406+
},
407+
{
408+
name: "only one ipmode set",
409+
l: &v1.LoadBalancerStatus{
410+
Ingress: []v1.LoadBalancerIngress{
411+
{IP: "192.168.0.1"},
412+
},
413+
},
414+
r: &v1.LoadBalancerStatus{
415+
Ingress: []v1.LoadBalancerIngress{
416+
{IP: "192.168.0.1", IPMode: ptr.To(v1.LoadBalancerIPModeVIP)},
417+
},
418+
},
419+
want: false,
420+
},
421+
{
422+
name: "different ipmode",
423+
l: &v1.LoadBalancerStatus{
424+
Ingress: []v1.LoadBalancerIngress{
425+
{IP: "192.168.0.1", IPMode: ptr.To(v1.LoadBalancerIPModeProxy)},
426+
},
427+
},
428+
r: &v1.LoadBalancerStatus{
429+
Ingress: []v1.LoadBalancerIngress{
430+
{IP: "192.168.0.1", IPMode: ptr.To(v1.LoadBalancerIPModeVIP)},
431+
},
432+
},
433+
want: false,
434+
},
435+
{
436+
name: "same ports",
437+
l: &v1.LoadBalancerStatus{
438+
Ingress: []v1.LoadBalancerIngress{
439+
{IP: "192.168.0.1", Ports: []v1.PortStatus{{Port: 80, Protocol: v1.ProtocolTCP}}},
440+
},
441+
},
442+
r: &v1.LoadBalancerStatus{
443+
Ingress: []v1.LoadBalancerIngress{
444+
{IP: "192.168.0.1", Ports: []v1.PortStatus{{Port: 80, Protocol: v1.ProtocolTCP}}},
445+
},
446+
},
447+
want: true,
448+
},
449+
{
450+
name: "same ports different protocol",
451+
l: &v1.LoadBalancerStatus{
452+
Ingress: []v1.LoadBalancerIngress{
453+
{IP: "192.168.0.1", Ports: []v1.PortStatus{{Port: 80, Protocol: v1.ProtocolTCP}}},
454+
},
455+
},
456+
r: &v1.LoadBalancerStatus{
457+
Ingress: []v1.LoadBalancerIngress{
458+
{IP: "192.168.0.1", Ports: []v1.PortStatus{{Port: 80, Protocol: v1.ProtocolUDP}}},
459+
},
460+
},
461+
want: false,
462+
},
463+
{
464+
name: "only one port",
465+
l: &v1.LoadBalancerStatus{
466+
Ingress: []v1.LoadBalancerIngress{
467+
{IP: "192.168.0.1", Ports: []v1.PortStatus{{Port: 80, Protocol: v1.ProtocolTCP}}},
468+
},
469+
},
470+
r: &v1.LoadBalancerStatus{
471+
Ingress: []v1.LoadBalancerIngress{
472+
{IP: "192.168.0.1", Ports: []v1.PortStatus{}},
473+
},
474+
},
475+
want: false,
476+
},
477+
{
478+
name: "only one port",
479+
l: &v1.LoadBalancerStatus{
480+
Ingress: []v1.LoadBalancerIngress{
481+
{IP: "192.168.0.1", Ports: []v1.PortStatus{{Port: 80, Protocol: v1.ProtocolTCP}}},
482+
},
483+
},
484+
r: &v1.LoadBalancerStatus{
485+
Ingress: []v1.LoadBalancerIngress{
486+
{IP: "192.168.0.1", Ports: []v1.PortStatus{}},
487+
},
488+
},
489+
want: false,
490+
},
491+
}
492+
for _, tt := range tests {
493+
t.Run(tt.name, func(t *testing.T) {
494+
if got := LoadBalancerStatusEqual(tt.l, tt.r); got != tt.want {
495+
t.Errorf("LoadBalancerStatusEqual() = %v, want %v", got, tt.want)
496+
}
497+
})
498+
}
499+
}

0 commit comments

Comments
 (0)