🐛 Manager.Elected() should beclosed after runnables are started

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 <[email protected]>

Author: vincepri

pkg/manager/internal.go

@@ -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()
 		}
 	}()
 
@@ -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,
 		},