Controller: Correctly identify other pods on shutdown.

charts/ingress-nginx/Chart.yaml

@@ -1,6 +1,6 @@
 annotations:
   artifacthub.io/changes: |
-    - Update Ingress-Nginx version controller-v1.12.2
+    - Add electionID label to controller pods when leader election is enabled
   artifacthub.io/prerelease: "false"
 apiVersion: v2
 appVersion: 1.12.2
@@ -20,4 +20,4 @@ maintainers:
 name: ingress-nginx
 sources:
 - https://github.com/kubernetes/ingress-nginx
-version: 4.12.2
+version: 4.12.3

charts/ingress-nginx/changelog/helm-chart-4.12.3.md

@@ -0,0 +1,9 @@
+# Changelog
+
+This file documents all notable changes to [ingress-nginx](https://github.com/kubernetes/ingress-nginx) Helm Chart. The release numbering uses [semantic versioning](http://semver.org).
+
+### 4.12.3
+
+* Add electionID label to controller pods when leader election is enabled
+
+**Full Changelog**: https://github.com/kubernetes/ingress-nginx/compare/helm-chart-4.12.2...helm-chart-4.12.3

charts/ingress-nginx/templates/controller-daemonset.yaml

@@ -34,6 +34,9 @@ spec:
       labels:
         {{- include "ingress-nginx.labels" . | nindent 8 }}
         app.kubernetes.io/component: controller
+        {{- if not .Values.controller.disableLeaderElection }}
+        nginx.ingress.kubernetes.io/electionID: {{ include "ingress-nginx.controller.electionID" . }}
+        {{- end }}
         {{- with .Values.controller.labels }}
         {{- toYaml . | nindent 8 }}
         {{- end }}

charts/ingress-nginx/templates/controller-deployment.yaml

@@ -40,6 +40,9 @@ spec:
       labels:
         {{- include "ingress-nginx.labels" . | nindent 8 }}
         app.kubernetes.io/component: controller
+        {{- if not .Values.controller.disableLeaderElection }}
+        nginx.ingress.kubernetes.io/electionID: {{ include "ingress-nginx.controller.electionID" . }}
+        {{- end }}
         {{- with .Values.controller.labels }}
         {{- toYaml . | nindent 8 }}
         {{- end }}

charts/ingress-nginx/tests/controller-daemonset_test.yaml

@@ -208,3 +208,30 @@ tests:
       - equal:
           path: spec.template.spec.runtimeClassName
           value: myClass
+
+  - it: should create a DaemonSet with the electionID label on the pod template when leader election is enabled
+    set:
+      controller.kind: DaemonSet
+      controller.disableLeaderElection: false
+    asserts:
+      - equal:
+          path: spec.template.metadata.labels.[nginx.ingress.kubernetes.io/electionID]
+          value: RELEASE-NAME-ingress-nginx-leader
+
+  - it: should create a DaemonSet with the custom electionID label on the pod template when controller.electionID is set and leader election is enabled
+    set:
+      controller.kind: DaemonSet
+      controller.disableLeaderElection: false
+      controller.electionID: custom-election-id
+    asserts:
+      - equal:
+          path: spec.template.metadata.labels.[nginx.ingress.kubernetes.io/electionID]
+          value: custom-election-id
+
+  - it: should create a DaemonSet without the electionID label on the pod template when leader election is disabled
+    set:
+      controller.kind: DaemonSet
+      controller.disableLeaderElection: true
+    asserts:
+      - notExists:
+          path: spec.template.metadata.labels.[nginx.ingress.kubernetes.io/electionID]

charts/ingress-nginx/tests/controller-deployment_test.yaml

@@ -231,3 +231,27 @@ tests:
       - equal:
           path: spec.template.spec.runtimeClassName
           value: myClass
+
+  - it: should create a Deployment with the electionID label on the pod template when leader election is enabled
+    set:
+      controller.disableLeaderElection: false
+    asserts:
+      - equal:
+          path: spec.template.metadata.labels.[nginx.ingress.kubernetes.io/electionID]
+          value: RELEASE-NAME-ingress-nginx-leader
+
+  - it: should create a Deployment with the custom electionID label on the pod template when controller.electionID is set and leader election is enabled
+    set:
+      controller.disableLeaderElection: false
+      controller.electionID: custom-election-id
+    asserts:
+      - equal:
+          path: spec.template.metadata.labels.[nginx.ingress.kubernetes.io/electionID]
+          value: custom-election-id
+
+  - it: should create a Deployment without the electionID label on the pod template when leader election is disabled
+    set:
+      controller.disableLeaderElection: true
+    asserts:
+      - notExists:
+          path: spec.template.metadata.labels.[nginx.ingress.kubernetes.io/electionID]

internal/ingress/controller/controller.go

@@ -143,6 +143,8 @@ type Configuration struct {
 
 	DisableSyncEvents bool
 
+	UseElectionIDSelectorOnShutdown bool
+
 	EnableTopologyAwareRouting bool
 }
 

internal/ingress/controller/nginx.go

@@ -152,6 +152,8 @@
 			IngressLister:          n.store,
 			UpdateStatusOnShutdown: config.UpdateStatusOnShutdown,
 			UseNodeInternalIP:      config.UseNodeInternalIP,
+			UseElectionIDSelectorOnShutdown: config.UseElectionIDSelectorOnShutdown,
+			ElectionID:             config.ElectionID,
 		})
 	} else {
 		klog.Warning("Update of Ingress status is disabled (flag --update-status)")

internal/ingress/status/status.go

@@ -40,6 +40,11 @@
 	"k8s.io/ingress-nginx/pkg/apis/ingress"
 )
 
