Skip to content

Commit 634c9cd

Browse files
stttssttts
committed
Address comments
Signed-off-by: Dr. Stefan Schimanski <[email protected]>
1 parent d092513 commit 634c9cd

File tree

1 file changed

+36
-35
lines changed

1 file changed

+36
-35
lines changed

‎pkg/controlplane/controller/leaderelection/leaderelection_controller_test.go‎

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222
"math/rand"
23+
"sync"
2324
"testing"
2425
"time"
2526

@@ -31,7 +32,6 @@ import (
3132
"k8s.io/apimachinery/pkg/util/wait"
3233
"k8s.io/client-go/informers"
3334
"k8s.io/client-go/kubernetes/fake"
34-
"k8s.io/client-go/tools/cache"
3535
testingclock "k8s.io/utils/clock/testing"
3636
"k8s.io/utils/ptr"
3737
)
@@ -175,7 +175,7 @@ func TestReconcileElectionStep(t *testing.T) {
175175
LeaseName: "component-A",
176176
EmulationVersion: "1.20.0",
177177
BinaryVersion: "1.20.0",
178-
PingTime: ptr.To(metav1.NewMicroTime(fakeClock.Now().Add(-1 * time.Millisecond))),
178+
PingTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())),
179179
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())),
180180
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
181181
},
@@ -440,8 +440,6 @@ func TestReconcileElectionStep(t *testing.T) {
440440
}
441441

