Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Panic in runnableGroup's Add and StopAndWait due to a data race #2756

Closed
pmalek opened this issue Apr 4, 2024 · 1 comment · Fixed by #2757
Closed

Panic in runnableGroup's Add and StopAndWait due to a data race #2756

pmalek opened this issue Apr 4, 2024 · 1 comment · Fixed by #2757
Assignees
@vincepri
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@pmalek
Copy link

pmalek commented Apr 4, 2024 •
edited
Loading

Problem statement

When running our test suite using envtest we've noticed a data race in controller-runtime's runnableGroup:

2024-04-03T21:14:32.9965940Z ==================
2024-04-03T21:14:32.9966429Z WARNING: DATA RACE
2024-04-03T21:14:32.9966932Z Write at 0x00c00228d3f0 by goroutine 19777:
2024-04-03T21:14:32.9967525Z   runtime.closechan()
2024-04-03T21:14:32.9968334Z       /opt/hostedtoolcache/go/1.21.6/x64/src/runtime/chan.go:357 +0x0
2024-04-03T21:14:32.9970073Z   sigs.k8s.io/controller-runtime/pkg/manager.(*controllerManager).engageStopProcedure.func3.(*runnableGroup).StopAndWait.func2.2()
2024-04-03T21:14:32.9972311Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/manager/runnable_group.go:275 +0x33
2024-04-03T21:14:32.9973457Z   runtime.deferreturn()
2024-04-03T21:14:32.9974396Z       /opt/hostedtoolcache/go/1.21.6/x64/src/runtime/panic.go:477 +0x30
2024-04-03T21:14:32.9975384Z   sync.(*Once).doSlow()
2024-04-03T21:14:32.9977007Z       /opt/hostedtoolcache/go/1.21.6/x64/src/sync/once.go:74 +0xf0
2024-04-03T21:14:32.9977847Z   sync.(*Once).Do()
2024-04-03T21:14:32.9978786Z       /opt/hostedtoolcache/go/1.21.6/x64/src/sync/once.go:65 +0x44
2024-04-03T21:14:32.9980074Z   sigs.k8s.io/controller-runtime/pkg/manager.(*runnableGroup).StopAndWait()
2024-04-03T21:14:32.9981931Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/manager/runnable_group.go:273 +0x31c
2024-04-03T21:14:32.9983864Z   sigs.k8s.io/controller-runtime/pkg/manager.(*controllerManager).engageStopProcedure.func3()
2024-04-03T21:14:32.9985888Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/manager/internal.go:521 +0x214
2024-04-03T21:14:32.9986931Z 
2024-04-03T21:14:32.9987238Z Previous read at 0x00c00228d3f0 by goroutine 19310:
2024-04-03T21:14:32.9987932Z   runtime.chansend()
2024-04-03T21:14:32.9988770Z       /opt/hostedtoolcache/go/1.21.6/x64/src/runtime/chan.go:160 +0x0
2024-04-03T21:14:32.9990004Z   sigs.k8s.io/controller-runtime/pkg/manager.(*runnableGroup).Add()
2024-04-03T21:14:32.9991783Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/manager/runnable_group.go:267 +0x33e
2024-04-03T21:14:32.9993268Z   sigs.k8s.io/controller-runtime/pkg/manager.(*runnables).Add()
2024-04-03T21:14:32.9995034Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/manager/runnable_group.go:69 +0x204
2024-04-03T21:14:32.9996595Z   sigs.k8s.io/controller-runtime/pkg/manager.(*controllerManager).add()
2024-04-03T21:14:32.9998427Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/manager/internal.go:179 +0xd8
2024-04-03T21:14:33.0001987Z   sigs.k8s.io/controller-runtime/pkg/manager.(*controllerManager).Add()
2024-04-03T21:14:33.0004388Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/manager/internal.go:175 +0xd9
2024-04-03T21:14:33.0006162Z   sigs.k8s.io/controller-runtime/pkg/controller.New()
2024-04-03T21:14:33.0008478Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/controller/controller.go:100 +0x118
2024-04-03T21:14:33.0011269Z   github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/gateway.(*GatewayClassReconciler).SetupWithManager()
2024-04-03T21:14:33.0015214Z       /home/runner/work/kubernetes-ingress-controller/kubernetes-ingress-controller/internal/controllers/gateway/gatewayclass_controller.go:68 +0x224
2024-04-03T21:14:33.0018578Z   github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/gateway.(*GatewayReconciler).SetupWithManager()
2024-04-03T21:14:33.0022029Z       /home/runner/work/kubernetes-ingress-controller/kubernetes-ingress-controller/internal/controllers/gateway/gateway_controller.go:162 +0x1ff9
2024-04-03T21:14:33.0024976Z   github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/crds.(*DynamicCRDController).setupController()
2024-04-03T21:14:33.0028158Z       /home/runner/work/kubernetes-ingress-controller/kubernetes-ingress-controller/internal/controllers/crds/dynamic_controller.go:130 +0x132
2024-04-03T21:14:33.0030878Z   github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/crds.(*DynamicCRDController).Reconcile.func1()
2024-04-03T21:14:33.0033986Z       /home/runner/work/kubernetes-ingress-controller/kubernetes-ingress-controller/internal/controllers/crds/dynamic_controller.go:93 +0x91
2024-04-03T21:14:33.0035800Z   sync.(*Once).doSlow()
2024-04-03T21:14:33.0036747Z       /opt/hostedtoolcache/go/1.21.6/x64/src/sync/once.go:74 +0xf0
2024-04-03T21:14:33.0037614Z   sync.(*Once).Do()
2024-04-03T21:14:33.0038511Z       /opt/hostedtoolcache/go/1.21.6/x64/src/sync/once.go:65 +0x44
2024-04-03T21:14:33.0040653Z   github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/crds.(*DynamicCRDController).Reconcile()
2024-04-03T21:14:33.0043505Z       /home/runner/work/kubernetes-ingress-controller/kubernetes-ingress-controller/internal/controllers/crds/dynamic_controller.go:91 +0x6be
2024-04-03T21:14:33.0045865Z   sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile()
2024-04-03T21:14:33.0047869Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/internal/controller/controller.go:119 +0x1a1
2024-04-03T21:14:33.0049837Z   sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler()
2024-04-03T21:14:33.0051849Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/internal/controller/controller.go:316 +0x58d
2024-04-03T21:14:33.0053574Z   sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem()
2024-04-03T21:14:33.0055588Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/internal/controller/controller.go:266 +0x350
2024-04-03T21:14:33.0057487Z   sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2()
2024-04-03T21:14:33.0059425Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/internal/controller/controller.go:227 +0xd5
2024-04-03T21:14:33.0060410Z 
2024-04-03T21:14:33.0060605Z Goroutine 19777 (running) created at:
2024-04-03T21:14:33.0061715Z   sigs.k8s.io/controller-runtime/pkg/manager.(*controllerManager).engageStopProcedure()
2024-04-03T21:14:33.0063495Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/manager/internal.go:514 +0x59e
2024-04-03T21:14:33.0064982Z   sigs.k8s.io/controller-runtime/pkg/manager.(*controllerManager).Start.func2()
2024-04-03T21:14:33.0066685Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/manager/internal.go:344 +0x59
2024-04-03T21:14:33.0067726Z   runtime.deferreturn()
2024-04-03T21:14:33.0068602Z       /opt/hostedtoolcache/go/1.21.6/x64/src/runtime/panic.go:477 +0x30
2024-04-03T21:14:33.0070239Z   github.com/kong/kubernetes-ingress-controller/v3/internal/manager.Run()
2024-04-03T21:14:33.0072203Z       /home/runner/work/kubernetes-ingress-controller/kubernetes-ingress-controller/internal/manager/run.go:340 +0x57e5
2024-04-03T21:14:33.0073966Z   github.com/kong/kubernetes-ingress-controller/v3/test/envtest.RunManager.func1()
2024-04-03T21:14:33.0075955Z       /home/runner/work/kubernetes-ingress-controller/kubernetes-ingress-controller/test/envtest/run.go:223 +0x2bd
2024-04-03T21:14:33.0076936Z 
2024-04-03T21:14:33.0077159Z Goroutine 19310 (running) created at:
2024-04-03T21:14:33.0078314Z   sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2()
2024-04-03T21:14:33.0080615Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/internal/controller/controller.go:223 +0x7f9
2024-04-03T21:14:33.0082425Z   sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start()
2024-04-03T21:14:33.0084420Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/internal/controller/controller.go:234 +0x2c4
2024-04-03T21:14:33.0086100Z   sigs.k8s.io/controller-runtime/pkg/manager.(*runnableGroup).reconcile.func1()
2024-04-03T21:14:33.0087945Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/manager/runnable_group.go:223 +0x1ce
2024-04-03T21:14:33.0089516Z   sigs.k8s.io/controller-runtime/pkg/manager.(*runnableGroup).reconcile.func2()
2024-04-03T21:14:33.0091340Z       /home/runner/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/manager/runnable_group.go:226 +0x41
2024-04-03T21:14:33.0092423Z
==================

Which apparently happens between https://github.com/kubernetes-sigs/controller-runtime/blob/v0.17.2/pkg/manager/runnable_group.go#L267 and https://github.com/kubernetes-sigs/controller-runtime/blob/v0.17.2/pkg/manager/runnable_group.go#L267

Full test suite logs: envtest.log

/kind bug

Expected behaviour

No data race.
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Apr 4, 2024
@vincepri
Copy link
Member

vincepri commented Apr 4, 2024

/assign

was able to reproduce with a test

@vincepri vincepri mentioned this issue Apr 4, 2024
Merged
@pmalek pmalek mentioned this issue Apr 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vincepri vincepri

Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

🐛 Runnable group should check if stopped before enqueueing vincepri/controller-runtime
3 participants
@pmalek @vincepri @k8s-ci-robot