[Bug]: panic nil pointer dereference for endpoint slice when namespace is no longer watched

[abirdZendesk]

Version

5.5.4

We're also currently running edge for the latest vulnerability patches and this error still exists there

What Kubernetes platforms are you running on?

EKS Amazon

Steps to reproduce

Context

This bug triggers when a namespace that was being watched by an ingress controller is no longer watched by that controller. This can occur when using the watch-namespace-label flag and the label state of the namespace changes.

If there is still a sync task in the queue related to that namespace, the task will still be picked up and run, which expects an informer for that namespace to exist, however the informer for that namespace has already been cleaned up as a result of it no longer being watched. This results in the informer being nil causing the nil pointer dereference

In our clusters we are hosting engineers cloud development environments, where each namespace is an environment. We scale these environments up and down as needed, using a label to indicate their scale status. Our hope was to only watch the scaled up namespaces with the ingress controller, to reduce load on the controller allowing for faster boot and less resource consumption. However due to the nature of our environments changing state often and this bug, by adding the watch namespace label filter to the controller we cause the controller to infinitely crashloop.

E0827 15:50:39.229920       1 panic.go:336] "Observed a panic" panic="runtime error: invalid memory address or nil pointer dereference" panicGoValue="\"invalid memory address or nil pointer dereference\"" stacktrace=<
	goroutine 545 [running]:
	k8s.io/apimachinery/pkg/util/runtime.logPanic({0x2e5ec70, 0x52fd8139b0}, {0x27eed40, 0x4aaf250})
		pkg/mod/k8s.io/apimachinery@v0.36.4/pkg/util/runtime/runtime.go:134 +0xbc
	k8s.io/apimachinery/pkg/util/runtime.handleCrash({0x2e5f588, 0x52fbdc79d0}, {0x27eed40, 0x4aaf250}, {0x0, 0x0, 0x4508a0?})
		pkg/mod/k8s.io/apimachinery@v0.36.4/pkg/util/runtime/runtime.go:109 +0x116
	k8s.io/apimachinery/pkg/util/runtime.HandleCrashWithContext({0x2e5f588, 0x52fbdc79d0}, {0x0, 0x0, 0x0})
		pkg/mod/k8s.io/apimachinery@v0.36.4/pkg/util/runtime/runtime.go:80 +0x55
	panic({0x27eed40?, 0x4aaf250?})
		$GOROOT/src/runtime/panic.go:860 +0x13a
	github.com/nginx/kubernetes-ingress/internal/k8s.(*LoadBalancerController).syncEndpointSlices(0x52fbb47808, {0x2cc3089?, {0x52fdcd99c0?, 0x52fbfe9d40?}})
		/home/runner/work/kubernetes-ingress/kubernetes-ingress/internal/k8s/endpoint_slice.go:70 +0xc6
	github.com/nginx/kubernetes-ingress/internal/k8s.(*LoadBalancerController).sync(0x52fbb47808, {0x10?, {0x52fdcd99c0?, 0x52fbfe9d01?}})
		/home/runner/work/kubernetes-ingress/kubernetes-ingress/internal/k8s/controller.go:1282 +0x26f
	github.com/nginx/kubernetes-ingress/internal/k8s.(*taskQueue).worker(0x52fbc12ac0)
		/home/runner/work/kubernetes-ingress/kubernetes-ingress/internal/k8s/task_queue.go:99 +0x95
	k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1({0x52fba943f4?, 0x52fb824640?})
		pkg/mod/k8s.io/apimachinery@v0.36.4/pkg/util/wait/backoff.go:233 +0x13
	k8s.io/apimachinery/pkg/util/wait.BackoffUntilWithContext.func1({0x2e5f588?, 0x52fbdc79d0?}, 0x424c14?)
		pkg/mod/k8s.io/apimachinery@v0.36.4/pkg/util/wait/backoff.go:255 +0x51
	k8s.io/apimachinery/pkg/util/wait.BackoffUntilWithContext({0x2e5f588, 0x52fbdc79d0}, 0x52fbfe9f40, {0x2e32e80, 0x52fc018600}, 0x1)
		pkg/mod/k8s.io/apimachinery@v0.36.4/pkg/util/wait/backoff.go:256 +0xe5
	k8s.io/apimachinery/pkg/util/wait.BackoffUntil(0x0?, {0x2e32e80?, 0x52fc018600?}, 0x0?, 0x52fb8247a0?)
		pkg/mod/k8s.io/apimachinery@v0.36.4/pkg/util/wait/backoff.go:233 +0x46
	k8s.io/apimachinery/pkg/util/wait.JitterUntil(0x52fb8247c0, 0x3b9aca00, 0x0, 0x1, 0x52fbdc79d0)
		pkg/mod/k8s.io/apimachinery@v0.36.4/pkg/util/wait/backoff.go:210 +0x7f
	k8s.io/apimachinery/pkg/util/wait.Until(...)
		pkg/mod/k8s.io/apimachinery@v0.36.4/pkg/util/wait/backoff.go:163
	github.com/nginx/kubernetes-ingress/internal/k8s.(*taskQueue).Run(...)
		/home/runner/work/kubernetes-ingress/kubernetes-ingress/internal/k8s/task_queue.go:48
	created by github.com/nginx/kubernetes-ingress/internal/k8s.(*LoadBalancerController).Run in goroutine 1
		/home/runner/work/kubernetes-ingress/kubernetes-ingress/internal/k8s/controller.go:870 +0xae8
 >