442442
func TestController(t *testing.T) {
443-
fakeClock := testingclock.NewFakeClock(time.Now())
444-
445443
cases := []struct {
446444
name string
447445
leases []*v1.Lease
@@ -462,7 +460,7 @@ func TestController(t *testing.T) {
462460
LeaseName: "component-A",
463461
EmulationVersion: "1.19.0",
464462
BinaryVersion: "1.19.0",
465-
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())),
463+
RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
466464
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
467465
},
468466
},
@@ -491,7 +489,7 @@ func TestController(t *testing.T) {
491489
LeaseName: "component-A",
492490
EmulationVersion: "1.19.0",
493491
BinaryVersion: "1.19.0",
494-
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())),
492+
RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
495493
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
496494
},
497495
},
@@ -504,7 +502,7 @@ func TestController(t *testing.T) {
504502
LeaseName: "component-A",
505503
EmulationVersion: "1.19.0",
506504
BinaryVersion: "1.20.0",
507-
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())),
505+
RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
508506
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
509507
},
510508
},
@@ -517,7 +515,7 @@ func TestController(t *testing.T) {
517515
LeaseName: "component-A",
518516
EmulationVersion: "1.20.0",
519517
BinaryVersion: "1.20.0",
520-
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())),
518+
RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
521519
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
522520
},
523521
},
@@ -544,7 +542,7 @@ func TestController(t *testing.T) {
544542
},
545543
Spec: v1.LeaseSpec{
546544
HolderIdentity: ptr.To("some-other-component"),
547-
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now().Add(time.Second))),
545+
RenewTime: ptr.To(metav1.NewMicroTime(time.Now().Add(time.Second))),
548546
LeaseDurationSeconds: ptr.To(int32(10)),
549547
},
550548
},
@@ -559,7 +557,7 @@ func TestController(t *testing.T) {
559557
LeaseName: "component-A",
560558
EmulationVersion: "1.19.0",
561559
BinaryVersion: "1.19.0",
562-
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())),
560+
RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
563561
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
564562
},
565563
},
@@ -589,7 +587,7 @@ func TestController(t *testing.T) {
589587
},
590588
Spec: v1.LeaseSpec{
591589
HolderIdentity: ptr.To("some-other-component"),
592-
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now().Add(-11 * time.Second))),
590+
RenewTime: ptr.To(metav1.NewMicroTime(time.Now().Add(-11 * time.Second))),
593591
LeaseDurationSeconds: ptr.To(int32(10)),
594592
Strategy: ptr.To[v1.CoordinatedLeaseStrategy]("OldestEmulationVersion"),
595593
},
@@ -605,7 +603,7 @@ func TestController(t *testing.T) {
605603
LeaseName: "component-A",
606604
EmulationVersion: "1.19.0",
607605
BinaryVersion: "1.19.0",
608-
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())),
606+
RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
609607
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
610608
},
611609
},
@@ -633,7 +631,7 @@ func TestController(t *testing.T) {
633631
Spec: v1.LeaseSpec{
634632
HolderIdentity: ptr.To("component-identity-1"),
635633
Strategy: ptr.To[v1.CoordinatedLeaseStrategy]("OldestEmulationVersion"),
636-
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now().Add(time.Second))),
634+
RenewTime: ptr.To(metav1.NewMicroTime(time.Now().Add(time.Second))),
637635
LeaseDurationSeconds: ptr.To(int32(10)),
638636
},
639637
},
@@ -648,7 +646,7 @@ func TestController(t *testing.T) {
648646
LeaseName: "component-A",
649647
EmulationVersion: "1.20.0",
650648
BinaryVersion: "1.20.0",
651-
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())),
649+
RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
652650
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
653651
},
654652
},
@@ -663,7 +661,7 @@ func TestController(t *testing.T) {
663661
LeaseName: "component-A",
664662
EmulationVersion: "1.19.0",
665663
BinaryVersion: "1.19.0",
666-
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())),
664+
RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
667665
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
668666
},
669667
},
@@ -684,6 +682,10 @@ func TestController(t *testing.T) {
684682

685683
for _, tc := range cases {
686684
t.Run(tc.name, func(t *testing.T) {
685+
// collect go routines using t.logf
686+
var wg sync.WaitGroup
687+
defer wg.Wait()
688+
687689
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
688690
defer cancel()
689691

@@ -698,7 +700,6 @@ func TestController(t *testing.T) {
698700
if err != nil {
699701
t.Fatal(err)
700702
}
701-
controller.clock = fakeClock
702703

703704
for _, obj := range tc.leases {
704705
t.Logf("Pre-creating lease %s/%s", obj.Namespace, obj.Name)
@@ -717,17 +718,16 @@ func TestController(t *testing.T) {
717718

718719
informerFactory.Start(ctx.Done())
719720
informerFactory.WaitForCacheSync(ctx.Done())
720-
if !cache.WaitForNamedCacheSync(controllerName, ctx.Done(), controller.leaseRegistration.HasSynced, controller.leaseCandidateRegistration.HasSynced) {
721-
return
722-
}
723721
go controller.Run(ctx, 1)
724722

725723
if rand.Intn(2) == 0 {
726724
t.Logf("Giving controller a chance to run")
727725
time.Sleep(time.Second)
728726
}
729727

728+
wg.Add(1)
730729
go func() {
730+
defer wg.Done()
731731
ticker := time.NewTicker(10 * time.Millisecond)
732732
// Mock out the removal of preferredHolder leases.
733733
// When controllers are running, they are expected to do this voluntarily
@@ -752,16 +752,18 @@ func TestController(t *testing.T) {
752752
continue // only candidate-aware controllers will follow preferredHolder
753753
}
754754

755-
fmt.Printf("Deleting lease %s/%s because of preferredHolder %q != %q", l.Namespace, l.Name, *ph, *l.Spec.HolderIdentity)
755+
t.Logf("Deleting lease %s/%s because of preferredHolder %q != %q", l.Namespace, l.Name, *ph, *l.Spec.HolderIdentity)
756756
if err = client.CoordinationV1().Leases(expectedLease.Namespace).Delete(ctx, expectedLease.Name, metav1.DeleteOptions{}); err != nil {
757-
fmt.Printf("Error deleting lease %s/%s: %v", l.Namespace, l.Name, err)
757+
t.Logf("Error deleting lease %s/%s: %v", l.Namespace, l.Name, err)
758758
}
759759
}
760760
}
761761
}
762762
}()
763763

764+
wg.Add(1)
764765
go func() {
766+
defer wg.Done()
765767
ticker := time.NewTicker(10 * time.Millisecond)
766768
// Mock out leasecandidate ack.
767769
// When controllers are running, they are expected to watch and ack
@@ -770,16 +772,18 @@ func TestController(t *testing.T) {
770772
case <-ctx.Done():
771773
return
772774
case <-ticker.C:
773-
for _, lc := range tc.createAfterControllerStart {
774-
c, err := client.CoordinationV1alpha1().LeaseCandidates(lc.Namespace).Get(ctx, lc.Name, metav1.GetOptions{})
775-
if err == nil {
776-
if c.Spec.PingTime != nil && c.Spec.PingTime.Before(c.Spec.RenewTime) {
777-
fmt.Printf("Answering ping for %s/%s", c.Namespace, c.Name)
778-
c.Spec.RenewTime = &metav1.MicroTime{Time: fakeClock.Now()}
779-
_, err = client.CoordinationV1alpha1().LeaseCandidates(lc.Namespace).Update(ctx, c, metav1.UpdateOptions{})
780-
if err != nil {
781-
fmt.Printf("Error updating lease candidate %s/%s: %v", c.Namespace, c.Name, err)
782-
}
775+
cs, err := client.CoordinationV1alpha1().LeaseCandidates("").List(ctx, metav1.ListOptions{})
776+
if err != nil {
777+
t.Logf("Error listing lease candidates: %v", err)
778+
continue
779+
}
780+
for _, c := range cs.Items {
781+
if c.Spec.PingTime != nil && (c.Spec.RenewTime == nil || c.Spec.PingTime.Time.After(c.Spec.RenewTime.Time)) {
782+
t.Logf("Answering ping for %s/%s", c.Namespace, c.Name)
783+
c.Spec.RenewTime = &metav1.MicroTime{Time: time.Now()}
784+
_, err = client.CoordinationV1alpha1().LeaseCandidates(c.Namespace).Update(ctx, &c, metav1.UpdateOptions{})
785+
if err != nil {
786+
t.Logf("Error updating lease candidate %s/%s: %v", c.Namespace, c.Name, err)
783787
}
784788
}
785789
}
@@ -820,10 +824,7 @@ func TestController(t *testing.T) {
820824
if lease.Spec.HolderIdentity == nil {
821825
return false, nil
822826
}
823-
if *expectedLease.Spec.HolderIdentity != *lease.Spec.HolderIdentity {
824-
return false, nil
825-
}
826-
return true, nil
827+
return *expectedLease.Spec.HolderIdentity == *lease.Spec.HolderIdentity, nil
827828
})
828829
if err != nil {
829830
if lease == nil {

0 commit comments

Comments
 (0)