Set LeaderElectionResourceLock to leases on (new) scaffolded main.go

[AlmogBaku]

What do you want to happen?

1. Kubernetes' Leases were introduced in 2019 (3y ago)
2. It's fair to assume that most clusters should support it by now.
3. Currently, The controller-runtime have a default value of a dual election mechanism(i.e.configmapsleases) that was aimed to help existing controllers with the migration (and avoid a dual lock when upgrading the controllers)

Given the above, it's safe to configure LeaderElectionResourceLock: "leases" as the default configuration for newer projects(when scaffolding main.go)

/hold until triage
/assign @AlmogBaku

References

kubernetes/kubernetes#80289

Extra Labels

No response

[camilamacedo86]

HI @AlmogBaku,

Thank you for your contribution and suggestion to move in this direction.
Let's see first what controller-runtime docs says:

⚠️ Change this value only if you know what you are doing.

// LeaderElectionResourceLock determines which resource lock to use for leader election,
// defaults to "configmapsleases". **Change this value only if you know what you are doing.**
// Otherwise, users of your controller might end up with multiple running instances that
// each acquired leadership through different resource locks during upgrades and thus
// act on the same resources concurrently.

Then see: ⚠️ (What are the detailed steps that are required here to achieve this goal? What Operator Authors using kubebuilder will need to do to ensure it? How they can ensure that? )

// If you want to migrate to the "leases" resource lock, you might do so by migrating to the
// respective multilock first ("configmapsleases" or "endpointsleases"), which will acquire a
// leader lock on both resources. After all your users have migrated to the multilock, you can
// go ahead and migrate to "leases". Please also keep in mind, that users might skip versions
// of your controller.

Also, it is not the far away:

// Note: before controller-runtime version v0.7, the resource lock was set to "configmaps".

⚠️ I am an Operator Author, providing critical solutions in production and I would like to move forward to adopt the changes safely in my next release, what are the requirements? How can I ensure it?

// Please keep this in mind, when planning a proper migration path for your controller. 

To do this change in the scaffold would also mean alerting the users about the need to properly migrate their solutions which IHMO would only fit well in a migration guide and we need to alert them about all.

WHY?

Many users will check the changes made in the scaffolds between the minor bumps of the Kubebuilder CLI and since we are not releasing major changes that will safely update their projects with what changed in the scaffolds. This scenario shows that could be problematic when they do not know what they are doing.

Also, I think would be very helpful if we get inputs from controller runtime before we try to change the default scaffolds on Kubebuilder to use any option that is not the default value. Then I'd like to suggest you begin with a thread in the controller-runtime channel and link it here, e.g.:

Thread to gather inputs

Note that the goal is to understand and know all that we need to do or not and if we should move in this direction.

Today the Kubebuilder scaffolds the default option for LeaderElectionResourceLock which uses configmapsleases. Knowing the controller runtime is moving forward to use "leases" what about we change the default scaffolds of Kubebuilder to move in this direction?

However, before we do so. Would be nice to understand with you folks all the pros/cons and get the current issues, docs and etc that we can use there to provide accurate info for users.

It shows for me, that the first step is we have the following info:

• Should we be suggesting for users stop using configmapsleases already?

• Have we planned to make the new option the default behaviour in controller-runtime? If yes, when?

• Did we deprecate the old option configmapsleases? If yes, have we any issue/doc design that speaks about it? If not, why not?

• What are the steps/changes that users need to do to move forward with their current project? Have we a doc that explains all steps? What are the consequences if they do not properly follow the steps? How can ensure the migration? How can they avoid problematic scenarios?

• What are the motivations for this change? What are the advantages to begin to use leases instead? When I should or not use leases? What are the circumstances?

Note that we will need to provide the answers to all these questions for Kubebuilder users in order to move in this direction and also produces good documentation that covers it in KB. So could that fit better under a new scaffold such as go/v4-alpha and not in the stable plugin? Could it be +1 change to address in the alpha instead where authors will expect major changes and plan to act on them?

Did you make that already? Have you all the above answers to supplement this RFE?
If not, let's begin from there?

[camilamacedo86]

c/c @varshaprasad96 @rashmigottipati @ryanbrainard @jmrodri

[AlmogBaku]

