Skip to content

Commit 66e07d5

Browse files
committed
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 <[email protected]>
1 parent fc423fc commit 66e07d5

File tree

9 files changed

+385
-477
lines changed

9 files changed

+385
-477
lines changed

pkg/cache/cache.go

Lines changed: 86 additions & 194 deletions
Large diffs are not rendered by default.

pkg/cache/cache_test.go

Lines changed: 50 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
kscheme "k8s.io/client-go/kubernetes/scheme"
3939
"k8s.io/client-go/rest"
4040
kcache "k8s.io/client-go/tools/cache"
41+
"k8s.io/utils/pointer"
4142

4243
"sigs.k8s.io/controller-runtime/pkg/cache"
4344
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -119,13 +120,10 @@ var _ = Describe("Informer Cache", func() {
119120
var _ = Describe("Multi-Namespace Informer Cache", func() {
120121
CacheTest(cache.MultiNamespacedCacheBuilder([]string{testNamespaceOne, testNamespaceTwo, "default"}), cache.Options{})
121122
})
122-
var _ = Describe("Informer Cache without DeepCopy", func() {
123+
124+
var _ = Describe("Informer Cache without global DeepCopy", func() {
123125
CacheTest(cache.New, cache.Options{
124-
View: cache.ViewOptions{
125-
ByObject: cache.ViewByObject{
126-
UnsafeDisableDeepCopy: cache.DisableDeepCopyByObject{cache.ObjectAll{}: true},
127-
},
128-
},
126+
UnsafeDisableDeepCopy: pointer.Bool(true),
129127
})
130128
})
131129

@@ -190,48 +188,46 @@ var _ = Describe("Cache with transformers", func() {
190188

191189
By("creating the informer cache")
192190
informerCache, err = cache.New(cfg, cache.Options{
193-
View: cache.ViewOptions{
194-
DefaultTransform: func(i interface{}) (interface{}, error) {
195-
obj := i.(runtime.Object)
196-
Expect(obj).NotTo(BeNil())
191+
DefaultTransform: func(i interface{}) (interface{}, error) {
192+
obj := i.(runtime.Object)
193+
Expect(obj).NotTo(BeNil())
197194

198-
accessor, err := meta.Accessor(obj)
199-
Expect(err).To(BeNil())
200-
annotations := accessor.GetAnnotations()
201-
202-
if _, exists := annotations["transformed"]; exists {
203-
// Avoid performing transformation multiple times.
204-
return i, nil
205-
}
195+
accessor, err := meta.Accessor(obj)
196+
Expect(err).To(BeNil())
197+
annotations := accessor.GetAnnotations()
206198

207-
if annotations == nil {
208-
annotations = make(map[string]string)
209-
}
210-
annotations["transformed"] = "default"
211-
accessor.SetAnnotations(annotations)
199+
if _, exists := annotations["transformed"]; exists {
200+
// Avoid performing transformation multiple times.
212201
return i, nil
213-
},
214-
ByObject: cache.ViewByObject{
215-
Transform: cache.TransformByObject{
216-
&corev1.Pod{}: func(i interface{}) (interface{}, error) {
217-
obj := i.(runtime.Object)
218-
Expect(obj).NotTo(BeNil())
219-
accessor, err := meta.Accessor(obj)
220-
Expect(err).To(BeNil())
221-
222-
annotations := accessor.GetAnnotations()
223-
if _, exists := annotations["transformed"]; exists {
224-
// Avoid performing transformation multiple times.
225-
return i, nil
226-
}
227-
228-
if annotations == nil {
229-
annotations = make(map[string]string)
230-
}
231-
annotations["transformed"] = "explicit"
232-
accessor.SetAnnotations(annotations)
202+
}
203+
204+
if annotations == nil {
205+
annotations = make(map[string]string)
206+
}
207+
annotations["transformed"] = "default"
208+
accessor.SetAnnotations(annotations)
209+
return i, nil
210+
},
211+
ByObject: map[client.Object]cache.ByObject{
212+
&corev1.Pod{}: {
213+
Transform: func(i interface{}) (interface{}, error) {
214+
obj := i.(runtime.Object)
215+
Expect(obj).NotTo(BeNil())
216+
accessor, err := meta.Accessor(obj)
217+
Expect(err).To(BeNil())
218+
219+
annotations := accessor.GetAnnotations()
220+
if _, exists := annotations["transformed"]; exists {
221+
// Avoid performing transformation multiple times.
233222
return i, nil
234-
},
223+
}
224+
225+
if annotations == nil {
226+
annotations = make(map[string]string)
227+
}
228+
annotations["transformed"] = "explicit"
229+
accessor.SetAnnotations(annotations)
230+
return i, nil
235231
},
236232
},
237233
},
@@ -370,13 +366,9 @@ var _ = Describe("Cache with selectors", func() {
370366
}
371367

372368
opts := cache.Options{
373-
View: cache.ViewOptions{
374-
DefaultSelector: cache.ObjectSelector{Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceTwo)},
375-
ByObject: cache.ViewByObject{
376-
Selectors: cache.SelectorsByObject{
377-
&corev1.ServiceAccount{}: {Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceOne)},
378-
},
379-
},
369+
DefaultSelector: cache.ObjectSelector{Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceTwo)},
370+
ByObject: map[client.Object]cache.ByObject{
371+
&corev1.ServiceAccount{}: {Field: fields.OneTermEqualSelector("metadata.namespace", testNamespaceOne)},
380372
},
381373
}
382374

@@ -798,7 +790,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
798790

799791
It("should be able to restrict cache to a namespace", func() {
800792
By("creating a namespaced cache")
801-
namespacedCache, err := cache.New(cfg, cache.Options{View: cache.ViewOptions{Namespaces: []string{testNamespaceOne}}})
793+
namespacedCache, err := cache.New(cfg, cache.Options{Namespaces: []string{testNamespaceOne}})
802794
Expect(err).NotTo(HaveOccurred())
803795

804796
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
10791071

10801072
It("should be able to restrict cache to a namespace", func() {
10811073
By("creating a namespaced cache")
1082-
namespacedCache, err := cache.New(cfg, cache.Options{View: cache.ViewOptions{Namespaces: []string{testNamespaceOne}}})
1074+
namespacedCache, err := cache.New(cfg, cache.Options{Namespaces: []string{testNamespaceOne}})
10831075
Expect(err).NotTo(HaveOccurred())
10841076

10851077
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
12331225
By("creating the cache")
12341226
builder := cache.BuilderWithOptions(
12351227
cache.Options{
1236-
View: cache.ViewOptions{
1237-
ByObject: cache.ViewByObject{
1238-
Selectors: cache.SelectorsByObject{
1239-
&corev1.Pod{}: {
1240-
Label: labels.Set(tc.labelSelectors).AsSelector(),
1241-
Field: fields.Set(tc.fieldSelectors).AsSelector(),
1242-
},
1243-
},
1228+
ByObject: map[client.Object]cache.ByObject{
1229+
&corev1.Pod{}: {
1230+
Label: labels.Set(tc.labelSelectors).AsSelector(),
1231+
Field: fields.Set(tc.fieldSelectors).AsSelector(),
12441232
},
12451233
},
12461234
},
@@ -1822,12 +1810,5 @@ func isKubeService(svc metav1.Object) bool {
18221810
}
18231811

18241812
func isPodDisableDeepCopy(opts cache.Options) bool {
1825-
if d, ok := opts.View.ByObject.UnsafeDisableDeepCopy[&corev1.Pod{}]; ok {
1826-
return d
1827-
} else if d, ok = opts.View.ByObject.UnsafeDisableDeepCopy[cache.ObjectAll{}]; ok {
1828-
return d
1829-
} else if d, ok = opts.View.ByObject.UnsafeDisableDeepCopy[&cache.ObjectAll{}]; ok {
1830-
return d
1831-
}
1832-
return false
1813+
return opts.ByObject[&corev1.Pod{}].UnsafeDisableDeepCopy || pointer.BoolDeref(opts.UnsafeDisableDeepCopy, false)
18331814
}

0 commit comments

Comments
 (0)