Skip to content

Commit

Permalink
🐛 Manager.Elected() should beclosed after runnables are started
Browse files Browse the repository at this point in the history
During debugging a weird issue with some tests failing if they were
running too fast. In other words, calling Start() and Close() on
a manager too fast throws an error where the informers haven't been able
to sync, which then makes Start() fail with an error.

In an effort to improve this behavior, tried to use Elected() to wait
for leader election to start, which also waits for the cache.

During some debugging, this issue happened again and upon digging a bit
more it seems that the channel was closed before starting the runnables
in some cases.

This change reorders the close on cm.elected, which should fix the above
issue.

Signed-off-by: Vince Prignano <vincepri@vmware.com>
  • Loading branch information
vincepri committed Jan 21, 2021
1 parent 73c52e8 commit c9e1c10
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions pkg/manager/internal.go
Expand Up @@ -473,8 +473,8 @@ func (cm *controllerManager) Start(ctx context.Context) (err error) {
}
} else {
// Treat not having leader election enabled the same as being elected.
cm.startLeaderElectionRunnables()
close(cm.elected)
go cm.startLeaderElectionRunnables()
}
}()

Expand Down Expand Up @@ -632,8 +632,8 @@ func (cm *controllerManager) startLeaderElection() (err error) {
RetryPeriod: cm.retryPeriod,
Callbacks: leaderelection.LeaderCallbacks{
OnStartedLeading: func(_ context.Context) {
close(cm.elected)
cm.startLeaderElectionRunnables()
close(cm.elected)
},
OnStoppedLeading: cm.onStoppedLeading,
},
Expand Down

0 comments on commit c9e1c10

Please sign in to comment.