[sbueringer]

I think we want a manager to shutdown as quickly as possible if we lose the leader lease.

What already worked is that if the manager is running and we lose the leader lease we ~ skip the graceful shutdown.

What didn't work is that if we were already shutting down gracefully losing the leader lease during that did not cancel the ongoing graceful shutdown.

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [sbueringer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sbueringer]

/hold
for Alvaro's review

/assign @alvaroaleman

[sbueringer]

This is the explanation why closing the channel below is not in a sync.Once. I'm fine either dropping the comment or with dropping the comment and using a sync.Once just to be safe

[sbueringer]

There is no reason (anymore, also before this PR) why this has to be a field on the struct instead of just a local variable, so I changed that to simplify

[alvaroaleman]

Have to look deeper but don't we cancel the leader lease ctx during shutdown and thus will always run into this case?

[sbueringer]

don't we cancel the leader lease ctx during shutdown and thus will always run into this case?

cm.leaderElectionCancel is only called in a defer func in engageStopProcedure
That defer func is only reached after <-shutdownCtx.Done() (l.615)
The select in the go func (l.525++) will return when the shutdownCtx is done instead of logging and calling shutdownCancel

(I'll flip the order in that select just in case it makes it more robust that we hit the shutdownCtx.Done() branch, not sure if it makes a difference)