Skip to content

Commit

Permalink
Refactor cache.Options, deprecate MultiNamespacedCacheBuilder
Browse files Browse the repository at this point in the history 
This changeset refactors the entire cache.Options struct to provide
a legible and clear way to configure parameters when a cache is created.
In addition, it handles under the hood the automatic multi-namespace
cache support we currently offer through the
MultiNamespacedCacheBuilder, which is now deprecated.

Signed-off-by: Vince Prignano <vincepri@redhat.com>
  • Loading branch information
@vincepri
vincepri committed Jan 31, 2023
1 parent fbaf768 commit 915b85d
Show file tree
Hide file tree
Showing 7 changed files with 253 additions and 189 deletions.
112 changes: 68 additions & 44 deletions pkg/cache/cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"time"

"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"
Expand Down Expand Up @@ -118,44 +119,59 @@ type Options struct {
// Mapper is the RESTMapper to use for mapping GroupVersionKinds to Resources
Mapper meta.RESTMapper

// Resync is the base frequency the informers are resynced.
// ResyncEvery is the base frequency the informers are resynced.
// Defaults to defaultResyncTime.
// A 10 percent jitter will be added to the Resync period between informers
// A 10 percent jitter will be added to the ResyncEvery period between informers
// So that all informers will not send list requests simultaneously.
Resync *time.Duration
ResyncEvery *time.Duration

// Namespace restricts the cache's ListWatch to the desired namespace
// Default watches all namespaces
Namespace string
// View restricts the cache's ListWatch to the desired fields per GVK
// Default watches all fields and all namespaces.
View ViewOptions
}

// SelectorsByObject restricts the cache's ListWatch to the desired
// fields per GVK at the specified object, the map's value must implement
// Selector [1] using for example a Set [2]
// [1] https://pkg.go.dev/k8s.io/apimachinery/pkg/fields#Selector
// [2] https://pkg.go.dev/k8s.io/apimachinery/pkg/fields#Set
SelectorsByObject SelectorsByObject
// 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
// that do not have a selector in SelectorsByObject defined.
// unless they have a more specific selector set in ByObject.
DefaultSelector ObjectSelector

// UnsafeDisableDeepCopyByObject 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.
UnsafeDisableDeepCopyByObject DisableDeepCopyByObject
// 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
}

// TransformByObject is a map from GVKs to transformer functions which
// 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

// Transform is a map from objects to transformer functions which
// get applied when objects of the transformation are about to be committed
// to cache.
//
// This function is called both for new objects to enter the cache,
// and for updated objects.
TransformByObject TransformByObject
// and for updated objects.
Transform TransformByObject

// DefaultTransform is the transform used for all GVKs which do
// not have an explicit transform func set in TransformByObject
DefaultTransform 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
}

var defaultResyncTime = 10 * time.Hour
Expand All @@ -166,15 +182,15 @@ func New(config *rest.Config, opts Options) (Cache, error) {
if err != nil {
return nil, err
}
selectorsByGVK, err := convertToByGVK(opts.SelectorsByObject, opts.DefaultSelector, opts.Scheme)
selectorsByGVK, err := convertToByGVK(opts.View.ByObject.Selectors, opts.View.DefaultSelector, opts.Scheme)
if err != nil {
return nil, err
}
disableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(opts.UnsafeDisableDeepCopyByObject, opts.Scheme)
disableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(opts.View.ByObject.UnsafeDisableDeepCopy, opts.Scheme)
if err != nil {
return nil, err
}
transformers, err := convertToByGVK(opts.TransformByObject, opts.DefaultTransform, opts.Scheme)
transformers, err := convertToByGVK(opts.View.ByObject.Transform, opts.View.DefaultTransform, opts.Scheme)
if err != nil {
return nil, err
}
Expand All @@ -185,14 +201,22 @@ func New(config *rest.Config, opts Options) (Cache, error) {
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)
}

