Skip to content

Commit

Permalink
Cache's ByObject is now a map Object -> options
Browse files Browse the repository at this point in the history 
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 <vincepri@redhat.com>
  • Loading branch information
@vincepri
vincepri committed Feb 2, 2023
1 parent fc423fc commit 0cb8f51
Show file tree
Hide file tree
Showing 9 changed files with 428 additions and 494 deletions.
304 changes: 102 additions & 202 deletions pkg/cache/cache.go
View file

Large diffs are not rendered by default.

121 changes: 54 additions & 67 deletions pkg/cache/cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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),
})
})

Expand Down Expand Up @@ -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
},
},
},
Expand Down Expand Up @@ -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)},
},
}

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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(),
},
},
},
Expand Down Expand Up @@ -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 && *opts.ByObject[&corev1.Pod{}].UnsafeDisableDeepCopy {
return true
}
if opts.UnsafeDisableDeepCopy != nil && *opts.UnsafeDisableDeepCopy {
return true
}
return false
}
Loading

0 comments on commit 0cb8f51

Please sign in to comment.