panic: runtime error: invalid memory address or nil pointer dereference [recovered, repanicked]
[signal SIGSEGV: segmentation violation code=0x1 addr=0x70 pc=0x23e2dc6]

• Deletion of namespace informer on watch label change/removal -

  kubernetes-ingress/internal/k8s/namespace.go

  Lines 80 to 85 in 5e7bf9a

  	// Watched label for namespace was removed
  	// delete any now unwatched namespaced informer groups if required
  	nsi := lbc.getNamespacedInformer(key)
  	if nsi != nil {
  	lbc.cleanupUnwatchedNamespacedResources(nsi)
  	delete(lbc.namespacedInformers, key)

• The point at which the nil pointer error is thrown -

  kubernetes-ingress/internal/k8s/endpoint_slice.go

  Line 70 in 5e7bf9a

  obj, endpointSliceExists, err = lbc.getNamespacedInformer(ns).endpointSliceLister.GetByKey(key)

  • lbc.getNamespacedInformer(ns).endpointSliceLister.GetByKey(key) is the call here and lbc.getNamespacedInformer(ns) is what returns nil due to the informer having been deleted
• The getNamespacedInformer definition which returns nil if it does not exist -

  kubernetes-ingress/internal/k8s/controller.go

  Lines 858 to 860 in 5e7bf9a

  	if !exists {
  	// we are not watching this namespace
  	return nil

Setup

1. Deploy the ingress controller using the -watch-namespace-label flag, e.g -watch-namespace-label=watch-me=true
2. Create or label a namespace with that label watch-me: true
3. Create a deployment in that namespace scaled up (any replica count above 0)
4. Create a service with selectors matching that deployment
5. You should see an endpoint slice created for this service once the pod for the deployment is running

Trigger bug

1. Start a loop to scale down and back up the deployment in your watched namespace. This will cause updates to the endpoint slice, which enqueues the task where we see the panic
2. At the same time, run a loop to change the value of the watch label on the namespace, so this namespace becomes watched and unwatched by the controller
3. You should soon see the controller crashloop and in the crashed logs you will find panic="runtime error: invalid memory address or nil pointer dereference"
4. This is triggered by a race condition where a task is still in the queue after the namespace informer is deleted, hence the looping behaviour to recreate it

Loop to scale the deployment

while true; do
  kubectl -n {namespace} scale deployment {deployment} --replicas=0
  sleep 1
  kubectl -n {namespace} scale deployment {deployment} --replicas=1
  sleep 1
done

Loop to modify the namespace label

while true; do
  kubectl label ns {namespace} watch-me=false --overwrite
  sleep 1
  kubectl label ns {namespace} watch-me=true --overwrite
  sleep 1
done

[github-actions]

Hi @abirdZendesk 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 🙂

If your issue concerns any of the enterprise features please open a ticket with F5 directly (For NGINX Plus customers NGINX Ingress Controller (when used with NGINX Plus) is covered by the support contract.)

Cheers!

[abirdZendesk]

I've not had a chance to test it but I believe a simple fix for this would be changing the sync_endpoint_slices from

	ns, _, _ := cache.SplitMetaNamespaceKey(key)
	obj, endpointSliceExists, err = lbc.getNamespacedInformer(ns).endpointSliceLister.GetByKey(key)
	if err != nil {
		lbc.syncQueue.Requeue(task, err)
		return false
	}

to

	ns, _, _ := cache.SplitMetaNamespaceKey(key)
	nsi := lbc.getNamespacedInformer(ns)

	if nsi == nil {
		return false
	}

	obj, endpointSliceExists, err = nsi.endpointSliceLister.GetByKey(key)
	if err != nil {
		lbc.syncQueue.Requeue(task, err)
		return false
	}

So we can do a quick nil check on the namespace informer and return early from the task