From f941ae41ac689cc916a3d6c4e27fefdfff6a384d Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Tue, 6 Sep 2022 08:19:13 -0400 Subject: [PATCH] address PR feedback Signed-off-by: Joe Lanford --- pkg/cache/cache_test.go | 4 ++-- pkg/cache/multi_namespace_cache.go | 37 +++++++++++++++--------------- pkg/manager/example_test.go | 2 +- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index 4aed0f0ae3..fc203e2aef 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -160,8 +160,8 @@ var _ = Describe("MultiNamespacedCacheWithOptionsBuilder", func() { pod3b = createPodWithLabels("pod-3b", testNamespaceThree, corev1.RestartPolicyNever, map[string]string{"other-match": "true"}) // matches (matches default cache label selector) By("creating the informer cache") informerCache, err = cache.MultiNamespacedCacheWithOptionsBuilder( - cache.WithNamespaceCache(testNamespaceOne, cache.New), - cache.WithNamespaceCache(testNamespaceTwo, cache.BuilderWithOptions(cache.Options{ + cache.WithNamespacedCache(testNamespaceOne, cache.New), + cache.WithNamespacedCache(testNamespaceTwo, cache.BuilderWithOptions(cache.Options{ DefaultSelector: cache.ObjectSelector{ Label: labels.Set{"ns2-match": "true"}.AsSelector(), }, diff --git a/pkg/cache/multi_namespace_cache.go b/pkg/cache/multi_namespace_cache.go index 1b82719255..a4b37f0eef 100644 --- a/pkg/cache/multi_namespace_cache.go +++ b/pkg/cache/multi_namespace_cache.go @@ -45,37 +45,37 @@ type MultiNamespacedOption func(*MultiNamespacedOptions) // MultiNamespacedOptions is used to configure the functions used to create caches // on a per-namespace basis. type MultiNamespacedOptions struct { - NewNamespaceCaches map[string]NewCacheFunc + NewNamespacedCaches map[string]NewCacheFunc NewClusterScopedCache NewCacheFunc NewDefaultNamespacedCache NewCacheFunc } -// WithLegacyNamespaceCaches configures the MultiNamespacedCacheWithOptionsBuilder +// WithLegacyNamespacedCaches configures the MultiNamespacedCacheWithOptionsBuilder // with standard caches in each of the namespaces provided as well as for cluster-scoped // objects. This option enables use of the MultiNamespacedCacheWithOptionsBuilder // to match the behavior of the deprecated MultiNamespacedCacheBuilder. -func WithLegacyNamespaceCaches(namespaces []string) MultiNamespacedOption { +func WithLegacyNamespacedCaches(namespaces []string) MultiNamespacedOption { return func(options *MultiNamespacedOptions) { - WithNamespaceCaches(namespaces, New)(options) + WithNamespacedCaches(namespaces, New)(options) WithClusterScopedCache(New)(options) } } -// WithNamespaceCaches configures MultiNamespacedCacheWithOptionsBuilder +// WithNamespacedCaches configures MultiNamespacedCacheWithOptionsBuilder // with namespace-specific caches that are created using the provided NewCacheFunc. -func WithNamespaceCaches(namespaces []string, f NewCacheFunc) MultiNamespacedOption { +func WithNamespacedCaches(namespaces []string, f NewCacheFunc) MultiNamespacedOption { return func(options *MultiNamespacedOptions) { for _, ns := range namespaces { - WithNamespaceCache(ns, f)(options) + WithNamespacedCache(ns, f)(options) } } } -// WithNamespaceCache configures MultiNamespacedCacheWithOptionsBuilder +// WithNamespacedCache configures MultiNamespacedCacheWithOptionsBuilder // with a namespace cache that uses the provided NewCacheFunc. -func WithNamespaceCache(namespace string, f NewCacheFunc) MultiNamespacedOption { +func WithNamespacedCache(namespace string, f NewCacheFunc) MultiNamespacedOption { return func(options *MultiNamespacedOptions) { - options.NewNamespaceCaches[namespace] = f + options.NewNamespacedCaches[namespace] = f } } @@ -89,7 +89,7 @@ func WithClusterScopedCache(f NewCacheFunc) MultiNamespacedOption { // WithDefaultNamespacedCache configures MultiNamespacedCacheWithOptionsBuilder // with a "catch-all" cache for namespace-scoped objects that are in namespaces -// explicitly configured on the cache builder. +// not explicitly configured on the cache builder. func WithDefaultNamespacedCache(f NewCacheFunc) MultiNamespacedOption { return func(options *MultiNamespacedOptions) { options.NewDefaultNamespacedCache = f @@ -112,7 +112,7 @@ func WithDefaultNamespacedCache(f NewCacheFunc) MultiNamespacedOption { // operations, reporting that a cluster-scoped cache is not defined. func MultiNamespacedCacheWithOptionsBuilder(opts ...MultiNamespacedOption) NewCacheFunc { multiNamespaceOpts := MultiNamespacedOptions{ - NewNamespaceCaches: map[string]NewCacheFunc{}, + NewNamespacedCaches: map[string]NewCacheFunc{}, } for _, opt := range opts { opt(&multiNamespaceOpts) @@ -134,14 +134,15 @@ func MultiNamespacedCacheWithOptionsBuilder(opts ...MultiNamespacedOption) NewCa nsToCache := map[string]Cache{} if multiNamespaceOpts.NewDefaultNamespacedCache != nil { - defaultNamespaceCache, err := multiNamespaceOpts.NewDefaultNamespacedCache(config, ignoreNamespaces(opts, multiNamespaceOpts.NewNamespaceCaches)) + defaultNamespacedOpts := setDefaultNamespacedCacheOpts(opts, multiNamespaceOpts.NewNamespacedCaches) + defaultNamespacedCache, err := multiNamespaceOpts.NewDefaultNamespacedCache(config, defaultNamespacedOpts) if err != nil { return nil, err } - nsToCache[corev1.NamespaceAll] = defaultNamespaceCache + nsToCache[corev1.NamespaceAll] = defaultNamespacedCache } - for ns, newCacheFunc := range multiNamespaceOpts.NewNamespaceCaches { + for ns, newCacheFunc := range multiNamespaceOpts.NewNamespacedCaches { opts.Namespace = ns nsToCache[ns], err = newCacheFunc(config, opts) if err != nil { @@ -158,7 +159,7 @@ func MultiNamespacedCacheWithOptionsBuilder(opts ...MultiNamespacedOption) NewCa } } -func ignoreNamespaces(opts Options, newObjectCaches map[string]NewCacheFunc) Options { +func setDefaultNamespacedCacheOpts(opts Options, newObjectCaches map[string]NewCacheFunc) Options { fieldSelectors := []fields.Selector{} if opts.DefaultSelector.Field != nil { fieldSelectors = append(fieldSelectors, opts.DefaultSelector.Field) @@ -180,11 +181,11 @@ func ignoreNamespaces(opts Options, newObjectCaches map[string]NewCacheFunc) Opt // Deprecated: Use MultiNamespacedCacheWithOptionsBuilder instead: // // cache.MultiNamespacedCacheWithOptionsBuilder( -// WithLegacyNamespaceCaches(namespaces), +// WithLegacyNamespacedCaches(namespaces), // ) func MultiNamespacedCacheBuilder(namespaces []string) NewCacheFunc { return MultiNamespacedCacheWithOptionsBuilder( - WithLegacyNamespaceCaches(namespaces), + WithLegacyNamespacedCaches(namespaces), ) } diff --git a/pkg/manager/example_test.go b/pkg/manager/example_test.go index 4fa79ace49..8248f4ad88 100644 --- a/pkg/manager/example_test.go +++ b/pkg/manager/example_test.go @@ -60,7 +60,7 @@ func ExampleNew_multinamespaceCache() { mgr, err := manager.New(cfg, manager.Options{ NewCache: cache.MultiNamespacedCacheWithOptionsBuilder( - cache.WithLegacyNamespaceCaches([]string{"namespace1", "namespace2"}), + cache.WithLegacyNamespacedCaches([]string{"namespace1", "namespace2"}), ), }) if err != nil {