Flakes in periodic-controller-runtime-test

[sbueringer]

I took a look at testgrid (https://testgrid.k8s.io/sig-api-machinery-kubebuilder#controller-runtime-periodic-main). In general, I think we're in a good state. But we have a few flakes that we should ideally fix.

*when context is cancelled

(The failures are not all at the same location, but I think the issues are similar)

Expected an error to have occurred.  Got:
    <nil>: nil
[FAILED] Expected an error to have occurred.  Got:
    <nil>: nil
In [It] at: /home/prow/go/src/github.com/kubernetes-sigs/controller-runtime/pkg/cache/cache_test.go:2097 @ 08/11/25 11:53:22.239

8/18/2025, 7:48:54 AM periodic-controller-runtime-test Cache Suite [It] Informer Cache without global DeepCopy Cache test as an Informer with structured objects should allow getting an informer by group/version/kind to be cancelled
8/15/2025, 7:48:51 PM periodic-controller-runtime-test Cache Suite [It] Informer Cache Cache test as a Reader with structured objects should return an error when context is cancelled
8/11/2025, 1:46:47 PM periodic-controller-runtime-test Cache Suite [It] Informer Cache without global DeepCopy Cache test as an Informer with structured objects should allow for get informer to be cancelled
8/6/2025, 7:44:38 AM periodic-controller-runtime-test Cache Suite [It] Informer Cache without global DeepCopy Cache test as a Reader with structured objects should return an error when context is cancelled

https://storage.googleapis.com/k8s-triage/index.html?job=periodic-controller-runtime-test#34b4162d9c125fe4f22d

A few others that occured only once in periodics in the last 2 weeks

https://storage.googleapis.com/k8s-triage/index.html?job=periodic-controller-runtime-test

[Okabe-Junya]

If my understanding is correct, it appears that cache.WaitForCacheSync is called in L316, which might cause i.Informer.HasSynced() to return true even when the context has been canceled.

I'm not sure if this will resolve all flaky tests, but it seems to be one of the potential causes.

controller-runtime/pkg/cache/internal/informers.go

Lines 316 to 318 in 9f93124

	if !cache.WaitForCacheSync(ctx.Done(), i.Informer.HasSynced) {
	return started, nil, apierrors.NewTimeoutError(fmt.Sprintf("failed waiting for %T Informer to sync", obj), 0)
	}

controller-runtime/pkg/cache/internal/informers.go

Lines 282 to 287 in 9f93124

	func (ip *Informers) WaitForCacheSync(ctx context.Context) bool {
	if !ip.waitForStarted(ctx) {
	return false
	}
	return cache.WaitForCacheSync(ctx.Done(), ip.getHasSyncedFuncs()...)
	}

[Okabe-Junya]

If needed, will create the PR to patch

[sbueringer]

cache.WaitForCacheSync calls the WaitForCacheSync func in the client-go/tools/cache package. But yeah I suspect that might just return without checking the context, e.g. if the informer has already synced. But I think if that's the issue it's not really a prod code issue. More a problem of the test

[Okabe-Junya]

...
But I think if that's the issue it's not really a prod code issue. More a problem of the test

Thanks. I see, in that case, we should fix the test code (rather than production).

I would like to look into this issue. but due to in a busy, may be next week 🙇
(If you are in a hurry, or if anyone else is interested, please go ahead and workon it right away.)

[sbueringer]

Sounds good, no rush from my side