Helm: add leader election flags, use lease-based election

changelog/fragments/helm-leader-election.yaml

@@ -0,0 +1,7 @@
+entries:
+  - description: >
+      In the Helm operator, use controller-runtime's lease-based leader
+      election instead of SDK's leader-for-life mechanism.
+
+    kind: "change"
+    breaking: false

cmd/helm-operator/main.go

@@ -41,7 +41,6 @@ import (
 	"github.com/operator-framework/operator-sdk/pkg/helm/watches"
 	"github.com/operator-framework/operator-sdk/pkg/k8sutil"
 	kubemetrics "github.com/operator-framework/operator-sdk/pkg/kube-metrics"
-	"github.com/operator-framework/operator-sdk/pkg/leader"
 	"github.com/operator-framework/operator-sdk/pkg/log/zap"
 	"github.com/operator-framework/operator-sdk/pkg/metrics"
 	sdkVersion "github.com/operator-framework/operator-sdk/version"
@@ -74,9 +73,23 @@ func main() {
 		os.Exit(1)
 	}
 
+	// Deprecated: OPERATOR_NAME environment variable is an artifact of the legacy operator-sdk project scaffolding.
+	//   Flag `--leader-election-id` should be used instead.
+	if operatorName, found := os.LookupEnv("OPERATOR_NAME"); found {
+		log.Info("Environment variable OPERATOR_NAME has been deprecated, use --leader-election-id instead.")
+		if pflag.CommandLine.Lookup("leader-election-id").Changed {
+			log.Info("Ignoring OPERATOR_NAME environment variable since --leader-election-id is set")
+		} else {
+			f.LeaderElectionID = operatorName
+		}
+	}
+
 	// Set default manager options
 	options := manager.Options{
-		MetricsBindAddress: f.MetricsAddress,
+		MetricsBindAddress:      f.MetricsAddress,
+		LeaderElection:          f.EnableLeaderElection,
+		LeaderElectionID:        f.LeaderElectionID,
+		LeaderElectionNamespace: f.LeaderElectionNamespace,
 		NewClient: func(cache cache.Cache, config *rest.Config, options crclient.Options) (crclient.Client, error) {
 			c, err := crclient.New(config, options)
 			if err != nil {
@@ -141,21 +154,6 @@ func main() {
 		gvks = append(gvks, w.GroupVersionKind)
 	}
 
-	operatorName, err := k8sutil.GetOperatorName()
-	if err != nil {
-		log.Error(err, "Failed to get operator name")
-		os.Exit(1)
-	}
-
-	ctx := context.TODO()
-
-	// Become the leader before proceeding
-	err = leader.Become(ctx, operatorName+"-lock")
-	if err != nil {
-		log.Error(err, "Failed to become leader.")
-		os.Exit(1)
-	}
-
 	addMetrics(context.TODO(), cfg, gvks)
 
 	// Start the Cmd

hack/tests/e2e-helm.sh

@@ -7,28 +7,30 @@ source hack/lib/image_lib.sh
 
 DEST_IMAGE="quay.io/example/nginx-operator:v0.0.2"
 TMPDIR="$(mktemp -d)"
+pushd $TMPDIR
 trap_add 'rm -rf $TMPDIR' EXIT
 
 setup_envs $tmp_sdk_root
 
 # kind has an issue with certain image registries (ex. redhat's), so use a
 # different test pod image.
-METRICS_TEST_IMAGE="fedora:latest"
+METRICS_TEST_IMAGE="curlimages/curl:latest"
 
 test_namespace="nginx-cr-system"
 operator_namespace="nginx-operator-system"
 
 deploy_operator() {
     make install
     make deploy IMG="$DEST_IMAGE"
+    kubectl create clusterrolebinding nginx-operator-system-metrics-reader --clusterrole=nginx-operator-metrics-reader --serviceaccount=${operator_namespace}:default
     kubectl create namespace ${test_namespace}
 }
 
 remove_operator() {
-    kubectl delete --ignore-not-found=true clusterrolebinding nginx-operator-system-metrics-reader
     kubectl delete --ignore-not-found=true --namespace=${test_namespace} -f "$OPERATORDIR/config/samples/helm.example_v1alpha1_nginx.yaml"
     kubectl delete --ignore-not-found=true namespace ${test_namespace}
-    kubectl delete --ignore-not-found=true namespace ${operator_namespace}
+    kubectl delete --ignore-not-found=true clusterrolebinding nginx-operator-system-metrics-reader
+    make undeploy
 }
 
 operator_logs() {
@@ -50,19 +52,22 @@ test_operator() {
         exit 1
     fi
 
+    metrics_service="nginx-operator-controller-manager-metrics-service"
+
     # verify that metrics service was created
-    if ! timeout 60s bash -c -- "until kubectl get service/nginx-operator-metrics --namespace=${operator_namespace} > /dev/null 2>&1; do sleep 1; done";
+    if ! timeout 60s bash -c -- "until kubectl get service/${metrics_service} --namespace=${operator_namespace} > /dev/null 2>&1; do sleep 1; done";
     then
         error_text "Failed to get metrics service"
         operator_logs
         exit 1
     fi
 
-    # give permissions to reach the metrics endpoint
-    kubectl create clusterrolebinding nginx-operator-system-metrics-reader --clusterrole=nginx-operator-metrics-reader --serviceaccount=nginx-operator-system:default
+
+    serviceaccount_secret=$(kubectl get serviceaccounts default -n ${operator_namespace} -o jsonpath='{.secrets[0].name}')
+    token=$(kubectl get secret ${serviceaccount_secret} -n ${operator_namespace} -o jsonpath={.data.token} | base64 -d)
 
     # verify that the metrics endpoint exists
-    if ! kubectl run --attach --rm --restart=Never --namespace=${operator_namespace} test-metrics --image=${metrics_test_image} -- /bin/bash -c 'curl -sfo /dev/null -v -s -k -H "Authorization: Bearer `cat /var/run/secrets/kubernetes.io/serviceaccount/token`" https://nginx-operator-controller-manager-metrics-service:8443/metrics';
+    if ! timeout 60s bash -c -- "until kubectl run --attach --rm --restart=Never --namespace=${operator_namespace} test-metrics --image=${metrics_test_image} -- -sfkH \"Authorization: Bearer ${token}\" https://${metrics_service}:8443/metrics; do sleep 1; done";
     then
         error_text "Failed to verify that metrics endpoint exists"
         operator_logs
@@ -130,7 +135,7 @@ test_operator() {
 
 # create and build the operator
 mkdir nginx-operator
-cd nginx-operator
+pushd nginx-operator
 log=$(operator-sdk init --plugins=helm.operator-sdk.io/v1 \
   --domain=com --group=helm.example --version=v1alpha1 --kind=Nginx \
   2>&1)
@@ -157,4 +162,3 @@ OPERATORDIR="$(pwd)"
 deploy_operator
 trap_add 'remove_operator' EXIT
 test_operator
-remove_operator

internal/plugins/helm/v1/scaffolds/init.go

@@ -17,7 +17,6 @@ limitations under the License.
 package scaffolds
 
 import (
-	"fmt"
 	"os"
 	"strings"
 
@@ -67,18 +66,13 @@ func (s *initScaffolder) newUniverse() *model.Universe {
 
 // Scaffold implements Scaffolder
 func (s *initScaffolder) Scaffold() error {
-	switch {
-	case s.config.IsV3():
-		if err := s.scaffold(); err != nil {
-			return err
-		}
-		if s.apiScaffolder != nil {
-			return s.apiScaffolder.Scaffold()
-		}
-		return nil
-	default:
-		return fmt.Errorf("unknown project version %v", s.config.Version)
+	if err := s.scaffold(); err != nil {
+		return err
+	}
+	if s.apiScaffolder != nil {
+		return s.apiScaffolder.Scaffold()
 	}
+	return nil
 }
 
 func (s *initScaffolder) scaffold() error {

internal/plugins/helm/v1/scaffolds/templates/makefile.go

@@ -74,7 +74,7 @@ all: docker-build
 
 # Run against the configured Kubernetes cluster in ~/.kube/config
 run: helm-operator
-	$(HELM_OPERATOR) run
+	$(HELM_OPERATOR)
 
 # Install CRDs into a cluster
 install: kustomize

internal/plugins/helm/v1/scaffolds/templates/manager/config.go

@@ -49,19 +49,13 @@ func (f *Config) SetTemplateDefaults() error {
 	if f.OperatorName == "" {
 		dir, err := os.Getwd()
 		if err != nil {
-			return fmt.Errorf("error to get the current path: %v", err)
+			return fmt.Errorf("error getting working directory: %v", err)
 		}
 		f.OperatorName = filepath.Base(dir)
 	}
 	return nil
 }
 
-// todo(camilamacedo86): add the arg --enable-leader-election for the manager
-// More info: https://github.com/operator-framework/operator-sdk/issues/3356
-
-// todo(camilamacedo86): add the arg --metrics-addr for the manager
-// More info: https://github.com/operator-framework/operator-sdk/issues/3358
-
 const configTemplate = `apiVersion: v1
 kind: Namespace
 metadata:
@@ -88,6 +82,9 @@ spec:
     spec:
       containers:
       - image: {{ .Image }}
+        args:
+        - "--enable-leader-election"
+        - "--leader-election-id={{ .OperatorName }}"
         name: manager
         resources:
           limits:
@@ -104,6 +101,6 @@ spec:
               fieldRef:
                 fieldPath: metadata.name
           - name: OPERATOR_NAME
-            value: {{ .OperatorName }}
+            value: "{{ .OperatorName }}"
       terminationGracePeriodSeconds: 10
 `

internal/plugins/helm/v1/scaffolds/templates/metricsauth/auth_proxy_patch.go

@@ -18,6 +18,8 @@ limitations under the License.
 package metricsauth
 
 import (
+	"fmt"
+	"os"
 	"path/filepath"
 
 	"sigs.k8s.io/kubebuilder/pkg/model/file"
@@ -29,6 +31,8 @@ var _ file.Template = &AuthProxyPatch{}
 // prometheus metrics for manager Pod.
 type AuthProxyPatch struct {
 	file.TemplateMixin
+
+	OperatorName string
 }
 
 // SetTemplateDefaults implements input.Template
@@ -41,12 +45,17 @@ func (f *AuthProxyPatch) SetTemplateDefaults() error {
 
 	f.IfExistsAction = file.Error
 
+	if f.OperatorName == "" {
+		dir, err := os.Getwd()
+		if err != nil {
+			return fmt.Errorf("error to get the current path: %v", err)
+		}
+		f.OperatorName = filepath.Base(dir)
+	}
+
 	return nil
 }
 
-// todo(camilamacedo86): add the arg --enable-leader-election for the manager
-// More info: https://github.com/operator-framework/operator-sdk/issues/3356
-
 const kustomizeAuthProxyPatchTemplate = `# This patch inject a sidecar container which is a HTTP proxy for the 
 # controller manager, it performs RBAC authorization against the Kubernetes API using SubjectAccessReviews.
 apiVersion: apps/v1
@@ -71,4 +80,6 @@ spec:
       - name: manager
         args:
         - "--metrics-addr=127.0.0.1:8080"
+        - "--enable-leader-election"
+        - "--leader-election-id={{ .OperatorName }}"
 `

pkg/helm/flags/flag.go

@@ -25,10 +25,13 @@ import (
 
 // Flags - Options to be used by a helm operator
 type Flags struct {
-	ReconcilePeriod time.Duration
-	WatchesFile     string
-	MaxWorkers      int
-	MetricsAddress  string
+	ReconcilePeriod         time.Duration
+	WatchesFile             string
+	MaxWorkers              int
+	MetricsAddress          string
+	EnableLeaderElection    bool
+	LeaderElectionID        string
+	LeaderElectionNamespace string
 }
 
 // AddTo - Add the helm operator flags to the the flagset
@@ -54,4 +57,19 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
 		":8080",
 		"The address the metric endpoint binds to",
 	)
+	flagSet.BoolVar(&f.EnableLeaderElection,
+		"enable-leader-election",
+		false,
+		"Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager.",
+	)
+	flagSet.StringVar(&f.LeaderElectionID,
+		"leader-election-id",
+		"",
+		"Name of the configmap that is used for holding the leader lock.",
+	)
+	flagSet.StringVar(&f.LeaderElectionNamespace,
+		"leader-election-namespace",
+		"",
+		"Namespace in which to create the leader election configmap for holding the leader lock (required if running locally with leader election enabled).",
+	)
 }