This suggestion is only for new scaffolded projects. This way, changing it will not impact older versions that need to migrate (because it's new!)

a.

Many users will check the changes made in the scaffolds between the minor bumps of the Kubebuilder CLI and since we are not releasing major changes that will safely update their projects with what changed in the scaffolds. This scenario shows that could be problematic when they do not know what they are doing.

Do we have users that actually do that? that sounds a bit crazy. The code is evolving and changing and getting out of sync with the original scaffolded code.
I.e. - in my project, I changed the file structure completely (somehow similar to v2). When I'm using Kubebuilder to scaffold new files, I manually change that, but not the whole main.go

b. Since you already wrote the whole "discussion thread" - I'm linking this issue on the slack instead ;-) This will also help us to document this discussion for further reference.

[camilamacedo86]

This suggestion is only for new scaffolded projects. This way, changing it will not impact older versions that need to migrate (because it's new!)

For those who already scaffold the project using the old versions and want to update their project to what is new, changed in a new version we must provide guidance.

Do we have users that actually do that? that sounds a bit crazy.

Yes, and it is not crazy. New releases of stable plugins ought to have new features and bug fixes. The Kubebuilde CLI/API as all plugins provided by it respect semver : https://semver.org/. Consumers does not expect to be affected if they decide to update their projects with what changed between MINOR releases.

Additionally, note that:

• Projects such as SDK (which consumes Kubebuilder and is integrated with) to help the users keep their projects updated between the MINOR releases provide migration guidance. see: https://sdk.operatorframework.io/docs/upgrading-sdk-version/.
• Also, the community already request we do the same for Kubebuilder which was tracked in: Migration guide for minor releases #2191

PS.: Plugins/layouts provided are very mature and stable and are kept for long periods. So, if users do not keep the project updated in the MINOR releases and wait for a major bump to do that it could means usually more than a year to be done.

I.e. - in my project, I changed the file structure completely (somehow similar to v2). When I'm using Kubebuilder to scaffold new files, I manually change that, but not the whole main.go

From v2 to v3 we introduce MAJOR changes: user follows the migration guide: https://book.kubebuilder.io/migration/v2vsv3.html and we have a policy that ensures that any stable plugin does not suffer breaking changes: https://github.com/kubernetes-sigs/kubebuilder/blob/master/docs/book/src/plugins/plugins-versioning.md.

Also, if you use the tool to do the scaffolds and help you out, you probably might want to stick with the layout proposed to make it easier to keep your project upgradable with the latest changes and not do changes that might break its helpers. However, if you decide to change everything completed you are still able to check what changed from one version to another very easily by just diff the samples under the test data directory between the tags release. For example, if you want to check what were the bug fixes, changes, and additions between the release 3.0.0 to 3.1.0 you can compare the sample on both and check it.

[camilamacedo86]

@AlmogBaku we might be able to begin by looking in C+R to find the PR that introduces the changes and the linked issues. It might have a lot of what we need to begin to check already. WDYT? Could you help us here by gathering all info first?

[AlmogBaku]

Sure.
It'll take me a few days to make time because of the Passover holiday though.

[AlmogBaku]

It seems that @joelanford already updated this in master to be

options.LeaderElectionResourceLock = resourcelock.LeasesResourceLock

https://github.com/kubernetes-sigs/controller-runtime/blob/eb39b8eb28cfe920fa2450eb38f814fc9e8003e8/pkg/leaderelection/leader_election.go#L66

In his commit: kubernetes-sigs/controller-runtime@6c4d947

I'm not sure why is it not part of the controller-runtime 0.11.2 though?

[camilamacedo86]

Hi @AlmogBaku,

Following the comments inline.

It seems that @joelanford already updated this in master to be
options.LeaderElectionResourceLock = resourcelock.LeasesResourceLock
https://github.com/kubernetes-sigs/controller-runtime/blob/eb39b8eb28cfe920fa2450eb38f814fc9e8003e8/pkg/leaderelection/leader_election.go#L66
In his commit: kubernetes-sigs/controller-runtime@6c4d947

So then when we bump Kubebuilder go/v3 plugin to use the new controller-runtime version its users will be able to check that the controller-runtime latest version address breaking changes and that they need be careful as well.

For Kubebuilder we will also need to make it very clear in the release notes when we bump this controller-runtime version so that, we can help its users be aware of and properly deal with the scenario. The PR kubernetes-sigs/controller-runtime#1773 contains helpful information and I think we will need to highlight that.

I'm not sure why is it not part of the controller-runtime 0.11.2 though?

Because 0.11.2 is a patch release. So that, follow semver we have only bugs addressed in z stream releases. We should address a breaking change in a z release.

Additionally, as shared in another RFE raised by you:

Kubebuilder provides a scaffold based on the default controller runtime configuration. We have no intention of adding optional features and options that should be used to address specific needs to the default scaffold. That goes against its goals

Ditto so, I do NOT agree that we should change the default scaffold to pass this option as requested here.
My vote is -1 for this one.

[AlmogBaku]

I agree. Since this is going to be pushed to ctrl-runtime, we can close it