Skip to content

Commit

Permalink
Update LeaderElection Manager Option type to *bool
Browse files Browse the repository at this point in the history 
- Changed the datatype of leader election for manager option from bool to *bool

- Changed the flag parsing logic in Webhook main according to the new datatype
If the flag is not passed, it will be nil, otherwise false or true

- Added test to check the condition if true in config file and false in flag
  • Loading branch information
Ayush Rangwala committed Apr 25, 2021
1 parent b2c90ab commit 993681f
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 20 deletions.
9 changes: 5 additions & 4 deletions pkg/manager/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/leaderelection/resourcelock"
"k8s.io/client-go/tools/record"

"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/cluster"
Expand Down Expand Up @@ -138,7 +139,7 @@ type Options struct {

// LeaderElection determines whether or not to use leader election when
// starting the manager.
LeaderElection bool
LeaderElection *bool

// LeaderElectionResourceLock determines which resource lock to use for leader election,
// defaults to "configmapsleases". Change this value only if you know what you are doing.
Expand Down Expand Up @@ -332,7 +333,7 @@ func New(config *rest.Config, options Options) (Manager, error) {
leaderConfig = rest.CopyConfig(config)
}
resourceLock, err := options.newResourceLock(leaderConfig, recorderProvider, leaderelection.Options{
LeaderElection: options.LeaderElection,
LeaderElection: *options.LeaderElection,
LeaderElectionResourceLock: options.LeaderElectionResourceLock,
LeaderElectionID: options.LeaderElectionID,
LeaderElectionNamespace: options.LeaderElectionNamespace,
Expand Down Expand Up @@ -459,8 +460,8 @@ func (o Options) AndFromOrDie(loader config.ControllerManagerConfiguration) Opti
}

func (o Options) setLeaderElectionConfig(obj v1alpha1.ControllerManagerConfigurationSpec) Options {
if o.LeaderElection == false && obj.LeaderElection.LeaderElect != nil {
o.LeaderElection = *obj.LeaderElection.LeaderElect
if o.LeaderElection == nil && obj.LeaderElection.LeaderElect != nil {
o.LeaderElection = obj.LeaderElection.LeaderElect
}

if o.LeaderElectionResourceLock == "" && obj.LeaderElection.ResourceLock != "" {
Expand Down
109 changes: 97 additions & 12 deletions pkg/manager/manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ var _ = Describe("manger.Manager", func() {

It("should be able to load Options from cfg.ControllerManagerConfiguration type", func(done Done) {
duration := metav1.Duration{Duration: 48 * time.Hour}
port := int(6090)
port := 6090
leaderElect := false

ccfg := &v1alpha1.ControllerManagerConfiguration{
Expand Down Expand Up @@ -164,7 +164,7 @@ var _ = Describe("manger.Manager", func() {
Expect(err).To(BeNil())

Expect(*m.SyncPeriod).To(Equal(duration.Duration))
Expect(m.LeaderElection).To(Equal(leaderElect))
Expect(*m.LeaderElection).To(Equal(leaderElect))
Expect(m.LeaderElectionResourceLock).To(Equal("leases"))
Expect(m.LeaderElectionNamespace).To(Equal("default"))
Expect(m.LeaderElectionID).To(Equal("ctrl-lease"))
Expand Down Expand Up @@ -218,9 +218,86 @@ var _ = Describe("manger.Manager", func() {
},
}

leaderElectOp := true
m, err := Options{
SyncPeriod: &optDuration,
LeaderElection: &leaderElectOp,
LeaderElectionResourceLock: "configmaps",
LeaderElectionNamespace: "ctrl",
LeaderElectionID: "ctrl-configmap",
LeaseDuration: &optDuration,
RenewDeadline: &optDuration,
RetryPeriod: &optDuration,
Namespace: "ctrl",
MetricsBindAddress: ":7000",
HealthProbeBindAddress: "5000",
ReadinessEndpointName: "/readiness",
LivenessEndpointName: "/liveness",
Port: 8080,
Host: "example.com",
CertDir: "/pki",
}.AndFrom(&fakeDeferredLoader{ccfg})
Expect(err).To(BeNil())

Expect(m.SyncPeriod.String()).To(Equal(optDuration.String()))
Expect(*m.LeaderElection).To(Equal(true))
Expect(m.LeaderElectionResourceLock).To(Equal("configmaps"))
Expect(m.LeaderElectionNamespace).To(Equal("ctrl"))
Expect(m.LeaderElectionID).To(Equal("ctrl-configmap"))
Expect(m.LeaseDuration.String()).To(Equal(optDuration.String()))
Expect(m.RenewDeadline.String()).To(Equal(optDuration.String()))
Expect(m.RetryPeriod.String()).To(Equal(optDuration.String()))
Expect(m.Namespace).To(Equal("ctrl"))
Expect(m.MetricsBindAddress).To(Equal(":7000"))
Expect(m.HealthProbeBindAddress).To(Equal("5000"))
Expect(m.ReadinessEndpointName).To(Equal("/readiness"))
Expect(m.LivenessEndpointName).To(Equal("/liveness"))
Expect(m.Port).To(Equal(8080))
Expect(m.Host).To(Equal("example.com"))
Expect(m.CertDir).To(Equal("/pki"))

close(done)
})

It("should be able to keep Options leaderElection false when cfg.ControllerManagerConfiguration set to true", func(done Done) {
optDuration := time.Duration(2)
duration := metav1.Duration{Duration: 48 * time.Hour}
port := 6090
leaderElect := true

ccfg := &v1alpha1.ControllerManagerConfiguration{
ControllerManagerConfigurationSpec: v1alpha1.ControllerManagerConfigurationSpec{
SyncPeriod: &duration,
LeaderElection: &configv1alpha1.LeaderElectionConfiguration{
LeaderElect: &leaderElect,
ResourceLock: "leases",
ResourceNamespace: "default",
ResourceName: "ctrl-lease",
LeaseDuration: duration,
RenewDeadline: duration,
RetryPeriod: duration,
},
CacheNamespace: "default",
Metrics: v1alpha1.ControllerMetrics{
BindAddress: ":6000",
},
Health: v1alpha1.ControllerHealth{
HealthProbeBindAddress: "6060",
ReadinessEndpointName: "/readyz",
LivenessEndpointName: "/livez",
},
Webhook: v1alpha1.ControllerWebhook{
Port: &port,
Host: "localhost",
CertDir: "/certs",
},
},
}

leaderElectOp := false
m, err := Options{
SyncPeriod: &optDuration,
LeaderElection: true,
LeaderElection: &leaderElectOp,
LeaderElectionResourceLock: "configmaps",
LeaderElectionNamespace: "ctrl",
LeaderElectionID: "ctrl-configmap",
Expand All @@ -239,7 +316,7 @@ var _ = Describe("manger.Manager", func() {
Expect(err).To(BeNil())

Expect(m.SyncPeriod.String()).To(Equal(optDuration.String()))
Expect(m.LeaderElection).To(Equal(true))
Expect(*m.LeaderElection).To(Equal(leaderElectOp))
Expect(m.LeaderElectionResourceLock).To(Equal("configmaps"))
Expect(m.LeaderElectionNamespace).To(Equal("ctrl"))
Expect(m.LeaderElectionID).To(Equal("ctrl-configmap"))
Expand Down Expand Up @@ -275,8 +352,9 @@ var _ = Describe("manger.Manager", func() {

Context("with leader election enabled", func() {
It("should only cancel the leader election after all runnables are done", func() {
leaderElectOpt := true
m, err := New(cfg, Options{
LeaderElection: true,
LeaderElection: &leaderElectOpt,
LeaderElectionNamespace: "default",
LeaderElectionID: "test-leader-election-id-2",
HealthProbeBindAddress: "0",
Expand Down Expand Up @@ -320,8 +398,9 @@ var _ = Describe("manger.Manager", func() {

})
It("should disable gracefulShutdown when stopping to lead", func() {
leaderElectOpt := true
m, err := New(cfg, Options{
LeaderElection: true,
LeaderElection: &leaderElectOpt,
LeaderElectionNamespace: "default",
LeaderElectionID: "test-leader-election-id-3",
HealthProbeBindAddress: "0",
Expand Down Expand Up @@ -349,8 +428,9 @@ var _ = Describe("manger.Manager", func() {
})
It("should default ID to controller-runtime if ID is not set", func() {
var rl resourcelock.Interface
leaderElectOpt := true
m1, err := New(cfg, Options{
LeaderElection: true,
LeaderElection: &leaderElectOpt,
LeaderElectionNamespace: "default",
LeaderElectionID: "test-leader-election-id",
newResourceLock: func(config *rest.Config, recorderProvider recorder.Provider, options leaderelection.Options) (resourcelock.Interface, error) {
Expand All @@ -368,9 +448,10 @@ var _ = Describe("manger.Manager", func() {
m1cm, ok := m1.(*controllerManager)
Expect(ok).To(BeTrue())
m1cm.onStoppedLeading = func() {}
leaderElectOpt = true

m2, err := New(cfg, Options{
LeaderElection: true,
LeaderElection: &leaderElectOpt,
LeaderElectionNamespace: "default",
LeaderElectionID: "test-leader-election-id",
newResourceLock: func(config *rest.Config, recorderProvider recorder.Provider, options leaderelection.Options) (resourcelock.Interface, error) {
Expand Down Expand Up @@ -437,7 +518,8 @@ var _ = Describe("manger.Manager", func() {
Expect(err).To(MatchError(ContainSubstring("expected error")))
})
It("should return an error if namespace not set and not running in cluster", func() {
m, err := New(cfg, Options{LeaderElection: true, LeaderElectionID: "controller-runtime"})
leaderElectOpt := true
m, err := New(cfg, Options{LeaderElection: &leaderElectOpt, LeaderElectionID: "controller-runtime"})
Expect(m).To(BeNil())
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("unable to find leader election namespace: not running in-cluster, please specify LeaderElectionNamespace"))
Expand All @@ -447,7 +529,8 @@ var _ = Describe("manger.Manager", func() {
// ConfigMap lock to a controller-runtime version that has this new default. Many users of controller-runtime skip
// versions, so we should be extremely conservative here.
It("should default to ConfigMapsLeasesResourceLock", func() {
m, err := New(cfg, Options{LeaderElection: true, LeaderElectionID: "controller-runtime", LeaderElectionNamespace: "my-ns"})
leaderElectOpt := true
m, err := New(cfg, Options{LeaderElection: &leaderElectOpt, LeaderElectionID: "controller-runtime", LeaderElectionNamespace: "my-ns"})
Expect(m).ToNot(BeNil())
Expect(err).ToNot(HaveOccurred())
cm, ok := m.(*controllerManager)
Expand All @@ -461,8 +544,9 @@ var _ = Describe("manger.Manager", func() {

})
It("should use the specified ResourceLock", func() {
leaderElectOpt := true
m, err := New(cfg, Options{
LeaderElection: true,
LeaderElection: &leaderElectOpt,
LeaderElectionResourceLock: resourcelock.LeasesResourceLock,
LeaderElectionID: "controller-runtime",
LeaderElectionNamespace: "my-ns",
Expand Down Expand Up @@ -990,9 +1074,10 @@ var _ = Describe("manger.Manager", func() {
})

Context("with leaderelection enabled", func() {
leaderElectOpt := true
startSuite(
Options{
LeaderElection: true,
LeaderElection: &leaderElectOpt,
LeaderElectionID: "controller-runtime",
LeaderElectionNamespace: "default",
newResourceLock: fakeleaderelection.NewResourceLock,
Expand Down
18 changes: 14 additions & 4 deletions pkg/webhook/conversion/testdata/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"context"
"flag"
"os"
"strconv"

"k8s.io/apimachinery/pkg/runtime"
_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
Expand Down Expand Up @@ -46,10 +47,19 @@ func init() {

func main() {
var metricsAddr string
var enableLeaderElection bool
var enableLeaderElection *bool
flag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.")
flag.BoolVar(&enableLeaderElection, "enable-leader-election", false,
"Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager.")
flag.Func("enable-leader-election",
"Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager.",
func(flagVal string) error {
boolVal, err := strconv.ParseBool(flagVal)
if err != nil {
return err
}
enableLeaderElection = &boolVal
return nil
},
)
flag.Parse()

ctrl.SetLogger(zap.Logger(true))
Expand All @@ -67,7 +77,7 @@ func main() {
// +kubebuilder:scaffold:builder

setupLog.Info("starting manager")
if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
if err = mgr.Start(ctrl.SetupSignalHandler()); err != nil {
setupLog.Error(err, "problem running manager")
os.Exit(1)
}
Expand Down

0 comments on commit 993681f

Please sign in to comment.