Use Lease for leader election (#4276)

nginx · Oct 24, 2023 · f168625 · f168625
1 parent 3347c8f
commit f168625
Show file tree

Hide file tree

Showing 6 changed files with 144 additions and 106 deletions.
diff --git a/charts/nginx-ingress/templates/rbac.yaml → .../nginx-ingress/templates/clusterrole.yaml b/charts/nginx-ingress/templates/rbac.yaml → .../nginx-ingress/templates/clusterrole.yaml
@@ -6,79 +6,14 @@ metadata:
   labels:
     {{- include "nginx-ingress.labels" . | nindent 4 }}
 rules:
-{{- if .Values.controller.appprotect.enable }}
-- apiGroups:
-  - appprotect.f5.com
-  resources:
-  - appolicies
-  - aplogconfs
-  - apusersigs
-  verbs:
-  - get
-  - watch
-  - list
-{{- end }}
-{{- if .Values.controller.appprotectdos.enable }}
-- apiGroups:
-    - appprotectdos.f5.com
-  resources:
-    - apdospolicies
-    - apdoslogconfs
-    - dosprotectedresources
-  verbs:
-    - get
-    - watch
-    - list
-{{- end }}
-- apiGroups:
-  - discovery.k8s.io
-  resources:
-  - endpointslices
-  verbs:
-  - get
-  - list
-  - watch
-- apiGroups:
-  - ""
-  resources:
-  - services
-  verbs:
-  - get
-  - list
-  - watch
-- apiGroups:
-  - ""
-  resources:
-  - secrets
-  verbs:
-  - get
-  - list
-  - watch
 - apiGroups:
   - ""
   resources:
   - configmaps
-  verbs:
-  - get
-  - list
-  - watch
-{{- if .Values.controller.reportIngressStatus.enableLeaderElection }}
-  - update
-  - create
-{{- end }}
-- apiGroups:
-  - ""
-  resources:
-  - pods
-  verbs:
-  - get
-  - list
-  - watch
-  - update
-- apiGroups:
-  - ""
-  resources:
   - namespaces
+  - pods
+  - secrets
+  - services
   verbs:
   - get
   - list
@@ -96,11 +31,16 @@ rules:
   resources:
   - leases
   verbs:
+  - list
+  - watch
+- apiGroups:
+  - discovery.k8s.io
+  resources:
+  - endpointslices
+  verbs:
   - get
   - list
   - watch
-  - update
-  - create
 - apiGroups:
   - networking.k8s.io
   resources:
@@ -123,6 +63,30 @@ rules:
   verbs:
   - update
 {{- end }}
+{{- if .Values.controller.appprotect.enable }}
+- apiGroups:
+  - appprotect.f5.com
+  resources:
+  - appolicies
+  - aplogconfs
+  - apusersigs
+  verbs:
+  - get
+  - watch
+  - list
+{{- end }}
+{{- if .Values.controller.appprotectdos.enable }}
+- apiGroups:
+    - appprotectdos.f5.com
+  resources:
+    - apdospolicies
+    - apdoslogconfs
+    - dosprotectedresources
+  verbs:
+    - get
+    - watch
+    - list
+{{- end }}
 {{- if .Values.controller.enableCustomResources }}
 - apiGroups:
   - k8s.nginx.org
@@ -188,19 +152,4 @@ rules:
   verbs:
   - update
 {{- end }}
----
-kind: ClusterRoleBinding
-apiVersion: rbac.authorization.k8s.io/v1
-metadata:
-  name: {{ include "nginx-ingress.fullname" . }}
-  labels:
-    {{- include "nginx-ingress.labels" . | nindent 4 }}
-subjects:
-- kind: ServiceAccount
-  name: {{ include "nginx-ingress.serviceAccountName" . }}
-  namespace: {{ .Release.Namespace }}
-roleRef:
-  kind: ClusterRole
-  name: {{ include "nginx-ingress.fullname" . }}
-  apiGroup: rbac.authorization.k8s.io
-{{- end }}
+{{- end}}
diff --git a/charts/nginx-ingress/templates/clusterrolebiding.yaml b/charts/nginx-ingress/templates/clusterrolebiding.yaml
@@ -0,0 +1,16 @@
+{{- if .Values.rbac.create }}
+kind: ClusterRoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: {{ include "nginx-ingress.fullname" . }}
+  labels:
+    {{- include "nginx-ingress.labels" . | nindent 4 }}
+subjects:
+- kind: ServiceAccount
+  name: {{ include "nginx-ingress.serviceAccountName" . }}
+  namespace: {{ .Release.Namespace }}
+roleRef:
+  kind: ClusterRole
+  name: {{ include "nginx-ingress.fullname" . }}
+  apiGroup: rbac.authorization.k8s.io
+{{- end }}
diff --git a/charts/nginx-ingress/templates/controller-role.yaml b/charts/nginx-ingress/templates/controller-role.yaml
@@ -0,0 +1,56 @@
+{{- if .Values.rbac.create }}
+kind: Role
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: {{ include "nginx-ingress.fullname" . }}
+  labels:
+    {{- include "nginx-ingress.labels" . | nindent 4 }}
+  namespace: {{ .Release.Namespace }}
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - configmaps
+  - pods
+  - secrets
+  - services
+  verbs:
+  - get
+  - list
+  - watch
+- apiGroups:
+    - ""
+  resources:
+    - namespaces
+  verbs:
+    - get
+- apiGroups:
+  - ""
+  resources:
+  - pods
+  verbs:
+  - update
+- apiGroups:
+  - ""
+  resources:
+  - events
+  verbs:
+  - create
+  - patch
+  - list
+- apiGroups:
+  - coordination.k8s.io
+  resources:
+  - leases
+  resourceNames:
+  - {{ .Values.controller.reportIngressStatus.leaderElectionLockName }}
+  verbs:
+  - get
+  - update
+- apiGroups:
+  - coordination.k8s.io
+  resources:
+  - leases
+  verbs:
+  - create
+{{- end }}
diff --git a/charts/nginx-ingress/templates/controller-rolebiding.yaml b/charts/nginx-ingress/templates/controller-rolebiding.yaml
@@ -0,0 +1,17 @@
+{{- if .Values.rbac.create }}
+kind: RoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: {{ include "nginx-ingress.fullname" . }}
+  labels:
+    {{- include "nginx-ingress.labels" . | nindent 4 }}
+  namespace: {{ .Release.Namespace }}
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: {{ include "nginx-ingress.fullname" . }}
+subjects:
+- kind: ServiceAccount
+  name: {{ include "nginx-ingress.serviceAccountName" . }}
+  namespace: {{ .Release.Namespace }}
+{{- end }}
diff --git a/charts/nginx-ingress/values.yaml b/charts/nginx-ingress/values.yaml
@@ -410,9 +410,8 @@ controller:
     ## Enable Leader election to avoid multiple replicas of the controller reporting the status of Ingress resources. controller.reportIngressStatus.enable must be set to true.
     enableLeaderElection: true
 
-    ## Specifies the name of the ConfigMap, within the same namespace as the controller, used as the lock for leader election. controller.reportIngressStatus.enableLeaderElection must be set to true.
-    ## Autogenerated if not set or set to "".
-    # leaderElectionLockName: "nginx-ingress-leader-election"
+    ## Specifies the name to be used as the lock for leader election. controller.reportIngressStatus.enableLeaderElection must be set to true.
+    leaderElectionLockName: "nginx-ingress-leader"
 
     ## The annotations of the leader election configmap.
     annotations: {}

diff --git a/internal/k8s/leader.go b/internal/k8s/leader.go
@@ -2,13 +2,13 @@ package k8s
 
 import (
 	"context"
-	"fmt"
 	"os"
 	"time"
 
 	"github.com/golang/glog"
 
 	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/leaderelection"
@@ -31,25 +31,26 @@ func newLeaderElector(client kubernetes.Interface, callbacks leaderelection.Lead
 		EventRecorder: recorder,
 	}
 
-	lock, err := resourcelock.New(resourcelock.LeasesResourceLock,
-		namespace,
-		lockName,
-		client.CoreV1(),
-		client.CoordinationV1(),
-		lc,
-	)
-	if err != nil {
-		return nil, fmt.Errorf("error creating leader election lock: %w", err)
+	leaseMeta := metav1.ObjectMeta{
+		Namespace: namespace,
+		Name:      lockName,
+	}
+
+	lock := &resourcelock.LeaseLock{
+		LeaseMeta:  leaseMeta,
+		Client:     client.CoordinationV1(),
+		LockConfig: lc,
 	}
 
 	ttl := 30 * time.Second
-	return leaderelection.NewLeaderElector(leaderelection.LeaderElectionConfig{
-		Lock:          lock,
-		LeaseDuration: ttl,
-		RenewDeadline: ttl / 2,
-		RetryPeriod:   ttl / 4,
-		Callbacks:     callbacks,
-	})
+	return leaderelection.NewLeaderElector(
+		leaderelection.LeaderElectionConfig{
+			Lock:          lock,
+			LeaseDuration: ttl,
+			RenewDeadline: ttl / 2,
+			RetryPeriod:   ttl / 4,
+			Callbacks:     callbacks,
+		})
 }
 
 // createLeaderHandler builds the handler funcs for leader handling