From d092513685996d21dfd538ab1869d95d35e2fad6 Mon Sep 17 00:00:00 2001 From: Jefftree Date: Mon, 29 Jul 2024 19:36:53 +0000 Subject: [PATCH] Use fake clock for controller/leaderelection:TestController --- .../leaderelection/leaderelection_controller.go | 5 +++-- .../leaderelection_controller_test.go | 15 ++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/pkg/controlplane/controller/leaderelection/leaderelection_controller.go b/pkg/controlplane/controller/leaderelection/leaderelection_controller.go index dde1d8287e6..cb53df92e81 100644 --- a/pkg/controlplane/controller/leaderelection/leaderelection_controller.go +++ b/pkg/controlplane/controller/leaderelection/leaderelection_controller.go @@ -278,7 +278,8 @@ func (c *Controller) reconcileElectionStep(ctx context.Context, leaseNN types.Na if candidate.Spec.PingTime == nil || // If PingTime is outdated, send another PingTime only if it already acked the first one. - (candidate.Spec.PingTime.Add(electionDuration).Before(now) && candidate.Spec.PingTime.Before(candidate.Spec.RenewTime)) { + // This checks for pingTime <= renewTime because equality is possible in unit tests using a fake clock. + (candidate.Spec.PingTime.Add(electionDuration).Before(now) && !candidate.Spec.RenewTime.Before(candidate.Spec.PingTime)) { // TODO(jefftree): We should randomize the order of sending pings and do them in parallel // so that all candidates have equal opportunity to ack. clone := candidate.DeepCopy() @@ -300,7 +301,7 @@ func (c *Controller) reconcileElectionStep(ctx context.Context, leaseNN types.Na continue // shouldn't be the case after the above } - if candidate.Spec.RenewTime != nil && candidate.Spec.PingTime.Before(candidate.Spec.RenewTime) { + if candidate.Spec.RenewTime != nil && !candidate.Spec.RenewTime.Before(candidate.Spec.PingTime) { continue // this has renewed already } diff --git a/pkg/controlplane/controller/leaderelection/leaderelection_controller_test.go b/pkg/controlplane/controller/leaderelection/leaderelection_controller_test.go index 093d40faea2..24b4161e6fe 100644 --- a/pkg/controlplane/controller/leaderelection/leaderelection_controller_test.go +++ b/pkg/controlplane/controller/leaderelection/leaderelection_controller_test.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/cache" testingclock "k8s.io/utils/clock/testing" "k8s.io/utils/ptr" ) @@ -697,6 +698,7 @@ func TestController(t *testing.T) { if err != nil { t.Fatal(err) } + controller.clock = fakeClock for _, obj := range tc.leases { t.Logf("Pre-creating lease %s/%s", obj.Namespace, obj.Name) @@ -715,6 +717,9 @@ func TestController(t *testing.T) { informerFactory.Start(ctx.Done()) informerFactory.WaitForCacheSync(ctx.Done()) + if !cache.WaitForNamedCacheSync(controllerName, ctx.Done(), controller.leaseRegistration.HasSynced, controller.leaseCandidateRegistration.HasSynced) { + return + } go controller.Run(ctx, 1) if rand.Intn(2) == 0 { @@ -747,9 +752,9 @@ func TestController(t *testing.T) { continue // only candidate-aware controllers will follow preferredHolder } - t.Logf("Deleting lease %s/%s because of preferredHolder %q != %q", l.Namespace, l.Name, *ph, *l.Spec.HolderIdentity) + fmt.Printf("Deleting lease %s/%s because of preferredHolder %q != %q", l.Namespace, l.Name, *ph, *l.Spec.HolderIdentity) if err = client.CoordinationV1().Leases(expectedLease.Namespace).Delete(ctx, expectedLease.Name, metav1.DeleteOptions{}); err != nil { - t.Logf("Error deleting lease %s/%s: %v", l.Namespace, l.Name, err) + fmt.Printf("Error deleting lease %s/%s: %v", l.Namespace, l.Name, err) } } } @@ -768,12 +773,12 @@ func TestController(t *testing.T) { for _, lc := range tc.createAfterControllerStart { c, err := client.CoordinationV1alpha1().LeaseCandidates(lc.Namespace).Get(ctx, lc.Name, metav1.GetOptions{}) if err == nil { - if c.Spec.PingTime != nil { - t.Logf("Answering ping for %s/%s", c.Namespace, c.Name) + if c.Spec.PingTime != nil && c.Spec.PingTime.Before(c.Spec.RenewTime) { + fmt.Printf("Answering ping for %s/%s", c.Namespace, c.Name) c.Spec.RenewTime = &metav1.MicroTime{Time: fakeClock.Now()} _, err = client.CoordinationV1alpha1().LeaseCandidates(lc.Namespace).Update(ctx, c, metav1.UpdateOptions{}) if err != nil { - t.Logf("Error updating lease candidate %s/%s: %v", c.Namespace, c.Name, err) + fmt.Printf("Error updating lease candidate %s/%s: %v", c.Namespace, c.Name, err) } } }