nginxinc / kubernetes-ingress

NGINX and NGINX Plus Ingress Controllers for Kubernetes
https://docs.nginx.com/nginx-ingress-controller
Apache License 2.0
4.67k stars 1.97k forks source link

Identify potential memory leaks in our goroutines #5373

Open shaun-nx opened 7 months ago

shaun-nx commented 7 months ago

As a maintainer of the NGINX Ingress Controller, I would like to know if there are any potential memory leaks in our go routine so that the overall security posture of the product is better.

Resources: https://github.com/uber-go/goleak https://medium.com/@hatronix/title-demystifying-gos-runtime-numgoroutine-93fddbb67527#:~:text=NumGoroutine()%20is%20a%20simple,the%20concurrency%20of%20your%20application.

### UACs
- [x] Identify all go routines in our code base
- [ ] Create at least one integration test that targets each go routine
github-actions[bot] commented 7 months ago

Hi @shaun-nx 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!

shaun-nx commented 7 months ago

List of known goroutines

This will be a list of which .go file which contains a goroutune covers that goroutine with a test

File Total goroutines Goroutines covered by tests
cmd/nginx-ingress/main.go 7 0
internal/certmanager/cm_controller.go 4 0
internal/externalcnd/controller.go 2 0
internal/k8s/controller.go 15 0
internal/k8s/task_queue.go 1 0
internal/metrics/syslog_listener.go 1 0
internal/nginx/manager.go 4 0
internal/pkg/client/informers/externalversions/factory.go 1 0

Detailed list of known goroutines

kubernetes-ingress  (35 usages found)
            cmd/nginx-ingress  (7 usages found)
                main.go  (7 usages found)
                    90 go updateSelfWithVersionInfo(kubeClient, version, appProtectVersion, agentVersion, nginxVersion, 10, time.Second*5)
                    222 go func() {
                    230 go handleTermination(lbc, nginxManager, syslogListener, process)
                    722 go metrics.RunPrometheusListenerForNginxPlus(*prometheusMetricsListenPort, plusCollector, registry, prometheusSecret)
                    726 go metrics.RunPrometheusListenerForNginx(*prometheusMetricsListenPort, client, registry, constLabels, prometheusSecret)
                    734 go syslogListener.Run()
                    754 go healthcheck.RunHealthCheck(*serviceInsightListenPort, plusClient, cnf, serviceInsightSecret)
            internal/certmanager  (4 usages found)
                cm_controller.go  (4 usages found)
                    255 go c.runWorker(ctx)
                    266 go nsi.vsSharedInformerFactory.Start(nsi.stopCh)
                    267 go nsi.cmSharedInformerFactory.Start(nsi.stopCh)
                    268 go nsi.kubeSharedInformerFactory.Start(nsi.stopCh)
            internal/externaldns  (2 usages found)
                controller.go  (2 usages found)
                    132 go c.runWorker(ctx)
                    143 go nsi.sharedInformerFactory.Start(nsi.stopCh)
            internal/k8s  (16 usages found)
                controller.go  (15 usages found)
                    691 go lbc.namespaceWatcherController.Run(lbc.ctx.Done())
                    703 go func() { time.Sleep(time.Second * 30); timeoutch <- true }()
                    711 go func() {
                    724 go lbc.certManagerController.Run(lbc.ctx.Done())
                    727 go lbc.externalDNSController.Run(lbc.ctx.Done())
                    731 go lbc.leaderElector.Run(lbc.ctx)
                    735 go func(ctx context.Context) {
                    750 go lbc.configMapController.Run(lbc.ctx.Done())
                    754 go lbc.globalConfigurationController.Run(lbc.ctx.Done())
                    757 go lbc.ingressLinkInformer.Run(lbc.ctx.Done())
                    776 go lbc.syncQueue.Run(time.Second, lbc.ctx.Done())
                    790 go nsi.sharedInformerFactory.Start(nsi.stopCh)
                    793 go nsi.secretInformerFactory.Start(nsi.stopCh)
                    797 go nsi.confSharedInformerFactory.Start(nsi.stopCh)
                    801 go nsi.dynInformerFactory.Start(nsi.stopCh)
                task_queue.go  (1 usage found)
                    80 go func(t task, after time.Duration) {
            internal/metrics  (1 usage found)
                syslog_listener.go  (1 usage found)
                    52 go l.collector.RecordLatency(string(buffer[:n]))
            internal/nginx  (4 usages found)
                manager.go  (4 usages found)
                    305 go func() {
                    508 go func() {
                    569 go func() {
                    591 go func() {
            pkg/client/informers/externalversions  (1 usage found)
                factory.go  (1 usage found)
                    116 go func() {
jjngx commented 2 months ago

The concept behind goleak check is to compare number of goroutines at the start of the test suite, and at the end of all tests. If the number is the same, happy days. If it's not, it means we have potential problem. We could either leverage the goleak pkg, or add counter ourselves in the test.Main().