Controller-runtime: It seems that controller-runtime closes its channel twice

[dulltz]

When I closed a controller, panic: close of closed channel is returned occasionally.
It seems that controller-runtime closes its channel twice.
Is it a bug of controller-runtime ?

E0527 19:09:23.200313  185967 runtime.go:69] Observed a panic: "close of closed channel" (close of closed channel)
/home/takahiro-tsuruda/go/src/github.com/cybozu-go/contour-plus/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:76
/home/takahiro-tsuruda/go/src/github.com/cybozu-go/contour-plus/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:65
/home/takahiro-tsuruda/go/src/github.com/cybozu-go/contour-plus/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:51
/usr/local/go/src/runtime/panic.go:522
/usr/local/go/src/runtime/chan.go:342
/home/takahiro-tsuruda/go/src/github.com/cybozu-go/contour-plus/vendor/k8s.io/client-go/util/workqueue/delaying_queue.go:139
/home/takahiro-tsuruda/go/src/github.com/cybozu-go/contour-plus/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:168
/home/takahiro-tsuruda/go/src/github.com/cybozu-go/contour-plus/vendor/sigs.k8s.io/controller-runtime/pkg/manager/internal.go:148
/usr/local/go/src/runtime/asm_amd64.s:1337
panic: close of closed channel [recovered]
        panic: close of closed channel

[adohe-zz]

@dulltz IIUC no need to close controller manually. What's ur case that need to close a controller?

[ymmt2005]

@adohe
We test controller-manager with different configurations.
In each test case, a controller-manager starts and stops.

Unfortunately, we got panics sometimes as @dulltz described.
This is an instance of such failure: https://circleci.com/gh/cybozu-go/contour-plus/53

Our test code is:
https://github.com/cybozu-go/contour-plus/blob/afd32e07a100bb55b5f20f211ad9e41f04446101/controllers/ingressroute_controller_test.go

Any help is appreciated. Thank you!

[adohe-zz]

@dulltz @ymmt2005 I take a deep look into this problem, imho the controller-runtime just ShutDown internal queue at once, potential cause could be add same controller to manager twice, thus the same work queue could be shutdown twice.

[dulltz]

@adohe
We noticed that our test codes' setup was wrong: the controller was added after the manager started.
I fixed the ordering, then it seems that our test code is stabilized.

Thank you for taking your time responding to my issue.

[adohe-zz]

but I did notice this issue when run CI, as u can see here: https://travis-ci.org/kubernetes-sigs/controller-runtime/builds/537689015?utm_source=github_status&utm_medium=notification, seems add controller after manager started will run into this issue.

[DirectXMan12]

That looks to be a green test run -- did the panic not cause a test failure

[mengqiy]

We noticed that our test codes' setup was wrong: the controller was added after the manager started.

Controller-runtime supports adding new controller after the manager has started. There seems to be a bug somewhere.

[DirectXMan12]

/kind bug

[mengqiy]

@dulltz @ymmt2005 what version of controller-runtime do you use?

[ymmt2005]

@mengqiy
It was v0.2.0-beta.1
https://github.com/cybozu-go/contour-plus/blob/afd32e07a100bb55b5f20f211ad9e41f04446101/go.mod#L25

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[DirectXMan12]

I believe this has been fixed. Please re-open if you observe it again