Helm chart: `controller.reportIngressStatus.leaderElectionLockName` is not auto-generated

[kyrofa]

The helm chart README contains the following line:

|controller.reportIngressStatus.leaderElectionLockName | 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 |

And indeed, this needs to be auto-generated in order to support multiple ingress controllers in the same namespace. However, it's actually hard-coded in the values.yaml to nginx-ingress-leader, which means by default you cannot install multiple ingress controllers in the same namespace.

It does look like the nginx-ingress.leaderElectionName definition does actually support auto-generating it, but the fact that the value is hard-coded means it's not done by default.

I propose updating the the default leaderElectionLockName value to be an empty string so that the README is telling the truth.

[github-actions[bot]]

Hi @kyrofa thanks for reporting!

Be sure to check out the docs and the Contributing Guidelines while you wait for a human to take a look at this :slightly_smiling_face:

Cheers!

[brianehlert]

Leader election lock name was invalidated when this project moved to using the leases API. This bit of code causes two leader election methods to be active at the same time and thus causes inappropriate load on the K8s API. The change that started this shift was released with 3.4 https://github.com/nginxinc/kubernetes-ingress/pull/4276

[kyrofa]

I'm not completely following. Are you saying we can actually remove this configmap altogether? Because the way it stands today, if you try to install two ingress controllers alongside each other, you get something along the lines of:

Error: rendered manifests contain a resource that already exists. Unable to continue with install: ConfigMap "nginx-ingress-leader" in namespace "nginx-ingress" exists and cannot be imported into the current release

[vepatel]

Hi @kyrofa, thanks for opening the issue. autogenerating name makes sense!

We're keeping the both objects i.e. old configmap and new lease type to provide some overlap after it was introduced in 3.4.0.

Now just leaving leaderElectionLockName as "" won't fix it as lease object directly reads it and doesn't have a template like configmap , so probably have to create a template like:

{{- if .Values.controller.reportIngressStatus.enableLeaderElection }}
apiVersion: coordination.k8s.io/v1
kind: Lease
metadata:
  name: {{ include "nginx-ingress.leaderElectionName" . }}
  namespace: {{ .Release.Namespace }}
  labels:
    {{- include "nginx-ingress.labels" . | nindent 4 }}
{{- end }}

We will be talking about this issue in our next community call, please feel free to join us on 6th May 3pm GMT (Zoom details on home page)

[danielnginx]

We should implement https://github.com/nginxinc/kubernetes-ingress/issues/5388 before fixing this issue.

[ksemele]

+1 do you plan to resolve this task in nearest future?

[shaun-nx]

Closing this issue Fix addressed in https://github.com/nginxinc/kubernetes-ingress/pull/6372