From 12f94f15e6beb7c1489fdfd9a4ccc72bbf0b6513 Mon Sep 17 00:00:00 2001 From: Varsha Date: Wed, 26 May 2021 05:45:21 -0700 Subject: [PATCH] :bug: filter global namespace while looking for cluster scoped resources (#1520) * :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. * create a cluser scoped cache separately Signed-off-by: varshaprasad96 --- pkg/cache/multi_namespace_cache.go | 81 ++++++++++++++++++++++++--- pkg/internal/objectutil/objectutil.go | 8 ++- 2 files changed, 80 insertions(+), 9 deletions(-) diff --git a/pkg/cache/multi_namespace_cache.go b/pkg/cache/multi_namespace_cache.go index 7e3f67d8d4..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 { @@ -50,9 +50,15 @@ 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) + } + for _, ns := range namespaces { opts.Namespace = ns c, err := New(config, opts) @@ -61,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 } } @@ -73,6 +79,7 @@ type multiNamespaceCache struct { namespaceToCache map[string]Cache Scheme *runtime.Scheme RESTMapper meta.RESTMapper + clusterCache Cache } var _ Cache = &multiNamespaceCache{} @@ -80,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 { @@ -87,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 { @@ -99,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) @@ -111,6 +163,7 @@ func (c *multiNamespaceCache) Start(ctx context.Context) error { } }(ns, cache) } + <-ctx.Done() return nil } @@ -122,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 @@ -142,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] @@ -165,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 { 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) }