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.

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

test: Ignore waitqueue.waitingLoop goroutine leak #28557

Merged

Conversation

dylandreimerink
Copy link
Member

We use goleak in certain tests to ensure we do not leak goroutines which might cause unexpected behavior during tests or shutdown in production.

Normally we expect our own goroutines to leak, but since we started using the delayed workqueue, we also leak the waitqueue.waitingLoop. This goroutine does get a shutdown signal when hive stops, but it isn't guaranteed to have exited before hive does so it sometimes triggers in CI.

This PR adds and exception to all goleak invocation that use resource.Resource[T] to ignore the waitqueue.waitingLoop goroutine.

Fixes: #28477

Add workqueue.(delayingType).waitingLoop to goleak exception list

We use goleak in certain tests to ensure we do not leak goroutines which
might cause unexpected behavior during tests or shutdown in production.

Normally we expect our own goroutines to leak, but since we started
using the delayed workqueue, we also leak the waitqueue.waitingLoop.
This goroutine does get a shutdown signal when hive stops, but it isn't
guaranteed to have exited before hive does so it sometimes triggers in
CI.

This commit adds and exception to all goleak invocation that use
resource.Resource[T] to ignore the waitqueue.waitingLoop goroutine.

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
@dylandreimerink dylandreimerink added area/CI Continuous Integration testing issue or flake area/CI-improvement Topic or proposal to improve the Continuous Integration workflow release-note/misc This PR makes changes that have no direct user impact. ci/flake This is a known failure that occurs in the tree. Please investigate me! labels Oct 12, 2023
@dylandreimerink dylandreimerink requested review from a team as code owners October 12, 2023 10:05
@dylandreimerink
Copy link
Member Author

/test

Copy link
Contributor

@thorn3r thorn3r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hit this a couple times. thanks for fixing it!

@aditighag aditighag merged commit d7b50cc into cilium:main Oct 16, 2023
63 checks passed
@julianwiedmann
Copy link
Member

@dylandreimerink looks like v1.14 is also seeing hitting this (https://github.com/cilium/cilium/actions/runs/6850169361/job/18623961162). Any concern about backporting - and if so, how far?

@dylandreimerink
Copy link
Member Author

Had not considered this might impact older releases, but backporting this should be easy, adding the label now

@dylandreimerink dylandreimerink added the needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch label Nov 13, 2023
@gandro gandro mentioned this pull request Nov 15, 2023
6 tasks
@gandro gandro added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Nov 15, 2023
@github-actions github-actions bot added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake area/CI-improvement Topic or proposal to improve the Continuous Integration workflow backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. ci/flake This is a known failure that occurs in the tree. Please investigate me! release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
6 participants