diff --git a/controller/controller_test.go b/controller/controller_test.go index 2e9fb45b3..6ce2e3dd0 100644 --- a/controller/controller_test.go +++ b/controller/controller_test.go @@ -941,15 +941,15 @@ func (cr *countingLeaderAwareReconciler) Reconcile(ctx context.Context, key stri } func TestStartAndShutdownWithLeaderAwareNoElection(t *testing.T) { - promoted := make(chan struct{}) r := &countingLeaderAwareReconciler{ LeaderAwareFuncs: reconciler.LeaderAwareFuncs{ PromoteFunc: func(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error { - close(promoted) + t.Error("Promote should not be called when no leader election is enabled.") return nil }, }, } + impl := NewContext(context.TODO(), r, ControllerOptions{ Logger: TestLogger(t), WorkQueueName: "Testing", @@ -968,13 +968,10 @@ func TestStartAndShutdownWithLeaderAwareNoElection(t *testing.T) { }) select { - case <-promoted: - // We expect to be promoted immediately, since there is no - // ElectorBuilder attached to the context. case <-doneCh: t.Fatal("StartAll finished early.") case <-time.After(10 * time.Second): - t.Error("Timed out waiting for StartAll.") + // Give it some time to run. } cancel() diff --git a/leaderelection/context.go b/leaderelection/context.go index d30ecd462..4758ccba5 100644 --- a/leaderelection/context.go +++ b/leaderelection/context.go @@ -94,7 +94,11 @@ func BuildElector(ctx context.Context, la reconciler.LeaderAware, queueName stri return &unopposedElector{ la: la, bkt: reconciler.UniversalBucket(), - enq: enq, + // The UniversalBucket owns everything, so there is never a need to + // enqueue things (no possible state change). We pass nil here to + // avoid filling the queue for an extra resynce at startup along + // this path. + enq: nil, }, nil } diff --git a/leaderelection/context_test.go b/leaderelection/context_test.go index 271bab215..1732e9557 100644 --- a/leaderelection/context_test.go +++ b/leaderelection/context_test.go @@ -323,3 +323,35 @@ func TestWithStatefulSetBuilder(t *testing.T) { t.Fatal("Timed out waiting for promotion.") } } + +func TestWithUnopposedElector(t *testing.T) { + laf := &reconciler.LeaderAwareFuncs{ + PromoteFunc: func(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error { + t.Error("Unexpected call to PromoteFunc with unopposedElector") + return nil + }, + } + + ctx := context.Background() + if HasLeaderElection(ctx) { + t.Error("HasLeaderElection() = true, wanted false") + } + + le, err := BuildElector(ctx, laf, "name", func(reconciler.Bucket, types.NamespacedName) { + t.Error("Unexpected call to enqueue function.") + }) + if err != nil { + t.Fatal("BuildElector() =", err) + } + + if _, ok := le.(*unopposedElector); !ok { + t.Fatalf("BuildElector() = %T, wanted an unopposedElector", le) + } + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + go le.Run(ctx) + + // Wait to see if PromoteFunc is called with nil or our enq function. + <-time.After(time.Second) +} diff --git a/reconciler/leader.go b/reconciler/leader.go index 6fece8526..ed7254c46 100644 --- a/reconciler/leader.go +++ b/reconciler/leader.go @@ -97,7 +97,7 @@ func (laf *LeaderAwareFuncs) Promote(b Bucket, enq func(Bucket, types.Namespaced laf.buckets[b.Name()] = b }() - if promote := laf.PromoteFunc; promote != nil { + if promote := laf.PromoteFunc; promote != nil && enq != nil { return promote(b, enq) } return nil diff --git a/reconciler/leader_test.go b/reconciler/leader_test.go index 34d6eefe5..82f32ce1a 100644 --- a/reconciler/leader_test.go +++ b/reconciler/leader_test.go @@ -42,6 +42,11 @@ func TestLeaderAwareFuncs(t *testing.T) { } laf.PromoteFunc = func(bkt Bucket, gotFunc func(Bucket, types.NamespacedName)) error { + if gotFunc == nil { + t.Error("PromoteFunc should not be called with nil gotFunc") + return nil + } + gotFunc(bkt, wantKey) if !called { t.Error("gotFunc didn't call wantFunc!") @@ -79,4 +84,7 @@ func TestLeaderAwareFuncs(t *testing.T) { if laf.IsLeaderFor(wantKey) { t.Error("IsLeaderFor() = true, wanted false") } + + // We should elide calls with a nil enq + laf.Promote(wantBkt, nil) }