From 50bfd90b0bdd922b2e4bee38cb76ecc0dbe7ee2d Mon Sep 17 00:00:00 2001 From: varshaprasad96 Date: Sat, 8 May 2021 11:04:13 -0700 Subject: [PATCH 1/2] :bug: filter global namespace while looking for cluster scoped resources In the current implementation of multinamespaced cache, we look at global namespace while listing objects from all namespaced. Global namespace should not be looked at while fetching namespaced resources. --- pkg/cache/multi_namespace_cache.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/pkg/cache/multi_namespace_cache.go b/pkg/cache/multi_namespace_cache.go index 7e3f67d8d4..ed37b6dd7f 100644 --- a/pkg/cache/multi_namespace_cache.go +++ b/pkg/cache/multi_namespace_cache.go @@ -50,9 +50,18 @@ func MultiNamespacedCacheBuilder(namespaces []string) NewCacheFunc { if err != nil { return nil, err } - // create a cache for cluster scoped resources - namespaces = append(namespaces, globalCache) + caches := map[string]Cache{} + + // create a cache for cluster scoped resources + gCache, err := New(config, opts) + if err != nil { + return nil, fmt.Errorf("error creating global cache %v", err) + } + + // add global cache to the cacheMap + caches[globalCache] = gCache + for _, ns := range namespaces { opts.Namespace = ns c, err := New(config, opts) @@ -190,7 +199,10 @@ func (c *multiNamespaceCache) List(ctx context.Context, list client.ObjectList, limitSet := listOpts.Limit > 0 var resourceVersion string - for _, cache := range c.namespaceToCache { + for ns, cache := range c.namespaceToCache { + if ns == globalCache { + continue + } listObj := list.DeepCopyObject().(client.ObjectList) err = cache.List(ctx, listObj, &listOpts) if err != nil { From 201ab803b53c1bbc261981d0690cdac32736409b Mon Sep 17 00:00:00 2001 From: varshaprasad96 Date: Mon, 24 May 2021 19:12:18 -0700 Subject: [PATCH 2/2] create a cluser scoped cache separately Signed-off-by: varshaprasad96 --- pkg/cache/multi_namespace_cache.go | 79 ++++++++++++++++++++++----- pkg/internal/objectutil/objectutil.go | 8 ++- 2 files changed, 73 insertions(+), 14 deletions(-) diff --git a/pkg/cache/multi_namespace_cache.go b/pkg/cache/multi_namespace_cache.go index ed37b6dd7f..f3520bf8d7 100644 --- a/pkg/cache/multi_namespace_cache.go +++ b/pkg/cache/multi_namespace_cache.go @@ -41,7 +41,7 @@ const globalCache = "_cluster-scope" // MultiNamespacedCacheBuilder - Builder function to create a new multi-namespaced cache. // This will scope the cache to a list of namespaces. Listing for all namespaces // will list for all the namespaces that this knows about. By default this will create -// a global cache for cluster scoped resource (having empty namespace). Note that this is not intended +// a global cache for cluster scoped resource. Note that this is not intended // to be used for excluding namespaces, this is better done via a Predicate. Also note that // you may face performance issues when using this with a high number of namespaces. func MultiNamespacedCacheBuilder(namespaces []string) NewCacheFunc { @@ -59,9 +59,6 @@ func MultiNamespacedCacheBuilder(namespaces []string) NewCacheFunc { return nil, fmt.Errorf("error creating global cache %v", err) } - // add global cache to the cacheMap - caches[globalCache] = gCache - for _, ns := range namespaces { opts.Namespace = ns c, err := New(config, opts) @@ -70,7 +67,7 @@ func MultiNamespacedCacheBuilder(namespaces []string) NewCacheFunc { } caches[ns] = c } - return &multiNamespaceCache{namespaceToCache: caches, Scheme: opts.Scheme, RESTMapper: opts.Mapper}, nil + return &multiNamespaceCache{namespaceToCache: caches, Scheme: opts.Scheme, RESTMapper: opts.Mapper, clusterCache: gCache}, nil } } @@ -82,6 +79,7 @@ type multiNamespaceCache struct { namespaceToCache map[string]Cache Scheme *runtime.Scheme RESTMapper meta.RESTMapper + clusterCache Cache } var _ Cache = &multiNamespaceCache{} @@ -89,6 +87,23 @@ var _ Cache = &multiNamespaceCache{} // Methods for multiNamespaceCache to conform to the Informers interface func (c *multiNamespaceCache) GetInformer(ctx context.Context, obj client.Object) (Informer, error) { informers := map[string]Informer{} + + // If the object is clusterscoped, get the informer from clusterCache, + // if not use the namespaced caches. + isNamespaced, err := objectutil.IsAPINamespaced(obj, c.Scheme, c.RESTMapper) + if err != nil { + return nil, err + } + if !isNamespaced { + clusterCacheInf, err := c.clusterCache.GetInformer(ctx, obj) + if err != nil { + return nil, err + } + informers[globalCache] = clusterCacheInf + + return &multiNamespaceInformer{namespaceToInformer: informers}, nil + } + for ns, cache := range c.namespaceToCache { informer, err := cache.GetInformer(ctx, obj) if err != nil { @@ -96,11 +111,29 @@ func (c *multiNamespaceCache) GetInformer(ctx context.Context, obj client.Object } informers[ns] = informer } + return &multiNamespaceInformer{namespaceToInformer: informers}, nil } func (c *multiNamespaceCache) GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind) (Informer, error) { informers := map[string]Informer{} + + // If the object is clusterscoped, get the informer from clusterCache, + // if not use the namespaced caches. + isNamespaced, err := objectutil.IsAPINamespacedWithGVK(gvk, c.Scheme, c.RESTMapper) + if err != nil { + return nil, err + } + if !isNamespaced { + clusterCacheInf, err := c.clusterCache.GetInformerForKind(ctx, gvk) + if err != nil { + return nil, err + } + informers[globalCache] = clusterCacheInf + + return &multiNamespaceInformer{namespaceToInformer: informers}, nil + } + for ns, cache := range c.namespaceToCache { informer, err := cache.GetInformerForKind(ctx, gvk) if err != nil { @@ -108,10 +141,20 @@ func (c *multiNamespaceCache) GetInformerForKind(ctx context.Context, gvk schema } informers[ns] = informer } + return &multiNamespaceInformer{namespaceToInformer: informers}, nil } func (c *multiNamespaceCache) Start(ctx context.Context) error { + // start global cache + go func() { + err := c.clusterCache.Start(ctx) + if err != nil { + log.Error(err, "cluster scoped cache failed to start") + } + }() + + // start namespaced caches for ns, cache := range c.namespaceToCache { go func(ns string, cache Cache) { err := cache.Start(ctx) @@ -120,6 +163,7 @@ func (c *multiNamespaceCache) Start(ctx context.Context) error { } }(ns, cache) } + <-ctx.Done() return nil } @@ -131,10 +175,24 @@ func (c *multiNamespaceCache) WaitForCacheSync(ctx context.Context) bool { synced = s } } + + // check if cluster scoped cache has synced + if !c.clusterCache.WaitForCacheSync(ctx) { + synced = false + } return synced } func (c *multiNamespaceCache) IndexField(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error { + isNamespaced, err := objectutil.IsAPINamespaced(obj, c.Scheme, c.RESTMapper) + if err != nil { + return nil + } + + if !isNamespaced { + return c.clusterCache.IndexField(ctx, obj, field, extractValue) + } + for _, cache := range c.namespaceToCache { if err := cache.IndexField(ctx, obj, field, extractValue); err != nil { return err @@ -151,8 +209,7 @@ func (c *multiNamespaceCache) Get(ctx context.Context, key client.ObjectKey, obj if !isNamespaced { // Look into the global cache to fetch the object - cache := c.namespaceToCache[globalCache] - return cache.Get(ctx, key, obj) + return c.clusterCache.Get(ctx, key, obj) } cache, ok := c.namespaceToCache[key.Namespace] @@ -174,8 +231,7 @@ func (c *multiNamespaceCache) List(ctx context.Context, list client.ObjectList, if !isNamespaced { // Look at the global cache to get the objects with the specified GVK - cache := c.namespaceToCache[globalCache] - return cache.List(ctx, list, opts...) + return c.clusterCache.List(ctx, list, opts...) } if listOpts.Namespace != corev1.NamespaceAll { @@ -199,10 +255,7 @@ func (c *multiNamespaceCache) List(ctx context.Context, list client.ObjectList, limitSet := listOpts.Limit > 0 var resourceVersion string - for ns, cache := range c.namespaceToCache { - if ns == globalCache { - continue - } + for _, cache := range c.namespaceToCache { listObj := list.DeepCopyObject().(client.ObjectList) err = cache.List(ctx, listObj, &listOpts) if err != nil { diff --git a/pkg/internal/objectutil/objectutil.go b/pkg/internal/objectutil/objectutil.go index baa62f0bfa..5264da3cc1 100644 --- a/pkg/internal/objectutil/objectutil.go +++ b/pkg/internal/objectutil/objectutil.go @@ -55,7 +55,13 @@ func IsAPINamespaced(obj runtime.Object, scheme *runtime.Scheme, restmapper apim return false, err } - restmapping, err := restmapper.RESTMapping(schema.GroupKind{Group: gvk.Group, Kind: gvk.Kind}) + return IsAPINamespacedWithGVK(gvk, scheme, restmapper) +} + +// IsAPINamespacedWithGVK returns true if the object having the provided +// GVK is namespace scoped. +func IsAPINamespacedWithGVK(gk schema.GroupVersionKind, scheme *runtime.Scheme, restmapper apimeta.RESTMapper) (bool, error) { + restmapping, err := restmapper.RESTMapping(schema.GroupKind{Group: gk.Group, Kind: gk.Kind}) if err != nil { return false, fmt.Errorf("failed to get restmapping: %w", err) }