Leader election seems broken for Kubernetes

[michaelbeaumont]

What happened?

When updating kuma-cp, the new control plane starts the controller-runtime.Manager immediately, leading to reconciliation errors because of concurrent resource updates.

This likely has to do with LeaderElection being disabled in the controller-runtime.Manager. It may be possible to hook our leader-for-life election into the Manager.

[michaelbeaumont]

Leader election is either handled by controller-runtime, without an easy way to "plug in" leader-for-life election, or it's skipped like we have it now.

When we start the Manager with LeaderElection: false, it runs all registered components unconditionally (including those that need leader election).
We have components that rely on the Manager being started but don't need leader election, and so we must call Manager.Start without waiting for leader election (i.e. leader.Become returning).

The solution we have now is to delay calling Manager.Add on certain components until after we call Start and call leader.Become.

Our controllers also end up as components and unfortunately, there seems to be no way with pkg/builder.Builder to setup a controller and not immediately add it as a component to the Manager with Manager.Add.

I think the best fix is to change this upstream in controller-runtime/pkg/builder/controller.go, so that it supports building a Controller by calling pkg/controller.NewUnmanaged instead of pkg/controller.New (where Manager.Add is called). Then we can call Manager.Add on our controllers ourselves after we call Start, along with the rest of the leader election components.

[jpeach]

Leader election was changed in #3023

[michaelbeaumont]

Created kubernetes-sigs/controller-runtime#1766 with proposed change

[jpeach]

I think the best fix is to change this upstream in controller-runtime/pkg/builder/controller.go, so that it supports building a Controller by calling pkg/controller.NewUnmanaged instead of pkg/controller.New (where Manager.Add is called). Then we can call Manager.Add on our controllers ourselves after we call Start, along with the rest of the leader election components.

Don't we need leader election to be working for the next release though?

[michaelbeaumont]

@jpeach yeah I think we can maintain a fork until it's upstreamed, it's not a very significant change.

[michaelbeaumont]

Never mind, there's an easy fix that works for our use case that doesn't require an upstream change!