tigerquoll opened a new pull request, #1061: URL: https://github.com/apache/yunikorn-k8shim/pull/1061
### What is this PR for? Shim half of YUNIKORN-3357 (core PR: apache/yunikorn-core#1124). Adds goroutine leak detection via [uber-go/goleak](https://github.com/uber-go/goleak) to all 18 test packages under `pkg/`, through a shared `pkg/common/leakcheck` helper mirroring the core design. - **Scoped exemptions**: only the genuinely multi-package entry (DRA resource-slice tracker) is shared; the 21 `pkg/shim`-specific entries are passed only from that package's `TestMain` via `leakcheck.ShimSchedulerOptions()`, so the other 17 packages run with a 1-entry list and the check binds tightly there - **Two-line test fix included**: `defer apiProvider.Stop()` at two `pkg/cache` call sites, which deletes the exemption those tests caused - `test/e2e` ginkgo suites deliberately not instrumented (live-cluster suites that leave informers running by design; documented in the package doc) - No production code modified; goleak was already a transitive dependency (`go.sum` unchanged) and becomes a direct test-only one ### Findings surfaced (separate JIRAs to follow) Suspected production defects, all reproduced and documented in the exemption comments: 1. `KubernetesShim.Stop()` stops only one of the two `doScheduling` loops — it sends a single value to the shared unbuffered `stopChan` instead of closing it; the surviving loop keeps scheduling on a stopped shim 2. `Stop()` after a failed `Run()` is a no-op (the select's `default:` branch), leaking the dispatcher and placeholder manager on every failed startup 3. `AsyncRMCallback.UpdateAllocation`'s AssumePod retry (30-step backoff) runs on the RM proxy event loop with no context/stop channel — a pod that cannot be assumed stalls all RM event handling and shutdown cannot interrupt it 4. yunikorn-core services are not restartable in-process (`EventSystemImpl.Stop()` is one-way), so `MockScheduler.stop()` cannot call `coreContext.StopAll()` without introducing an intermittent failure at `-count>1` — measured and documented in the exemption comments. The 17 inherited core-service exemptions burn down after a core-side fix ### What type of PR is it? * [ ] - Bug Fix * [x] - Improvement * [ ] - Feature * [ ] - Documentation * [ ] - Hot Fix * [ ] - Refactoring ### What is the Jira issue? https://issues.apache.org/jira/browse/YUNIKORN-3357 ### How should this be tested? - `go test ./pkg/... -tags deadlock -count=3`: 18/18 ok - `go test ./pkg/shim/ ./pkg/cache/ -tags deadlock -count=5`: ok (the racy packages) - `go test ./pkg/... -race -tags deadlock`: 18/18 ok, no data races - `golangci-lint run`, `make license-check`, `go vet`, `gofmt`: clean - The check was sanity-tested with a deliberately planted leak (caught, then removed) Generated by the Author with assistance from Claude Code -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
