K8SPXC-1805 make election params configurable

cmd/manager/main.go

@@ -8,14 +8,17 @@ import (
 	"runtime"
 	"strconv"
 	"strings"
+	"time"
 
 	_ "github.com/Percona-Lab/percona-version-service/api"
 	certmgrscheme "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned/scheme"
 	"github.com/go-logr/logr"
+	"github.com/kelseyhightower/envconfig"
 	uzap "go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
 	eventsv1 "k8s.io/api/events/v1"
 	k8sruntime "k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
 	"k8s.io/klog/v2"
@@ -46,14 +49,10 @@ var (
 
 func main() {
 	var metricsAddr string
-	var enableLeaderElection bool
 	var probeAddr string
 
 	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
-	flag.BoolVar(&enableLeaderElection, "leader-elect", true,
-		"Enable leader election for controller manager. "+
-			"Enabling this will ensure there is only one active controller manager.")
 
 	opts := zap.Options{
 		Encoder: getLogEncoder(setupLog),
@@ -90,8 +89,14 @@ func main() {
 		os.Exit(1)
 	}
 
+	envs := new(envConfig)
+	if err := envconfig.Process("", envs); err != nil {
+		setupLog.Error(err, "failed to parse env vars")
+		os.Exit(1)
+	}
+
 	fg := features.NewGate()
-	if err := fg.Set(os.Getenv("PXCO_FEATURE_GATES")); err != nil {
+	if err := fg.Set(envs.FeatureGates); err != nil {
 		setupLog.Error(err, "failed to set feature gates")
 		os.Exit(1)
 	}
@@ -109,8 +114,6 @@ func main() {
 			BindAddress: metricsAddr,
 		},
 		HealthProbeBindAddress: probeAddr,
-		LeaderElection:         enableLeaderElection,
-		LeaderElectionID:       "08db1feb.percona.com",
 		WebhookServer: ctrlWebhook.NewServer(ctrlWebhook.Options{
 			Port: 9443,
 		}),
@@ -119,7 +122,13 @@ func main() {
 		},
 	}
 
-	err = configureGroupKindConcurrency(&options)
+	err = configureLeaderElection(&options, envs, operatorNamespace)
+	if err != nil {
+		setupLog.Error(err, "failed to configure leader election")
+		os.Exit(1)
+	}
+
+	err = configureGroupKindConcurrency(&options, envs)
 	if err != nil {
 		setupLog.Error(err, "failed to configure group kind concurrency")
 		os.Exit(1)
@@ -252,7 +261,42 @@ func getLogLevel(log logr.Logger) zapcore.LevelEnabler {
 	}
 }
 
-func configureGroupKindConcurrency(options *ctrl.Options) error {
+const defaultElectionID = "08db1feb.percona.com"
+
+type envConfig struct {
+	LeaderElection   bool          `default:"true" envconfig:"PXCO_LEADER_ELECTION_ENABLED"`
+	LeaderElectionID string        `envconfig:"PXCO_LEADER_ELECTION_LEASE_NAME"`
+	LeaseDuration    time.Duration `default:"15s" envconfig:"PXCO_LEADER_ELECTION_LEASE_DURATION"`
+	RenewDeadline    time.Duration `default:"10s" envconfig:"PXCO_LEADER_ELECTION_RENEW_DEADLINE"`
+	RetryPeriod      time.Duration `default:"2s" envconfig:"PXCO_LEADER_ELECTION_RETRY_PERIOD"`
+
+	FeatureGates string `envconfig:"PXCO_FEATURE_GATES"`
+
+	Workers *int `envconfig:"MAX_CONCURRENT_RECONCILES"`
+}
+
+func configureLeaderElection(options *ctrl.Options, envs *envConfig, operatorNamespace string) error {
+	options.LeaderElection = envs.LeaderElection
+	if envs.LeaderElection {
+		options.LeaderElectionID = defaultElectionID
+	}
+
+	options.LeaseDuration = &envs.LeaseDuration
+	options.RenewDeadline = &envs.RenewDeadline
+	options.RetryPeriod = &envs.RetryPeriod
+
+	if lease := envs.LeaderElectionID; envs.LeaderElection && len(lease) > 0 {
+		if errs := validation.IsDNS1123Subdomain(lease); len(errs) > 0 {
+			return fmt.Errorf("value for PXCO_LEADER_ELECTION_LEASE_NAME is invalid: %v", errs)
+		}
+		options.LeaderElectionID = lease
+		options.LeaderElectionNamespace = operatorNamespace
+	}
+
+	return nil
+}
+
+func configureGroupKindConcurrency(options *ctrl.Options, envs *envConfig) error {
 	groupKinds := []string{
 		"PerconaXtraDBCluster." + pxcv1.SchemeGroupVersion.Group,
 		"PerconaXtraDBClusterBackup." + pxcv1.SchemeGroupVersion.Group,
@@ -265,16 +309,12 @@ func configureGroupKindConcurrency(options *ctrl.Options) error {
 		options.Controller.GroupKindConcurrency[gk] = defaultConcurrency
 	}
 
-	if s := os.Getenv("MAX_CONCURRENT_RECONCILES"); s != "" {
-		i, err := strconv.Atoi(s)
-		if err != nil {
-			return fmt.Errorf("MAX_CONCURRENT_RECONCILES must be a valid integer: %s", s)
-		}
-		if i <= 0 {
-			return fmt.Errorf("MAX_CONCURRENT_RECONCILES must be a positive number: %d", i)
+	if envs.Workers != nil {
+		if *envs.Workers <= 0 {
+			return fmt.Errorf("MAX_CONCURRENT_RECONCILES must be a positive number: %d", *envs.Workers)
 		}
 		for _, gk := range groupKinds {
-			options.Controller.GroupKindConcurrency[gk] = i
+			options.Controller.GroupKindConcurrency[gk] = *envs.Workers
 		}
 	}
 	return nil

cmd/manager/main_test.go

@@ -2,19 +2,125 @@ package main
 
 import (
 	"testing"
+	"time"
 
+	"github.com/kelseyhightower/envconfig"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	ctrl "sigs.k8s.io/controller-runtime"
 
 	pxcv1 "github.com/percona/percona-xtradb-cluster-operator/pkg/apis/pxc/v1"
 	metricsServer "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 )
 
+func parseEnvConfig(t *testing.T) *envConfig {
+	t.Helper()
+	envs := new(envConfig)
+	require.NoError(t, envconfig.Process("", envs))
+	return envs
+}
+
+func TestConfigureLeaderElection(t *testing.T) {
+	t.Run("defaults", func(t *testing.T) {
+		envs := parseEnvConfig(t)
+		options := ctrl.Options{}
+		err := configureLeaderElection(&options, envs, "test-ns")
+		require.NoError(t, err)
+
+		assert.True(t, options.LeaderElection)
+		assert.Equal(t, defaultElectionID, options.LeaderElectionID)
+		assert.Equal(t, 15*time.Second, *options.LeaseDuration)
+		assert.Equal(t, 10*time.Second, *options.RenewDeadline)
+		assert.Equal(t, 2*time.Second, *options.RetryPeriod)
+		assert.Empty(t, options.LeaderElectionNamespace)
+	})
+
+	t.Run("custom durations", func(t *testing.T) {
+		t.Setenv("PXCO_LEADER_ELECTION_LEASE_DURATION", "120s")
+		t.Setenv("PXCO_LEADER_ELECTION_RENEW_DEADLINE", "80s")
+		t.Setenv("PXCO_LEADER_ELECTION_RETRY_PERIOD", "20s")
+
+		envs := parseEnvConfig(t)
+		options := ctrl.Options{}
+		err := configureLeaderElection(&options, envs, "test-ns")
+		require.NoError(t, err)
+
+		assert.Equal(t, 120*time.Second, *options.LeaseDuration)
+		assert.Equal(t, 80*time.Second, *options.RenewDeadline)
+		assert.Equal(t, 20*time.Second, *options.RetryPeriod)
+	})
+
+	t.Run("invalid duration", func(t *testing.T) {
+		t.Setenv("PXCO_LEADER_ELECTION_LEASE_DURATION", "invalid")
+
+		envs := new(envConfig)
+		err := envconfig.Process("", envs)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "PXCO_LEADER_ELECTION_LEASE_DURATION")
+	})
+
+	t.Run("leader election disabled", func(t *testing.T) {
+		t.Setenv("PXCO_LEADER_ELECTION_ENABLED", "false")
+
+		envs := parseEnvConfig(t)
+		options := ctrl.Options{}
+		err := configureLeaderElection(&options, envs, "test-ns")
+		require.NoError(t, err)
+
+		assert.False(t, options.LeaderElection)
+		assert.Empty(t, options.LeaderElectionID)
+	})
+
+	t.Run("invalid boolean for enabled", func(t *testing.T) {
+		t.Setenv("PXCO_LEADER_ELECTION_ENABLED", "not-a-bool")
+
+		envs := new(envConfig)
+		err := envconfig.Process("", envs)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "PXCO_LEADER_ELECTION_ENABLED")
+	})
+
+	t.Run("custom lease name valid", func(t *testing.T) {
+		t.Setenv("PXCO_LEADER_ELECTION_LEASE_NAME", "my-custom-lease")
+
+		envs := parseEnvConfig(t)
+		options := ctrl.Options{}
+		err := configureLeaderElection(&options, envs, "operator-ns")
+		require.NoError(t, err)
+
+		assert.True(t, options.LeaderElection)
+		assert.Equal(t, "my-custom-lease", options.LeaderElectionID)
+		assert.Equal(t, "operator-ns", options.LeaderElectionNamespace)
+	})
+
+	t.Run("custom lease name invalid", func(t *testing.T) {
+		t.Setenv("PXCO_LEADER_ELECTION_LEASE_NAME", "INVALID_NAME")
+
+		envs := parseEnvConfig(t)
+		options := ctrl.Options{}
+		err := configureLeaderElection(&options, envs, "test-ns")
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "PXCO_LEADER_ELECTION_LEASE_NAME")
+	})
+
+	t.Run("invalid lease name with election disabled", func(t *testing.T) {
+		t.Setenv("PXCO_LEADER_ELECTION_ENABLED", "false")
+		t.Setenv("PXCO_LEADER_ELECTION_LEASE_NAME", "INVALID_NAME")
+
+		envs := parseEnvConfig(t)
+		options := ctrl.Options{}
+		err := configureLeaderElection(&options, envs, "test-ns")
+		require.NoError(t, err)
+
+		assert.False(t, options.LeaderElection)
+	})
+}
+
 func TestConfigureGroupKindConcurrency(t *testing.T) {
 	tests := map[string]struct {
 		envValue      string
-		expectedError string
 		expectedVal   map[string]int
+		expectedError bool
 	}{
 		"default concurrency when env not set": {
 			envValue: "",
@@ -32,32 +138,13 @@ func TestConfigureGroupKindConcurrency(t *testing.T) {
 				"PerconaXtraDBClusterRestore." + pxcv1.SchemeGroupVersion.Group: 5,
 			},
 		},
-		"invalid non-integer value": {
-			envValue: "invalid",
-			expectedVal: map[string]int{
-				"PerconaXtraDBCluster." + pxcv1.SchemeGroupVersion.Group:        1,
-				"PerconaXtraDBClusterBackup." + pxcv1.SchemeGroupVersion.Group:  1,
-				"PerconaXtraDBClusterRestore." + pxcv1.SchemeGroupVersion.Group: 1,
-			},
-			expectedError: "valid integer",
-		},
 		"zero value rejected": {
-			envValue: "0",
-			expectedVal: map[string]int{
-				"PerconaXtraDBCluster." + pxcv1.SchemeGroupVersion.Group:        1,
-				"PerconaXtraDBClusterBackup." + pxcv1.SchemeGroupVersion.Group:  1,
-				"PerconaXtraDBClusterRestore." + pxcv1.SchemeGroupVersion.Group: 1,
-			},
-			expectedError: "positive number",
+			envValue:      "0",
+			expectedError: true,
 		},
 		"negative value rejected": {
-			envValue: "-1",
-			expectedVal: map[string]int{
-				"PerconaXtraDBCluster." + pxcv1.SchemeGroupVersion.Group:        1,
-				"PerconaXtraDBClusterBackup." + pxcv1.SchemeGroupVersion.Group:  1,
-				"PerconaXtraDBClusterRestore." + pxcv1.SchemeGroupVersion.Group: 1,
-			},
-			expectedError: "positive number",
+			envValue:      "-1",
+			expectedError: true,
 		},
 	}
 
@@ -67,6 +154,7 @@ func TestConfigureGroupKindConcurrency(t *testing.T) {
 				t.Setenv("MAX_CONCURRENT_RECONCILES", tt.envValue)
 			}
 
+			envs := parseEnvConfig(t)
 			options := ctrl.Options{
 				Scheme: scheme,
 				Metrics: metricsServer.Options{
@@ -77,23 +165,33 @@ func TestConfigureGroupKindConcurrency(t *testing.T) {
 				LeaderElectionID:       "election-id",
 			}
 
-			err := configureGroupKindConcurrency(&options)
+			err := configureGroupKindConcurrency(&options, envs)
 
-			if tt.expectedError != "" {
+			if tt.expectedError {
 				assert.Error(t, err)
-				assert.Contains(t, err.Error(), tt.expectedError)
-			} else {
-				assert.NoError(t, err)
-				// ensure that the original options are not affected
-				assert.Equal(t, scheme, options.Scheme)
-				assert.Equal(t, metricsServer.Options{
-					BindAddress: "bind-address",
-				}, options.Metrics)
-				assert.Equal(t, "probe-address", options.HealthProbeBindAddress)
-				assert.Equal(t, "election-id", options.LeaderElectionID)
-				assert.True(t, options.LeaderElection)
+				return
 			}
+
+			require.NoError(t, err)
+
+			// ensure that the original options are not affected
+			assert.Equal(t, scheme, options.Scheme)
+			assert.Equal(t, metricsServer.Options{
+				BindAddress: "bind-address",
+			}, options.Metrics)
+			assert.Equal(t, "probe-address", options.HealthProbeBindAddress)
+			assert.Equal(t, "election-id", options.LeaderElectionID)
+			assert.True(t, options.LeaderElection)
 			assert.Equal(t, tt.expectedVal, options.Controller.GroupKindConcurrency)
 		})
 	}
+
+	t.Run("invalid non-integer value", func(t *testing.T) {
+		t.Setenv("MAX_CONCURRENT_RECONCILES", "invalid")
+
+		envs := new(envConfig)
+		err := envconfig.Process("", envs)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "MAX_CONCURRENT_RECONCILES")
+	})
 }

deploy/bundle.yaml

@@ -12533,6 +12533,16 @@ spec:
           value: "1"
         - name: PXCO_FEATURE_GATES
           value: ""
+        - name: PXCO_LEADER_ELECTION_ENABLED
+          value: "true"
+        - name: PXCO_LEADER_ELECTION_LEASE_NAME
+          value: ""
+        - name: PXCO_LEADER_ELECTION_LEASE_DURATION
+          value: "15s"
+        - name: PXCO_LEADER_ELECTION_RENEW_DEADLINE
+          value: "10s"
+        - name: PXCO_LEADER_ELECTION_RETRY_PERIOD
+          value: "2s"
         image: perconalab/percona-xtradb-cluster-operator:main
         imagePullPolicy: Always
         livenessProbe:

deploy/cw-bundle.yaml

@@ -12543,6 +12543,16 @@ spec:
           value: "1"
         - name: PXCO_FEATURE_GATES
           value: ""
+        - name: PXCO_LEADER_ELECTION_ENABLED
+          value: "true"
+        - name: PXCO_LEADER_ELECTION_LEASE_NAME
+          value: ""
+        - name: PXCO_LEADER_ELECTION_LEASE_DURATION
+          value: "15s"
+        - name: PXCO_LEADER_ELECTION_RENEW_DEADLINE
+          value: "10s"
+        - name: PXCO_LEADER_ELECTION_RETRY_PERIOD
+          value: "2s"
         image: perconalab/percona-xtradb-cluster-operator:main
         imagePullPolicy: Always
         resources: