From 3e35cab1ea29145d8699259b079633a2b8cfc116 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Sun, 23 Jul 2023 20:05:57 -0700 Subject: [PATCH] :sparkles: Allow configuring more granular cache filtering This change implements most of the [cache options design][0], in particular it is now possible to: * Configure Namespaces per type * Configure filtering per namespace What is still missing is to allow configuring namespace-level default settings that will be used for all namespaces not explicitly configured. There is nothing in the way of doing that as a follow-up. The implementation slightly derivates from the design document in that the filter settings in `ByGVK` do not use the `Config` struct but instead are directly embedded. This allows us to not break compatibility and is more ergonomic to use. The implementation is based on a new and internal per-type delegating "meta cache". [0]: https://github.com/kubernetes-sigs/controller-runtime/blob/main/designs/cache_options.md --- examples/scratch-env/go.sum | 1 + go.mod | 3 +- go.sum | 2 + pkg/cache/cache.go | 254 ++++++++++++----- pkg/cache/cache_test.go | 390 ++++++++++++++++++++------- pkg/cache/defaulting_test.go | 290 ++++++++++++++++++++ pkg/cache/delegating_by_gvk_cache.go | 127 +++++++++ pkg/cache/internal/informers.go | 86 ++---- pkg/cache/multi_namespace_cache.go | 57 ++-- pkg/manager/example_test.go | 5 +- pkg/manager/manager.go | 4 +- pkg/manager/manager_test.go | 8 +- 12 files changed, 974 insertions(+), 253 deletions(-) create mode 100644 pkg/cache/defaulting_test.go create mode 100644 pkg/cache/delegating_by_gvk_cache.go diff --git a/examples/scratch-env/go.sum b/examples/scratch-env/go.sum index c96d3e7c3c..630ed4f474 100644 --- a/examples/scratch-env/go.sum +++ b/examples/scratch-env/go.sum @@ -1251,6 +1251,7 @@ golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e/go.mod h1:Kr81I6Kryrl9sr8s2FK3vxD90NdsKWRuOIl2O4CvYbA= +golang.org/x/exp v0.0.0-20220827204233-334a2380cb91 h1:tnebWN09GYg9OLPss1KXj8txwZc6X6uMr6VFdcGNbHw= golang.org/x/exp v0.0.0-20220827204233-334a2380cb91/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE= golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81/go.mod h1:ux5Hcp/YLpHSI86hEcLt0YII63i6oz57MZXIpbrjZUs= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= diff --git a/go.mod b/go.mod index a6f057706e..cd7987e638 100644 --- a/go.mod +++ b/go.mod @@ -9,12 +9,14 @@ require ( github.com/go-logr/logr v1.2.4 github.com/go-logr/zapr v1.2.4 github.com/google/go-cmp v0.5.9 + github.com/google/gofuzz v1.2.0 github.com/onsi/ginkgo/v2 v2.11.0 github.com/onsi/gomega v1.27.10 github.com/prometheus/client_golang v1.16.0 github.com/prometheus/client_model v0.4.0 go.uber.org/goleak v1.2.1 go.uber.org/zap v1.24.0 + golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e golang.org/x/sys v0.10.0 gomodules.xyz/jsonpatch/v2 v2.3.0 k8s.io/api v0.28.0-beta.0 @@ -40,7 +42,6 @@ require ( github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/google/gnostic-models v0.6.8 // indirect - github.com/google/gofuzz v1.2.0 // indirect github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 // indirect github.com/google/uuid v1.3.0 // indirect github.com/imdario/mergo v0.3.6 // indirect diff --git a/go.sum b/go.sum index 6a2bba6f76..c543cef7d2 100644 --- a/go.sum +++ b/go.sum @@ -128,6 +128,8 @@ go.uber.org/zap v1.24.0/go.mod h1:2kMP+WWQ8aoFoedH3T2sq6iJ2yDWpHbP0f6MQbS9Gkg= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e h1:+WEEuIdZHnUeJJmEUjyYC2gfUMj69yZXw17EnHg/otA= +golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e/go.mod h1:Kr81I6Kryrl9sr8s2FK3vxD90NdsKWRuOIl2O4CvYbA= golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index e849e16f72..33c5b52f60 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -22,8 +22,8 @@ import ( "net/http" "time" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -31,6 +31,7 @@ import ( "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" toolscache "k8s.io/client-go/tools/cache" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/cache/internal" "sigs.k8s.io/controller-runtime/pkg/client" @@ -144,36 +145,60 @@ type Options struct { // instead of `reconcile.Result{}`. SyncPeriod *time.Duration - // Namespaces restricts the cache's ListWatch to the desired namespaces - // Per default ListWatch watches all namespaces - Namespaces []string + // DefaultNamespaces maps namespace names to cache configs. If set, only + // the namespaces in here will be watched and it will by used to default + // ByObject.Namespaces for all objects if that is nil. + // + // The options in the Config that are nil will be defaulted from + // the respective Default* settings. + DefaultNamespaces map[string]Config - // DefaultLabelSelector will be used as a label selector for all object types - // unless they have a more specific selector set in ByObject. + // DefaultLabelSelector will be used as a label selector for all objects + // unless there is already one set in ByObject or DefaultNamespaces. DefaultLabelSelector labels.Selector // DefaultFieldSelector will be used as a field selector for all object types - // unless they have a more specific selector set in ByObject. + // unless there is already one set in ByObject or DefaultNamespaces. DefaultFieldSelector fields.Selector // DefaultTransform will be used as transform for all object types - // unless they have a more specific transform set in ByObject. + // unless there is already one set in ByObject or DefaultNamespaces. DefaultTransform toolscache.TransformFunc - // UnsafeDisableDeepCopy indicates not to deep copy objects during get or - // list objects for EVERY object. + // DefaultUnsafeDisableDeepCopy is the default for UnsafeDisableDeepCopy + // for everything that doesn't specify this. + // // Be very careful with this, when enabled you must DeepCopy any object before mutating it, // otherwise you will mutate the object in the cache. // - // This is a global setting for all objects, and can be overridden by the ByObject setting. - UnsafeDisableDeepCopy *bool + // This will be used for all object types, unless it is set in ByObject or + // DefaultNamespaces. + DefaultUnsafeDisableDeepCopy *bool // ByObject restricts the cache's ListWatch to the desired fields per GVK at the specified object. + // object, this will fall through to Default* settings. ByObject map[client.Object]ByObject } // ByObject offers more fine-grained control over the cache's ListWatch by object. type ByObject struct { + // Namespaces maps a namespace name to cache configs. If set, only the + // namespaces in this map will be cached. + // + // Settings in the map value that are unset will be defaulted. + // Use an empty value for the specific setting to prevent that. + // + // A nil map allows to default this to the cache's DefaultNamespaces setting. + // An empty map prevents this and means that all namespaces will be cached. + // + // The defaulting follows the following precedence order: + // 1. ByObject + // 2. DefaultNamespaces[namespace] + // 3. Default* + // + // This must be unset for cluster-scoped objects. + Namespaces map[string]Config + // Label represents a label selector for the object. Label labels.Selector @@ -194,48 +219,118 @@ type ByObject struct { UnsafeDisableDeepCopy *bool } +// Config describes all potential options for a given watch. +type Config struct { + // LabelSelector specifies a label selector. A nil value allows to + // default this. + // + // Set to labels.Everything() if you don't want this defaulted. + LabelSelector labels.Selector + + // FieldSelector specifics a field selector. A nil value allows to + // default this. + // + // Set to fields.Everything() if you don't want this defaulted. + FieldSelector fields.Selector + + // Transform specifies a transform func. A nil value allows to default + // this. + // + // Set to an empty func to prevent this: + // func(in interface{}) (interface{}, error) { return in, nil } + Transform toolscache.TransformFunc + + // UnsafeDisableDeepCopy specifies if List and Get requests against the + // cache should not DeepCopy. A nil value allows to default this. + UnsafeDisableDeepCopy *bool +} + // NewCacheFunc - Function for creating a new cache from the options and a rest config. type NewCacheFunc func(config *rest.Config, opts Options) (Cache, error) // New initializes and returns a new Cache. -func New(config *rest.Config, opts Options) (Cache, error) { - if len(opts.Namespaces) == 0 { - opts.Namespaces = []string{metav1.NamespaceAll} +func New(cfg *rest.Config, opts Options) (Cache, error) { + opts, err := defaultOpts(cfg, opts) + if err != nil { + return nil, err } - if len(opts.Namespaces) > 1 { - return newMultiNamespaceCache(config, opts) + + newCacheFunc := newCache(cfg, opts) + + var defaultCache Cache + if len(opts.DefaultNamespaces) > 0 { + defaultConfig := optionDefaultsToConfig(&opts) + defaultCache = newMultiNamespaceCache(newCacheFunc, opts.Scheme, opts.Mapper, opts.DefaultNamespaces, &defaultConfig) + } else { + defaultCache = newCacheFunc(optionDefaultsToConfig(&opts), corev1.NamespaceAll) } - opts, err := defaultOpts(config, opts) - if err != nil { - return nil, err + if len(opts.ByObject) == 0 { + return defaultCache, nil } - byGVK, err := convertToInformerOptsByGVK(opts.ByObject, opts.Scheme) - if err != nil { - return nil, err + delegating := &delegatingByGVKCache{ + scheme: opts.Scheme, + caches: make(map[schema.GroupVersionKind]Cache, len(opts.ByObject)), + defaultCache: defaultCache, + } + + for obj, config := range opts.ByObject { + gvk, err := apiutil.GVKForObject(obj, opts.Scheme) + if err != nil { + return nil, fmt.Errorf("failed to get GVK for type %T: %w", obj, err) + } + var cache Cache + if len(config.Namespaces) > 0 { + cache = newMultiNamespaceCache(newCacheFunc, opts.Scheme, opts.Mapper, config.Namespaces, nil) + } else { + cache = newCacheFunc(byObjectToConfig(config), corev1.NamespaceAll) + } + delegating.caches[gvk] = cache } - // Set the default selector and transform. - byGVK[schema.GroupVersionKind{}] = internal.InformersOptsByGVK{ - Selector: internal.Selector{ - Label: opts.DefaultLabelSelector, - Field: opts.DefaultFieldSelector, - }, + + return delegating, nil +} + +func optionDefaultsToConfig(opts *Options) Config { + return Config{ + LabelSelector: opts.DefaultLabelSelector, + FieldSelector: opts.DefaultFieldSelector, Transform: opts.DefaultTransform, - UnsafeDisableDeepCopy: opts.UnsafeDisableDeepCopy, + UnsafeDisableDeepCopy: opts.DefaultUnsafeDisableDeepCopy, + } +} + +func byObjectToConfig(byObject ByObject) Config { + return Config{ + LabelSelector: byObject.Label, + FieldSelector: byObject.Field, + Transform: byObject.Transform, + UnsafeDisableDeepCopy: byObject.UnsafeDisableDeepCopy, } +} - return &informerCache{ - scheme: opts.Scheme, - Informers: internal.NewInformers(config, &internal.InformersOpts{ - HTTPClient: opts.HTTPClient, - Scheme: opts.Scheme, - Mapper: opts.Mapper, - ResyncPeriod: *opts.SyncPeriod, - Namespace: opts.Namespaces[0], - ByGVK: byGVK, - }), - }, nil +type newCacheFunc func(config Config, namespace string) Cache + +func newCache(restConfig *rest.Config, opts Options) newCacheFunc { + return func(config Config, namespace string) Cache { + return &informerCache{ + scheme: opts.Scheme, + Informers: internal.NewInformers(restConfig, &internal.InformersOpts{ + HTTPClient: opts.HTTPClient, + Scheme: opts.Scheme, + Mapper: opts.Mapper, + ResyncPeriod: *opts.SyncPeriod, + Namespace: namespace, + Selector: internal.Selector{ + Label: config.LabelSelector, + Field: config.FieldSelector, + }, + Transform: config.Transform, + UnsafeDisableDeepCopy: pointer.BoolDeref(config.UnsafeDisableDeepCopy, false), + }), + } + } } func defaultOpts(config *rest.Config, opts Options) (Options, error) { @@ -262,6 +357,50 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) { } } + for namespace, cfg := range opts.DefaultNamespaces { + cfg = defaultConfig(cfg, optionDefaultsToConfig(&opts)) + opts.DefaultNamespaces[namespace] = cfg + } + + for obj, byObject := range opts.ByObject { + isNamespaced, err := apiutil.IsObjectNamespaced(obj, opts.Scheme, opts.Mapper) + if err != nil { + return opts, fmt.Errorf("failed to determine if %T is namespaced: %w", obj, err) + } + if !isNamespaced && byObject.Namespaces != nil { + return opts, fmt.Errorf("type %T is not namespaced, but its ByObject.Namespaces setting is not nil", obj) + } + + // Default the namespace-level configs first, because they need to use the undefaulted type-level config. + for namespace, config := range byObject.Namespaces { + // 1. Default from the undefaulted type-level config + config = defaultConfig(config, byObjectToConfig(byObject)) + + // 2. Default from the namespace-level config. This was defaulted from the global default config earlier, but + // might not have an entry for the current namespace. + if defaultNamespaceSettings, hasDefaultNamespace := opts.DefaultNamespaces[namespace]; hasDefaultNamespace { + config = defaultConfig(config, defaultNamespaceSettings) + } + + // 3. Default from the global defaults + config = defaultConfig(config, optionDefaultsToConfig(&opts)) + + byObject.Namespaces[namespace] = config + } + + defaultedConfig := defaultConfig(byObjectToConfig(byObject), optionDefaultsToConfig(&opts)) + byObject.Label = defaultedConfig.LabelSelector + byObject.Field = defaultedConfig.FieldSelector + byObject.Transform = defaultedConfig.Transform + byObject.UnsafeDisableDeepCopy = defaultedConfig.UnsafeDisableDeepCopy + + if byObject.Namespaces == nil { + byObject.Namespaces = opts.DefaultNamespaces + } + + opts.ByObject[obj] = byObject + } + // Default the resync period to 10 hours if unset if opts.SyncPeriod == nil { opts.SyncPeriod = &defaultSyncPeriod @@ -269,24 +408,19 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) { return opts, nil } -func convertToInformerOptsByGVK(in map[client.Object]ByObject, scheme *runtime.Scheme) (map[schema.GroupVersionKind]internal.InformersOptsByGVK, error) { - out := map[schema.GroupVersionKind]internal.InformersOptsByGVK{} - for object, byObject := range in { - gvk, err := apiutil.GVKForObject(object, scheme) - if err != nil { - return nil, err - } - if _, ok := out[gvk]; ok { - return nil, fmt.Errorf("duplicate cache options for GVK %v, cache.Options.ByObject has multiple types with the same GroupVersionKind", gvk) - } - out[gvk] = internal.InformersOptsByGVK{ - Selector: internal.Selector{ - Field: byObject.Field, - Label: byObject.Label, - }, - Transform: byObject.Transform, - UnsafeDisableDeepCopy: byObject.UnsafeDisableDeepCopy, - } +func defaultConfig(toDefault, defaultFrom Config) Config { + if toDefault.LabelSelector == nil { + toDefault.LabelSelector = defaultFrom.LabelSelector } - return out, nil + if toDefault.FieldSelector == nil { + toDefault.FieldSelector = defaultFrom.FieldSelector + } + if toDefault.Transform == nil { + toDefault.Transform = defaultFrom.Transform + } + if toDefault.UnsafeDisableDeepCopy == nil { + toDefault.UnsafeDisableDeepCopy = defaultFrom.UnsafeDisableDeepCopy + } + + return toDefault } diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index a23e11e73e..27c1c63cfe 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -119,13 +119,17 @@ var _ = Describe("Informer Cache", func() { }) var _ = Describe("Multi-Namespace Informer Cache", func() { CacheTest(cache.New, cache.Options{ - Namespaces: []string{testNamespaceOne, testNamespaceTwo, "default"}, + DefaultNamespaces: map[string]cache.Config{ + testNamespaceOne: {}, + testNamespaceTwo: {}, + "default": {}, + }, }) }) var _ = Describe("Informer Cache without global DeepCopy", func() { CacheTest(cache.New, cache.Options{ - UnsafeDisableDeepCopy: pointer.Bool(true), + DefaultUnsafeDisableDeepCopy: pointer.Bool(true), }) }) @@ -370,7 +374,9 @@ var _ = Describe("Cache with selectors", func() { opts := cache.Options{ DefaultFieldSelector: fields.OneTermEqualSelector("metadata.namespace", testNamespaceTwo), ByObject: map[client.Object]cache.ByObject{ - &corev1.ServiceAccount{}: {Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceOne)}, + &corev1.ServiceAccount{}: { + Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceOne), + }, }, } @@ -790,61 +796,89 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca } }) - It("should be able to restrict cache to a namespace", func() { - By("creating a namespaced cache") - namespacedCache, err := cache.New(cfg, cache.Options{Namespaces: []string{testNamespaceOne}}) - Expect(err).NotTo(HaveOccurred()) - - By("running the cache and waiting for it to sync") - go func() { - defer GinkgoRecover() - Expect(namespacedCache.Start(informerCacheCtx)).To(Succeed()) - }() - Expect(namespacedCache.WaitForCacheSync(informerCacheCtx)).To(BeTrue()) + cacheRestrictSubTests := []struct { + nameSuffix string + cacheOpts cache.Options + }{ + { + nameSuffix: "by using the per-gvk setting", + cacheOpts: cache.Options{ + ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: { + Namespaces: map[string]cache.Config{ + testNamespaceOne: {}, + }, + }, + }, + }, + }, + { + nameSuffix: "by using the global DefaultNamespaces setting", + cacheOpts: cache.Options{ + DefaultNamespaces: map[string]cache.Config{ + testNamespaceOne: {}, + }, + }, + }, + } - By("listing pods in all namespaces") - out := &unstructured.UnstructuredList{} - out.SetGroupVersionKind(schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "PodList", - }) - Expect(namespacedCache.List(context.Background(), out)).To(Succeed()) + for _, tc := range cacheRestrictSubTests { + It("should be able to restrict cache to a namespace "+tc.nameSuffix, func() { + By("creating a namespaced cache") + namespacedCache, err := cache.New(cfg, tc.cacheOpts) + Expect(err).NotTo(HaveOccurred()) + + By("running the cache and waiting for it to sync") + go func() { + defer GinkgoRecover() + Expect(namespacedCache.Start(informerCacheCtx)).To(Succeed()) + }() + Expect(namespacedCache.WaitForCacheSync(informerCacheCtx)).To(BeTrue()) + + By("listing pods in all namespaces") + out := &unstructured.UnstructuredList{} + out.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "PodList", + }) + Expect(namespacedCache.List(context.Background(), out)).To(Succeed()) - By("verifying the returned pod is from the watched namespace") - Expect(out.Items).NotTo(BeEmpty()) - Expect(out.Items).Should(HaveLen(2)) - for _, item := range out.Items { - Expect(item.GetNamespace()).To(Equal(testNamespaceOne)) - } - By("listing all nodes - should still be able to list a cluster-scoped resource") - nodeList := &unstructured.UnstructuredList{} - nodeList.SetGroupVersionKind(schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "NodeList", - }) - Expect(namespacedCache.List(context.Background(), nodeList)).To(Succeed()) + By("verifying the returned pod is from the watched namespace") + Expect(out.Items).NotTo(BeEmpty()) + Expect(out.Items).Should(HaveLen(2)) + for _, item := range out.Items { + Expect(item.GetNamespace()).To(Equal(testNamespaceOne)) + } + By("listing all nodes - should still be able to list a cluster-scoped resource") + nodeList := &unstructured.UnstructuredList{} + nodeList.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "NodeList", + }) + Expect(namespacedCache.List(context.Background(), nodeList)).To(Succeed()) - By("verifying the node list is not empty") - Expect(nodeList.Items).NotTo(BeEmpty()) + By("verifying the node list is not empty") + Expect(nodeList.Items).NotTo(BeEmpty()) - By("getting a node - should still be able to get a cluster-scoped resource") - node := &unstructured.Unstructured{} - node.SetGroupVersionKind(schema.GroupVersionKind{ - Group: "", - Version: "v1", - Kind: "Node", - }) + By("getting a node - should still be able to get a cluster-scoped resource") + node := &unstructured.Unstructured{} + node.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "Node", + }) - By("verifying that getting the node works with an empty namespace") - key1 := client.ObjectKey{Namespace: "", Name: testNodeOne} - Expect(namespacedCache.Get(context.Background(), key1, node)).To(Succeed()) + By("verifying that getting the node works with an empty namespace") + key1 := client.ObjectKey{Namespace: "", Name: testNodeOne} + Expect(namespacedCache.Get(context.Background(), key1, node)).To(Succeed()) - By("verifying that the namespace is ignored when getting a cluster-scoped resource") - key2 := client.ObjectKey{Namespace: "random", Name: testNodeOne} - Expect(namespacedCache.Get(context.Background(), key2, node)).To(Succeed()) - }) + By("verifying that the namespace is ignored when getting a cluster-scoped resource") + key2 := client.ObjectKey{Namespace: "random", Name: testNodeOne} + Expect(namespacedCache.Get(context.Background(), key2, node)).To(Succeed()) + }) + } if !isPodDisableDeepCopy(opts) { It("should deep copy the object unless told otherwise", func() { @@ -934,7 +968,10 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca It("test multinamespaced cache for cluster scoped resources", func() { By("creating a multinamespaced cache to watch specific namespaces") m, err := cache.New(cfg, cache.Options{ - Namespaces: []string{"default", testNamespaceOne}, + DefaultNamespaces: map[string]cache.Config{ + "default": {}, + testNamespaceOne: {}, + }, }) Expect(err).NotTo(HaveOccurred()) @@ -1074,7 +1111,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca It("should be able to restrict cache to a namespace", func() { By("creating a namespaced cache") - namespacedCache, err := cache.New(cfg, cache.Options{Namespaces: []string{testNamespaceOne}}) + namespacedCache, err := cache.New(cfg, cache.Options{DefaultNamespaces: map[string]cache.Config{testNamespaceOne: {}}}) Expect(err).NotTo(HaveOccurred()) By("running the cache and waiting for it to sync") @@ -1220,20 +1257,12 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca }) }) type selectorsTestCase struct { - fieldSelectors map[string]string - labelSelectors map[string]string - expectedPods []string + options cache.Options + expectedPods []string } DescribeTable(" and cache with selectors", func(tc selectorsTestCase) { By("creating the cache") - informer, err := cache.New(cfg, cache.Options{ - ByObject: map[client.Object]cache.ByObject{ - &corev1.Pod{}: { - Label: labels.Set(tc.labelSelectors).AsSelector(), - Field: fields.Set(tc.fieldSelectors).AsSelector(), - }, - }, - }) + informer, err := cache.New(cfg, tc.options) Expect(err).NotTo(HaveOccurred()) By("running the cache and waiting for it to sync") @@ -1289,38 +1318,215 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca }, ConsistOf(tc.expectedPods))) }, Entry("when selectors are empty it has to inform about all the pods", selectorsTestCase{ - fieldSelectors: map[string]string{}, - labelSelectors: map[string]string{}, - expectedPods: []string{"test-pod-1", "test-pod-2", "test-pod-3", "test-pod-4", "test-pod-5", "test-pod-6"}, + expectedPods: []string{"test-pod-1", "test-pod-2", "test-pod-3", "test-pod-4", "test-pod-5", "test-pod-6"}, + }), + Entry("type-level field selector matches one pod", selectorsTestCase{ + options: cache.Options{ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: {Field: fields.SelectorFromSet(map[string]string{ + "metadata.name": "test-pod-2", + })}, + }}, + expectedPods: []string{"test-pod-2"}, + }), + Entry("global field selector matches one pod", selectorsTestCase{ + options: cache.Options{ + DefaultFieldSelector: fields.SelectorFromSet(map[string]string{ + "metadata.name": "test-pod-2", + }), + }, + expectedPods: []string{"test-pod-2"}, + }), + Entry("type-level field selectors matches multiple pods", selectorsTestCase{ + options: cache.Options{ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: {Field: fields.SelectorFromSet(map[string]string{ + "metadata.namespace": testNamespaceTwo, + })}, + }}, + expectedPods: []string{"test-pod-2", "test-pod-3", "test-pod-6"}, + }), + Entry("global field selectors matches multiple pods", selectorsTestCase{ + options: cache.Options{ + DefaultFieldSelector: fields.SelectorFromSet(map[string]string{ + "metadata.namespace": testNamespaceTwo, + }), + }, + expectedPods: []string{"test-pod-2", "test-pod-3", "test-pod-6"}, + }), + Entry("type-level label selector matches one pod", selectorsTestCase{ + options: cache.Options{ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: {Label: labels.SelectorFromSet(map[string]string{ + "test-label": "test-pod-4", + })}, + }}, + expectedPods: []string{"test-pod-4"}, + }), + Entry("global label selector matches one pod", selectorsTestCase{ + options: cache.Options{ + DefaultLabelSelector: labels.SelectorFromSet(map[string]string{ + "test-label": "test-pod-4", + }), + }, + expectedPods: []string{"test-pod-4"}, + }), + Entry("type-level label selector matches multiple pods", selectorsTestCase{ + options: cache.Options{ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: {Label: labels.SelectorFromSet(map[string]string{ + "common-label": "common", + })}, + }}, + expectedPods: []string{"test-pod-3", "test-pod-4"}, + }), + Entry("global label selector matches multiple pods", selectorsTestCase{ + options: cache.Options{ + DefaultLabelSelector: labels.SelectorFromSet(map[string]string{ + "common-label": "common", + }), + }, + expectedPods: []string{"test-pod-3", "test-pod-4"}, + }), + Entry("type-level label and field selector, matches one pod", selectorsTestCase{ + options: cache.Options{ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: { + Label: labels.SelectorFromSet(map[string]string{"common-label": "common"}), + Field: fields.SelectorFromSet(map[string]string{"metadata.namespace": testNamespaceTwo}), + }, + }}, + expectedPods: []string{"test-pod-3"}, }), - Entry("when field matches one pod it has to inform about it", selectorsTestCase{ - fieldSelectors: map[string]string{"metadata.name": "test-pod-2"}, - expectedPods: []string{"test-pod-2"}, + Entry("global label and field selector, matches one pod", selectorsTestCase{ + options: cache.Options{ + DefaultLabelSelector: labels.SelectorFromSet(map[string]string{"common-label": "common"}), + DefaultFieldSelector: fields.SelectorFromSet(map[string]string{"metadata.namespace": testNamespaceTwo}), + }, + expectedPods: []string{"test-pod-3"}, }), - Entry("when field matches multiple pods it has to inform about all of them", selectorsTestCase{ - fieldSelectors: map[string]string{"metadata.namespace": testNamespaceTwo}, - expectedPods: []string{"test-pod-2", "test-pod-3", "test-pod-6"}, + Entry("type-level label selector does not match, no results", selectorsTestCase{ + options: cache.Options{ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: {Label: labels.SelectorFromSet(map[string]string{ + "new-label": "new", + })}, + }}, + expectedPods: []string{}, }), - Entry("when label matches one pod it has to inform about it", selectorsTestCase{ - labelSelectors: map[string]string{"test-label": "test-pod-4"}, - expectedPods: []string{"test-pod-4"}, + Entry("global label selector does not match, no results", selectorsTestCase{ + options: cache.Options{ + DefaultLabelSelector: labels.SelectorFromSet(map[string]string{ + "new-label": "new", + }), + }, + expectedPods: []string{}, }), - Entry("when label matches multiple pods it has to inform about all of them", selectorsTestCase{ - labelSelectors: map[string]string{"common-label": "common"}, - expectedPods: []string{"test-pod-3", "test-pod-4"}, + Entry("type-level field selector does not match, no results", selectorsTestCase{ + options: cache.Options{ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: {Field: fields.SelectorFromSet(map[string]string{ + "metadata.namespace": "new", + })}, + }}, + expectedPods: []string{}, }), - Entry("when label and field matches one pod it has to inform about about it", selectorsTestCase{ - labelSelectors: map[string]string{"common-label": "common"}, - fieldSelectors: map[string]string{"metadata.namespace": testNamespaceTwo}, - expectedPods: []string{"test-pod-3"}, + Entry("global field selector does not match, no results", selectorsTestCase{ + options: cache.Options{ + DefaultFieldSelector: fields.SelectorFromSet(map[string]string{ + "metadata.namespace": "new", + }), + }, + expectedPods: []string{}, }), - Entry("when label does not match it does not has to inform", selectorsTestCase{ - labelSelectors: map[string]string{"new-label": "new"}, - expectedPods: []string{}, + Entry("type-level field selector on namespace matches one pod", selectorsTestCase{ + options: cache.Options{ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: {Namespaces: map[string]cache.Config{ + testNamespaceTwo: { + FieldSelector: fields.SelectorFromSet(map[string]string{ + "metadata.name": "test-pod-2", + }), + }, + }}, + }}, + expectedPods: []string{"test-pod-2"}, }), - Entry("when field does not match it does not has to inform", selectorsTestCase{ - fieldSelectors: map[string]string{"metadata.namespace": "new"}, - expectedPods: []string{}, + Entry("type-level field selector on namespace doesn't match", selectorsTestCase{ + options: cache.Options{ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: {Namespaces: map[string]cache.Config{ + testNamespaceTwo: { + FieldSelector: fields.SelectorFromSet(map[string]string{ + "metadata.name": "test-pod-doesn-exist", + }), + }, + }}, + }}, + expectedPods: []string{}, + }), + Entry("global field selector on namespace matches one pod", selectorsTestCase{ + options: cache.Options{ + DefaultNamespaces: map[string]cache.Config{ + testNamespaceTwo: { + FieldSelector: fields.SelectorFromSet(map[string]string{ + "metadata.name": "test-pod-2", + }), + }, + }, + }, + expectedPods: []string{"test-pod-2"}, + }), + Entry("global field selector on namespace doesn't match", selectorsTestCase{ + options: cache.Options{ + DefaultNamespaces: map[string]cache.Config{ + testNamespaceTwo: { + FieldSelector: fields.SelectorFromSet(map[string]string{ + "metadata.name": "test-pod-doesn-exist", + }), + }, + }, + }, + expectedPods: []string{}, + }), + Entry("type-level label selector on namespace matches one pod", selectorsTestCase{ + options: cache.Options{ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: {Namespaces: map[string]cache.Config{ + testNamespaceTwo: { + LabelSelector: labels.SelectorFromSet(map[string]string{ + "test-label": "test-pod-2", + }), + }, + }}, + }}, + expectedPods: []string{"test-pod-2"}, + }), + Entry("type-level label selector on namespace doesn't match", selectorsTestCase{ + options: cache.Options{ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: {Namespaces: map[string]cache.Config{ + testNamespaceTwo: { + LabelSelector: labels.SelectorFromSet(map[string]string{ + "test-label": "test-pod-doesn-exist", + }), + }, + }}, + }}, + expectedPods: []string{}, + }), + Entry("global label selector on namespace matches one pod", selectorsTestCase{ + options: cache.Options{ + DefaultNamespaces: map[string]cache.Config{ + testNamespaceTwo: { + LabelSelector: labels.SelectorFromSet(map[string]string{ + "test-label": "test-pod-2", + }), + }, + }, + }, + expectedPods: []string{"test-pod-2"}, + }), + Entry("global label selector on namespace doesn't match", selectorsTestCase{ + options: cache.Options{ + DefaultNamespaces: map[string]cache.Config{ + testNamespaceTwo: { + LabelSelector: labels.SelectorFromSet(map[string]string{ + "test-label": "test-pod-doesn-exist", + }), + }, + }, + }, + expectedPods: []string{}, }), ) }) @@ -1813,8 +2019,8 @@ func isPodDisableDeepCopy(opts cache.Options) bool { if opts.ByObject[&corev1.Pod{}].UnsafeDisableDeepCopy != nil { return *opts.ByObject[&corev1.Pod{}].UnsafeDisableDeepCopy } - if opts.UnsafeDisableDeepCopy != nil { - return *opts.UnsafeDisableDeepCopy + if opts.DefaultUnsafeDisableDeepCopy != nil { + return *opts.DefaultUnsafeDisableDeepCopy } return false } diff --git a/pkg/cache/defaulting_test.go b/pkg/cache/defaulting_test.go new file mode 100644 index 0000000000..6cbde247c3 --- /dev/null +++ b/pkg/cache/defaulting_test.go @@ -0,0 +1,290 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cache + +import ( + "reflect" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + fuzz "github.com/google/gofuzz" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/cache" + "k8s.io/utils/pointer" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func TestDefaultOpts(t *testing.T) { + t.Parallel() + + pod := &corev1.Pod{} + testCases := []struct { + name string + in Options + + verification func(Options) string + }{ + { + name: "ByObject.Namespaces gets defaulted from ByObject", + in: Options{ + ByObject: map[client.Object]ByObject{pod: { + Namespaces: map[string]Config{ + "default": {}, + }, + Label: labels.SelectorFromSet(map[string]string{"from": "by-object"}), + }}, + DefaultNamespaces: map[string]Config{ + "default": {LabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-namespaces"})}, + }, + DefaultLabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-label-selector"}), + }, + + verification: func(o Options) string { + expected := map[string]Config{ + "default": {LabelSelector: labels.SelectorFromSet(map[string]string{"from": "by-object"})}, + } + return cmp.Diff(expected, o.ByObject[pod].Namespaces) + }, + }, + { + name: "ByObject.Namespaces gets defaulted from DefaultNamespaces", + in: Options{ + ByObject: map[client.Object]ByObject{pod: { + Namespaces: map[string]Config{ + "default": {}, + }, + }}, + DefaultNamespaces: map[string]Config{ + "default": {LabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-namespaces"})}, + }, + DefaultLabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-label-selector"}), + }, + + verification: func(o Options) string { + expected := map[string]Config{ + "default": {LabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-namespaces"})}, + } + return cmp.Diff(expected, o.ByObject[pod].Namespaces) + }, + }, + { + name: "ByObject.Namespaces gets defaulted from DefaultLabelSelector", + in: Options{ + ByObject: map[client.Object]ByObject{pod: { + Namespaces: map[string]Config{ + "default": {}, + }, + }}, + DefaultLabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-label-selector"}), + }, + + verification: func(o Options) string { + expected := map[string]Config{ + "default": {LabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-label-selector"})}, + } + return cmp.Diff(expected, o.ByObject[pod].Namespaces) + }, + }, + { + name: "ByObject.Namespaces gets defaulted from DefaultNamespaces", + in: Options{ + ByObject: map[client.Object]ByObject{pod: {}}, + DefaultNamespaces: map[string]Config{ + "default": {LabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-namespaces"})}, + }, + }, + + verification: func(o Options) string { + expected := map[string]Config{ + "default": {LabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-namespaces"})}, + } + return cmp.Diff(expected, o.ByObject[pod].Namespaces) + }, + }, + { + name: "ByObject.Namespaces doesn't get defaulted when its empty", + in: Options{ + ByObject: map[client.Object]ByObject{pod: {Namespaces: map[string]Config{}}}, + DefaultNamespaces: map[string]Config{ + "default": {LabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-namespaces"})}, + }, + }, + + verification: func(o Options) string { + expected := map[string]Config{} + return cmp.Diff(expected, o.ByObject[pod].Namespaces) + }, + }, + { + name: "ByObject.Labels gets defaulted from DefautLabelSelector", + in: Options{ + ByObject: map[client.Object]ByObject{pod: {}}, + DefaultLabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-label-selector"}), + }, + + verification: func(o Options) string { + expected := labels.SelectorFromSet(map[string]string{"from": "default-label-selector"}) + return cmp.Diff(expected, o.ByObject[pod].Label) + }, + }, + { + name: "ByObject.Labels doesn't get defaulted when set", + in: Options{ + ByObject: map[client.Object]ByObject{pod: {Label: labels.SelectorFromSet(map[string]string{"from": "by-object"})}}, + DefaultLabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-label-selector"}), + }, + + verification: func(o Options) string { + expected := labels.SelectorFromSet(map[string]string{"from": "by-object"}) + return cmp.Diff(expected, o.ByObject[pod].Label) + }, + }, + { + name: "ByObject.Fields gets defaulted from DefaultFieldSelector", + in: Options{ + ByObject: map[client.Object]ByObject{pod: {}}, + DefaultFieldSelector: fields.SelectorFromSet(map[string]string{"from": "default-field-selector"}), + }, + + verification: func(o Options) string { + expected := fields.SelectorFromSet(map[string]string{"from": "default-field-selector"}) + return cmp.Diff(expected, o.ByObject[pod].Field, cmp.Exporter(func(reflect.Type) bool { return true })) + }, + }, + { + name: "ByObject.Fields doesn't get defaulted when set", + in: Options{ + ByObject: map[client.Object]ByObject{pod: {Field: fields.SelectorFromSet(map[string]string{"from": "by-object"})}}, + DefaultFieldSelector: fields.SelectorFromSet(map[string]string{"from": "default-field-selector"}), + }, + + verification: func(o Options) string { + expected := fields.SelectorFromSet(map[string]string{"from": "by-object"}) + return cmp.Diff(expected, o.ByObject[pod].Field, cmp.Exporter(func(reflect.Type) bool { return true })) + }, + }, + { + name: "ByObject.UnsafeDisableDeepCopy gets defaulted from DefaultUnsafeDisableDeepCopy", + in: Options{ + ByObject: map[client.Object]ByObject{pod: {}}, + DefaultUnsafeDisableDeepCopy: pointer.Bool(true), + }, + + verification: func(o Options) string { + expected := pointer.Bool(true) + return cmp.Diff(expected, o.ByObject[pod].UnsafeDisableDeepCopy) + }, + }, + { + name: "ByObject.UnsafeDisableDeepCopy doesn't get defaulted when set", + in: Options{ + ByObject: map[client.Object]ByObject{pod: {UnsafeDisableDeepCopy: pointer.Bool(false)}}, + DefaultUnsafeDisableDeepCopy: pointer.Bool(true), + }, + + verification: func(o Options) string { + expected := pointer.Bool(false) + return cmp.Diff(expected, o.ByObject[pod].UnsafeDisableDeepCopy) + }, + }, + { + name: "DefaultNamespace label selector gets defaulted from DefaultLabelSelector", + in: Options{ + DefaultNamespaces: map[string]Config{"default": {}}, + DefaultLabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-label-selector"}), + }, + + verification: func(o Options) string { + expected := map[string]Config{ + "default": {LabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-label-selector"})}, + } + return cmp.Diff(expected, o.DefaultNamespaces) + }, + }, + { + name: "DefaultNamespace label selector doesn't get defaulted when set", + in: Options{ + DefaultNamespaces: map[string]Config{"default": {LabelSelector: labels.Everything()}}, + DefaultLabelSelector: labels.SelectorFromSet(map[string]string{"from": "default-label-selector"}), + }, + + verification: func(o Options) string { + expected := map[string]Config{ + "default": {LabelSelector: labels.Everything()}, + } + return cmp.Diff(expected, o.DefaultNamespaces) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tc.in.Mapper = &fakeRESTMapper{} + + defaulted, err := defaultOpts(&rest.Config{}, tc.in) + if err != nil { + t.Fatal(err) + } + + if diff := tc.verification(defaulted); diff != "" { + t.Errorf("expected config differs from actual: %s", diff) + } + }) + } +} + +type fakeRESTMapper struct { + meta.RESTMapper +} + +func (f *fakeRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { + return &meta.RESTMapping{Scope: meta.RESTScopeNamespace}, nil +} + +func TestDefaultConfigConsidersAllFields(t *testing.T) { + t.Parallel() + seed := time.Now().UnixNano() + t.Logf("Seed is %d", seed) + f := fuzz.NewWithSeed(seed).Funcs( + func(ls *labels.Selector, _ fuzz.Continue) { + *ls = labels.SelectorFromSet(map[string]string{"foo": "bar"}) + }, + func(fs *fields.Selector, _ fuzz.Continue) { + *fs = fields.SelectorFromSet(map[string]string{"foo": "bar"}) + }, + func(tf *cache.TransformFunc, _ fuzz.Continue) { + // never default this, as functions can not be compared so we fail down the line + }, + ) + + for i := 0; i < 100; i++ { + fuzzed := Config{} + f.Fuzz(&fuzzed) + + defaulted := defaultConfig(Config{}, fuzzed) + + if diff := cmp.Diff(fuzzed, defaulted, cmp.Exporter(func(reflect.Type) bool { return true })); diff != "" { + t.Errorf("Defaulted config doesn't match fuzzed one: %s", diff) + } + } +} diff --git a/pkg/cache/delegating_by_gvk_cache.go b/pkg/cache/delegating_by_gvk_cache.go new file mode 100644 index 0000000000..6d640216a0 --- /dev/null +++ b/pkg/cache/delegating_by_gvk_cache.go @@ -0,0 +1,127 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cache + +import ( + "context" + "strings" + "sync" + + "golang.org/x/exp/maps" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" +) + +// delegatingByGVKCache delegates to a type-specific cache if present +// and uses the defaultCache otherwise. +type delegatingByGVKCache struct { + scheme *runtime.Scheme + caches map[schema.GroupVersionKind]Cache + defaultCache Cache +} + +func (dbt *delegatingByGVKCache) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + cache, err := dbt.cacheForObject(obj) + if err != nil { + return err + } + return cache.Get(ctx, key, obj, opts...) +} + +func (dbt *delegatingByGVKCache) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + cache, err := dbt.cacheForObject(list) + if err != nil { + return err + } + return cache.List(ctx, list, opts...) +} + +func (dbt *delegatingByGVKCache) GetInformer(ctx context.Context, obj client.Object) (Informer, error) { + cache, err := dbt.cacheForObject(obj) + if err != nil { + return nil, err + } + return cache.GetInformer(ctx, obj) +} + +func (dbt *delegatingByGVKCache) GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind) (Informer, error) { + return dbt.cacheForGVK(gvk).GetInformerForKind(ctx, gvk) +} + +func (dbt *delegatingByGVKCache) Start(ctx context.Context) error { + allCaches := maps.Values(dbt.caches) + allCaches = append(allCaches, dbt.defaultCache) + + wg := &sync.WaitGroup{} + errs := make(chan error) + for idx := range allCaches { + cache := allCaches[idx] + wg.Add(1) + go func() { + defer wg.Done() + if err := cache.Start(ctx); err != nil { + errs <- err + } + }() + } + + select { + case err := <-errs: + return err + case <-ctx.Done(): + wg.Wait() + return nil + } +} + +func (dbt *delegatingByGVKCache) WaitForCacheSync(ctx context.Context) bool { + synced := true + for _, cache := range append(maps.Values(dbt.caches), dbt.defaultCache) { + if !cache.WaitForCacheSync(ctx) { + synced = false + } + } + + return synced +} + +func (dbt *delegatingByGVKCache) IndexField(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error { + cache, err := dbt.cacheForObject(obj) + if err != nil { + return err + } + return cache.IndexField(ctx, obj, field, extractValue) +} + +func (dbt *delegatingByGVKCache) cacheForObject(o runtime.Object) (Cache, error) { + gvk, err := apiutil.GVKForObject(o, dbt.scheme) + if err != nil { + return nil, err + } + gvk.Kind = strings.TrimSuffix(gvk.Kind, "List") + return dbt.cacheForGVK(gvk), nil +} + +func (dbt *delegatingByGVKCache) cacheForGVK(gvk schema.GroupVersionKind) Cache { + if specific, hasSpecific := dbt.caches[gvk]; hasSpecific { + return specific + } + + return dbt.defaultCache +} diff --git a/pkg/cache/internal/informers.go b/pkg/cache/internal/informers.go index c01d70a940..ea72a5bf43 100644 --- a/pkg/cache/internal/informers.go +++ b/pkg/cache/internal/informers.go @@ -35,26 +35,19 @@ import ( "k8s.io/client-go/metadata" "k8s.io/client-go/rest" "k8s.io/client-go/tools/cache" - "sigs.k8s.io/controller-runtime/pkg/client/apiutil" ) // InformersOpts configures an InformerMap. type InformersOpts struct { - HTTPClient *http.Client - Scheme *runtime.Scheme - Mapper meta.RESTMapper - ResyncPeriod time.Duration - Namespace string - ByGVK map[schema.GroupVersionKind]InformersOptsByGVK -} - -// InformersOptsByGVK configures additionally by group version kind (or object) -// in an InformerMap. -type InformersOptsByGVK struct { + HTTPClient *http.Client + Scheme *runtime.Scheme + Mapper meta.RESTMapper + ResyncPeriod time.Duration + Namespace string Selector Selector Transform cache.TransformFunc - UnsafeDisableDeepCopy *bool + UnsafeDisableDeepCopy bool } // NewInformers creates a new InformersMap that can create informers under the hood. @@ -69,12 +62,14 @@ func NewInformers(config *rest.Config, options *InformersOpts) *Informers { Unstructured: make(map[schema.GroupVersionKind]*Cache), Metadata: make(map[schema.GroupVersionKind]*Cache), }, - codecs: serializer.NewCodecFactory(options.Scheme), - paramCodec: runtime.NewParameterCodec(options.Scheme), - resync: options.ResyncPeriod, - startWait: make(chan struct{}), - namespace: options.Namespace, - byGVK: options.ByGVK, + codecs: serializer.NewCodecFactory(options.Scheme), + paramCodec: runtime.NewParameterCodec(options.Scheme), + resync: options.ResyncPeriod, + startWait: make(chan struct{}), + namespace: options.Namespace, + selector: options.Selector, + transform: options.Transform, + unsafeDisableDeepCopy: options.UnsafeDisableDeepCopy, } } @@ -145,46 +140,9 @@ type Informers struct { // default or empty string means all namespaces namespace string - byGVK map[schema.GroupVersionKind]InformersOptsByGVK -} - -func (ip *Informers) getSelector(gvk schema.GroupVersionKind) Selector { - if ip.byGVK == nil { - return Selector{} - } - if res, ok := ip.byGVK[gvk]; ok { - return res.Selector - } - if res, ok := ip.byGVK[schema.GroupVersionKind{}]; ok { - return res.Selector - } - return Selector{} -} - -func (ip *Informers) getTransform(gvk schema.GroupVersionKind) cache.TransformFunc { - if ip.byGVK == nil { - return nil - } - if res, ok := ip.byGVK[gvk]; ok { - return res.Transform - } - if res, ok := ip.byGVK[schema.GroupVersionKind{}]; ok { - return res.Transform - } - return nil -} - -func (ip *Informers) getDisableDeepCopy(gvk schema.GroupVersionKind) bool { - if ip.byGVK == nil { - return false - } - if res, ok := ip.byGVK[gvk]; ok && res.UnsafeDisableDeepCopy != nil { - return *res.UnsafeDisableDeepCopy - } - if res, ok := ip.byGVK[schema.GroupVersionKind{}]; ok && res.UnsafeDisableDeepCopy != nil { - return *res.UnsafeDisableDeepCopy - } - return false + selector Selector + transform cache.TransformFunc + unsafeDisableDeepCopy bool } // Start calls Run on each of the informers and sets started to true. Blocks on the context. @@ -331,11 +289,11 @@ func (ip *Informers) addInformerToMap(gvk schema.GroupVersionKind, obj runtime.O } sharedIndexInformer := cache.NewSharedIndexInformer(&cache.ListWatch{ ListFunc: func(opts metav1.ListOptions) (runtime.Object, error) { - ip.getSelector(gvk).ApplyToList(&opts) + ip.selector.ApplyToList(&opts) return listWatcher.ListFunc(opts) }, WatchFunc: func(opts metav1.ListOptions) (watch.Interface, error) { - ip.getSelector(gvk).ApplyToList(&opts) + ip.selector.ApplyToList(&opts) opts.Watch = true // Watch needs to be set to true separately return listWatcher.WatchFunc(opts) }, @@ -344,7 +302,7 @@ func (ip *Informers) addInformerToMap(gvk schema.GroupVersionKind, obj runtime.O }) // Check to see if there is a transformer for this gvk - if err := sharedIndexInformer.SetTransform(ip.getTransform(gvk)); err != nil { + if err := sharedIndexInformer.SetTransform(ip.transform); err != nil { return nil, false, err } @@ -360,7 +318,7 @@ func (ip *Informers) addInformerToMap(gvk schema.GroupVersionKind, obj runtime.O indexer: sharedIndexInformer.GetIndexer(), groupVersionKind: gvk, scopeName: mapping.Scope.Name(), - disableDeepCopy: ip.getDisableDeepCopy(gvk), + disableDeepCopy: ip.unsafeDisableDeepCopy, }, } ip.informersByType(obj)[gvk] = i @@ -384,7 +342,7 @@ func (ip *Informers) makeListWatcher(gvk schema.GroupVersionKind, obj runtime.Ob // Figure out if the GVK we're dealing with is global, or namespace scoped. var namespace string if mapping.Scope.Name() == meta.RESTScopeNameNamespace { - namespace = restrictNamespaceBySelector(ip.namespace, ip.getSelector(gvk)) + namespace = restrictNamespaceBySelector(ip.namespace, ip.selector) } switch obj.(type) { diff --git a/pkg/cache/multi_namespace_cache.go b/pkg/cache/multi_namespace_cache.go index 920e7c9309..f767ddd951 100644 --- a/pkg/cache/multi_namespace_cache.go +++ b/pkg/cache/multi_namespace_cache.go @@ -25,7 +25,6 @@ import ( apimeta "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/rest" toolscache "k8s.io/client-go/tools/cache" "sigs.k8s.io/controller-runtime/pkg/client" @@ -35,34 +34,31 @@ import ( // a new global namespaced cache to handle cluster scoped resources. const globalCache = "_cluster-scope" -func newMultiNamespaceCache(config *rest.Config, opts Options) (Cache, error) { - if len(opts.Namespaces) < 2 { - return nil, fmt.Errorf("must specify more than one namespace to use multi-namespace cache") - } - opts, err := defaultOpts(config, opts) - if err != nil { - return nil, err - } - +func newMultiNamespaceCache( + newCache newCacheFunc, + scheme *runtime.Scheme, + restMapper apimeta.RESTMapper, + namespaces map[string]Config, + globalConfig *Config, // may be nil in which case no cache for cluster-scoped objects will be created +) Cache { // Create every namespace cache. caches := map[string]Cache{} - for _, ns := range opts.Namespaces { - opts.Namespaces = []string{ns} - c, err := New(config, opts) - if err != nil { - return nil, err - } - caches[ns] = c + for namespace, config := range namespaces { + caches[namespace] = newCache(config, namespace) } - // Create a cache for cluster scoped resources. - opts.Namespaces = []string{} - clusterCache, err := New(config, opts) - if err != nil { - return nil, fmt.Errorf("error creating global cache: %w", err) + // Create a cache for cluster scoped resources if requested + var clusterCache Cache + if globalConfig != nil { + clusterCache = newCache(*globalConfig, corev1.NamespaceAll) } - return &multiNamespaceCache{namespaceToCache: caches, Scheme: opts.Scheme, RESTMapper: opts.Mapper, clusterCache: clusterCache}, nil + return &multiNamespaceCache{ + namespaceToCache: caches, + Scheme: scheme, + RESTMapper: restMapper, + clusterCache: clusterCache, + } } // multiNamespaceCache knows how to handle multiple namespaced caches @@ -146,11 +142,14 @@ func (c *multiNamespaceCache) GetInformerForKind(ctx context.Context, gvk schema func (c *multiNamespaceCache) Start(ctx context.Context) error { // start global cache - go func() { - if err := c.clusterCache.Start(ctx); err != nil { - log.Error(err, "multi-namespace cache failed to start cluster scoped cache") - } - }() + if c.clusterCache != nil { + 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 { @@ -174,7 +173,7 @@ func (c *multiNamespaceCache) WaitForCacheSync(ctx context.Context) bool { } // check if cluster scoped cache has synced - if !c.clusterCache.WaitForCacheSync(ctx) { + if c.clusterCache != nil && !c.clusterCache.WaitForCacheSync(ctx) { synced = false } return synced diff --git a/pkg/manager/example_test.go b/pkg/manager/example_test.go index 06712d7171..2ca2332df1 100644 --- a/pkg/manager/example_test.go +++ b/pkg/manager/example_test.go @@ -61,7 +61,10 @@ func ExampleNew_limitToNamespaces() { mgr, err := manager.New(cfg, manager.Options{ NewCache: func(config *rest.Config, opts cache.Options) (cache.Cache, error) { - opts.Namespaces = []string{"namespace1", "namespace2"} + opts.DefaultNamespaces = map[string]cache.Config{ + "namespace1": {}, + "namespace2": {}, + } return cache.New(config, opts) }}, ) diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 360c9a2a44..719b8425ac 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -451,8 +451,8 @@ func (o Options) AndFrom(loader config.ControllerManagerConfiguration) (Options, o.Cache.SyncPeriod = &newObj.SyncPeriod.Duration } - if len(o.Cache.Namespaces) == 0 && newObj.CacheNamespace != "" { - o.Cache.Namespaces = []string{newObj.CacheNamespace} + if len(o.Cache.DefaultNamespaces) == 0 && newObj.CacheNamespace != "" { + o.Cache.DefaultNamespaces = map[string]cache.Config{newObj.CacheNamespace: {}} } if o.MetricsBindAddress == "" && newObj.Metrics.BindAddress != "" { diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index dd6d3b2470..198574c144 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -164,7 +164,7 @@ var _ = Describe("manger.Manager", func() { Expect(m.LeaseDuration.String()).To(Equal(duration.Duration.String())) Expect(m.RenewDeadline.String()).To(Equal(duration.Duration.String())) Expect(m.RetryPeriod.String()).To(Equal(duration.Duration.String())) - Expect(m.Cache.Namespaces).To(Equal([]string{"default"})) + Expect(m.Cache.DefaultNamespaces).To(Equal(map[string]cache.Config{"default": {}})) Expect(m.MetricsBindAddress).To(Equal(":6000")) Expect(m.HealthProbeBindAddress).To(Equal("6060")) Expect(m.ReadinessEndpointName).To(Equal("/readyz")) @@ -214,8 +214,8 @@ var _ = Describe("manger.Manager", func() { } m, err := Options{ Cache: cache.Options{ - SyncPeriod: &optDuration, - Namespaces: []string{"ctrl"}, + SyncPeriod: &optDuration, + DefaultNamespaces: map[string]cache.Config{"ctrl": {}}, }, LeaderElection: true, LeaderElectionResourceLock: "configmaps", @@ -245,7 +245,7 @@ var _ = Describe("manger.Manager", func() { Expect(m.LeaseDuration.String()).To(Equal(optDuration.String())) Expect(m.RenewDeadline.String()).To(Equal(optDuration.String())) Expect(m.RetryPeriod.String()).To(Equal(optDuration.String())) - Expect(m.Cache.Namespaces).To(Equal([]string{"ctrl"})) + Expect(m.Cache.DefaultNamespaces).To(Equal(map[string]cache.Config{"ctrl": {}})) Expect(m.MetricsBindAddress).To(Equal(":7000")) Expect(m.HealthProbeBindAddress).To(Equal("5000")) Expect(m.ReadinessEndpointName).To(Equal("/readiness"))