From 9134dd3359af2e919c05328a2a7499b9764b00fe Mon Sep 17 00:00:00 2001 From: Derek Brown <6845676+DerekTBrown@users.noreply.github.com> Date: Thu, 29 May 2025 07:34:27 -0700 Subject: [PATCH 1/5] Fixes 11087 --- charts/ingress-nginx/Chart.yaml | 4 +- .../changelog/helm-chart-4.12.3.md | 9 + .../templates/controller-daemonset.yaml | 3 + .../templates/controller-deployment.yaml | 3 + .../tests/controller-daemonset_test.yaml | 27 +++ .../tests/controller-deployment_test.yaml | 24 ++ internal/ingress/controller/controller.go | 2 + internal/ingress/controller/nginx.go | 2 + internal/ingress/status/status.go | 59 +++-- internal/ingress/status/status_test.go | 217 ++++++++++++++++++ pkg/flags/flags.go | 3 + 11 files changed, 333 insertions(+), 20 deletions(-) create mode 100644 charts/ingress-nginx/changelog/helm-chart-4.12.3.md diff --git a/charts/ingress-nginx/Chart.yaml b/charts/ingress-nginx/Chart.yaml index 06faa41706..c8b402d98a 100644 --- a/charts/ingress-nginx/Chart.yaml +++ b/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 diff --git a/charts/ingress-nginx/changelog/helm-chart-4.12.3.md b/charts/ingress-nginx/changelog/helm-chart-4.12.3.md new file mode 100644 index 0000000000..551eed7252 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/charts/ingress-nginx/templates/controller-daemonset.yaml b/charts/ingress-nginx/templates/controller-daemonset.yaml index a9a3dee399..86ef46194b 100644 --- a/charts/ingress-nginx/templates/controller-daemonset.yaml +++ b/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 }} diff --git a/charts/ingress-nginx/templates/controller-deployment.yaml b/charts/ingress-nginx/templates/controller-deployment.yaml index 224694d1b3..d81ffb9b9d 100644 --- a/charts/ingress-nginx/templates/controller-deployment.yaml +++ b/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 }} diff --git a/charts/ingress-nginx/tests/controller-daemonset_test.yaml b/charts/ingress-nginx/tests/controller-daemonset_test.yaml index 9f79a3b23d..b511ed8afd 100644 --- a/charts/ingress-nginx/tests/controller-daemonset_test.yaml +++ b/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] diff --git a/charts/ingress-nginx/tests/controller-deployment_test.yaml b/charts/ingress-nginx/tests/controller-deployment_test.yaml index 37b6908853..7187e64aea 100644 --- a/charts/ingress-nginx/tests/controller-deployment_test.yaml +++ b/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] diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 0894c0dfc1..322bebab8e 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -143,6 +143,8 @@ type Configuration struct { DisableSyncEvents bool + UseElectionIDSelectorOnShutdown bool + EnableTopologyAwareRouting bool } diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 20fad5afb8..7646e65b69 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -152,6 +152,8 @@ func NewNGINXController(config *Configuration, mc metric.Collector) *NGINXContro 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)") diff --git a/internal/ingress/status/status.go b/internal/ingress/status/status.go index ef01cdd248..ce80e97459 100644 --- a/internal/ingress/status/status.go +++ b/internal/ingress/status/status.go @@ -40,6 +40,11 @@ import ( "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 @@ type Config struct { UseNodeInternalIP bool + UseElectionIDSelectorOnShutdown bool + + ElectionID string + IngressLister ingressLister } @@ -175,6 +184,36 @@ func nameOrIPToLoadBalancerIngress(nameOrIP string) v1.IngressLoadBalancerIngres // 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 @@ func (s *statusSync) runningAddresses() ([]v1.IngressLoadBalancerIngress, error) } // 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) runningAddresses() ([]v1.IngressLoadBalancerIngress, error) } 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 } diff --git a/internal/ingress/status/status_test.go b/internal/ingress/status/status_test.go index 01419708b3..a97a356a35 100644 --- a/internal/ingress/status/status_test.go +++ b/internal/ingress/status/status_test.go @@ -18,6 +18,7 @@ package status import ( "context" + "fmt" "reflect" "testing" "time" @@ -740,3 +741,219 @@ func TestIngressSliceEqual(t *testing.T) { } } } + +func TestListControllerPods(t *testing.T) { + testCases := []struct { + name string + useElectionIDSelector bool + electionID string + podLabels map[string]string + expectedLabelSelectorContains string + }{ + { + name: "use election ID selector", + useElectionIDSelector: true, + electionID: "test-election-id", + podLabels: map[string]string{ + "app": "ingress-nginx", + "pod-template-hash": "abc123", + ElectionIDLabelKey: "test-election-id", + }, + expectedLabelSelectorContains: ElectionIDLabelKey, + }, + { + name: "don't use election ID selector", + useElectionIDSelector: false, + electionID: "test-election-id", + podLabels: map[string]string{ + "app": "ingress-nginx", + "pod-template-hash": "abc123", + }, + expectedLabelSelectorContains: "app=ingress-nginx", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Create a client with a single pod + client := testclient.NewSimpleClientset( + &apiv1.PodList{Items: []apiv1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress-controller", + Namespace: apiv1.NamespaceDefault, + Labels: tc.podLabels, + }, + }, + }}, + ) + + // Store the old pod details and set new ones for testing + origPodDetails := k8s.IngressPodDetails + defer func() { k8s.IngressPodDetails = origPodDetails }() + k8s.IngressPodDetails = &k8s.PodInfo{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress-controller", + Namespace: apiv1.NamespaceDefault, + Labels: tc.podLabels, + }, + } + + // Create a status sync with our test configuration + st := &statusSync{ + Config: Config{ + Client: client, + UseElectionIDSelectorOnShutdown: tc.useElectionIDSelector, + ElectionID: tc.electionID, + }, + } + + // Call listControllerPods + podList, err := st.listControllerPods(tc.useElectionIDSelector) + if err != nil { + t.Fatalf("listControllerPods returned an error: %v", err) + } + + // Verify the result + if tc.useElectionIDSelector { + // Should have exactly one pod when electionID selector matches + if len(podList.Items) != 1 && tc.podLabels[ElectionIDLabelKey] == tc.electionID { + t.Errorf("expected 1 pod but got %d", len(podList.Items)) + } + // Should have no pods when electionID selector doesn't match + if len(podList.Items) != 0 && tc.podLabels[ElectionIDLabelKey] != tc.electionID { + t.Errorf("expected 0 pods but got %d", len(podList.Items)) + } + } else { + // Should have exactly one pod + if len(podList.Items) != 1 { + t.Errorf("expected 1 pod but got %d", len(podList.Items)) + } + } + }) + } +} + +// TestIsRunningMultiplePods tests the isRunningMultiplePods function with different label selectors +func TestIsRunningMultiplePods(t *testing.T) { + testCases := []struct { + name string + useElectionID bool + electionID string + podLabels []map[string]string + expectedMultiple bool + }{ + { + name: "multiple pods with same electionID", + useElectionID: true, + electionID: "test-election-id", + podLabels: []map[string]string{ + { + "app": "ingress-nginx", + "pod-template-hash": "abc123", + ElectionIDLabelKey: "test-election-id", + }, + { + "app": "ingress-nginx", + "pod-template-hash": "def456", + ElectionIDLabelKey: "test-election-id", + }, + }, + expectedMultiple: true, + }, + { + name: "multiple pods with different electionIDs", + useElectionID: true, + electionID: "test-election-id", + podLabels: []map[string]string{ + { + "app": "ingress-nginx", + "pod-template-hash": "abc123", + ElectionIDLabelKey: "test-election-id", + }, + { + "app": "ingress-nginx", + "pod-template-hash": "def456", + ElectionIDLabelKey: "other-election-id", + }, + }, + expectedMultiple: false, + }, + { + name: "single pod with matching electionID", + useElectionID: true, + electionID: "test-election-id", + podLabels: []map[string]string{ + { + "app": "ingress-nginx", + "pod-template-hash": "abc123", + ElectionIDLabelKey: "test-election-id", + }, + }, + expectedMultiple: false, + }, + { + name: "multiple pods when not using electionID", + useElectionID: false, + electionID: "test-election-id", + podLabels: []map[string]string{ + { + "app": "ingress-nginx", + "pod-template-hash": "abc123", + }, + { + "app": "ingress-nginx", + "pod-template-hash": "def456", + }, + }, + expectedMultiple: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Create pods for this test case + var pods []apiv1.Pod + for i, labels := range tc.podLabels { + pods = append(pods, apiv1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("ingress-controller-%d", i), + Namespace: apiv1.NamespaceDefault, + Labels: labels, + }, + Status: apiv1.PodStatus{ + Phase: apiv1.PodRunning, + }, + }) + } + + client := testclient.NewSimpleClientset(&apiv1.PodList{Items: pods}) + + // Store the old pod details and set new ones for testing + origPodDetails := k8s.IngressPodDetails + defer func() { k8s.IngressPodDetails = origPodDetails }() + k8s.IngressPodDetails = &k8s.PodInfo{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress-controller-0", + Namespace: apiv1.NamespaceDefault, + Labels: tc.podLabels[0], + }, + } + + // Create a status sync with our test configuration + st := &statusSync{ + Config: Config{ + Client: client, + UseElectionIDSelectorOnShutdown: tc.useElectionID, + ElectionID: tc.electionID, + }, + } + + // Test the isRunningMultiplePods method + result := st.isRunningMultiplePods() + if result != tc.expectedMultiple { + t.Errorf("isRunningMultiplePods returned %v but expected %v", result, tc.expectedMultiple) + } + }) + } +} diff --git a/pkg/flags/flags.go b/pkg/flags/flags.go index 376484649b..9ab4b0005f 100644 --- a/pkg/flags/flags.go +++ b/pkg/flags/flags.go @@ -232,6 +232,8 @@ Takes the form ":port". If not provided, no admission controller is starte disableSyncEvents = flags.Bool("disable-sync-events", false, "Disables the creation of 'Sync' event resources") + useElectionIDSelectorOnShutdown = flags.Bool("use-election-id-selector-on-shutdown", true, "Determine if other pods are running based on the electionID label, rather than all pod labels.") + enableTopologyAwareRouting = flags.Bool("enable-topology-aware-routing", false, "Enable topology aware routing feature, needs service object annotation service.kubernetes.io/topology-mode sets to auto or trafficDistribution.") ) @@ -377,6 +379,7 @@ https://blog.maxmind.com/2019/12/significant-changes-to-accessing-and-using-geol HealthCheckHost: *healthzHost, DynamicConfigurationRetries: *dynamicConfigurationRetries, EnableTopologyAwareRouting: *enableTopologyAwareRouting, + UseElectionIDSelectorOnShutdown: *useElectionIDSelectorOnShutdown, ListenPorts: &ngx_config.ListenPorts{ Default: *defServerPort, Health: *healthzPort, From e5b5eb5673d7905786e9cd2110ccb990e10e6eaf Mon Sep 17 00:00:00 2001 From: Derek Brown <6845676+DerekTBrown@users.noreply.github.com> Date: Thu, 29 May 2025 08:09:45 -0700 Subject: [PATCH 2/5] tweaks --- charts/ingress-nginx/README.md | 2 +- docs/user-guide/cli-arguments.md | 1 + internal/ingress/status/status.go | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/charts/ingress-nginx/README.md b/charts/ingress-nginx/README.md index c415c2b899..2237a84ede 100644 --- a/charts/ingress-nginx/README.md +++ b/charts/ingress-nginx/README.md @@ -2,7 +2,7 @@ [ingress-nginx](https://github.com/kubernetes/ingress-nginx) Ingress controller for Kubernetes using NGINX as a reverse proxy and load balancer -![Version: 4.12.2](https://img.shields.io/badge/Version-4.12.2-informational?style=flat-square) ![AppVersion: 1.12.2](https://img.shields.io/badge/AppVersion-1.12.2-informational?style=flat-square) +![Version: 4.12.3](https://img.shields.io/badge/Version-4.12.3-informational?style=flat-square) ![AppVersion: 1.12.2](https://img.shields.io/badge/AppVersion-1.12.2-informational?style=flat-square) To use, add `ingressClassName: nginx` spec field or the `kubernetes.io/ingress.class: nginx` annotation to your Ingress resources. diff --git a/docs/user-guide/cli-arguments.md b/docs/user-guide/cli-arguments.md index a33e75159b..d8ae56abb8 100644 --- a/docs/user-guide/cli-arguments.md +++ b/docs/user-guide/cli-arguments.md @@ -68,6 +68,7 @@ They are set in the container spec of the `ingress-nginx-controller` Deployment | `--udp-services-configmap` | Name of the ConfigMap containing the definition of the UDP services to expose. The key in the map indicates the external port to be used. The value is a reference to a Service in the form "namespace/name:port", where "port" can either be a port name or number. | | `--update-status` | Update the load-balancer status of Ingress objects this controller satisfies. Requires setting the publish-service parameter to a valid Service reference. (default true) | | `--update-status-on-shutdown` | Update the load-balancer status of Ingress objects when the controller shuts down. Requires the update-status parameter. (default true) | +| `--use-election-id-selector-on-shutdown` | Determine if other pods are running based on the electionID label, rather than all pod labels. When true, the controller looks for pods with the specific nginx.ingress.kubernetes.io/electionID label matching the controller's election ID. When false, it uses all standard Kubernetes app labels to identify other controller pods. (default true) | | `--shutdown-grace-period` | Seconds to wait after receiving the shutdown signal, before stopping the nginx process. (default 0) | | `--size-buckets` | Set of buckets which will be used for prometheus histogram metrics such as BytesSent. (default `[10, 100, 1000, 10000, 100000, 1e+06, 1e+07]`) | | `-v, --v Level` | number for the log level verbosity | diff --git a/internal/ingress/status/status.go b/internal/ingress/status/status.go index ce80e97459..2e35b0c8cd 100644 --- a/internal/ingress/status/status.go +++ b/internal/ingress/status/status.go @@ -230,7 +230,7 @@ func (s *statusSync) runningAddresses() ([]v1.IngressLoadBalancerIngress, error) } // get information about all the pods running the ingress controller - pods, err := s.listControllerPods(false) + pods, err := s.listControllerPods(s.UseElectionIDSelectorOnShutdown) if err != nil { return nil, err } From a0423d09bd44f94f930095664d0a0f2ec4df4aff Mon Sep 17 00:00:00 2001 From: Derek Brown <6845676+DerekTBrown@users.noreply.github.com> Date: Thu, 29 May 2025 08:11:26 -0700 Subject: [PATCH 3/5] revert changes to chart version --- charts/ingress-nginx/Chart.yaml | 4 ++-- charts/ingress-nginx/README.md | 2 +- charts/ingress-nginx/changelog/helm-chart-4.12.3.md | 9 --------- 3 files changed, 3 insertions(+), 12 deletions(-) delete mode 100644 charts/ingress-nginx/changelog/helm-chart-4.12.3.md diff --git a/charts/ingress-nginx/Chart.yaml b/charts/ingress-nginx/Chart.yaml index c8b402d98a..06faa41706 100644 --- a/charts/ingress-nginx/Chart.yaml +++ b/charts/ingress-nginx/Chart.yaml @@ -1,6 +1,6 @@ annotations: artifacthub.io/changes: | - - Add electionID label to controller pods when leader election is enabled + - Update Ingress-Nginx version controller-v1.12.2 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.3 +version: 4.12.2 diff --git a/charts/ingress-nginx/README.md b/charts/ingress-nginx/README.md index 2237a84ede..c415c2b899 100644 --- a/charts/ingress-nginx/README.md +++ b/charts/ingress-nginx/README.md @@ -2,7 +2,7 @@ [ingress-nginx](https://github.com/kubernetes/ingress-nginx) Ingress controller for Kubernetes using NGINX as a reverse proxy and load balancer -![Version: 4.12.3](https://img.shields.io/badge/Version-4.12.3-informational?style=flat-square) ![AppVersion: 1.12.2](https://img.shields.io/badge/AppVersion-1.12.2-informational?style=flat-square) +![Version: 4.12.2](https://img.shields.io/badge/Version-4.12.2-informational?style=flat-square) ![AppVersion: 1.12.2](https://img.shields.io/badge/AppVersion-1.12.2-informational?style=flat-square) To use, add `ingressClassName: nginx` spec field or the `kubernetes.io/ingress.class: nginx` annotation to your Ingress resources. diff --git a/charts/ingress-nginx/changelog/helm-chart-4.12.3.md b/charts/ingress-nginx/changelog/helm-chart-4.12.3.md deleted file mode 100644 index 551eed7252..0000000000 --- a/charts/ingress-nginx/changelog/helm-chart-4.12.3.md +++ /dev/null @@ -1,9 +0,0 @@ -# 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 \ No newline at end of file From 0c1b9fa93741c4cee26a6ad824af9befbaf515be Mon Sep 17 00:00:00 2001 From: Derek Brown <6845676+DerekTBrown@users.noreply.github.com> Date: Thu, 29 May 2025 08:48:21 -0700 Subject: [PATCH 4/5] fix fix --- charts/ingress-nginx/tests/controller-daemonset_test.yaml | 6 +++--- charts/ingress-nginx/tests/controller-deployment_test.yaml | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/charts/ingress-nginx/tests/controller-daemonset_test.yaml b/charts/ingress-nginx/tests/controller-daemonset_test.yaml index b511ed8afd..303a4e133d 100644 --- a/charts/ingress-nginx/tests/controller-daemonset_test.yaml +++ b/charts/ingress-nginx/tests/controller-daemonset_test.yaml @@ -215,7 +215,7 @@ tests: controller.disableLeaderElection: false asserts: - equal: - path: spec.template.metadata.labels.[nginx.ingress.kubernetes.io/electionID] + 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 @@ -225,7 +225,7 @@ tests: controller.electionID: custom-election-id asserts: - equal: - path: spec.template.metadata.labels.[nginx.ingress.kubernetes.io/electionID] + 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 @@ -234,4 +234,4 @@ tests: controller.disableLeaderElection: true asserts: - notExists: - path: spec.template.metadata.labels.[nginx.ingress.kubernetes.io/electionID] + path: spec.template.metadata.labels['nginx.ingress.kubernetes.io/electionID'] diff --git a/charts/ingress-nginx/tests/controller-deployment_test.yaml b/charts/ingress-nginx/tests/controller-deployment_test.yaml index 7187e64aea..a52f70ff02 100644 --- a/charts/ingress-nginx/tests/controller-deployment_test.yaml +++ b/charts/ingress-nginx/tests/controller-deployment_test.yaml @@ -237,7 +237,7 @@ tests: controller.disableLeaderElection: false asserts: - equal: - path: spec.template.metadata.labels.[nginx.ingress.kubernetes.io/electionID] + 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 @@ -246,7 +246,7 @@ tests: controller.electionID: custom-election-id asserts: - equal: - path: spec.template.metadata.labels.[nginx.ingress.kubernetes.io/electionID] + 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 @@ -254,4 +254,4 @@ tests: controller.disableLeaderElection: true asserts: - notExists: - path: spec.template.metadata.labels.[nginx.ingress.kubernetes.io/electionID] + path: spec.template.metadata.labels['nginx.ingress.kubernetes.io/electionID'] From 59e0c50db8df2c5d92de14a7b9d3386b5ac825eb Mon Sep 17 00:00:00 2001 From: Derek Brown <6845676+DerekTBrown@users.noreply.github.com> Date: Thu, 29 May 2025 09:56:19 -0700 Subject: [PATCH 5/5] fmt --- internal/ingress/controller/nginx.go | 14 +++---- internal/ingress/status/status.go | 7 ++-- internal/ingress/status/status_test.go | 57 ++++++++++++-------------- 3 files changed, 37 insertions(+), 41 deletions(-) diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 7646e65b69..8e96b4d151 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -146,14 +146,14 @@ func NewNGINXController(config *Configuration, mc metric.Collector) *NGINXContro if config.UpdateStatus { n.syncStatus = status.NewStatusSyncer(status.Config{ - Client: config.Client, - PublishService: config.PublishService, - PublishStatusAddress: config.PublishStatusAddress, - IngressLister: n.store, - UpdateStatusOnShutdown: config.UpdateStatusOnShutdown, - UseNodeInternalIP: config.UseNodeInternalIP, + Client: config.Client, + PublishService: config.PublishService, + PublishStatusAddress: config.PublishStatusAddress, + IngressLister: n.store, + UpdateStatusOnShutdown: config.UpdateStatusOnShutdown, + UseNodeInternalIP: config.UseNodeInternalIP, UseElectionIDSelectorOnShutdown: config.UseElectionIDSelectorOnShutdown, - ElectionID: config.ElectionID, + ElectionID: config.ElectionID, }) } else { klog.Warning("Update of Ingress status is disabled (flag --update-status)") diff --git a/internal/ingress/status/status.go b/internal/ingress/status/status.go index 2e35b0c8cd..b7ceb6e1b0 100644 --- a/internal/ingress/status/status.go +++ b/internal/ingress/status/status.go @@ -187,12 +187,12 @@ func nameOrIPToLoadBalancerIngress(nameOrIP string) v1.IngressLoadBalancerIngres // 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 + podLabel[ElectionIDLabelKey] = s.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 @@ -206,9 +206,8 @@ func (s *statusSync) listControllerPods(useElectionID bool) (*apiv1.PodList, err } } } - return s.Client.CoreV1().Pods(k8s.IngressPodDetails.Namespace).List( - context.TODO(), + context.TODO(), metav1.ListOptions{ LabelSelector: labels.SelectorFromSet(podLabel).String(), }) diff --git a/internal/ingress/status/status_test.go b/internal/ingress/status/status_test.go index a97a356a35..7cbbd25247 100644 --- a/internal/ingress/status/status_test.go +++ b/internal/ingress/status/status_test.go @@ -755,9 +755,9 @@ func TestListControllerPods(t *testing.T) { useElectionIDSelector: true, electionID: "test-election-id", podLabels: map[string]string{ - "app": "ingress-nginx", - "pod-template-hash": "abc123", - ElectionIDLabelKey: "test-election-id", + "app": "ingress-nginx", + "pod-template-hash": "abc123", + ElectionIDLabelKey: "test-election-id", }, expectedLabelSelectorContains: ElectionIDLabelKey, }, @@ -766,7 +766,7 @@ func TestListControllerPods(t *testing.T) { useElectionIDSelector: false, electionID: "test-election-id", podLabels: map[string]string{ - "app": "ingress-nginx", + "app": "ingress-nginx", "pod-template-hash": "abc123", }, expectedLabelSelectorContains: "app=ingress-nginx", @@ -802,9 +802,9 @@ func TestListControllerPods(t *testing.T) { // Create a status sync with our test configuration st := &statusSync{ Config: Config{ - Client: client, + Client: client, UseElectionIDSelectorOnShutdown: tc.useElectionIDSelector, - ElectionID: tc.electionID, + ElectionID: tc.electionID, }, } @@ -824,11 +824,8 @@ func TestListControllerPods(t *testing.T) { if len(podList.Items) != 0 && tc.podLabels[ElectionIDLabelKey] != tc.electionID { t.Errorf("expected 0 pods but got %d", len(podList.Items)) } - } else { - // Should have exactly one pod - if len(podList.Items) != 1 { - t.Errorf("expected 1 pod but got %d", len(podList.Items)) - } + } else if len(podList.Items) != 1 { + t.Errorf("expected 1 pod but got %d", len(podList.Items)) } }) } @@ -837,11 +834,11 @@ func TestListControllerPods(t *testing.T) { // TestIsRunningMultiplePods tests the isRunningMultiplePods function with different label selectors func TestIsRunningMultiplePods(t *testing.T) { testCases := []struct { - name string - useElectionID bool - electionID string - podLabels []map[string]string - expectedMultiple bool + name string + useElectionID bool + electionID string + podLabels []map[string]string + expectedMultiple bool }{ { name: "multiple pods with same electionID", @@ -849,14 +846,14 @@ func TestIsRunningMultiplePods(t *testing.T) { electionID: "test-election-id", podLabels: []map[string]string{ { - "app": "ingress-nginx", + "app": "ingress-nginx", "pod-template-hash": "abc123", - ElectionIDLabelKey: "test-election-id", + ElectionIDLabelKey: "test-election-id", }, { - "app": "ingress-nginx", + "app": "ingress-nginx", "pod-template-hash": "def456", - ElectionIDLabelKey: "test-election-id", + ElectionIDLabelKey: "test-election-id", }, }, expectedMultiple: true, @@ -867,14 +864,14 @@ func TestIsRunningMultiplePods(t *testing.T) { electionID: "test-election-id", podLabels: []map[string]string{ { - "app": "ingress-nginx", + "app": "ingress-nginx", "pod-template-hash": "abc123", - ElectionIDLabelKey: "test-election-id", + ElectionIDLabelKey: "test-election-id", }, { - "app": "ingress-nginx", + "app": "ingress-nginx", "pod-template-hash": "def456", - ElectionIDLabelKey: "other-election-id", + ElectionIDLabelKey: "other-election-id", }, }, expectedMultiple: false, @@ -885,9 +882,9 @@ func TestIsRunningMultiplePods(t *testing.T) { electionID: "test-election-id", podLabels: []map[string]string{ { - "app": "ingress-nginx", + "app": "ingress-nginx", "pod-template-hash": "abc123", - ElectionIDLabelKey: "test-election-id", + ElectionIDLabelKey: "test-election-id", }, }, expectedMultiple: false, @@ -898,11 +895,11 @@ func TestIsRunningMultiplePods(t *testing.T) { electionID: "test-election-id", podLabels: []map[string]string{ { - "app": "ingress-nginx", + "app": "ingress-nginx", "pod-template-hash": "abc123", }, { - "app": "ingress-nginx", + "app": "ingress-nginx", "pod-template-hash": "def456", }, }, @@ -943,9 +940,9 @@ func TestIsRunningMultiplePods(t *testing.T) { // Create a status sync with our test configuration st := &statusSync{ Config: Config{ - Client: client, + Client: client, UseElectionIDSelectorOnShutdown: tc.useElectionID, - ElectionID: tc.electionID, + ElectionID: tc.electionID, }, }