Skip to content

Commit

Permalink
Merge pull request #2166 from vincepri/rework-cache-sanity
Browse files Browse the repository at this point in the history 
⚠ Cache's ByObject is now a map Object -> options
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Feb 2, 2023
2 parents fc423fc + 7e2dbaf commit 6adc01f
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 {
return *opts.ByObject[&corev1.Pod{}].UnsafeDisableDeepCopy
}
if opts.UnsafeDisableDeepCopy != nil {
return *opts.UnsafeDisableDeepCopy
}
return false
}
Loading

0 comments on commit 6adc01f

Please sign in to comment.