Start function of manager object not always returns error if it happens.

[Alexseij]

In my case error happened during initialization l object:

controller-runtime/pkg/manager/internal.go

Lines 572 to 603 in e28a842

	l, err := leaderelection.NewLeaderElector(leaderelection.LeaderElectionConfig{
	Lock: cm.resourceLock,
	LeaseDuration: cm.leaseDuration,
	RenewDeadline: cm.renewDeadline,
	RetryPeriod: cm.retryPeriod,
	Callbacks: leaderelection.LeaderCallbacks{
	OnStartedLeading: func(_ context.Context) {
	if err := cm.startLeaderElectionRunnables(); err != nil {
	cm.errChan <- err
	return
	}
	close(cm.elected)
	},
	OnStoppedLeading: func() {
	if cm.onStoppedLeading != nil {
	cm.onStoppedLeading()
	}
	// Make sure graceful shutdown is skipped if we lost the leader lock without
	// intending to.
	cm.gracefulShutdownTimeout = time.Duration(0)
	// Most implementations of leader election log.Fatal() here.
	// Since Start is wrapped in log.Fatal when called, we can just return
	// an error here which will cause the program to exit.
	cm.errChan <- errors.New("leader election lost")
	},
	},
	ReleaseOnCancel: cm.leaderElectionReleaseOnCancel,
	Name: cm.leaderElectionID,
	})
	if err != nil {
	return err
	}

Error contains this message:
leaseDuration must be greater than renewDeadline

After that error, function of manager object Start dont return it.

Because this code call here by function startLeaderElection:

controller-runtime/pkg/manager/internal.go

Lines 432 to 449 in e28a842

	// Start the leader election and all required runnables.
	{
	ctx, cancel := context.WithCancel(context.Background())
	cm.leaderElectionCancel = cancel
	go func() {
	if cm.resourceLock != nil {
	if err := cm.startLeaderElection(ctx); err != nil {
	cm.errChan <- err
	}
	} else {
	// Treat not having leader election enabled the same as being elected.
	if err := cm.startLeaderElectionRunnables(); err != nil {
	cm.errChan <- err
	}
	close(cm.elected)
	}
	}()
	}

After this defers:

controller-runtime/pkg/manager/internal.go

Lines 355 to 371 in e28a842

	stopComplete := make(chan struct{})
	defer close(stopComplete)
	// This must be deferred after closing stopComplete, otherwise we deadlock.
	defer func() {
	// https://hips.hearstapps.com/hmg-prod.s3.amazonaws.com/images/gettyimages-459889618-1533579787.jpg
	stopErr := cm.engageStopProcedure(stopComplete)
	if stopErr != nil {
	if err != nil {
	// Utilerrors.Aggregate allows to use errors.Is for all contained errors
	// whereas fmt.Errorf allows wrapping at most one error which means the
	// other one can not be found anymore.
	err = kerrors.NewAggregate([]error{err, stopErr})
	} else {
	err = stopErr
	}
	}
	}()

And this defers call another defers one of which waiting for value in channel:

controller-runtime/pkg/manager/internal.go

Lines 511 to 521 in e28a842

	defer func() {
	// Cancel leader election only after we waited. It will os.Exit() the app for safety.
	if cm.resourceLock != nil {
	// After asking the context to be cancelled, make sure
	// we wait for the leader stopped channel to be closed, otherwise
	// we might encounter race conditions between this code
	// and the event recorder, which is used within leader election code.
	cm.leaderElectionCancel()
	<-cm.leaderElectionStopped
	}
	}()

But this channel is never being close because close function calls only when there is no error when l object initializing

controller-runtime/pkg/manager/internal.go

Lines 601 to 611 in e28a842

	if err != nil {
	return err
	}
	// Start the leader elector process
	go func() {
	l.Run(ctx)
	<-ctx.Done()
	close(cm.leaderElectionStopped)
	}()
	return nil

I fix this issue in my fork repository, if this issue will be accepted I can do pull request.

[alvaroaleman]

/kind bug
Please feel free to submit a fix