From fbc6d2832f05ad7283e8a2059cb6f0e14e956772 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 --- go.mod | 1 + go.sum | 2 + pkg/cache/cache.go | 236 +++++++++++++---- pkg/cache/cache_test.go | 395 ++++++++++++++++++++++------- pkg/cache/defaulting_test.go | 290 +++++++++++++++++++++ pkg/cache/delegate_by_type.go | 127 ++++++++++ pkg/cache/internal/informers.go | 85 ++----- pkg/cache/multi_namespace_cache.go | 72 ++---- pkg/cluster/cluster.go | 4 +- pkg/manager/example_test.go | 5 +- 10 files changed, 959 insertions(+), 258 deletions(-) create mode 100644 pkg/cache/defaulting_test.go create mode 100644 pkg/cache/delegate_by_type.go diff --git a/go.mod b/go.mod index a5d7da8406..20256af665 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( 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-alpha.3 diff --git a/go.sum b/go.sum index cee66b77d3..242d48a55e 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..4ff8ef3f97 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" @@ -144,9 +144,13 @@ 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 settings. 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. @@ -160,20 +164,40 @@ type Options struct { // unless they have a more specific transform set in ByObject. 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 + 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 setting. If set, only the + // namespaces in this map will be cached. + // + // Settings in the map value that are unset because either the value as a + // whole is nil or because the specific setting is nil will be defaulted. + // Use an empty value for the specific setting to prevent that. + // + // It is possible to have specific Config for just some namespaces + // but cache all namespaces by using the AllNamespaces const as the map key. + // This wil then include all namespaces that do not have a more specific + // setting. + // + // A nil map allows to default this to the cache's DefaultNamespaces setting. + // An empty map prevents this. + // + // 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 +218,111 @@ 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. + LabelSelector labels.Selector + + // FieldSelector specifics a field selector. A nil value allows to + // default this. + FieldSelector fields.Selector + + // Transform specifies a transform func. A nil value allows to default + // this. + 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) + + newCache := newCache(cfg, opts) + + var defaultCache Cache + if len(opts.DefaultNamespaces) > 0 { + defaultConfig := optionDefaultsToConfig(&opts) + defaultCache = newMultiNamespaceCache(newCache, opts.Scheme, opts.Mapper, opts.DefaultNamespaces, &defaultConfig) + } else { + defaultCache = newCache(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 := &delegatingByTypeCache{ + scheme: opts.Scheme, + caches: make(map[schema.GroupVersionKind]Cache, len(opts.ByObject)), + defaultCache: defaultCache, } - // Set the default selector and transform. - byGVK[schema.GroupVersionKind{}] = internal.InformersOptsByGVK{ - Selector: internal.Selector{ - Label: opts.DefaultLabelSelector, - Field: opts.DefaultFieldSelector, - }, + + for tp, config := range opts.ByObject { + gvk, err := apiutil.GVKForObject(tp, opts.Scheme) + if err != nil { + return nil, fmt.Errorf("failed to get GVK for type %T: %w", tp, err) + } + var cache Cache + if len(config.Namespaces) > 0 { + cache = newMultiNamespaceCache(newCache, opts.Scheme, opts.Mapper, config.Namespaces, nil) + } else { + cache = newCache(byObjectToConfig(config), corev1.NamespaceAll) + } + delegating.caches[gvk] = cache + } + + 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(cfg *rest.Config, opts Options) newCacheFunc { + return func(config Config, namespace string) Cache { + return &informerCache{ + scheme: opts.Scheme, + Informers: internal.NewInformers(cfg, &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: config.UnsafeDisableDeepCopy, + }), + } + } } func defaultOpts(config *rest.Config, opts Options) (Options, error) { @@ -262,6 +349,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 tp, cfg := range opts.ByObject { + isNamespaced, err := apiutil.IsObjectNamespaced(tp, opts.Scheme, opts.Mapper) + if err != nil { + return opts, fmt.Errorf("failed to determine if %T is namespaced: %w", tp, err) + } + if !isNamespaced && cfg.Namespaces != nil { + return opts, fmt.Errorf("type %T is not namespaced, but its ByObject.Namespaces setting is not nil", tp) + } + + // Default the namespace-level configs first, because they need to use the undefaulted type-level config. + for namespace, config := range cfg.Namespaces { + // 1. Default from the undefaulted type-level config + config = defaultConfig(config, byObjectToConfig(cfg)) + + // 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)) + + cfg.Namespaces[namespace] = config + } + + defaultedConfig := defaultConfig(byObjectToConfig(cfg), optionDefaultsToConfig(&opts)) + cfg.Label = defaultedConfig.LabelSelector + cfg.Field = defaultedConfig.FieldSelector + cfg.Transform = defaultedConfig.Transform + cfg.UnsafeDisableDeepCopy = defaultedConfig.UnsafeDisableDeepCopy + + if cfg.Namespaces == nil { + cfg.Namespaces = opts.DefaultNamespaces + } + + opts.ByObject[tp] = cfg + } + // Default the resync period to 10 hours if unset if opts.SyncPeriod == nil { opts.SyncPeriod = &defaultSyncPeriod @@ -269,24 +400,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 + } + if toDefault.FieldSelector == nil { + toDefault.FieldSelector = defaultFrom.FieldSelector } - return out, nil + 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 100825854a..27c1c63cfe 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -118,12 +118,18 @@ var _ = Describe("Informer Cache", func() { CacheTest(cache.New, cache.Options{}) }) var _ = Describe("Multi-Namespace Informer Cache", func() { - CacheTest(cache.MultiNamespacedCacheBuilder([]string{testNamespaceOne, testNamespaceTwo, "default"}), cache.Options{}) + CacheTest(cache.New, cache.Options{ + 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), }) }) @@ -368,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), + }, }, } @@ -788,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() { @@ -931,8 +967,12 @@ 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") - multi := cache.MultiNamespacedCacheBuilder([]string{"default", testNamespaceOne}) - m, err := multi(cfg, cache.Options{}) + m, err := cache.New(cfg, cache.Options{ + DefaultNamespaces: map[string]cache.Config{ + "default": {}, + testNamespaceOne: {}, + }, + }) Expect(err).NotTo(HaveOccurred()) By("running the cache and waiting it for sync") @@ -1071,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") @@ -1217,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") @@ -1286,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("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("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("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("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("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 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("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 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("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 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("when label does not match it does not has to inform", selectorsTestCase{ - labelSelectors: map[string]string{"new-label": "new"}, - expectedPods: []string{}, + 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("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 does not match it does not has to inform", selectorsTestCase{ - fieldSelectors: map[string]string{"metadata.namespace": "new"}, - expectedPods: []string{}, + 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("global label selector does not match, no results", selectorsTestCase{ + options: cache.Options{ + DefaultLabelSelector: labels.SelectorFromSet(map[string]string{ + "new-label": "new", + }), + }, + expectedPods: []string{}, + }), + 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("global field selector does not match, no results", selectorsTestCase{ + options: cache.Options{ + DefaultFieldSelector: fields.SelectorFromSet(map[string]string{ + "metadata.namespace": "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("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{}, }), ) }) @@ -1810,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/delegate_by_type.go b/pkg/cache/delegate_by_type.go new file mode 100644 index 0000000000..7e52a45805 --- /dev/null +++ b/pkg/cache/delegate_by_type.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" +) + +// delegatingByTypeCache delegates to a type-specific cache if present +// and uses the defaultCache otherwise. +type delegatingByTypeCache struct { + scheme *runtime.Scheme + caches map[schema.GroupVersionKind]Cache + defaultCache Cache +} + +func (dbt *delegatingByTypeCache) 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 *delegatingByTypeCache) 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 *delegatingByTypeCache) 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 *delegatingByTypeCache) GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind) (Informer, error) { + return dbt.cacheForGVK(gvk).GetInformerForKind(ctx, gvk) +} + +func (dbt *delegatingByTypeCache) 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 *delegatingByTypeCache) 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 *delegatingByTypeCache) 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 *delegatingByTypeCache) 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 *delegatingByTypeCache) 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..bd48f157a0 100644 --- a/pkg/cache/internal/informers.go +++ b/pkg/cache/internal/informers.go @@ -35,23 +35,17 @@ import ( "k8s.io/client-go/metadata" "k8s.io/client-go/rest" "k8s.io/client-go/tools/cache" - + "k8s.io/utils/pointer" "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 @@ -69,12 +63,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: pointer.BoolDeref(options.UnsafeDisableDeepCopy, false), } } @@ -145,46 +141,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 +290,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 +303,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 +319,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 +343,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 36dcf3d4e5..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,49 +34,31 @@ import ( // a new global namespaced cache to handle cluster scoped resources. 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. 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. -// -// Deprecated: Use cache.Options.Namespaces instead. -func MultiNamespacedCacheBuilder(namespaces []string) NewCacheFunc { - return func(config *rest.Config, opts Options) (Cache, error) { - opts.Namespaces = namespaces - return newMultiNamespaceCache(config, opts) - } -} - -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 @@ -161,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 { @@ -189,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/cluster/cluster.go b/pkg/cluster/cluster.go index 7d00c3c4b0..32e785695d 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -211,8 +211,8 @@ func New(config *rest.Config, opts ...Option) (Cluster, error) { if cacheOpts.SyncPeriod == nil { cacheOpts.SyncPeriod = options.SyncPeriod } - if len(cacheOpts.Namespaces) == 0 && options.Namespace != "" { - cacheOpts.Namespaces = []string{options.Namespace} + if len(cacheOpts.DefaultNamespaces) == 0 && options.Namespace != "" { + cacheOpts.DefaultNamespaces = map[string]cache.Config{options.Namespace: {}} } } cache, err := options.NewCache(config, cacheOpts) 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) }}, )