kubernetes-ingress icon indicating copy to clipboard operation
kubernetes-ingress copied to clipboard

Identify potential memory leaks in our goroutines

Open shaun-nx opened this issue 10 months ago • 3 comments

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

shaun-nx avatar Apr 11 '24 09:04 shaun-nx

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!

github-actions[bot] avatar Apr 11 '24 09:04 github-actions[bot]

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() {

shaun-nx avatar Apr 16 '24 12:04 shaun-nx

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().

jjngx avatar Sep 13 '24 18:09 jjngx