return &informerCache{
scheme: opts.Scheme,
Informers: internal.NewInformers(config, &internal.InformersOpts{
HTTPClient: opts.HTTPClient,
Scheme: opts.Scheme,
Mapper: opts.Mapper,
ResyncPeriod: *opts.Resync,
Namespace: opts.Namespace,
ResyncPeriod: *opts.ResyncEvery,
Namespace: opts.View.Namespaces[0],
ByGVK: internal.InformersOptsByGVK{
Selectors: internalSelectorsByGVK,
DisableDeepCopy: disableDeepCopyByGVK,
Expand Down Expand Up @@ -235,17 +259,17 @@ func (options Options) inheritFrom(inherited Options) (*Options, error) {
)
combined.Scheme = combineScheme(inherited.Scheme, options.Scheme)
combined.Mapper = selectMapper(inherited.Mapper, options.Mapper)
combined.Resync = selectResync(inherited.Resync, options.Resync)
combined.Namespace = selectNamespace(inherited.Namespace, options.Namespace)
combined.SelectorsByObject, combined.DefaultSelector, err = combineSelectors(inherited, options, combined.Scheme)
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.UnsafeDisableDeepCopyByObject, err = combineUnsafeDeepCopy(inherited, options, combined.Scheme)
combined.View.ByObject.UnsafeDisableDeepCopy, err = combineUnsafeDeepCopy(inherited, options, combined.Scheme)
if err != nil {
return nil, err
}
combined.TransformByObject, combined.DefaultTransform, err = combineTransforms(inherited, options, combined.Scheme)
combined.View.ByObject.Transform, combined.View.DefaultTransform, err = combineTransforms(inherited, options, combined.Scheme)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -282,8 +306,8 @@ func selectResync(def, override *time.Duration) *time.Duration {
return def
}

func selectNamespace(def, override string) string {
if override != "" {
func selectNamespaces(def, override []string) []string {
if len(override) > 0 {
return override
}
return def
Expand All @@ -297,11 +321,11 @@ func combineSelectors(inherited, options Options, scheme *runtime.Scheme) (Selec
//
// 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.SelectorsByObject, options.DefaultSelector, scheme)
optionsSelectorsByGVK, err := convertToByGVK(options.View.ByObject.Selectors, options.View.DefaultSelector, scheme)
if err != nil {
return nil, ObjectSelector{}, err
}
inheritedSelectorsByGVK, err := convertToByGVK(inherited.SelectorsByObject, inherited.DefaultSelector, inherited.Scheme)
inheritedSelectorsByGVK, err := convertToByGVK(inherited.View.ByObject.Selectors, inherited.View.DefaultSelector, inherited.Scheme)
if err != nil {
return nil, ObjectSelector{}, err
}
Expand Down Expand Up @@ -360,11 +384,11 @@ func combineFieldSelectors(fs ...fields.Selector) fields.Selector {
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.UnsafeDisableDeepCopyByObject, options.Scheme)
optionsDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(options.View.ByObject.UnsafeDisableDeepCopy, options.Scheme)
if err != nil {
return nil, err
}
inheritedDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(inherited.UnsafeDisableDeepCopyByObject, inherited.Scheme)
inheritedDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(inherited.View.ByObject.UnsafeDisableDeepCopy, inherited.Scheme)
if err != nil {
return nil, err
}
Expand All @@ -384,11 +408,11 @@ func combineTransforms(inherited, options Options, scheme *runtime.Scheme) (Tran
// 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.TransformByObject, options.DefaultTransform, options.Scheme)
optionsTransformByGVK, err := convertToByGVK(options.View.ByObject.Transform, options.View.DefaultTransform, options.Scheme)
if err != nil {
return nil, nil, err
}
inheritedTransformByGVK, err := convertToByGVK(inherited.TransformByObject, inherited.DefaultTransform, inherited.Scheme)
inheritedTransformByGVK, err := convertToByGVK(inherited.View.ByObject.Transform, inherited.View.DefaultTransform, inherited.Scheme)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -447,8 +471,8 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) {
}

// Default the resync period to 10 hours if unset
if opts.Resync == nil {
opts.Resync = &defaultResyncTime
if opts.ResyncEvery == nil {
opts.ResyncEvery = &defaultResyncTime
}
return opts, nil
}
Expand Down
92 changes: 55 additions & 37 deletions pkg/cache/cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,13 @@ var _ = Describe("Multi-Namespace Informer Cache", func() {
CacheTest(cache.MultiNamespacedCacheBuilder([]string{testNamespaceOne, testNamespaceTwo, "default"}), cache.Options{})
})
var _ = Describe("Informer Cache without DeepCopy", func() {
CacheTest(cache.New, cache.Options{UnsafeDisableDeepCopyByObject: cache.DisableDeepCopyByObject{cache.ObjectAll{}: true}})
CacheTest(cache.New, cache.Options{
View: cache.ViewOptions{
ByObject: cache.ViewByObject{
UnsafeDisableDeepCopy: cache.DisableDeepCopyByObject{cache.ObjectAll{}: true},
},
},
})
})

var _ = Describe("Cache with transformers", func() {
Expand Down Expand Up @@ -184,34 +190,15 @@ var _ = Describe("Cache with transformers", func() {

By("creating the informer cache")
informerCache, err = cache.New(cfg, cache.Options{
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
}

if annotations == nil {
annotations = make(map[string]string)
}
annotations["transformed"] = "default"
accessor.SetAnnotations(annotations)
return i, nil
},
TransformByObject: cache.TransformByObject{
&corev1.Pod{}: func(i interface{}) (interface{}, error) {
View: cache.ViewOptions{
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
Expand All @@ -220,10 +207,33 @@ var _ = Describe("Cache with transformers", func() {
if annotations == nil {
annotations = make(map[string]string)
}
annotations["transformed"] = "explicit"
annotations["transformed"] = "default"
accessor.SetAnnotations(annotations)
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)
return i, nil
},
},
},
},
})
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -360,10 +370,14 @@ var _ = Describe("Cache with selectors", func() {
}

opts := cache.Options{
SelectorsByObject: cache.SelectorsByObject{
&corev1.ServiceAccount{}: {Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceOne)},
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)},
},
},
},
DefaultSelector: cache.ObjectSelector{Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceTwo)},
}

By("creating the informer cache")
Expand Down Expand Up @@ -784,7 +798,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{Namespace: testNamespaceOne})
namespacedCache, err := cache.New(cfg, cache.Options{View: cache.ViewOptions{Namespaces: []string{testNamespaceOne}}})
Expect(err).NotTo(HaveOccurred())

By("running the cache and waiting for it to sync")
Expand Down Expand Up @@ -1065,7 +1079,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{Namespace: testNamespaceOne})
namespacedCache, err := cache.New(cfg, cache.Options{View: cache.ViewOptions{Namespaces: []string{testNamespaceOne}}})
Expect(err).NotTo(HaveOccurred())

By("running the cache and waiting for it to sync")
Expand Down Expand Up @@ -1219,10 +1233,14 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
By("creating the cache")
builder := cache.BuilderWithOptions(
cache.Options{
SelectorsByObject: cache.SelectorsByObject{
&corev1.Pod{}: {
Label: labels.Set(tc.labelSelectors).AsSelector(),
Field: fields.Set(tc.fieldSelectors).AsSelector(),
View: cache.ViewOptions{
ByObject: cache.ViewByObject{
Selectors: cache.SelectorsByObject{
&corev1.Pod{}: {
Label: labels.Set(tc.labelSelectors).AsSelector(),
Field: fields.Set(tc.fieldSelectors).AsSelector(),
},
},
},
},
},
Expand Down Expand Up @@ -1804,11 +1822,11 @@ func isKubeService(svc metav1.Object) bool {
}

func isPodDisableDeepCopy(opts cache.Options) bool {
if d, ok := opts.UnsafeDisableDeepCopyByObject[&corev1.Pod{}]; ok {
if d, ok := opts.View.ByObject.UnsafeDisableDeepCopy[&corev1.Pod{}]; ok {
return d
} else if d, ok = opts.UnsafeDisableDeepCopyByObject[cache.ObjectAll{}]; ok {
} else if d, ok = opts.View.ByObject.UnsafeDisableDeepCopy[cache.ObjectAll{}]; ok {
return d
} else if d, ok = opts.UnsafeDisableDeepCopyByObject[&cache.ObjectAll{}]; ok {
} else if d, ok = opts.View.ByObject.UnsafeDisableDeepCopy[&cache.ObjectAll{}]; ok {
return d
}
return false
Expand Down
Loading

0 comments on commit 915b85d

Please sign in to comment.