diff --git a/changelog/fragments/helm-leader-election.yaml b/changelog/fragments/helm-leader-election.yaml new file mode 100644 index 0000000000..21f56bec23 --- /dev/null +++ b/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 diff --git a/cmd/helm-operator/main.go b/cmd/helm-operator/main.go index cd4a3bc6aa..131d04859e 100644 --- a/cmd/helm-operator/main.go +++ b/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 diff --git a/hack/tests/e2e-helm.sh b/hack/tests/e2e-helm.sh index 7e1b9aaf0b..215b06f14d 100755 --- a/hack/tests/e2e-helm.sh +++ b/hack/tests/e2e-helm.sh @@ -7,13 +7,14 @@ 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" @@ -21,14 +22,15 @@ 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 diff --git a/internal/plugins/helm/v1/scaffolds/init.go b/internal/plugins/helm/v1/scaffolds/init.go index bdd3000f54..eb7ed5782d 100644 --- a/internal/plugins/helm/v1/scaffolds/init.go +++ b/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 { diff --git a/internal/plugins/helm/v1/scaffolds/templates/makefile.go b/internal/plugins/helm/v1/scaffolds/templates/makefile.go index bedd21e1a2..bad1b8f837 100644 --- a/internal/plugins/helm/v1/scaffolds/templates/makefile.go +++ b/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 diff --git a/internal/plugins/helm/v1/scaffolds/templates/manager/config.go b/internal/plugins/helm/v1/scaffolds/templates/manager/config.go index 7cf792331e..4690238097 100644 --- a/internal/plugins/helm/v1/scaffolds/templates/manager/config.go +++ b/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 ` diff --git a/internal/plugins/helm/v1/scaffolds/templates/metricsauth/auth_proxy_patch.go b/internal/plugins/helm/v1/scaffolds/templates/metricsauth/auth_proxy_patch.go index c1009a52bd..4c3347e726 100644 --- a/internal/plugins/helm/v1/scaffolds/templates/metricsauth/auth_proxy_patch.go +++ b/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 }}" ` diff --git a/pkg/helm/flags/flag.go b/pkg/helm/flags/flag.go index 3e392e3e6f..8ded6d94d8 100644 --- a/pkg/helm/flags/flag.go +++ b/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).", + ) }