+const (
+	// ElectionIDLabelKey is the label key used to identify controller pods by election ID
+	ElectionIDLabelKey = "nginx.ingress.kubernetes.io/electionID"
+)
+
 // UpdateInterval defines the time interval, in seconds, in
 // which the status should check if an update is required.
 var UpdateInterval = 60
@@ -68,6 +73,10 @@
 
 	UseNodeInternalIP bool
 
+	UseElectionIDSelectorOnShutdown bool
+
+	ElectionID string
+
 	IngressLister ingressLister
 }
 
@@ -175,6 +184,36 @@
 
 // runningAddresses returns a list of IP addresses and/or FQDN where the
 // ingress controller is currently running
+// listControllerPods returns a list of running pods with controller labels
+func (s *statusSync) listControllerPods(useElectionID bool) (*apiv1.PodList, error) {
+	podLabel := make(map[string]string)
+
+	if useElectionID {
+		// When using electionID, only look for pods with the electionID label
+		// This is more specific and will correctly identify pods belonging to the same controller group
+		// Note: This requires the electionID label to be set on the pods (done by helm chart)
+		podLabel[ElectionIDLabelKey] = s.Config.ElectionID
+	} else {
+		// As a standard, app.kubernetes.io are "reserved well-known" labels.
+		// In our case, we add those labels as identifiers of the Ingress
+		// deployment in this namespace, so we can select it as a set of Ingress instances.
+		// As those labels are also generated as part of a HELM deployment, we can be "safe" they
+		// cover 95% of the cases
+		for k, v := range k8s.IngressPodDetails.Labels {
+			// Skip labels that are frequently modified by deployment controllers
+			if k != "pod-template-hash" && k != "controller-revision-hash" && k != "pod-template-generation" {
+				podLabel[k] = v
+			}
+		}
+	}
+
+	return s.Client.CoreV1().Pods(k8s.IngressPodDetails.Namespace).List(
+		context.TODO(), 
+		metav1.ListOptions{
+			LabelSelector: labels.SelectorFromSet(podLabel).String(),
+		})
+}
+
 func (s *statusSync) runningAddresses() ([]v1.IngressLoadBalancerIngress, error) {
 	if s.PublishStatusAddress != "" {
 		re := regexp.MustCompile(`,\s*`)
@@ -191,9 +230,7 @@
 	}
 
 	// get information about all the pods running the ingress controller
-	pods, err := s.Client.CoreV1().Pods(k8s.IngressPodDetails.Namespace).List(context.TODO(), metav1.ListOptions{
-		LabelSelector: labels.SelectorFromSet(k8s.IngressPodDetails.Labels).String(),
-	})
+	pods, err := s.listControllerPods(false)
 	if err != nil {
 		return nil, err
 	}
@@ -230,21 +267,7 @@
 }
 
 func (s *statusSync) isRunningMultiplePods() bool {
-	// As a standard, app.kubernetes.io are "reserved well-known" labels.
-	// In our case, we add those labels as identifiers of the Ingress
-	// deployment in this namespace, so we can select it as a set of Ingress instances.
-	// As those labels are also generated as part of a HELM deployment, we can be "safe" they
-	// cover 95% of the cases
-	podLabel := make(map[string]string)
-	for k, v := range k8s.IngressPodDetails.Labels {
-		if k != "pod-template-hash" && k != "controller-revision-hash" && k != "pod-template-generation" {
-			podLabel[k] = v
-		}
-	}
-
-	pods, err := s.Client.CoreV1().Pods(k8s.IngressPodDetails.Namespace).List(context.TODO(), metav1.ListOptions{
-		LabelSelector: labels.SelectorFromSet(podLabel).String(),
-	})
+	pods, err := s.listControllerPods(s.UseElectionIDSelectorOnShutdown) // Use election ID-compatible labels if configured
 	if err != nil {
 		return false
 	}