Skip to content

Commit 1a46fb9

Browse files
vepatelvepatel
authored
fix status updates for VS endpoints (#8074)
1 parent 4f85c32 commit 1a46fb9

File tree

2 files changed

+189
-15
lines changed

2 files changed

+189
-15
lines changed

‎internal/k8s/status.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ func (su *statusUpdater) retryUpdateVirtualServerRouteStatus(vsrCopy *conf_v1.Vi
405405
return nil
406406
}
407407

408-
func hasVsStatusChanged(vs *conf_v1.VirtualServer, state string, reason string, message string) bool {
408+
func (su *statusUpdater) hasVsStatusChanged(vs *conf_v1.VirtualServer, state string, reason string, message string) bool {
409409
if vs.Status.State != state {
410410
return true
411411
}
@@ -418,6 +418,10 @@ func hasVsStatusChanged(vs *conf_v1.VirtualServer, state string, reason string,
418418
return true
419419
}
420420

421+
if !reflect.DeepEqual(vs.Status.ExternalEndpoints, su.externalEndpoints) {
422+
return true
423+
}
424+
421425
return false
422426
}
423427

@@ -486,7 +490,7 @@ func (su *statusUpdater) UpdateVirtualServerStatus(vs *conf_v1.VirtualServer, st
486490

487491
vsCopy := vsLatest.(*conf_v1.VirtualServer).DeepCopy()
488492

489-
if !hasVsStatusChanged(vsCopy, state, reason, message) {
493+
if !su.hasVsStatusChanged(vsCopy, state, reason, message) {
490494
return nil
491495
}
492496

@@ -503,7 +507,7 @@ func (su *statusUpdater) UpdateVirtualServerStatus(vs *conf_v1.VirtualServer, st
503507
return err
504508
}
505509

506-
func hasVsrStatusChanged(vsr *conf_v1.VirtualServerRoute, state string, reason string, message string, referencedByString string) bool {
510+
func (su *statusUpdater) hasVsrStatusChanged(vsr *conf_v1.VirtualServerRoute, state string, reason string, message string, referencedByString string) bool {
507511
if vsr.Status.State != state {
508512
return true
509513
}
@@ -520,6 +524,10 @@ func hasVsrStatusChanged(vsr *conf_v1.VirtualServerRoute, state string, reason s
520524
return true
521525
}
522526

527+
if !reflect.DeepEqual(vsr.Status.ExternalEndpoints, su.externalEndpoints) {
528+
return true
529+
}
530+
523531
return false
524532
}
525533

@@ -548,7 +556,7 @@ func (su *statusUpdater) UpdateVirtualServerRouteStatusWithReferencedBy(vsr *con
548556

549557
vsrCopy := vsrLatest.(*conf_v1.VirtualServerRoute).DeepCopy()
550558

551-
if !hasVsrStatusChanged(vsrCopy, state, reason, message, referencedByString) {
559+
if !su.hasVsrStatusChanged(vsrCopy, state, reason, message, referencedByString) {
552560
return nil
553561
}
554562

@@ -587,7 +595,7 @@ func (su *statusUpdater) UpdateVirtualServerRouteStatus(vsr *conf_v1.VirtualServ
587595

588596
vsrCopy := vsrLatest.(*conf_v1.VirtualServerRoute).DeepCopy()
589597

590-
if !hasVsrStatusChanged(vsrCopy, state, reason, message, "") {
598+
if !su.hasVsrStatusChanged(vsrCopy, state, reason, message, "") {
591599
return nil
592600
}
593601

‎internal/k8s/status_test.go

Lines changed: 176 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -490,59 +490,146 @@ func TestHasVsStatusChanged(t *testing.T) {
490490
reason := "AddedOrUpdated"
491491
msg := "Configuration was added or updated"
492492

493+
// Create a statusUpdater with some external endpoints
494+
su := &statusUpdater{
495+
externalEndpoints: []conf_v1.ExternalEndpoint{
496+
{
497+
IP: "1.2.3.4",
498+
Ports: "80,443",
499+
},
500+
},
501+
}
502+
493503
tests := []struct {
494504
expected bool
495505
vs conf_v1.VirtualServer
506+
desc string
496507
}{
497508
{
498509
expected: false,
510+
desc: "no change in status or external endpoints",
499511
vs: conf_v1.VirtualServer{
500512
Status: conf_v1.VirtualServerStatus{
501513
State: state,
502514
Reason: reason,
503515
Message: msg,
516+
ExternalEndpoints: []conf_v1.ExternalEndpoint{
517+
{
518+
IP: "1.2.3.4",
519+
Ports: "80,443",
520+
},
521+
},
504522
},
505523
},
506524
},
507525
{
508526
expected: true,
527+
desc: "different state",
509528
vs: conf_v1.VirtualServer{
510529
Status: conf_v1.VirtualServerStatus{
511-
State: "DifferentState",
530+
State: "Invalid",
512531
Reason: reason,
513532
Message: msg,
533+
ExternalEndpoints: []conf_v1.ExternalEndpoint{
534+
{
535+
IP: "1.2.3.4",
536+
Ports: "80,443",
537+
},
538+
},
514539
},
515540
},
516541
},
517542
{
518543
expected: true,
544+
desc: "different reason",
519545
vs: conf_v1.VirtualServer{
520546
Status: conf_v1.VirtualServerStatus{
521547
State: state,
522548
Reason: "DifferentReason",
523549
Message: msg,
550+
ExternalEndpoints: []conf_v1.ExternalEndpoint{
551+
{
552+
IP: "1.2.3.4",
553+
Ports: "80,443",
554+
},
555+
},
524556
},
525557
},
526558
},
527559
{
528560
expected: true,
561+
desc: "different message",
529562
vs: conf_v1.VirtualServer{
530563
Status: conf_v1.VirtualServerStatus{
531564
State: state,
532565
Reason: reason,
533566
Message: "DifferentMessage",
567+
ExternalEndpoints: []conf_v1.ExternalEndpoint{
568+
{
569+
IP: "1.2.3.4",
570+
Ports: "80,443",
571+
},
572+
},
573+
},
574+
},
575+
},
576+
{
577+
expected: true,
578+
desc: "different external endpoints - different IP",
579+
vs: conf_v1.VirtualServer{
580+
Status: conf_v1.VirtualServerStatus{
581+
State: state,
582+
Reason: reason,
583+
Message: msg,
584+
ExternalEndpoints: []conf_v1.ExternalEndpoint{
585+
{
586+
IP: "5.6.7.8",
587+
Ports: "80,443",
588+
},
589+
},
590+
},
591+
},
592+
},
593+
{
594+
expected: true,
595+
desc: "different external endpoints - different ports",
596+
vs: conf_v1.VirtualServer{
597+
Status: conf_v1.VirtualServerStatus{
598+
State: state,
599+
Reason: reason,
600+
Message: msg,
601+
ExternalEndpoints: []conf_v1.ExternalEndpoint{
602+
{
603+
IP: "1.2.3.4",
604+
Ports: "8080,8443",
605+
},
606+
},
607+
},
608+
},
609+
},
610+
{
611+
expected: true,
612+
desc: "missing external endpoints in VS",
613+
vs: conf_v1.VirtualServer{
614+
Status: conf_v1.VirtualServerStatus{
615+
State: state,
616+
Reason: reason,
617+
Message: msg,
618+
// No ExternalEndpoints
534619
},
535620
},
536621
},
537622
}
538623

539624
for _, test := range tests {
540625
test := test // address gosec G601
541-
changed := hasVsStatusChanged(&test.vs, state, reason, msg)
626+
t.Run(test.desc, func(t *testing.T) {
627+
changed := su.hasVsStatusChanged(&test.vs, state, reason, msg)
542628

543-
if changed != test.expected {
544-
t.Errorf("hasVsStatusChanged(%v, %v, %v, %v) returned %v but expected %v.", test.vs, state, reason, msg, changed, test.expected)
545-
}
629+
if changed != test.expected {
630+
t.Errorf("hasVsStatusChanged(%v, %v, %v, %v) returned %v but expected %v for test: %s", test.vs, state, reason, msg, changed, test.expected, test.desc)
631+
}
632+
})
546633
}
547634
}
548635

@@ -553,74 +640,153 @@ func TestHasVsrStatusChanged(t *testing.T) {
553640
reason := "AddedOrUpdated"
554641
msg := "Configuration was added or updated"
555642

643+
// Create a statusUpdater with some external endpoints
644+
su := &statusUpdater{
645+
externalEndpoints: []conf_v1.ExternalEndpoint{
646+
{
647+
IP: "1.2.3.4",
648+
Ports: "80,443",
649+
},
650+
},
651+
}
652+
556653
tests := []struct {
557654
expected bool
558655
vsr conf_v1.VirtualServerRoute
656+
desc string
559657
}{
560658
{
561659
expected: false,
660+
desc: "no change in status or external endpoints",
562661
vsr: conf_v1.VirtualServerRoute{
563662
Status: conf_v1.VirtualServerRouteStatus{
564663
State: state,
565664
Reason: reason,
566665
Message: msg,
567666
ReferencedBy: referencedBy,
667+
ExternalEndpoints: []conf_v1.ExternalEndpoint{
668+
{
669+
IP: "1.2.3.4",
670+
Ports: "80,443",
671+
},
672+
},
568673
},
569674
},
570675
},
571676
{
572677
expected: true,
678+
desc: "different state",
573679
vsr: conf_v1.VirtualServerRoute{
574680
Status: conf_v1.VirtualServerRouteStatus{
575-
State: "DifferentState",
681+
State: "Invalid",
576682
Reason: reason,
577683
Message: msg,
578684
ReferencedBy: referencedBy,
685+
ExternalEndpoints: []conf_v1.ExternalEndpoint{
686+
{
687+
IP: "1.2.3.4",
688+
Ports: "80,443",
689+
},
690+
},
579691
},
580692
},
581693
},
582694
{
583695
expected: true,
696+
desc: "different reason",
584697
vsr: conf_v1.VirtualServerRoute{
585698
Status: conf_v1.VirtualServerRouteStatus{
586699
State: state,
587700
Reason: "DifferentReason",
588701
Message: msg,
589702
ReferencedBy: referencedBy,
703+
ExternalEndpoints: []conf_v1.ExternalEndpoint{
704+
{
705+
IP: "1.2.3.4",
706+
Ports: "80,443",
707+
},
708+
},
590709
},
591710
},
592711
},
593712
{
594713
expected: true,
714+
desc: "different message",
595715
vsr: conf_v1.VirtualServerRoute{
596716
Status: conf_v1.VirtualServerRouteStatus{
597717
State: state,
598718
Reason: reason,
599719
Message: "DifferentMessage",
600720
ReferencedBy: referencedBy,
721+
ExternalEndpoints: []conf_v1.ExternalEndpoint{
722+
{
723+
IP: "1.2.3.4",
724+
Ports: "80,443",
725+
},
726+
},
601727
},
602728
},
603729
},
604730
{
605731
expected: true,
732+
desc: "different referenced by",
606733
vsr: conf_v1.VirtualServerRoute{
607734
Status: conf_v1.VirtualServerRouteStatus{
608735
State: state,
609736
Reason: reason,
610737
Message: msg,
611738
ReferencedBy: "DifferentReferencedBy",
739+
ExternalEndpoints: []conf_v1.ExternalEndpoint{
740+
{
741+
IP: "1.2.3.4",
742+
Ports: "80,443",
743+
},
744+
},
745+
},
746+
},
747+
},
748+
{
749+
expected: true,
750+
desc: "different external endpoints - different IP",
751+
vsr: conf_v1.VirtualServerRoute{
752+
Status: conf_v1.VirtualServerRouteStatus{
753+
State: state,
754+
Reason: reason,
755+
Message: msg,
756+
ReferencedBy: referencedBy,
757+
ExternalEndpoints: []conf_v1.ExternalEndpoint{
758+
{
759+
IP: "5.6.7.8",
760+
Ports: "80,443",
761+
},
762+
},
763+
},
764+
},
765+
},
766+
{
767+
expected: true,
768+
desc: "missing external endpoints in VSR",
769+
vsr: conf_v1.VirtualServerRoute{
770+
Status: conf_v1.VirtualServerRouteStatus{
771+
State: state,
772+
Reason: reason,
773+
Message: msg,
774+
ReferencedBy: referencedBy,
775+
// No ExternalEndpoints
612776
},
613777
},
614778
},
615779
}
616780

617781
for _, test := range tests {
618782
test := test // address gosec G601
619-
changed := hasVsrStatusChanged(&test.vsr, state, reason, msg, referencedBy)
783+
t.Run(test.desc, func(t *testing.T) {
784+
changed := su.hasVsrStatusChanged(&test.vsr, state, reason, msg, referencedBy)
620785

621-
if changed != test.expected {
622-
t.Errorf("hasVsrStatusChanged(%v, %v, %v, %v) returned %v but expected %v.", test.vsr, state, reason, msg, changed, test.expected)
623-
}
786+
if changed != test.expected {
787+
t.Errorf("hasVsrStatusChanged(%v, %v, %v, %v) returned %v but expected %v for test: %s", test.vsr, state, reason, msg, changed, test.expected, test.desc)
788+
}
789+
})
624790
}
625791
}
626792

0 commit comments

Comments
 (0)