From 7e2dbaff79520c26d81ffa21bc2e89f33ce37b32 Mon Sep 17 00:00:00 2001 From: Vince Prignano Date: Wed, 1 Feb 2023 10:50:02 -0800 Subject: [PATCH] Cache's ByObject is now a map Object -> options This is a breaking change and revisits some of the previous changes we've made. It brings the options ByObject to a top level map, which now contains all the options the internal informer can be configured with. In addition, it removes the ObjectAll object, which was super confusing and only used to disable the deep copy for every object; instead it's now possible to disable deep copy through a top level option. Signed-off-by: Vince Prignano --- pkg/cache/cache.go | 304 ++++++++-------------- pkg/cache/cache_test.go | 121 ++++----- pkg/cache/cache_unit_test.go | 354 +++++++++++++++----------- pkg/cache/internal/disabledeepcopy.go | 35 --- pkg/cache/internal/informers.go | 77 ++++-- pkg/cache/internal/selector.go | 15 -- pkg/cache/multi_namespace_cache.go | 10 +- pkg/cluster/cluster.go | 4 +- pkg/manager/example_test.go | 2 +- 9 files changed, 428 insertions(+), 494 deletions(-) delete mode 100644 pkg/cache/internal/disabledeepcopy.go diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index b01746b3cc..f74e8e1077 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -39,7 +39,10 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/internal/log" ) -var log = logf.RuntimeLog.WithName("object-cache") +var ( + log = logf.RuntimeLog.WithName("object-cache") + defaultResyncTime = 10 * time.Hour +) // Cache knows how to load Kubernetes objects, fetch informers to request // to receive events for Kubernetes objects (at a low-level), @@ -100,14 +103,6 @@ type Informer interface { HasSynced() bool } -// ObjectSelector is an alias name of internal.Selector. -type ObjectSelector internal.Selector - -// SelectorsByObject associate a client.Object's GVK to a field/label selector. -// There is also `DefaultSelector` to set a global default (which will be overridden by -// a more specific setting here, if any). -type SelectorsByObject map[client.Object]ObjectSelector - // Options are the optional arguments for creating a new InformersMap object. type Options struct { // HTTPClient is the http client to use for the REST client @@ -125,39 +120,41 @@ type Options struct { // So that all informers will not send list requests simultaneously. ResyncEvery *time.Duration - // View restricts the cache's ListWatch to the desired fields per GVK - // Default watches all fields and all namespaces. - View ViewOptions -} - -// ViewOptions are the optional arguments for creating a cache view. -// A cache view restricts the Get(), List(), and internal ListWatch operations -// to the desired parameter. -type ViewOptions struct { // Namespaces restricts the cache's ListWatch to the desired namespaces // Default watches all namespaces Namespaces []string - // DefaultSelector will be used as selectors for all object types + // DefaultLabelSelector will be used as a label selectors for all object types // unless they have a more specific selector set in ByObject. - DefaultSelector ObjectSelector + DefaultLabelSelector labels.Selector + + // DefaultFieldSelector will be used as a field selectors for all object types + // unless they have a more specific selector set in ByObject. + DefaultFieldSelector fields.Selector // DefaultTransform will be used as transform for all object types // unless they have a more specific transform set in ByObject. DefaultTransform toolscache.TransformFunc // ByObject restricts the cache's ListWatch to the desired fields per GVK at the specified object. - ByObject ViewByObject + ByObject map[client.Object]ByObject + + // UnsafeDisableDeepCopy indicates not to deep copy objects during get or + // list objects for EVERY object. + // 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 } -// ViewByObject offers more fine-grained control over the cache's ListWatch by object. -type ViewByObject struct { - // Selectors restricts the cache's ListWatch to the desired - // fields per GVK at the specified object, the map's value must implement - // Selectors [1] using for example a Set [2] - // [1] https://pkg.go.dev/k8s.io/apimachinery/pkg/fields#Selectors - // [2] https://pkg.go.dev/k8s.io/apimachinery/pkg/fields#Set - Selectors SelectorsByObject +// ByObject offers more fine-grained control over the cache's ListWatch by object. +type ByObject struct { + // Label represents a label selector for the object. + Label labels.Selector + + // Field represents a field selector for the object. + Field fields.Selector // Transform is a map from objects to transformer functions which // get applied when objects of the transformation are about to be committed @@ -165,48 +162,39 @@ type ViewByObject struct { // // This function is called both for new objects to enter the cache, // and for updated objects. - Transform TransformByObject + Transform toolscache.TransformFunc // UnsafeDisableDeepCopy indicates not to deep copy objects during get or // list objects per GVK at the specified object. // Be very careful with this, when enabled you must DeepCopy any object before mutating it, // otherwise you will mutate the object in the cache. - UnsafeDisableDeepCopy DisableDeepCopyByObject + UnsafeDisableDeepCopy *bool } -var defaultResyncTime = 10 * time.Hour - // New initializes and returns a new Cache. func New(config *rest.Config, opts Options) (Cache, error) { opts, err := defaultOpts(config, opts) if err != nil { return nil, err } - selectorsByGVK, err := convertToByGVK(opts.View.ByObject.Selectors, opts.View.DefaultSelector, opts.Scheme) - if err != nil { - return nil, err + if len(opts.Namespaces) == 0 { + opts.Namespaces = []string{metav1.NamespaceAll} } - disableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(opts.View.ByObject.UnsafeDisableDeepCopy, opts.Scheme) - if err != nil { - return nil, err + if len(opts.Namespaces) > 1 { + return newMultiNamespaceCache(config, opts) } - transformers, err := convertToByGVK(opts.View.ByObject.Transform, opts.View.DefaultTransform, opts.Scheme) + byGVK, err := convertToInformerOptsByGVK(opts.ByObject, opts.Scheme) if err != nil { return nil, err } - transformByGVK := internal.TransformFuncByGVKFromMap(transformers) - - internalSelectorsByGVK := internal.SelectorsByGVK{} - for gvk, selector := range selectorsByGVK { - internalSelectorsByGVK[gvk] = internal.Selector(selector) - } - - if len(opts.View.Namespaces) == 0 { - opts.View.Namespaces = []string{metav1.NamespaceAll} - } - - if len(opts.View.Namespaces) > 1 { - return newMultiNamespaceCache(config, opts) + // Set the default selector and transform. + byGVK[schema.GroupVersionKind{}] = internal.InformersOptsByGVK{ + Selector: internal.Selector{ + Label: opts.DefaultLabelSelector, + Field: opts.DefaultFieldSelector, + }, + Transform: opts.DefaultTransform, + UnsafeDisableDeepCopy: opts.UnsafeDisableDeepCopy, } return &informerCache{ @@ -216,22 +204,18 @@ func New(config *rest.Config, opts Options) (Cache, error) { Scheme: opts.Scheme, Mapper: opts.Mapper, ResyncPeriod: *opts.ResyncEvery, - Namespace: opts.View.Namespaces[0], - ByGVK: internal.InformersOptsByGVK{ - Selectors: internalSelectorsByGVK, - DisableDeepCopy: disableDeepCopyByGVK, - Transformers: transformByGVK, - }, + Namespace: opts.Namespaces[0], + ByGVK: byGVK, }), }, nil } // BuilderWithOptions returns a Cache constructor that will build a cache // honoring the options argument, this is useful to specify options like -// SelectorsByObject -// WARNING: If SelectorsByObject is specified, filtered out resources are not +// ByObjects, DefaultSelector, DefaultTransform, etc. +// WARNING: If ByObject selectors are specified, filtered out resources are not // returned. -// WARNING: If UnsafeDisableDeepCopy is enabled, you must DeepCopy any object +// WARNING: If ByObject UnsafeDisableDeepCopy is enabled, you must DeepCopy any object // returned from cache get/list before mutating it. func BuilderWithOptions(options Options) NewCacheFunc { return func(config *rest.Config, inherited Options) (Cache, error) { @@ -260,16 +244,22 @@ func (options Options) inheritFrom(inherited Options) (*Options, error) { combined.Scheme = combineScheme(inherited.Scheme, options.Scheme) combined.Mapper = selectMapper(inherited.Mapper, options.Mapper) combined.ResyncEvery = selectResync(inherited.ResyncEvery, options.ResyncEvery) - combined.View.Namespaces = selectNamespaces(inherited.View.Namespaces, options.View.Namespaces) - combined.View.ByObject.Selectors, combined.View.DefaultSelector, err = combineSelectors(inherited, options, combined.Scheme) - if err != nil { - return nil, err + combined.Namespaces = selectNamespaces(inherited.Namespaces, options.Namespaces) + combined.DefaultLabelSelector = combineSelector( + internal.Selector{Label: inherited.DefaultLabelSelector}, + internal.Selector{Label: options.DefaultLabelSelector}, + ).Label + combined.DefaultFieldSelector = combineSelector( + internal.Selector{Field: inherited.DefaultFieldSelector}, + internal.Selector{Field: options.DefaultFieldSelector}, + ).Field + combined.DefaultTransform = combineTransform(inherited.DefaultTransform, options.DefaultTransform) + combined.ByObject, err = combineByObject(inherited, options, combined.Scheme) + if options.UnsafeDisableDeepCopy != nil { + combined.UnsafeDisableDeepCopy = options.UnsafeDisableDeepCopy + } else { + combined.UnsafeDisableDeepCopy = inherited.UnsafeDisableDeepCopy } - combined.View.ByObject.UnsafeDisableDeepCopy, err = combineUnsafeDeepCopy(inherited, options, combined.Scheme) - if err != nil { - return nil, err - } - combined.View.ByObject.Transform, combined.View.DefaultTransform, err = combineTransforms(inherited, options, combined.Scheme) if err != nil { return nil, err } @@ -313,37 +303,37 @@ func selectNamespaces(def, override []string) []string { return def } -func combineSelectors(inherited, options Options, scheme *runtime.Scheme) (SelectorsByObject, ObjectSelector, error) { - // Selectors are combined via logical AND. - // - Combined label selector is a union of the selectors requirements from both sets of options. - // - Combined field selector uses fields.AndSelectors with the combined list of non-nil field selectors - // defined in both sets of options. - // - // There is a bunch of complexity here because we need to convert to SelectorsByGVK - // to be able to match keys between options and inherited and then convert back to SelectorsByObject - optionsSelectorsByGVK, err := convertToByGVK(options.View.ByObject.Selectors, options.View.DefaultSelector, scheme) +func combineByObject(inherited, options Options, scheme *runtime.Scheme) (map[client.Object]ByObject, error) { + optionsByGVK, err := convertToInformerOptsByGVK(options.ByObject, scheme) if err != nil { - return nil, ObjectSelector{}, err + return nil, err } - inheritedSelectorsByGVK, err := convertToByGVK(inherited.View.ByObject.Selectors, inherited.View.DefaultSelector, inherited.Scheme) + inheritedByGVK, err := convertToInformerOptsByGVK(inherited.ByObject, scheme) if err != nil { - return nil, ObjectSelector{}, err + return nil, err } - - for gvk, inheritedSelector := range inheritedSelectorsByGVK { - optionsSelectorsByGVK[gvk] = combineSelector(inheritedSelector, optionsSelectorsByGVK[gvk]) + for gvk, inheritedByGVK := range inheritedByGVK { + unsafeDisableDeepCopy := options.UnsafeDisableDeepCopy + if current, ok := optionsByGVK[gvk]; ok { + unsafeDisableDeepCopy = current.UnsafeDisableDeepCopy + } + optionsByGVK[gvk] = internal.InformersOptsByGVK{ + Selector: combineSelector(inheritedByGVK.Selector, optionsByGVK[gvk].Selector), + Transform: combineTransform(inheritedByGVK.Transform, optionsByGVK[gvk].Transform), + UnsafeDisableDeepCopy: unsafeDisableDeepCopy, + } } - return convertToByObject(optionsSelectorsByGVK, scheme) + return convertToByObject(optionsByGVK, scheme) } -func combineSelector(selectors ...ObjectSelector) ObjectSelector { +func combineSelector(selectors ...internal.Selector) internal.Selector { ls := make([]labels.Selector, 0, len(selectors)) fs := make([]fields.Selector, 0, len(selectors)) for _, s := range selectors { ls = append(ls, s.Label) fs = append(fs, s.Field) } - return ObjectSelector{ + return internal.Selector{ Label: combineLabelSelectors(ls...), Field: combineFieldSelectors(fs...), } @@ -381,51 +371,6 @@ func combineFieldSelectors(fs ...fields.Selector) fields.Selector { return fields.AndSelectors(nonNil...) } -func combineUnsafeDeepCopy(inherited, options Options, scheme *runtime.Scheme) (DisableDeepCopyByObject, error) { - // UnsafeDisableDeepCopyByObject is combined via precedence. Only if a value for a particular GVK is unset - // in options will a value from inherited be used. - optionsDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(options.View.ByObject.UnsafeDisableDeepCopy, options.Scheme) - if err != nil { - return nil, err - } - inheritedDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(inherited.View.ByObject.UnsafeDisableDeepCopy, inherited.Scheme) - if err != nil { - return nil, err - } - - for gvk, inheritedDeepCopy := range inheritedDisableDeepCopyByGVK { - if _, ok := optionsDisableDeepCopyByGVK[gvk]; !ok { - if optionsDisableDeepCopyByGVK == nil { - optionsDisableDeepCopyByGVK = map[schema.GroupVersionKind]bool{} - } - optionsDisableDeepCopyByGVK[gvk] = inheritedDeepCopy - } - } - return convertToDisableDeepCopyByObject(optionsDisableDeepCopyByGVK, scheme) -} - -func combineTransforms(inherited, options Options, scheme *runtime.Scheme) (TransformByObject, toolscache.TransformFunc, error) { - // Transform functions are combined via chaining. If both inherited and options define a transform - // function, the transform function from inherited will be called first, and the transform function from - // options will be called second. - optionsTransformByGVK, err := convertToByGVK(options.View.ByObject.Transform, options.View.DefaultTransform, options.Scheme) - if err != nil { - return nil, nil, err - } - inheritedTransformByGVK, err := convertToByGVK(inherited.View.ByObject.Transform, inherited.View.DefaultTransform, inherited.Scheme) - if err != nil { - return nil, nil, err - } - - for gvk, inheritedTransform := range inheritedTransformByGVK { - if optionsTransformByGVK == nil { - optionsTransformByGVK = map[schema.GroupVersionKind]toolscache.TransformFunc{} - } - optionsTransformByGVK[gvk] = combineTransform(inheritedTransform, optionsTransformByGVK[gvk]) - } - return convertToByObject(optionsTransformByGVK, scheme) -} - func combineTransform(inherited, current toolscache.TransformFunc) toolscache.TransformFunc { if inherited == nil { return current @@ -477,77 +422,32 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) { return opts, nil } -func convertToByGVK[T any](byObject map[client.Object]T, def T, scheme *runtime.Scheme) (map[schema.GroupVersionKind]T, error) { - byGVK := map[schema.GroupVersionKind]T{} - for object, value := range byObject { +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 } - byGVK[gvk] = value - } - byGVK[schema.GroupVersionKind{}] = def - return byGVK, nil -} - -func convertToByObject[T any](byGVK map[schema.GroupVersionKind]T, scheme *runtime.Scheme) (map[client.Object]T, T, error) { - var byObject map[client.Object]T - def := byGVK[schema.GroupVersionKind{}] - for gvk, value := range byGVK { - if gvk == (schema.GroupVersionKind{}) { - continue + 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) } - obj, err := scheme.New(gvk) - if err != nil { - return nil, def, err - } - cObj, ok := obj.(client.Object) - if !ok { - return nil, def, fmt.Errorf("object %T for GVK %q does not implement client.Object", obj, gvk) - } - if byObject == nil { - byObject = map[client.Object]T{} - } - byObject[cObj] = value - } - return byObject, def, nil -} - -// DisableDeepCopyByObject associate a client.Object's GVK to disable DeepCopy during get or list from cache. -type DisableDeepCopyByObject map[client.Object]bool - -var _ client.Object = &ObjectAll{} - -// ObjectAll is the argument to represent all objects' types. -type ObjectAll struct { - client.Object -} - -func convertToDisableDeepCopyByGVK(disableDeepCopyByObject DisableDeepCopyByObject, scheme *runtime.Scheme) (internal.DisableDeepCopyByGVK, error) { - disableDeepCopyByGVK := internal.DisableDeepCopyByGVK{} - for obj, disable := range disableDeepCopyByObject { - switch obj.(type) { - case ObjectAll, *ObjectAll: - disableDeepCopyByGVK[internal.GroupVersionKindAll] = disable - default: - gvk, err := apiutil.GVKForObject(obj, scheme) - if err != nil { - return nil, err - } - disableDeepCopyByGVK[gvk] = disable + out[gvk] = internal.InformersOptsByGVK{ + Selector: internal.Selector{ + Field: byObject.Field, + Label: byObject.Label, + }, + Transform: byObject.Transform, + UnsafeDisableDeepCopy: byObject.UnsafeDisableDeepCopy, } } - return disableDeepCopyByGVK, nil + return out, nil } -func convertToDisableDeepCopyByObject(byGVK internal.DisableDeepCopyByGVK, scheme *runtime.Scheme) (DisableDeepCopyByObject, error) { - var byObject DisableDeepCopyByObject - for gvk, value := range byGVK { - if byObject == nil { - byObject = DisableDeepCopyByObject{} - } +func convertToByObject(in map[schema.GroupVersionKind]internal.InformersOptsByGVK, scheme *runtime.Scheme) (map[client.Object]ByObject, error) { + out := map[client.Object]ByObject{} + for gvk, opts := range in { if gvk == (schema.GroupVersionKind{}) { - byObject[ObjectAll{}] = value continue } obj, err := scheme.New(gvk) @@ -558,12 +458,12 @@ func convertToDisableDeepCopyByObject(byGVK internal.DisableDeepCopyByGVK, schem if !ok { return nil, fmt.Errorf("object %T for GVK %q does not implement client.Object", obj, gvk) } - - byObject[cObj] = value + out[cObj] = ByObject{ + Field: opts.Selector.Field, + Label: opts.Selector.Label, + Transform: opts.Transform, + UnsafeDisableDeepCopy: opts.UnsafeDisableDeepCopy, + } } - return byObject, nil + return out, nil } - -// TransformByObject associate a client.Object's GVK to a transformer function -// to be applied when storing the object into the cache. -type TransformByObject map[client.Object]toolscache.TransformFunc diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index 4e882f0f8f..36213c6ffd 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -38,6 +38,7 @@ import ( kscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" kcache "k8s.io/client-go/tools/cache" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" @@ -119,13 +120,10 @@ var _ = Describe("Informer Cache", func() { var _ = Describe("Multi-Namespace Informer Cache", func() { CacheTest(cache.MultiNamespacedCacheBuilder([]string{testNamespaceOne, testNamespaceTwo, "default"}), cache.Options{}) }) -var _ = Describe("Informer Cache without DeepCopy", func() { + +var _ = Describe("Informer Cache without global DeepCopy", func() { CacheTest(cache.New, cache.Options{ - View: cache.ViewOptions{ - ByObject: cache.ViewByObject{ - UnsafeDisableDeepCopy: cache.DisableDeepCopyByObject{cache.ObjectAll{}: true}, - }, - }, + UnsafeDisableDeepCopy: pointer.Bool(true), }) }) @@ -190,48 +188,46 @@ var _ = Describe("Cache with transformers", func() { By("creating the informer cache") informerCache, err = cache.New(cfg, cache.Options{ - View: cache.ViewOptions{ - DefaultTransform: func(i interface{}) (interface{}, error) { - obj := i.(runtime.Object) - Expect(obj).NotTo(BeNil()) + DefaultTransform: func(i interface{}) (interface{}, error) { + obj := i.(runtime.Object) + Expect(obj).NotTo(BeNil()) - accessor, err := meta.Accessor(obj) - Expect(err).To(BeNil()) - annotations := accessor.GetAnnotations() - - if _, exists := annotations["transformed"]; exists { - // Avoid performing transformation multiple times. - return i, nil - } + accessor, err := meta.Accessor(obj) + Expect(err).To(BeNil()) + annotations := accessor.GetAnnotations() - if annotations == nil { - annotations = make(map[string]string) - } - annotations["transformed"] = "default" - accessor.SetAnnotations(annotations) + if _, exists := annotations["transformed"]; exists { + // Avoid performing transformation multiple times. return i, nil - }, - ByObject: cache.ViewByObject{ - Transform: cache.TransformByObject{ - &corev1.Pod{}: func(i interface{}) (interface{}, error) { - obj := i.(runtime.Object) - Expect(obj).NotTo(BeNil()) - accessor, err := meta.Accessor(obj) - Expect(err).To(BeNil()) - - annotations := accessor.GetAnnotations() - if _, exists := annotations["transformed"]; exists { - // Avoid performing transformation multiple times. - return i, nil - } - - if annotations == nil { - annotations = make(map[string]string) - } - annotations["transformed"] = "explicit" - accessor.SetAnnotations(annotations) + } + + if annotations == nil { + annotations = make(map[string]string) + } + annotations["transformed"] = "default" + accessor.SetAnnotations(annotations) + return i, nil + }, + ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: { + Transform: func(i interface{}) (interface{}, error) { + obj := i.(runtime.Object) + Expect(obj).NotTo(BeNil()) + accessor, err := meta.Accessor(obj) + Expect(err).To(BeNil()) + + annotations := accessor.GetAnnotations() + if _, exists := annotations["transformed"]; exists { + // Avoid performing transformation multiple times. return i, nil - }, + } + + if annotations == nil { + annotations = make(map[string]string) + } + annotations["transformed"] = "explicit" + accessor.SetAnnotations(annotations) + return i, nil }, }, }, @@ -370,13 +366,9 @@ var _ = Describe("Cache with selectors", func() { } opts := cache.Options{ - View: cache.ViewOptions{ - DefaultSelector: cache.ObjectSelector{Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceTwo)}, - ByObject: cache.ViewByObject{ - Selectors: cache.SelectorsByObject{ - &corev1.ServiceAccount{}: {Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceOne)}, - }, - }, + DefaultFieldSelector: fields.OneTermEqualSelector("metadata.namespace", testNamespaceTwo), + ByObject: map[client.Object]cache.ByObject{ + &corev1.ServiceAccount{}: {Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceOne)}, }, } @@ -798,7 +790,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{View: cache.ViewOptions{Namespaces: []string{testNamespaceOne}}}) + namespacedCache, err := cache.New(cfg, cache.Options{Namespaces: []string{testNamespaceOne}}) Expect(err).NotTo(HaveOccurred()) By("running the cache and waiting for it to sync") @@ -1079,7 +1071,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{View: cache.ViewOptions{Namespaces: []string{testNamespaceOne}}}) + namespacedCache, err := cache.New(cfg, cache.Options{Namespaces: []string{testNamespaceOne}}) Expect(err).NotTo(HaveOccurred()) By("running the cache and waiting for it to sync") @@ -1233,14 +1225,10 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca By("creating the cache") builder := cache.BuilderWithOptions( cache.Options{ - View: cache.ViewOptions{ - ByObject: cache.ViewByObject{ - Selectors: cache.SelectorsByObject{ - &corev1.Pod{}: { - Label: labels.Set(tc.labelSelectors).AsSelector(), - Field: fields.Set(tc.fieldSelectors).AsSelector(), - }, - }, + ByObject: map[client.Object]cache.ByObject{ + &corev1.Pod{}: { + Label: labels.Set(tc.labelSelectors).AsSelector(), + Field: fields.Set(tc.fieldSelectors).AsSelector(), }, }, }, @@ -1822,12 +1810,11 @@ func isKubeService(svc metav1.Object) bool { } func isPodDisableDeepCopy(opts cache.Options) bool { - if d, ok := opts.View.ByObject.UnsafeDisableDeepCopy[&corev1.Pod{}]; ok { - return d - } else if d, ok = opts.View.ByObject.UnsafeDisableDeepCopy[cache.ObjectAll{}]; ok { - return d - } else if d, ok = opts.View.ByObject.UnsafeDisableDeepCopy[&cache.ObjectAll{}]; ok { - return d + if opts.ByObject[&corev1.Pod{}].UnsafeDisableDeepCopy != nil { + return *opts.ByObject[&corev1.Pod{}].UnsafeDisableDeepCopy + } + if opts.UnsafeDisableDeepCopy != nil { + return *opts.UnsafeDisableDeepCopy } return false } diff --git a/pkg/cache/cache_unit_test.go b/pkg/cache/cache_unit_test.go index 8f74e2407f..1006c72812 100644 --- a/pkg/cache/cache_unit_test.go +++ b/pkg/cache/cache_unit_test.go @@ -14,7 +14,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/cache" "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" @@ -110,184 +109,231 @@ var _ = Describe("cache.inheritFrom", func() { }) }) Context("Namespace", func() { - It("has zero length when View.Namespaces specified and inherited are unset", func() { - Expect(checkError(specified.inheritFrom(inherited)).View.Namespaces).To(HaveLen(0)) + It("has zero length when Namespaces specified and inherited are unset", func() { + Expect(checkError(specified.inheritFrom(inherited)).Namespaces).To(HaveLen(0)) }) It("is unchanged when only specified is set", func() { - specified.View.Namespaces = []string{"specified"} - Expect(checkError(specified.inheritFrom(inherited)).View.Namespaces).To(Equal(specified.View.Namespaces)) + specified.Namespaces = []string{"specified"} + Expect(checkError(specified.inheritFrom(inherited)).Namespaces).To(Equal(specified.Namespaces)) }) It("is inherited when only inherited is set", func() { - inherited.View.Namespaces = []string{"inherited"} - Expect(checkError(specified.inheritFrom(inherited)).View.Namespaces).To(Equal(inherited.View.Namespaces)) + inherited.Namespaces = []string{"inherited"} + Expect(checkError(specified.inheritFrom(inherited)).Namespaces).To(Equal(inherited.Namespaces)) }) It("in unchanged when both inherited and specified are set", func() { - specified.View.Namespaces = []string{"specified"} - inherited.View.Namespaces = []string{"inherited"} - Expect(checkError(specified.inheritFrom(inherited)).View.Namespaces).To(Equal(specified.View.Namespaces)) + specified.Namespaces = []string{"specified"} + inherited.Namespaces = []string{"inherited"} + Expect(checkError(specified.inheritFrom(inherited)).Namespaces).To(Equal(specified.Namespaces)) }) }) Context("SelectorsByObject", func() { It("is unchanged when specified and inherited are unset", func() { - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors).To(BeNil()) + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(0)) }) It("is unchanged when only specified is set", func() { specified.Scheme = coreScheme - specified.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.Pod{}: {}} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors).To(HaveLen(1)) + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: {}, + } + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(1)) }) It("is inherited when only inherited is set", func() { inherited.Scheme = coreScheme - inherited.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.ConfigMap{}: {}} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors).To(HaveLen(1)) + inherited.ByObject = map[client.Object]ByObject{ + &corev1.ConfigMap{}: {}, + } + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(1)) }) It("is combined when both inherited and specified are set", func() { specified.Scheme = coreScheme inherited.Scheme = coreScheme - specified.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.Pod{}: {}} - inherited.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.ConfigMap{}: {}} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors).To(HaveLen(2)) + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: {}, + } + inherited.ByObject = map[client.Object]ByObject{ + &corev1.ConfigMap{}: {}, + } + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(2)) }) It("combines selectors if specified and inherited specify selectors for the same object", func() { specified.Scheme = coreScheme inherited.Scheme = coreScheme - specified.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.Pod{}: { - Label: labels.Set{"specified": "true"}.AsSelector(), - Field: fields.Set{"metadata.name": "specified"}.AsSelector(), - }} - inherited.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.Pod{}: { - Label: labels.Set{"inherited": "true"}.AsSelector(), - Field: fields.Set{"metadata.namespace": "inherited"}.AsSelector(), - }} - combined := checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + Label: labels.Set{"specified": "true"}.AsSelector(), + Field: fields.Set{"metadata.name": "specified"}.AsSelector(), + }, + } + inherited.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + Label: labels.Set{"inherited": "true"}.AsSelector(), + Field: fields.Set{"metadata.namespace": "inherited"}.AsSelector(), + }, + } + combined := checkError(specified.inheritFrom(inherited)).ByObject Expect(combined).To(HaveLen(1)) var ( obj client.Object - selector ObjectSelector + byObject ByObject ) - for obj, selector = range combined { + for obj, byObject = range combined { } Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) - Expect(selector.Label.Matches(labels.Set{"specified": "true"})).To(BeFalse()) - Expect(selector.Label.Matches(labels.Set{"inherited": "true"})).To(BeFalse()) - Expect(selector.Label.Matches(labels.Set{"specified": "true", "inherited": "true"})).To(BeTrue()) + Expect(byObject.Label.Matches(labels.Set{"specified": "true"})).To(BeFalse()) + Expect(byObject.Label.Matches(labels.Set{"inherited": "true"})).To(BeFalse()) + Expect(byObject.Label.Matches(labels.Set{"specified": "true", "inherited": "true"})).To(BeTrue()) - Expect(selector.Field.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "other"})).To(BeFalse()) - Expect(selector.Field.Matches(fields.Set{"metadata.name": "other", "metadata.namespace": "inherited"})).To(BeFalse()) - Expect(selector.Field.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "inherited"})).To(BeTrue()) + Expect(byObject.Field.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "other"})).To(BeFalse()) + Expect(byObject.Field.Matches(fields.Set{"metadata.name": "other", "metadata.namespace": "inherited"})).To(BeFalse()) + Expect(byObject.Field.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "inherited"})).To(BeTrue()) }) It("uses inherited scheme for inherited selectors", func() { inherited.Scheme = coreScheme - inherited.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.ConfigMap{}: {}} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors).To(HaveLen(1)) - }) - It("does not use specified scheme for inherited selectors", func() { - inherited.Scheme = runtime.NewScheme() - specified.Scheme = coreScheme - inherited.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.ConfigMap{}: {}} - _, err := specified.inheritFrom(inherited) - Expect(err).To(WithTransform(runtime.IsNotRegisteredError, BeTrue())) + inherited.ByObject = map[client.Object]ByObject{ + &corev1.ConfigMap{}: {}, + } + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(1)) }) It("uses inherited scheme for specified selectors", func() { inherited.Scheme = coreScheme - specified.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.ConfigMap{}: {}} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors).To(HaveLen(1)) + specified.ByObject = map[client.Object]ByObject{ + &corev1.ConfigMap{}: {}, + } + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(1)) }) It("uses specified scheme for specified selectors", func() { specified.Scheme = coreScheme - specified.View.ByObject.Selectors = map[client.Object]ObjectSelector{&corev1.ConfigMap{}: {}} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Selectors).To(HaveLen(1)) + specified.ByObject = map[client.Object]ByObject{ + &corev1.ConfigMap{}: {}, + } + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(1)) }) }) Context("DefaultSelector", func() { It("is unchanged when specified and inherited are unset", func() { - Expect(specified.View.DefaultSelector).To(Equal(ObjectSelector{})) - Expect(inherited.View.DefaultSelector).To(Equal(ObjectSelector{})) - Expect(checkError(specified.inheritFrom(inherited)).View.DefaultSelector).To(Equal(ObjectSelector{})) + Expect(specified.DefaultLabelSelector).To(BeNil()) + Expect(inherited.DefaultLabelSelector).To(BeNil()) + Expect(specified.DefaultFieldSelector).To(BeNil()) + Expect(inherited.DefaultFieldSelector).To(BeNil()) + Expect(checkError(specified.inheritFrom(inherited)).DefaultLabelSelector).To(BeNil()) + Expect(checkError(specified.inheritFrom(inherited)).DefaultFieldSelector).To(BeNil()) }) It("is unchanged when only specified is set", func() { - specified.View.DefaultSelector = ObjectSelector{Label: labels.Set{"specified": "true"}.AsSelector()} - Expect(checkError(specified.inheritFrom(inherited)).View.DefaultSelector).To(Equal(specified.View.DefaultSelector)) + specified.DefaultLabelSelector = labels.Set{"specified": "true"}.AsSelector() + specified.DefaultFieldSelector = fields.Set{"specified": "true"}.AsSelector() + Expect(checkError(specified.inheritFrom(inherited)).DefaultLabelSelector).To(Equal(specified.DefaultLabelSelector)) + Expect(checkError(specified.inheritFrom(inherited)).DefaultFieldSelector).To(Equal(specified.DefaultFieldSelector)) }) It("is inherited when only inherited is set", func() { - inherited.View.DefaultSelector = ObjectSelector{Label: labels.Set{"inherited": "true"}.AsSelector()} - Expect(checkError(specified.inheritFrom(inherited)).View.DefaultSelector).To(Equal(inherited.View.DefaultSelector)) + inherited.DefaultLabelSelector = labels.Set{"inherited": "true"}.AsSelector() + inherited.DefaultFieldSelector = fields.Set{"inherited": "true"}.AsSelector() + Expect(checkError(specified.inheritFrom(inherited)).DefaultLabelSelector).To(Equal(inherited.DefaultLabelSelector)) + Expect(checkError(specified.inheritFrom(inherited)).DefaultFieldSelector).To(Equal(inherited.DefaultFieldSelector)) }) It("is combined when both inherited and specified are set", func() { - specified.View.DefaultSelector = ObjectSelector{ - Label: labels.Set{"specified": "true"}.AsSelector(), - Field: fields.Set{"metadata.name": "specified"}.AsSelector(), + specified.DefaultLabelSelector = labels.Set{"specified": "true"}.AsSelector() + specified.DefaultFieldSelector = fields.Set{"metadata.name": "specified"}.AsSelector() + + inherited.DefaultLabelSelector = labels.Set{"inherited": "true"}.AsSelector() + inherited.DefaultFieldSelector = fields.Set{"metadata.namespace": "inherited"}.AsSelector() + { + combined := checkError(specified.inheritFrom(inherited)).DefaultLabelSelector + Expect(combined).NotTo(BeNil()) + Expect(combined.Matches(labels.Set{"specified": "true"})).To(BeFalse()) + Expect(combined.Matches(labels.Set{"inherited": "true"})).To(BeFalse()) + Expect(combined.Matches(labels.Set{"specified": "true", "inherited": "true"})).To(BeTrue()) } - inherited.View.DefaultSelector = ObjectSelector{ - Label: labels.Set{"inherited": "true"}.AsSelector(), - Field: fields.Set{"metadata.namespace": "inherited"}.AsSelector(), + + { + combined := checkError(specified.inheritFrom(inherited)).DefaultFieldSelector + Expect(combined.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "other"})).To(BeFalse()) + Expect(combined.Matches(fields.Set{"metadata.name": "other", "metadata.namespace": "inherited"})).To(BeFalse()) + Expect(combined.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "inherited"})).To(BeTrue()) } - combined := checkError(specified.inheritFrom(inherited)).View.DefaultSelector - Expect(combined).NotTo(BeNil()) - Expect(combined.Label.Matches(labels.Set{"specified": "true"})).To(BeFalse()) - Expect(combined.Label.Matches(labels.Set{"inherited": "true"})).To(BeFalse()) - Expect(combined.Label.Matches(labels.Set{"specified": "true", "inherited": "true"})).To(BeTrue()) - Expect(combined.Field.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "other"})).To(BeFalse()) - Expect(combined.Field.Matches(fields.Set{"metadata.name": "other", "metadata.namespace": "inherited"})).To(BeFalse()) - Expect(combined.Field.Matches(fields.Set{"metadata.name": "specified", "metadata.namespace": "inherited"})).To(BeTrue()) }) }) Context("UnsafeDisableDeepCopyByObject", func() { It("is unchanged when specified and inherited are unset", func() { - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.UnsafeDisableDeepCopy).To(BeNil()) + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(0)) }) It("is unchanged when only specified is set", func() { specified.Scheme = coreScheme - specified.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{ObjectAll{}: true} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.UnsafeDisableDeepCopy).To(HaveLen(1)) + specified.UnsafeDisableDeepCopy = pointer.Bool(true) + Expect(*(checkError(specified.inheritFrom(inherited)).UnsafeDisableDeepCopy)).To(BeTrue()) }) It("is inherited when only inherited is set", func() { inherited.Scheme = coreScheme - inherited.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{ObjectAll{}: true} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.UnsafeDisableDeepCopy).To(HaveLen(1)) + inherited.UnsafeDisableDeepCopy = pointer.Bool(true) + Expect(*(checkError(specified.inheritFrom(inherited)).UnsafeDisableDeepCopy)).To(BeTrue()) }) It("is combined when both inherited and specified are set for different keys", func() { specified.Scheme = coreScheme inherited.Scheme = coreScheme - specified.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{&corev1.Pod{}: true} - inherited.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{&corev1.ConfigMap{}: true} - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.UnsafeDisableDeepCopy).To(HaveLen(2)) + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + UnsafeDisableDeepCopy: pointer.Bool(true), + }, + } + inherited.ByObject = map[client.Object]ByObject{ + &corev1.ConfigMap{}: { + UnsafeDisableDeepCopy: pointer.Bool(true), + }, + } + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(2)) }) It("is true when inherited=false and specified=true for the same key", func() { specified.Scheme = coreScheme inherited.Scheme = coreScheme - specified.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{&corev1.Pod{}: true} - inherited.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{&corev1.Pod{}: false} - combined := checkError(specified.inheritFrom(inherited)).View.ByObject.UnsafeDisableDeepCopy + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + UnsafeDisableDeepCopy: pointer.Bool(true), + }, + } + inherited.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + UnsafeDisableDeepCopy: pointer.Bool(false), + }, + } + combined := checkError(specified.inheritFrom(inherited)).ByObject Expect(combined).To(HaveLen(1)) var ( - obj client.Object - disableDeepCopy bool + obj client.Object + byObject ByObject ) - for obj, disableDeepCopy = range combined { + for obj, byObject = range combined { } Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) - Expect(disableDeepCopy).To(BeTrue()) + Expect(byObject.UnsafeDisableDeepCopy).ToNot(BeNil()) + Expect(*byObject.UnsafeDisableDeepCopy).To(BeTrue()) }) It("is false when inherited=true and specified=false for the same key", func() { specified.Scheme = coreScheme inherited.Scheme = coreScheme - specified.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{&corev1.Pod{}: false} - inherited.View.ByObject.UnsafeDisableDeepCopy = map[client.Object]bool{&corev1.Pod{}: true} - combined := checkError(specified.inheritFrom(inherited)).View.ByObject.UnsafeDisableDeepCopy + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + UnsafeDisableDeepCopy: pointer.Bool(false), + }, + } + inherited.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + UnsafeDisableDeepCopy: pointer.Bool(true), + }, + } + combined := checkError(specified.inheritFrom(inherited)).ByObject Expect(combined).To(HaveLen(1)) var ( - obj client.Object - disableDeepCopy bool + obj client.Object + byObject ByObject ) - for obj, disableDeepCopy = range combined { + for obj, byObject = range combined { } Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) - Expect(disableDeepCopy).To(BeFalse()) + Expect(byObject.UnsafeDisableDeepCopy).ToNot(BeNil()) + Expect(*byObject.UnsafeDisableDeepCopy).To(BeFalse()) }) }) Context("TransformByObject", func() { @@ -302,20 +348,24 @@ var _ = Describe("cache.inheritFrom", func() { tx = transformed{} }) It("is unchanged when specified and inherited are unset", func() { - Expect(checkError(specified.inheritFrom(inherited)).View.ByObject.Transform).To(BeNil()) + Expect(checkError(specified.inheritFrom(inherited)).ByObject).To(HaveLen(0)) }) It("is unchanged when only specified is set", func() { specified.Scheme = coreScheme - specified.View.ByObject.Transform = map[client.Object]cache.TransformFunc{&corev1.Pod{}: func(i interface{}) (interface{}, error) { - ti := i.(transformed) - ti.podSpecified = true - return ti, nil - }} - combined := checkError(specified.inheritFrom(inherited)).View.ByObject.Transform + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + Transform: func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.podSpecified = true + return ti, nil + }, + }, + } + combined := checkError(specified.inheritFrom(inherited)).ByObject Expect(combined).To(HaveLen(1)) - for obj, fn := range combined { + for obj, byObject := range combined { Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) - out, _ := fn(tx) + out, _ := byObject.Transform(tx) Expect(out).To(And( BeAssignableToTypeOf(tx), WithTransform(func(i transformed) bool { return i.podSpecified }, BeTrue()), @@ -325,16 +375,20 @@ var _ = Describe("cache.inheritFrom", func() { }) It("is inherited when only inherited is set", func() { inherited.Scheme = coreScheme - inherited.View.ByObject.Transform = map[client.Object]cache.TransformFunc{&corev1.Pod{}: func(i interface{}) (interface{}, error) { - ti := i.(transformed) - ti.podInherited = true - return ti, nil - }} - combined := checkError(specified.inheritFrom(inherited)).View.ByObject.Transform + inherited.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + Transform: func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.podInherited = true + return ti, nil + }, + }, + } + combined := checkError(specified.inheritFrom(inherited)).ByObject Expect(combined).To(HaveLen(1)) - for obj, fn := range combined { + for obj, byObject := range combined { Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) - out, _ := fn(tx) + out, _ := byObject.Transform(tx) Expect(out).To(And( BeAssignableToTypeOf(tx), WithTransform(func(i transformed) bool { return i.podSpecified }, BeFalse()), @@ -345,20 +399,28 @@ var _ = Describe("cache.inheritFrom", func() { It("is combined when both inherited and specified are set for different keys", func() { specified.Scheme = coreScheme inherited.Scheme = coreScheme - specified.View.ByObject.Transform = map[client.Object]cache.TransformFunc{&corev1.Pod{}: func(i interface{}) (interface{}, error) { - ti := i.(transformed) - ti.podSpecified = true - return ti, nil - }} - inherited.View.ByObject.Transform = map[client.Object]cache.TransformFunc{&corev1.ConfigMap{}: func(i interface{}) (interface{}, error) { - ti := i.(transformed) - ti.configmapInherited = true - return ti, nil - }} - combined := checkError(specified.inheritFrom(inherited)).View.ByObject.Transform + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + Transform: func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.podSpecified = true + return ti, nil + }, + }, + } + inherited.ByObject = map[client.Object]ByObject{ + &corev1.ConfigMap{}: { + Transform: func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.configmapInherited = true + return ti, nil + }, + }, + } + combined := checkError(specified.inheritFrom(inherited)).ByObject Expect(combined).To(HaveLen(2)) - for obj, fn := range combined { - out, _ := fn(tx) + for obj, byObject := range combined { + out, _ := byObject.Transform(tx) if reflect.TypeOf(obj) == reflect.TypeOf(&corev1.Pod{}) { Expect(out).To(And( BeAssignableToTypeOf(tx), @@ -382,21 +444,29 @@ var _ = Describe("cache.inheritFrom", func() { It("is combined into a single transform function when both inherited and specified are set for the same key", func() { specified.Scheme = coreScheme inherited.Scheme = coreScheme - specified.View.ByObject.Transform = map[client.Object]cache.TransformFunc{&corev1.Pod{}: func(i interface{}) (interface{}, error) { - ti := i.(transformed) - ti.podSpecified = true - return ti, nil - }} - inherited.View.ByObject.Transform = map[client.Object]cache.TransformFunc{&corev1.Pod{}: func(i interface{}) (interface{}, error) { - ti := i.(transformed) - ti.podInherited = true - return ti, nil - }} - combined := checkError(specified.inheritFrom(inherited)).View.ByObject.Transform + specified.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + Transform: func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.podSpecified = true + return ti, nil + }, + }, + } + inherited.ByObject = map[client.Object]ByObject{ + &corev1.Pod{}: { + Transform: func(i interface{}) (interface{}, error) { + ti := i.(transformed) + ti.podInherited = true + return ti, nil + }, + }, + } + combined := checkError(specified.inheritFrom(inherited)).ByObject Expect(combined).To(HaveLen(1)) - for obj, fn := range combined { + for obj, byObject := range combined { Expect(obj).To(BeAssignableToTypeOf(&corev1.Pod{})) - out, _ := fn(tx) + out, _ := byObject.Transform(tx) Expect(out).To(And( BeAssignableToTypeOf(tx), WithTransform(func(i transformed) bool { return i.podSpecified }, BeTrue()), @@ -417,15 +487,15 @@ var _ = Describe("cache.inheritFrom", func() { tx = transformed{} }) It("is unchanged when specified and inherited are unset", func() { - Expect(checkError(specified.inheritFrom(inherited)).View.DefaultTransform).To(BeNil()) + Expect(checkError(specified.inheritFrom(inherited)).DefaultTransform).To(BeNil()) }) It("is unchanged when only specified is set", func() { - specified.View.DefaultTransform = func(i interface{}) (interface{}, error) { + specified.DefaultTransform = func(i interface{}) (interface{}, error) { ti := i.(transformed) ti.specified = true return ti, nil } - combined := checkError(specified.inheritFrom(inherited)).View.DefaultTransform + combined := checkError(specified.inheritFrom(inherited)).DefaultTransform out, _ := combined(tx) Expect(out).To(And( BeAssignableToTypeOf(tx), @@ -434,12 +504,12 @@ var _ = Describe("cache.inheritFrom", func() { )) }) It("is inherited when only inherited is set", func() { - inherited.View.DefaultTransform = func(i interface{}) (interface{}, error) { + inherited.DefaultTransform = func(i interface{}) (interface{}, error) { ti := i.(transformed) ti.inherited = true return ti, nil } - combined := checkError(specified.inheritFrom(inherited)).View.DefaultTransform + combined := checkError(specified.inheritFrom(inherited)).DefaultTransform out, _ := combined(tx) Expect(out).To(And( BeAssignableToTypeOf(tx), @@ -448,17 +518,17 @@ var _ = Describe("cache.inheritFrom", func() { )) }) It("is combined when the transform function is defined in both inherited and specified", func() { - specified.View.DefaultTransform = func(i interface{}) (interface{}, error) { + specified.DefaultTransform = func(i interface{}) (interface{}, error) { ti := i.(transformed) ti.specified = true return ti, nil } - inherited.View.DefaultTransform = func(i interface{}) (interface{}, error) { + inherited.DefaultTransform = func(i interface{}) (interface{}, error) { ti := i.(transformed) ti.inherited = true return ti, nil } - combined := checkError(specified.inheritFrom(inherited)).View.DefaultTransform + combined := checkError(specified.inheritFrom(inherited)).DefaultTransform Expect(combined).NotTo(BeNil()) out, _ := combined(tx) Expect(out).To(And( diff --git a/pkg/cache/internal/disabledeepcopy.go b/pkg/cache/internal/disabledeepcopy.go deleted file mode 100644 index 54bd7eec93..0000000000 --- a/pkg/cache/internal/disabledeepcopy.go +++ /dev/null @@ -1,35 +0,0 @@ -/* -Copyright 2021 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 internal - -import "k8s.io/apimachinery/pkg/runtime/schema" - -// GroupVersionKindAll is the argument to represent all GroupVersionKind types. -var GroupVersionKindAll = schema.GroupVersionKind{} - -// DisableDeepCopyByGVK associate a GroupVersionKind to disable DeepCopy during get or list from cache. -type DisableDeepCopyByGVK map[schema.GroupVersionKind]bool - -// IsDisabled returns whether a GroupVersionKind is disabled DeepCopy. -func (disableByGVK DisableDeepCopyByGVK) IsDisabled(gvk schema.GroupVersionKind) bool { - if d, ok := disableByGVK[gvk]; ok { - return d - } else if d, ok = disableByGVK[GroupVersionKindAll]; ok { - return d - } - return false -} diff --git a/pkg/cache/internal/informers.go b/pkg/cache/internal/informers.go index c69c4985ef..bc3e5f23da 100644 --- a/pkg/cache/internal/informers.go +++ b/pkg/cache/internal/informers.go @@ -50,15 +50,15 @@ type InformersOpts struct { Mapper meta.RESTMapper ResyncPeriod time.Duration Namespace string - ByGVK InformersOptsByGVK + ByGVK map[schema.GroupVersionKind]InformersOptsByGVK } // InformersOptsByGVK configured additional by group version kind (or object) // in an InformerMap. type InformersOptsByGVK struct { - Selectors SelectorsByGVK - Transformers TransformFuncByGVK - DisableDeepCopy DisableDeepCopyByGVK + Selector Selector + Transform cache.TransformFunc + UnsafeDisableDeepCopy *bool } // NewInformers creates a new InformersMap that can create informers under the hood. @@ -73,14 +73,12 @@ 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, - selectorByGVK: options.ByGVK.Selectors.forGVK, - disableDeepCopyByGVK: options.ByGVK.DisableDeepCopy, - transformersByGVK: options.ByGVK.Transformers, + codecs: serializer.NewCodecFactory(options.Scheme), + paramCodec: runtime.NewParameterCodec(options.Scheme), + resync: options.ResyncPeriod, + startWait: make(chan struct{}), + namespace: options.Namespace, + byGVK: options.ByGVK, } } @@ -151,15 +149,46 @@ type Informers struct { // default or empty string means all namespaces namespace string - // selectorByGVK are the label or field selectorByGVK that will be added to the - // ListWatch ListOptions. - selectorByGVK func(gvk schema.GroupVersionKind) Selector + byGVK map[schema.GroupVersionKind]InformersOptsByGVK +} - // disableDeepCopyByGVK indicates not to deep copy objects during get or list objects. - disableDeepCopyByGVK DisableDeepCopyByGVK +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{} +} - // transform funcs are applied to objects before they are committed to the cache - transformersByGVK TransformFuncByGVK +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 } // Start calls Run on each of the informers and sets started to true. Blocks on the context. @@ -305,11 +334,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.selectorByGVK(gvk).ApplyToList(&opts) + ip.getSelector(gvk).ApplyToList(&opts) return listWatcher.ListFunc(opts) }, WatchFunc: func(opts metav1.ListOptions) (watch.Interface, error) { - ip.selectorByGVK(gvk).ApplyToList(&opts) + ip.getSelector(gvk).ApplyToList(&opts) opts.Watch = true // Watch needs to be set to true separately return listWatcher.WatchFunc(opts) }, @@ -318,7 +347,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.transformersByGVK.Get(gvk)); err != nil { + if err := sharedIndexInformer.SetTransform(ip.getTransform(gvk)); err != nil { return nil, false, err } @@ -334,7 +363,7 @@ func (ip *Informers) addInformerToMap(gvk schema.GroupVersionKind, obj runtime.O indexer: sharedIndexInformer.GetIndexer(), groupVersionKind: gvk, scopeName: mapping.Scope.Name(), - disableDeepCopy: ip.disableDeepCopyByGVK.IsDisabled(gvk), + disableDeepCopy: ip.getDisableDeepCopy(gvk), }, } ip.informersByType(obj)[gvk] = i @@ -358,7 +387,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.selectorByGVK(gvk)) + namespace = restrictNamespaceBySelector(ip.namespace, ip.getSelector(gvk)) } switch obj.(type) { diff --git a/pkg/cache/internal/selector.go b/pkg/cache/internal/selector.go index 4eff32fb35..c674379b99 100644 --- a/pkg/cache/internal/selector.go +++ b/pkg/cache/internal/selector.go @@ -20,23 +20,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime/schema" ) -// SelectorsByGVK associate a GroupVersionKind to a field/label selector. -type SelectorsByGVK map[schema.GroupVersionKind]Selector - -func (s SelectorsByGVK) forGVK(gvk schema.GroupVersionKind) Selector { - if specific, found := s[gvk]; found { - return specific - } - if defaultSelector, found := s[schema.GroupVersionKind{}]; found { - return defaultSelector - } - - return Selector{} -} - // Selector specify the label/field selector to fill in ListOptions. type Selector struct { Label labels.Selector diff --git a/pkg/cache/multi_namespace_cache.go b/pkg/cache/multi_namespace_cache.go index 1bd2a8c96b..ee33fb0dd5 100644 --- a/pkg/cache/multi_namespace_cache.go +++ b/pkg/cache/multi_namespace_cache.go @@ -47,13 +47,13 @@ const globalCache = "_cluster-scope" // Deprecated: Use cache.Options.View.Namespaces instead. func MultiNamespacedCacheBuilder(namespaces []string) NewCacheFunc { return func(config *rest.Config, opts Options) (Cache, error) { - opts.View.Namespaces = namespaces + opts.Namespaces = namespaces return newMultiNamespaceCache(config, opts) } } func newMultiNamespaceCache(config *rest.Config, opts Options) (Cache, error) { - if len(opts.View.Namespaces) < 2 { + 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) @@ -63,8 +63,8 @@ func newMultiNamespaceCache(config *rest.Config, opts Options) (Cache, error) { // Create every namespace cache. caches := map[string]Cache{} - for _, ns := range opts.View.Namespaces { - opts.View.Namespaces = []string{ns} + for _, ns := range opts.Namespaces { + opts.Namespaces = []string{ns} c, err := New(config, opts) if err != nil { return nil, err @@ -73,7 +73,7 @@ func newMultiNamespaceCache(config *rest.Config, opts Options) (Cache, error) { } // Create a cache for cluster scoped resources. - opts.View.Namespaces = []string{} + opts.Namespaces = []string{} gCache, err := New(config, opts) if err != nil { return nil, fmt.Errorf("error creating global cache: %w", err) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 2bf61b7141..332c7a5e5d 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -176,9 +176,7 @@ func New(config *rest.Config, opts ...Option) (Cluster, error) { Scheme: options.Scheme, Mapper: mapper, ResyncEvery: options.SyncPeriod, - View: cache.ViewOptions{ - Namespaces: []string{options.Namespace}, - }, + Namespaces: []string{options.Namespace}, }) if err != nil { return nil, err diff --git a/pkg/manager/example_test.go b/pkg/manager/example_test.go index 12c421bb83..360008e72b 100644 --- a/pkg/manager/example_test.go +++ b/pkg/manager/example_test.go @@ -60,7 +60,7 @@ func ExampleNew_limitToNamespaces() { mgr, err := manager.New(cfg, manager.Options{ NewCache: func(config *rest.Config, opts cache.Options) (cache.Cache, error) { - opts.View.Namespaces = []string{"namespace1", "namespace2"} + opts.Namespaces = []string{"namespace1", "namespace2"} return cache.New(config, opts) }}, )