[bug] Serial worker reloads can potentially block dynamic endpoint changes
What happened:
We have an ingress-nginx deployment that uses enable-serial-reloads to prevent too many nginx workers from spawning when many ingress changes are made within close proximity to one another. This configuration does not seem to have detailed documentation in the website docs, but you can see the docs in the code here: https://github.com/kubernetes/ingress-nginx/blob/d1dc3e827f818ee23a08af09e9a7be0b12af1736/internal/ingress/controller/config/config.go#L475-L480
I think the intention behind enable-serial-reloads is to prevent too many configuration reloads that require new nginx worker processes to be launched. However, this configuration option seems to also unintentionally prevent dynamic configuration reloads (reloads that do not require a new nginx worker process). This seems to be a bug, because it can lead to a situation where a particularly long nginx reload blocks all dynamic endpoint changes for the controller. If a deployment rollout happens during a reload that requires a new nginx worker process, the updated list of endpoints does not seem to be picked up by the controller until the reload is finished.
What you expected to happen:
If enable-serial-reloads is set to true, I expect reloads that require new nginx worker processes to be re-queued if another such reload is already in progress, but I expect for dynamic reloads to remain intact. Endpoint updates should still happen even during a particularly long reload.
Code path:
Given a pending change that requires a configuration reload, which also includes endpoint changes:
syncIngressesis called from the task queue: https://github.com/kubernetes/ingress-nginx/blob/d1dc3e827f818ee23a08af09e9a7be0b12af1736/internal/ingress/controller/controller.go#L175utilingress.IsDynamicConfigurationEnoughis used to determine if a nginx reload is needed: https://github.com/kubernetes/ingress-nginx/blob/d1dc3e827f818ee23a08af09e9a7be0b12af1736/internal/ingress/controller/controller.go#L195onUpdateis called: https://github.com/kubernetes/ingress-nginx/blob/d1dc3e827f818ee23a08af09e9a7be0b12af1736/internal/ingress/controller/controller.go#L207-L214- Inside
onUpdate, an error is thrown due to an nginx reload already happening: https://github.com/kubernetes/ingress-nginx/blob/dc3acbd7864816c464f3536f433aaabd3ea9a37e/internal/ingress/controller/nginx.go#L682-L685 - Thus, the dynamic configuration never happens, lower in the
syncIngressesfunction: https://github.com/kubernetes/ingress-nginx/blob/d1dc3e827f818ee23a08af09e9a7be0b12af1736/internal/ingress/controller/controller.go#L240
NGINX Ingress controller version (exec into the pod and run /nginx-ingress-controller --version): v1.12.0
Kubernetes version (use kubectl version):
Client Version: v1.31.3
Kustomize Version: v5.4.2
Server Version: v1.29.12-eks-2d5f260Environment:
- Cloud provider or hardware configuration: AWS EKS
- OS (e.g. from /etc/os-release): Amazon Linux 2
- Kernel (e.g.
uname -a):Linux ip-10-2-52-194.us-west-2.compute.internal 5.10.223-212.873.amzn2.x86_64 #1 SMP Wed Aug 7 16:53:32 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux - Install tools:
- Helm
Source: kubernetes/ingress-nginx