🐛 Cache reader: Wait for cache sync when ReaderFailOnMissingInformer is true by troy0820 · Pull Request #3425 · kubernetes-sigs/controller-runtime

troy0820 · 2026-01-21T19:39:35Z

Resolves #3424

pkg/cache/informer_cache.go

sbueringer · 2026-01-22T07:15:37Z

pkg/cache/informer_cache.go

 		if !ok {
 			return false, nil, &ErrResourceNotCached{GVK: gvk}
 		}
+		if !cache.Informer.HasSynced() {


This might still differ too much from the behavior in Informers.Get

Maybe ic.readerFailOnMissingInformer should be passed into Informers.Get and handled there to make it easier to keep both branches in sync.

@alvaroaleman WDYT?

Yeah, we should use the same code to determine and do the syncing regardless of if readerFailOnMissingInformer is set or not, that it was added outside of that was basically the main mistake back then.

Yeah, we should use the same code to determine and do the syncing regardless of if readerFailOnMissingInformer is set or not, that it was added outside of that was basically the main mistake back then.

Wouldn't that change the test that is with it?

Describe("as a Reader", func() { Context("with structured objects", func() { It("should not be able to list objects that haven't been watched previously", func(ctx SpecContext) { By("listing all services in the cluster") listObj := &corev1.ServiceList{} Expect(errors.As(informerCache.List(ctx, listObj), &errNotCached)).To(BeTrue()) })

I am trying to understand the behavior we want because if we don't care whether this is set or not:

Should we fail if this is set?

Should we add to the informerMap and then wait for sync, then fail?

Assuming the above test is for a cache that has ReaderFailOnMissingInformer it needs to keep passing, yes. The only difference in Get between that option being set and not should be that we error out if the informer is not in the map already

Gotcha, that's how I understood it after I re-read the issue and the setting. I am dealing with a context cancelled test failure at the moment, that I am hunting down.

alvaroaleman · 2026-01-24T17:31:58Z

pkg/cache/internal/informers.go

 	i, started, ok := ip.Peek(gvk, obj)
 	if !ok {
+		if readerFailOnMissingInformer {
+			return false, nil, fmt.Errorf("failed to get informer for %v: informer not found", gvk)


This is wrong, this here needs to return ErrResourceNotCached. That in turn requires to move that to internal, something like this will do that without breaking the export:

+++ b/pkg/cache/informer_cache.go @@ -51,16 +51,7 @@ var _ error = (*ErrCacheNotStarted)(nil) // ErrResourceNotCached indicates that the resource type // the client asked the cache for is not cached, i.e. the // corresponding informer does not exist yet. -type ErrResourceNotCached struct { - GVK schema.GroupVersionKind -} - -// Error returns the error -func (r ErrResourceNotCached) Error() string { - return fmt.Sprintf("%s is not cached", r.GVK.String()) -} - -var _ error = (*ErrResourceNotCached)(nil) +type ErrResourceNotCached = internal.ErrResourceNotCached

alvaroaleman · 2026-01-24T17:32:47Z

pkg/cache/informer_cache.go

-		return started, cache, nil
+	started, cache, err := ic.Informers.Get(ctx, gvk, obj, ic.readerFailOnMissingInformer, &internal.GetOptions{})
+	if err != nil {
+		return false, nil, &ErrResourceNotCached{GVK: gvk}


This here needs to just return the error and not overwrite it with ErrResourceNotCached, as there is a range of other errors that Informers.Get may return

alvaroaleman · 2026-01-25T14:53:27Z

pkg/cache/informer_cache.go

 	return i.Informer, nil
 }

 func (ic *informerCache) getInformerForKind(ctx context.Context, gvk schema.GroupVersionKind, obj runtime.Object) (bool, *internal.Cache, error) {


Its not very obvious from the code but this is the only place where we need to pass ic.readerFailOnMissingInformer, in the two others we need to pass false.
This is because this here is only used by the cache reader whereas GetInformer and GetInformerForKind are methods on the cache. If those error out when readerFailOnMissingInformer is true, it becomes impossible to construct an informer at all

Signed-off-by: Troy Connor <troy0820@users.noreply.github.com>

k8s-ci-robot · 2026-01-25T15:41:59Z

@troy0820: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-controller-runtime-apidiff	`8a2b6fe`	link	false	`/test pull-controller-runtime-apidiff`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

alvaroaleman

thanks!

k8s-ci-robot · 2026-01-25T16:15:28Z

LGTM label has been added.

Details

Git tree hash: 04a492b074bb478257b0a0557e2adc0ea2fbe882

k8s-ci-robot · 2026-01-25T16:15:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, troy0820

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [alvaroaleman]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

alvaroaleman · 2026-01-25T16:15:41Z

/cherrypick release-0.23
/cherrypick release-0.22

k8s-infra-cherrypick-robot · 2026-01-25T16:15:44Z

@alvaroaleman: once the present PR merges, I will cherry-pick it on top of release-0.22, release-0.23 in new PRs and assign them to you.

Details

In response to this:

/cherrypick release-0.23
/cherrypick release-0.22

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-infra-cherrypick-robot · 2026-01-25T16:17:47Z

@alvaroaleman: new pull request created: #3432

Details

In response to this:

/cherrypick release-0.23
/cherrypick release-0.22

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-infra-cherrypick-robot · 2026-01-25T16:18:20Z

@alvaroaleman: new pull request created: #3433

Details

In response to this:

/cherrypick release-0.23
/cherrypick release-0.22

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot assigned alvaroaleman Jan 21, 2026

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 21, 2026

k8s-ci-robot requested review from inteon and vincepri January 21, 2026 19:39

k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 21, 2026

troy0820 force-pushed the troy0820/cache-bug branch from cd58e14 to 81fb5ab Compare January 21, 2026 19:55

troy0820 commented Jan 21, 2026

View reviewed changes

pkg/cache/informer_cache.go Outdated Show resolved Hide resolved

troy0820 force-pushed the troy0820/cache-bug branch from 81f3497 to 2a68a6b Compare January 21, 2026 22:02

sbueringer reviewed Jan 22, 2026

View reviewed changes

troy0820 force-pushed the troy0820/cache-bug branch from 2a68a6b to 651a146 Compare January 22, 2026 16:26

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 22, 2026

troy0820 force-pushed the troy0820/cache-bug branch from 651a146 to 7ae5df3 Compare January 23, 2026 19:48

alvaroaleman reviewed Jan 24, 2026

View reviewed changes

troy0820 force-pushed the troy0820/cache-bug branch from 7ae5df3 to 8ede9cf Compare January 25, 2026 05:05

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 25, 2026

alvaroaleman reviewed Jan 25, 2026

View reviewed changes

check to see if informer is synced and started before returning cache

8a2b6fe

Signed-off-by: Troy Connor <troy0820@users.noreply.github.com>

troy0820 force-pushed the troy0820/cache-bug branch from 8ede9cf to 8a2b6fe Compare January 25, 2026 15:37

alvaroaleman changed the title ~~🐛 Check to see if informer is synced and started before returning cache~~ 🐛 Cache reader: Wait for cache sync when ReaderFailOnMissingInformer is true Jan 25, 2026

alvaroaleman approved these changes Jan 25, 2026

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 25, 2026

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 25, 2026

k8s-ci-robot merged commit 82cc073 into kubernetes-sigs:main Jan 25, 2026
13 of 14 checks passed

k8s-infra-cherrypick-robot mentioned this pull request Jan 25, 2026

[release-0.22] 🐛 Cache reader: Wait for cache sync when ReaderFailOnMissingInformer is true #3432

Merged

k8s-infra-cherrypick-robot mentioned this pull request Jan 25, 2026

[release-0.23] 🐛 Cache reader: Wait for cache sync when ReaderFailOnMissingInformer is true #3433

Merged

Comments

Conversation

troy0820 commented Jan 21, 2026

Uh oh!

Uh oh!

sbueringer Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

alvaroaleman Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

troy0820 Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

alvaroaleman Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

troy0820 Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

alvaroaleman Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

alvaroaleman Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

alvaroaleman Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alvaroaleman left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jan 25, 2026

Uh oh!

k8s-ci-robot commented Jan 25, 2026

Uh oh!

alvaroaleman commented Jan 25, 2026

Uh oh!

k8s-infra-cherrypick-robot commented Jan 25, 2026

Uh oh!

Uh oh!

k8s-infra-cherrypick-robot commented Jan 25, 2026

Uh oh!

k8s-infra-cherrypick-robot commented Jan 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

alvaroaleman Jan 22, 2026 •

edited

Loading

k8s-ci-robot commented Jan 25, 2026 •

edited

Loading