CRS associated secret updates will not trigger CRS Reconcile

[Levi080513]

What steps did you take and what happened?

1. Update the secret associated with CRS.
2. CRS Controller will not trigger reconcile.

What did you expect to happen?

CRS Controller triggers Reconcile immediately.

Cluster API version

1.6.4

Kubernetes version

1.25.15

Anything else you would like to add?

I guess the problem may lie here:

cluster-api/main.go

Lines 306 to 317 in 6d400b5

	Cache: cache.Options{
	DefaultNamespaces: watchNamespaces,
	SyncPeriod: &syncPeriod,
	ByObject: map[client.Object]cache.ByObject{
	// Note: Only Secrets with the cluster name label are cached.
	// The default client of the manager won't use the cache for secrets at all (see Client.Cache.DisableFor).
	// The cached secrets will only be used by the secretCachingClient we create below.
	&corev1.Secret{}: {
	Label: clusterSecretCacheSelector,
	},
	},
	},

The secret CR cache filtered cluster.x-k8s.io/cluster-name label, This configuration is passed through the method manager.New => cluster.New => cache.New => cache.newCache => internal.NewInformers and finally takes effect the Informers object for secret CR.
When the ClusterResourceSetReconciler.SetupWithManager function is executed, we observe the secret CR.
And when the controller starts, it will execute the function manager.Cache.GetInformer => delegatingByGVKCache.GetInformer => informerCache.GetInformer => Informers.Get => Informers.addInformerToMap => Informers.newInformer Gets the informer object. Since the filtering configuration takes effect on the secret CR Informers Object, the final generated Informers Object will filter out all object events without the cluster.x-k8s.io/cluster-name" label.

https://github.com/kubernetes-sigs/controller-runtime/blob/dca5e8b2b00a1ab10a617f6c4a6fd0d40f9a974e/pkg/cache/internal/informers.go#L296-L354

And i test when the secret configured cluster.x-k8s.io/cluster-name label, if the secret updated, CRS Controller triggers Reconcile immediately.

Label(s) to be applied

/kind bug
One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels.

[k8s-ci-robot]

This issue is currently awaiting triage.

CAPI contributors will take a look as soon as possible, apply one of the triage/* labels and provide further guidance.

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.

[sbueringer]

Hm. I would have assumed that PartialObjectMeta is handled separately

[Levi080513]

Hm. I would have assumed that PartialObjectMeta is handled separately

But the selector filter is effective. Is there something wrong with my understanding?
https://github.com/kubernetes-sigs/controller-runtime/blob/dca5e8b2b00a1ab10a617f6c4a6fd0d40f9a974e/pkg/cache/internal/informers.go#L308-L324

[sbueringer]

No probably not. I just meant that was my initial assumption. I'll take a closer look soon

/assign

[sbueringer]

(@chrischdi Ah just remembered that we talked about this issue yesterday, let me know if you want to take it over)

[jessehu]

when the secret configured cluster.x-k8s.io/cluster-name label, if the secret updated, CRS Controller triggers Reconcile immediately.

Since CRS uses the cluster-name label selector, would it be reasonable to require the secrets used by CRS to have the same cluster-name label?

  clusterSelector:
    matchLabels:
      cluster.x-k8s.io/cluster-name:

[sbueringer]

It would work as a workaround, but seems not ideal because crs applies to multiple clusters

[chrischdi]

I created a PR which fixes this. To note: there had been other options too:

current state:

• in main.go: we configure the LabelSelector which configures the cache
  • this LabelSelector gets propagated to the informer's list/watch functions and is to filter the on server-side (APIServer)
  • this filters out all secrets which don't match the LabelSelector (which was done to optimize resource usage and because we don't want to have all secrets of the management cluster in our cache)
  • this config (LabelSelector) gets applied to the structured, unstructured and partialObjectMeta watches of for the type, so also to the WatchesMetadata for Secrets in the controller for ClusterResourceSets.

Fix:

1. (as implemented in 🐛 Use separate cache for partial metadata watches on secrets to include all secrets #10633): Use a custom cache when doing WatchesMetadata for secrets in the controller

• by that we don't inherit the above described LabelSelector. This is okay because we only watch for PartialObjectMetadata here anyway.

2. Drop the LabelSelector and use a predicate to filter instead + transform function to drop information

• this is not optimal because dropping the label selector would get effective for all Watches on Secrets (not only the one used for ClusterResourceSets).