Skip to content

Commit 1ea2fc6

Browse files
vinceprivincepri
committed
Refactor cache.Options, deprecate MultiNamespacedCacheBuilder
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 <[email protected]>
1 parent 81199b9 commit 1ea2fc6

File tree

7 files changed

+252
-189
lines changed

7 files changed

+252
-189
lines changed

‎pkg/cache/cache.go

Lines changed: 68 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"time"
2424

2525
"k8s.io/apimachinery/pkg/api/meta"
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2627
"k8s.io/apimachinery/pkg/fields"
2728
"k8s.io/apimachinery/pkg/labels"
2829
"k8s.io/apimachinery/pkg/runtime"
@@ -114,44 +115,59 @@ type Options struct {
114115
// Mapper is the RESTMapper to use for mapping GroupVersionKinds to Resources
115116
Mapper meta.RESTMapper
116117

117-
// Resync is the base frequency the informers are resynced.
118+
// ResyncEvery is the base frequency the informers are resynced.
118119
// Defaults to defaultResyncTime.
119-
// A 10 percent jitter will be added to the Resync period between informers
120+
// A 10 percent jitter will be added to the ResyncEvery period between informers
120121
// So that all informers will not send list requests simultaneously.
121-
Resync *time.Duration
122+
ResyncEvery *time.Duration
122123

123-
// Namespace restricts the cache's ListWatch to the desired namespace
124-
// Default watches all namespaces
125-
Namespace string
124+
// View restricts the cache's ListWatch to the desired fields per GVK
125+
// Default watches all fields and all namespaces.
126+
View ViewOptions
127+
}
126128

127-
// SelectorsByObject restricts the cache's ListWatch to the desired
128-
// fields per GVK at the specified object, the map's value must implement
129-
// Selector [1] using for example a Set [2]
130-
// [1] https://pkg.go.dev/k8s.io/apimachinery/pkg/fields#Selector
131-
// [2] https://pkg.go.dev/k8s.io/apimachinery/pkg/fields#Set
132-
SelectorsByObject SelectorsByObject
129+
// ViewOptions are the optional arguments for creating a cache view.
130+
// A cache view restricts the Get(), List(), and internal ListWatch operations
131+
// to the desired parameter.
132+
type ViewOptions struct {
133+
// Namespaces restricts the cache's ListWatch to the desired namespaces
134+
// Default watches all namespaces
135+
Namespaces []string
133136

134137
// DefaultSelector will be used as selectors for all object types
135-
// that do not have a selector in SelectorsByObject defined.
138+
// unless they have a more specific selector set in ByObject.
136139
DefaultSelector ObjectSelector
137140

138-
// UnsafeDisableDeepCopyByObject indicates not to deep copy objects during get or
139-
// list objects per GVK at the specified object.
140-
// Be very careful with this, when enabled you must DeepCopy any object before mutating it,
141-
// otherwise you will mutate the object in the cache.
142-
UnsafeDisableDeepCopyByObject DisableDeepCopyByObject
141+
// DefaultTransform will be used as transform for all object types
142+
// unless they have a more specific transform set in ByObject.
143+
DefaultTransform toolscache.TransformFunc
144+
145+
// ByObject restricts the cache's ListWatch to the desired fields per GVK at the specified object.
146+
ByObject ViewByObject
147+
}
143148

144-
// TransformByObject is a map from GVKs to transformer functions which
149+
// ViewByObject offers more fine-grained control over the cache's ListWatch by object.
150+
type ViewByObject struct {
151+
// Selectors restricts the cache's ListWatch to the desired
152+
// fields per GVK at the specified object, the map's value must implement
153+
// Selectors [1] using for example a Set [2]
154+
// [1] https://pkg.go.dev/k8s.io/apimachinery/pkg/fields#Selectors
155+
// [2] https://pkg.go.dev/k8s.io/apimachinery/pkg/fields#Set
156+
Selectors SelectorsByObject
157+
158+
// Transform is a map from objects to transformer functions which
145159
// get applied when objects of the transformation are about to be committed
146160
// to cache.
147161
//
148162
// This function is called both for new objects to enter the cache,
149-
// and for updated objects.
150-
TransformByObject TransformByObject
163+
// and for updated objects.
164+
Transform TransformByObject
151165

152-
// DefaultTransform is the transform used for all GVKs which do
153-
// not have an explicit transform func set in TransformByObject
154-
DefaultTransform toolscache.TransformFunc
166+
// UnsafeDisableDeepCopy indicates not to deep copy objects during get or
167+
// list objects per GVK at the specified object.
168+
// Be very careful with this, when enabled you must DeepCopy any object before mutating it,
169+
// otherwise you will mutate the object in the cache.
170+
UnsafeDisableDeepCopy DisableDeepCopyByObject
155171
}
156172

