Skip to content

Commit

Permalink
Drop leader-election-resource-lock flag for extensions cause only p…
Browse files Browse the repository at this point in the history 
…ossible value now is leases

For gardener it will be done in seperate PR
  • Loading branch information
@acumino
acumino committed Sep 13, 2023
1 parent d45f053 commit 7531c19
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 79 deletions.
16 changes: 7 additions & 9 deletions cmd/gardener-extension-provider-local/app/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (
"k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
vpaautoscalingv1 "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
"k8s.io/client-go/tools/leaderelection/resourcelock"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/cluster"
"sigs.k8s.io/controller-runtime/pkg/healthz"
Expand Down Expand Up @@ -88,14 +87,13 @@ func NewControllerManagerCommand(ctx context.Context) *cobra.Command {
var (
restOpts = &extensionscmdcontroller.RESTOptions{}
mgrOpts = &extensionscmdcontroller.ManagerOptions{
LeaderElection: true,
LeaderElectionResourceLock: resourcelock.LeasesResourceLock,
LeaderElectionID: extensionscmdcontroller.LeaderElectionNameID(local.Name),
LeaderElectionNamespace: os.Getenv("LEADER_ELECTION_NAMESPACE"),
WebhookServerPort: 443,
WebhookCertDir: "/tmp/gardener-extensions-cert",
MetricsBindAddress: ":8080",
HealthBindAddress: ":8081",
LeaderElection: true,
LeaderElectionID: extensionscmdcontroller.LeaderElectionNameID(local.Name),
LeaderElectionNamespace: os.Getenv("LEADER_ELECTION_NAMESPACE"),
WebhookServerPort: 443,
WebhookCertDir: "/tmp/gardener-extensions-cert",
MetricsBindAddress: ":8080",
HealthBindAddress: ":8081",
}
generalOpts = &extensionscmdcontroller.GeneralOptions{}

Expand Down
30 changes: 1 addition & 29 deletions extensions/pkg/controller/cmd/options.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ import (
const (
// LeaderElectionFlag is the name of the command line flag to specify whether to do leader election or not.
LeaderElectionFlag = "leader-election"
// LeaderElectionResourceLockFlag is the name of the command line flag to specify the resource type used for leader
// election.
LeaderElectionResourceLockFlag = "leader-election-resource-lock"
// LeaderElectionIDFlag is the name of the command line flag to specify the leader election ID.
LeaderElectionIDFlag = "leader-election-id"
// LeaderElectionNamespaceFlag is the name of the command line flag to specify the leader election namespace.
Expand Down Expand Up @@ -176,20 +173,6 @@ func (b *OptionAggregator) Complete() error {
type ManagerOptions struct {
// LeaderElection is whether leader election is turned on or not.
LeaderElection bool
// LeaderElectionResourceLock is the resource type used for leader election (defaults to `leases`).
//
// When changing the default resource lock, please make sure to migrate via multilocks to
// avoid situations where multiple running instances of your controller have each acquired leadership
// through different resource locks (e.g. during upgrades) and thus act on the same resources concurrently.
// For example, 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 one release with the multilock as a default, you can
// go ahead and migrate to "leases". Please also keep in mind, that users might skip versions
// of your controller, so at least add a flashy release note when changing the default lock.
//
// Note: before controller-runtime version v0.7, the resource lock was set to "configmaps".
// Please keep this in mind, when planning a proper migration path for your controller.
LeaderElectionResourceLock string
// LeaderElectionID is the id to do leader election with.
LeaderElectionID string
// LeaderElectionNamespace is the namespace to do leader election in.
Expand All @@ -214,15 +197,7 @@ type ManagerOptions struct {

// AddFlags implements Flagger.AddFlags.
func (m *ManagerOptions) AddFlags(fs *pflag.FlagSet) {
defaultLeaderElectionResourceLock := m.LeaderElectionResourceLock
if defaultLeaderElectionResourceLock == "" {
// explicitly default to leases if no default is specified
defaultLeaderElectionResourceLock = resourcelock.LeasesResourceLock
}

fs.BoolVar(&m.LeaderElection, LeaderElectionFlag, m.LeaderElection, "Whether to use leader election or not when running this controller manager.")
fs.StringVar(&m.LeaderElectionResourceLock, LeaderElectionResourceLockFlag, defaultLeaderElectionResourceLock, "Which resource type to use for leader election. "+
"Supported options are 'leases', 'endpointsleases' and 'configmapsleases'.")
fs.StringVar(&m.LeaderElectionID, LeaderElectionIDFlag, m.LeaderElectionID, "The leader election id to use.")
fs.StringVar(&m.LeaderElectionNamespace, LeaderElectionNamespaceFlag, m.LeaderElectionNamespace, "The namespace to do leader election in.")
fs.StringVar(&m.WebhookServerHost, WebhookServerHostFlag, m.WebhookServerHost, "The webhook server host.")
Expand Down Expand Up @@ -251,7 +226,6 @@ func (m *ManagerOptions) Complete() error {

m.config = &ManagerConfig{
m.LeaderElection,
m.LeaderElectionResourceLock,
m.LeaderElectionID,
m.LeaderElectionNamespace,
m.WebhookServerHost,
Expand All @@ -272,8 +246,6 @@ func (m *ManagerOptions) Completed() *ManagerConfig {
type ManagerConfig struct {
// LeaderElection is whether leader election is turned on or not.
LeaderElection bool
// LeaderElectionResourceLock is the resource type used for leader election.
LeaderElectionResourceLock string
// LeaderElectionID is the id to do leader election with.
LeaderElectionID string
// LeaderElectionNamespace is the namespace to do leader election in.
Expand All @@ -295,7 +267,7 @@ type ManagerConfig struct {
// Apply sets the values of this ManagerConfig in the given manager.Options.
func (c *ManagerConfig) Apply(opts *manager.Options) {
opts.LeaderElection = c.LeaderElection
opts.LeaderElectionResourceLock = c.LeaderElectionResourceLock
opts.LeaderElectionResourceLock = resourcelock.LeasesResourceLock
opts.LeaderElectionID = c.LeaderElectionID
opts.LeaderElectionNamespace = c.LeaderElectionNamespace
opts.Metrics = metricsserver.Options{BindAddress: c.MetricsBindAddress}
Expand Down
73 changes: 32 additions & 41 deletions extensions/pkg/controller/cmd/options_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,21 +201,19 @@ var _ = Describe("Options", func() {

Context("ManagerOptions", func() {
const (
name = "foo"
leaderElectionResourceLock = "leases"
leaderElectionID = "id"
leaderElectionNamespace = "namespace"
metricsBindAddress = ":8080"
healthBindAddress = ":8081"
logLevel = "debug"
logFormat = "text"
logLevelDefault = "info"
logFormatDefault = "json"
name = "foo"
leaderElectionID = "id"
leaderElectionNamespace = "namespace"
metricsBindAddress = ":8080"
healthBindAddress = ":8081"
logLevel = "debug"
logFormat = "text"
logLevelDefault = "info"
logFormatDefault = "json"
)
command := test.NewCommandBuilder(name).
Flags(
test.BoolFlag("leader-election", true),
test.StringFlag("leader-election-resource-lock", leaderElectionResourceLock),
test.StringFlag("leader-election-id", leaderElectionID),
test.StringFlag("leader-election-namespace", leaderElectionNamespace),
test.StringFlag("metrics-bind-address", metricsBindAddress),
Expand All @@ -235,14 +233,13 @@ var _ = Describe("Options", func() {

Expect(fs.Parse(command)).NotTo(HaveOccurred())
Expect(opts).To(Equal(ManagerOptions{
LeaderElection: true,
LeaderElectionResourceLock: leaderElectionResourceLock,
LeaderElectionID: leaderElectionID,
LeaderElectionNamespace: leaderElectionNamespace,
MetricsBindAddress: metricsBindAddress,
HealthBindAddress: healthBindAddress,
LogLevel: logLevel,
LogFormat: logFormat,
LeaderElection: true,
LeaderElectionID: leaderElectionID,
LeaderElectionNamespace: leaderElectionNamespace,
MetricsBindAddress: metricsBindAddress,
HealthBindAddress: healthBindAddress,
LogLevel: logLevel,
LogFormat: logFormat,
}))
})

Expand All @@ -263,14 +260,13 @@ var _ = Describe("Options", func() {
Slice(),
)).NotTo(HaveOccurred())
Expect(opts).To(Equal(ManagerOptions{
LeaderElection: true,
LeaderElectionResourceLock: "leases",
LeaderElectionID: leaderElectionID,
LeaderElectionNamespace: leaderElectionNamespace,
MetricsBindAddress: metricsBindAddress,
HealthBindAddress: healthBindAddress,
LogLevel: logLevelDefault,
LogFormat: logFormatDefault,
LeaderElection: true,
LeaderElectionID: leaderElectionID,
LeaderElectionNamespace: leaderElectionNamespace,
MetricsBindAddress: metricsBindAddress,
HealthBindAddress: healthBindAddress,
LogLevel: logLevelDefault,
LogFormat: logFormatDefault,
}))
})
})
Expand Down Expand Up @@ -331,7 +327,6 @@ var _ = Describe("Options", func() {
Expect(fs.Parse(command)).NotTo(HaveOccurred())
Expect(opts.Complete()).NotTo(HaveOccurred())
Expect(opts.Completed()).To(HaveField("LeaderElection", true))
Expect(opts.Completed()).To(HaveField("LeaderElectionResourceLock", leaderElectionResourceLock))
Expect(opts.Completed()).To(HaveField("LeaderElectionID", leaderElectionID))
Expect(opts.Completed()).To(HaveField("LeaderElectionNamespace", leaderElectionNamespace))
Expect(opts.Completed()).To(HaveField("MetricsBindAddress", metricsBindAddress))
Expand Down Expand Up @@ -591,26 +586,24 @@ var _ = Describe("Options", func() {

Context("ManagerConfig", func() {
const (
leaderElectionResourceLock = "leases"
leaderElectionID = "id"
leaderElectionNamespace = "namespace"
leaderElectionID = "id"
leaderElectionNamespace = "namespace"
)

Describe("#Apply", func() {
It("should apply the values to the given manager.Options", func() {
cfg := &ManagerConfig{
LeaderElection: true,
LeaderElectionResourceLock: leaderElectionResourceLock,
LeaderElectionID: leaderElectionID,
LeaderElectionNamespace: leaderElectionNamespace,
LeaderElection: true,
LeaderElectionID: leaderElectionID,
LeaderElectionNamespace: leaderElectionNamespace,
}

opts := manager.Options{}
cfg.Apply(&opts)

Expect(opts).To(Equal(manager.Options{
LeaderElection: true,
LeaderElectionResourceLock: leaderElectionResourceLock,
LeaderElectionResourceLock: "leases",
LeaderElectionID: leaderElectionID,
LeaderElectionNamespace: leaderElectionNamespace,
Controller: controllerconfig.Controller{
Expand All @@ -624,16 +617,14 @@ var _ = Describe("Options", func() {
Describe("#Options", func() {
It("should return manager.Options with the given values set", func() {
cfg := &ManagerConfig{
LeaderElection: true,
LeaderElectionResourceLock: leaderElectionResourceLock,
LeaderElectionID: leaderElectionID,
LeaderElectionNamespace: leaderElectionNamespace,
LeaderElection: true,
LeaderElectionID: leaderElectionID,
LeaderElectionNamespace: leaderElectionNamespace,
}

opts := cfg.Options()

Expect(opts.LeaderElection).To(BeTrue())
Expect(opts.LeaderElectionResourceLock).To(Equal(leaderElectionResourceLock))
Expect(opts.LeaderElectionID).To(Equal(leaderElectionID))
Expect(opts.LeaderElectionNamespace).To(Equal(leaderElectionNamespace))
})
Expand Down

0 comments on commit 7531c19

Please sign in to comment.