157173
var defaultResyncTime = 10 * time.Hour
@@ -162,15 +178,15 @@ func New(config *rest.Config, opts Options) (Cache, error) {
162178
if err != nil {
163179
return nil, err
164180
}
165-
selectorsByGVK, err := convertToByGVK(opts.SelectorsByObject, opts.DefaultSelector, opts.Scheme)
181+
selectorsByGVK, err := convertToByGVK(opts.View.ByObject.Selectors, opts.View.DefaultSelector, opts.Scheme)
166182
if err != nil {
167183
return nil, err
168184
}
169-
disableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(opts.UnsafeDisableDeepCopyByObject, opts.Scheme)
185+
disableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(opts.View.ByObject.UnsafeDisableDeepCopy, opts.Scheme)
170186
if err != nil {
171187
return nil, err
172188
}
173-
transformers, err := convertToByGVK(opts.TransformByObject, opts.DefaultTransform, opts.Scheme)
189+
transformers, err := convertToByGVK(opts.View.ByObject.Transform, opts.View.DefaultTransform, opts.Scheme)
174190
if err != nil {
175191
return nil, err
176192
}
@@ -181,13 +197,21 @@ func New(config *rest.Config, opts Options) (Cache, error) {
181197
internalSelectorsByGVK[gvk] = internal.Selector(selector)
182198
}
183199

200+
if len(opts.View.Namespaces) == 0 {
201+
opts.View.Namespaces = []string{metav1.NamespaceAll}
202+
}
203+
204+
if len(opts.View.Namespaces) > 1 {
205+
return newMultiNamespaceCache(config, opts)
206+
}
207+
184208
return &informerCache{
185209
scheme: opts.Scheme,
186210
Informers: internal.NewInformers(config, &internal.InformersOpts{
187211
Scheme: opts.Scheme,
188212
Mapper: opts.Mapper,
189-
ResyncPeriod: *opts.Resync,
190-
Namespace: opts.Namespace,
213+
ResyncPeriod: *opts.ResyncEvery,
214+
Namespace: opts.View.Namespaces[0],
191215
ByGVK: internal.InformersOptsByGVK{
192216
Selectors: internalSelectorsByGVK,
193217
DisableDeepCopy: disableDeepCopyByGVK,
@@ -230,17 +254,17 @@ func (options Options) inheritFrom(inherited Options) (*Options, error) {
230254
)
231255
combined.Scheme = combineScheme(inherited.Scheme, options.Scheme)
232256
combined.Mapper = selectMapper(inherited.Mapper, options.Mapper)
233-
combined.Resync = selectResync(inherited.Resync, options.Resync)
234-
combined.Namespace = selectNamespace(inherited.Namespace, options.Namespace)
235-
combined.SelectorsByObject, combined.DefaultSelector, err = combineSelectors(inherited, options, combined.Scheme)
257+
combined.ResyncEvery = selectResync(inherited.ResyncEvery, options.ResyncEvery)
258+
combined.View.Namespaces = selectNamespaces(inherited.View.Namespaces, options.View.Namespaces)
259+
combined.View.ByObject.Selectors, combined.View.DefaultSelector, err = combineSelectors(inherited, options, combined.Scheme)
236260
if err != nil {
237261
return nil, err
238262
}
239-
combined.UnsafeDisableDeepCopyByObject, err = combineUnsafeDeepCopy(inherited, options, combined.Scheme)
263+
combined.View.ByObject.UnsafeDisableDeepCopy, err = combineUnsafeDeepCopy(inherited, options, combined.Scheme)
240264
if err != nil {
241265
return nil, err
242266
}
243-
combined.TransformByObject, combined.DefaultTransform, err = combineTransforms(inherited, options, combined.Scheme)
267+
combined.View.ByObject.Transform, combined.View.DefaultTransform, err = combineTransforms(inherited, options, combined.Scheme)
244268
if err != nil {
245269
return nil, err
246270
}
@@ -277,8 +301,8 @@ func selectResync(def, override *time.Duration) *time.Duration {
277301
return def
278302
}
279303

280-
func selectNamespace(def, override string) string {
281-
if override != "" {
304+
func selectNamespaces(def, override []string) []string {
305+
if len(override) > 0 {
282306
return override
283307
}
284308
return def
@@ -292,11 +316,11 @@ func combineSelectors(inherited, options Options, scheme *runtime.Scheme) (Selec
292316
//
293317
// There is a bunch of complexity here because we need to convert to SelectorsByGVK
294318
// to be able to match keys between options and inherited and then convert back to SelectorsByObject
295-
optionsSelectorsByGVK, err := convertToByGVK(options.SelectorsByObject, options.DefaultSelector, scheme)
319+
optionsSelectorsByGVK, err := convertToByGVK(options.View.ByObject.Selectors, options.View.DefaultSelector, scheme)
296320
if err != nil {
297321
return nil, ObjectSelector{}, err
298322
}
299-
inheritedSelectorsByGVK, err := convertToByGVK(inherited.SelectorsByObject, inherited.DefaultSelector, inherited.Scheme)
323+
inheritedSelectorsByGVK, err := convertToByGVK(inherited.View.ByObject.Selectors, inherited.View.DefaultSelector, inherited.Scheme)
300324
if err != nil {
301325
return nil, ObjectSelector{}, err
302326
}
@@ -355,11 +379,11 @@ func combineFieldSelectors(fs ...fields.Selector) fields.Selector {
355379
func combineUnsafeDeepCopy(inherited, options Options, scheme *runtime.Scheme) (DisableDeepCopyByObject, error) {
356380
// UnsafeDisableDeepCopyByObject is combined via precedence. Only if a value for a particular GVK is unset
357381
// in options will a value from inherited be used.
358-
optionsDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(options.UnsafeDisableDeepCopyByObject, options.Scheme)
382+
optionsDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(options.View.ByObject.UnsafeDisableDeepCopy, options.Scheme)
359383
if err != nil {
360384
return nil, err
361385
}
362-
inheritedDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(inherited.UnsafeDisableDeepCopyByObject, inherited.Scheme)
386+
inheritedDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(inherited.View.ByObject.UnsafeDisableDeepCopy, inherited.Scheme)
363387
if err != nil {
364388
return nil, err
365389
}
@@ -379,11 +403,11 @@ func combineTransforms(inherited, options Options, scheme *runtime.Scheme) (Tran
379403
// Transform functions are combined via chaining. If both inherited and options define a transform
380404
// function, the transform function from inherited will be called first, and the transform function from
381405
// options will be called second.
382-
optionsTransformByGVK, err := convertToByGVK(options.TransformByObject, options.DefaultTransform, options.Scheme)
406+
optionsTransformByGVK, err := convertToByGVK(options.View.ByObject.Transform, options.View.DefaultTransform, options.Scheme)
383407
if err != nil {
384408
return nil, nil, err
385409
}
386-
inheritedTransformByGVK, err := convertToByGVK(inherited.TransformByObject, inherited.DefaultTransform, inherited.Scheme)
410+
inheritedTransformByGVK, err := convertToByGVK(inherited.View.ByObject.Transform, inherited.View.DefaultTransform, inherited.Scheme)
387411
if err != nil {
388412
return nil, nil, err
389413
}
@@ -430,8 +454,8 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) {
430454
}
431455

432456
// Default the resync period to 10 hours if unset
433-
if opts.Resync == nil {
434-
opts.Resync = &defaultResyncTime
457+
if opts.ResyncEvery == nil {
458+
opts.ResyncEvery = &defaultResyncTime
435459
}
436460
return opts, nil
437461
}

‎pkg/cache/cache_test.go

Lines changed: 55 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,13 @@ var _ = Describe("Multi-Namespace Informer Cache", func() {
120120
CacheTest(cache.MultiNamespacedCacheBuilder([]string{testNamespaceOne, testNamespaceTwo, "default"}), cache.Options{})
121121
})
122122
var _ = Describe("Informer Cache without DeepCopy", func() {
123-
CacheTest(cache.New, cache.Options{UnsafeDisableDeepCopyByObject: cache.DisableDeepCopyByObject{cache.ObjectAll{}: true}})
123+
CacheTest(cache.New, cache.Options{
124+
View: cache.ViewOptions{
125+
ByObject: cache.ViewByObject{
126+
UnsafeDisableDeepCopy: cache.DisableDeepCopyByObject{cache.ObjectAll{}: true},
127+
},
128+
},
129+
})
124130
})
125131

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

185191
By("creating the informer cache")
186192
informerCache, err = cache.New(cfg, cache.Options{
187-
DefaultTransform: func(i interface{}) (interface{}, error) {
188-
obj := i.(runtime.Object)
189-
Expect(obj).NotTo(BeNil())
190-
191-
accessor, err := meta.Accessor(obj)
192-
Expect(err).To(BeNil())
193-
annotations := accessor.GetAnnotations()
194-
195-
if _, exists := annotations["transformed"]; exists {
196-
// Avoid performing transformation multiple times.
197-
return i, nil
198-
}
199-
200-
if annotations == nil {
201-
annotations = make(map[string]string)
202-
}
203-
annotations["transformed"] = "default"
204-
accessor.SetAnnotations(annotations)
205-
return i, nil
206-
},
207-
TransformByObject: cache.TransformByObject{
208-
&corev1.Pod{}: func(i interface{}) (interface{}, error) {
193+
View: cache.ViewOptions{
194+
DefaultTransform: func(i interface{}) (interface{}, error) {
209195
obj := i.(runtime.Object)
210196
Expect(obj).NotTo(BeNil())
197+
211198
accessor, err := meta.Accessor(obj)
212199
Expect(err).To(BeNil())
213-
214200
annotations := accessor.GetAnnotations()
201+
215202
if _, exists := annotations["transformed"]; exists {
216203
// Avoid performing transformation multiple times.
217204
return i, nil
@@ -220,10 +207,33 @@ var _ = Describe("Cache with transformers", func() {
220207
if annotations == nil {
221208
annotations = make(map[string]string)
222209
}
223-
annotations["transformed"] = "explicit"
210+
annotations["transformed"] = "default"
224211
accessor.SetAnnotations(annotations)
225212
return i, nil
226213
},
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)
233+
return i, nil
234+
},
235+
},
236+
},
227237
},
228238
})
229239
Expect(err).NotTo(HaveOccurred())
@@ -360,10 +370,14 @@ var _ = Describe("Cache with selectors", func() {
360370
}
361371

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

369383
By("creating the informer cache")
@@ -784,7 +798,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
784798

785799
It("should be able to restrict cache to a namespace", func() {
786800
By("creating a namespaced cache")
787-
namespacedCache, err := cache.New(cfg, cache.Options{Namespace: testNamespaceOne})
801+
namespacedCache, err := cache.New(cfg, cache.Options{View: cache.ViewOptions{Namespaces: []string{testNamespaceOne}}})
788802
Expect(err).NotTo(HaveOccurred())
789803

790804
By("running the cache and waiting for it to sync")
@@ -1065,7 +1079,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
10651079

10661080
It("should be able to restrict cache to a namespace", func() {
10671081
By("creating a namespaced cache")
1068-
namespacedCache, err := cache.New(cfg, cache.Options{Namespace: testNamespaceOne})
1082+
namespacedCache, err := cache.New(cfg, cache.Options{View: cache.ViewOptions{Namespaces: []string{testNamespaceOne}}})
10691083
Expect(err).NotTo(HaveOccurred())
10701084

10711085
By("running the cache and waiting for it to sync")
@@ -1219,10 +1233,14 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
12191233
By("creating the cache")
12201234
builder := cache.BuilderWithOptions(
12211235
cache.Options{
1222-
SelectorsByObject: cache.SelectorsByObject{
1223-
&corev1.Pod{}: {
1224-
Label: labels.Set(tc.labelSelectors).AsSelector(),
1225-
Field: fields.Set(tc.fieldSelectors).AsSelector(),
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+
},
12261244
},
12271245
},
12281246
},
@@ -1804,11 +1822,11 @@ func isKubeService(svc metav1.Object) bool {
18041822
}
18051823

18061824
func isPodDisableDeepCopy(opts cache.Options) bool {
1807-
if d, ok := opts.UnsafeDisableDeepCopyByObject[&corev1.Pod{}]; ok {
1825+
if d, ok := opts.View.ByObject.UnsafeDisableDeepCopy[&corev1.Pod{}]; ok {
18081826
return d
1809-
} else if d, ok = opts.UnsafeDisableDeepCopyByObject[cache.ObjectAll{}]; ok {
1827+
} else if d, ok = opts.View.ByObject.UnsafeDisableDeepCopy[cache.ObjectAll{}]; ok {
18101828
return d
1811-
} else if d, ok = opts.UnsafeDisableDeepCopyByObject[&cache.ObjectAll{}]; ok {
1829+
} else if d, ok = opts.View.ByObject.UnsafeDisableDeepCopy[&cache.ObjectAll{}]; ok {
18121830
return d
18131831
}
18141832
return false

0 commit comments

Comments
 (0)