Skip to content

Commit e2dbb0e

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2157 from vincepri/refactor-cache-opts
⚠ Refactor cache.Options, deprecate MultiNamespacedCacheBuilder
2 parents 4d784d2 + 915b85d commit e2dbb0e

File tree

7 files changed

+253
-189
lines changed

7 files changed

+253
-189
lines changed

‎pkg/cache/cache.go

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

2626
"k8s.io/apimachinery/pkg/api/meta"
27+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2728
"k8s.io/apimachinery/pkg/fields"
2829
"k8s.io/apimachinery/pkg/labels"
2930
"k8s.io/apimachinery/pkg/runtime"
@@ -118,44 +119,59 @@ type Options struct {
118119
// Mapper is the RESTMapper to use for mapping GroupVersionKinds to Resources
119120
Mapper meta.RESTMapper
120121

121-
// Resync is the base frequency the informers are resynced.
122+
// ResyncEvery is the base frequency the informers are resynced.
122123
// Defaults to defaultResyncTime.
123-
// A 10 percent jitter will be added to the Resync period between informers
124+
// A 10 percent jitter will be added to the ResyncEvery period between informers
124125
// So that all informers will not send list requests simultaneously.
125-
Resync *time.Duration
126+
ResyncEvery *time.Duration
126127

127-
// Namespace restricts the cache's ListWatch to the desired namespace
128-
// Default watches all namespaces
129-
Namespace string
128+
// View restricts the cache's ListWatch to the desired fields per GVK
129+
// Default watches all fields and all namespaces.
130+
View ViewOptions
131+
}
130132

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

138141
// DefaultSelector will be used as selectors for all object types
139-
// that do not have a selector in SelectorsByObject defined.
142+
// unless they have a more specific selector set in ByObject.
140143
DefaultSelector ObjectSelector
141144

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

148-
// TransformByObject is a map from GVKs to transformer functions which
153+
// ViewByObject offers more fine-grained control over the cache's ListWatch by object.
154+
type ViewByObject struct {
155+
// Selectors restricts the cache's ListWatch to the desired
156+
// fields per GVK at the specified object, the map's value must implement
157+
// Selectors [1] using for example a Set [2]
158+
// [1] https://pkg.go.dev/k8s.io/apimachinery/pkg/fields#Selectors
159+
// [2] https://pkg.go.dev/k8s.io/apimachinery/pkg/fields#Set
160+
Selectors SelectorsByObject
161+
162+
// Transform is a map from objects to transformer functions which
149163
// get applied when objects of the transformation are about to be committed
150164
// to cache.
151165
//
152166
// This function is called both for new objects to enter the cache,
153-
// and for updated objects.
154-
TransformByObject TransformByObject
167+
// and for updated objects.
168+
Transform TransformByObject
155169

156-
// DefaultTransform is the transform used for all GVKs which do
157-
// not have an explicit transform func set in TransformByObject
158-
DefaultTransform toolscache.TransformFunc
170+
// UnsafeDisableDeepCopy indicates not to deep copy objects during get or
171+
// list objects per GVK at the specified object.
172+
// Be very careful with this, when enabled you must DeepCopy any object before mutating it,
173+
// otherwise you will mutate the object in the cache.
174+
UnsafeDisableDeepCopy DisableDeepCopyByObject
159175
}
160176

161177
var defaultResyncTime = 10 * time.Hour
@@ -166,15 +182,15 @@ func New(config *rest.Config, opts Options) (Cache, error) {
166182
if err != nil {
167183
return nil, err
168184
}
169-
selectorsByGVK, err := convertToByGVK(opts.SelectorsByObject, opts.DefaultSelector, opts.Scheme)
185+
selectorsByGVK, err := convertToByGVK(opts.View.ByObject.Selectors, opts.View.DefaultSelector, opts.Scheme)
170186
if err != nil {
171187
return nil, err
172188
}
173-
disableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(opts.UnsafeDisableDeepCopyByObject, opts.Scheme)
189+
disableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(opts.View.ByObject.UnsafeDisableDeepCopy, opts.Scheme)
174190
if err != nil {
175191
return nil, err
176192
}
177-
transformers, err := convertToByGVK(opts.TransformByObject, opts.DefaultTransform, opts.Scheme)
193+
transformers, err := convertToByGVK(opts.View.ByObject.Transform, opts.View.DefaultTransform, opts.Scheme)
178194
if err != nil {
179195
return nil, err
180196
}
@@ -185,14 +201,22 @@ func New(config *rest.Config, opts Options) (Cache, error) {
185201
internalSelectorsByGVK[gvk] = internal.Selector(selector)
186202
}
187203

204+
if len(opts.View.Namespaces) == 0 {
205+
opts.View.Namespaces = []string{metav1.NamespaceAll}
206+
}
207+
208+
if len(opts.View.Namespaces) > 1 {
209+
return newMultiNamespaceCache(config, opts)
210+
}
211+
188212
return &informerCache{
189213
scheme: opts.Scheme,
190214
Informers: internal.NewInformers(config, &internal.InformersOpts{
191215
HTTPClient: opts.HTTPClient,
192216
Scheme: opts.Scheme,
193217
Mapper: opts.Mapper,
194-
ResyncPeriod: *opts.Resync,
195-
Namespace: opts.Namespace,
218+
ResyncPeriod: *opts.ResyncEvery,
219+
Namespace: opts.View.Namespaces[0],
196220
ByGVK: internal.InformersOptsByGVK{
197221
Selectors: internalSelectorsByGVK,
198222
DisableDeepCopy: disableDeepCopyByGVK,
@@ -235,17 +259,17 @@ func (options Options) inheritFrom(inherited Options) (*Options, error) {
235259
)
236260
combined.Scheme = combineScheme(inherited.Scheme, options.Scheme)
237261
combined.Mapper = selectMapper(inherited.Mapper, options.Mapper)
238-
combined.Resync = selectResync(inherited.Resync, options.Resync)
239-
combined.Namespace = selectNamespace(inherited.Namespace, options.Namespace)
240-
combined.SelectorsByObject, combined.DefaultSelector, err = combineSelectors(inherited, options, combined.Scheme)
262+
combined.ResyncEvery = selectResync(inherited.ResyncEvery, options.ResyncEvery)
263+
combined.View.Namespaces = selectNamespaces(inherited.View.Namespaces, options.View.Namespaces)
264+
combined.View.ByObject.Selectors, combined.View.DefaultSelector, err = combineSelectors(inherited, options, combined.Scheme)
241265
if err != nil {
242266
return nil, err
243267
}
244-
combined.UnsafeDisableDeepCopyByObject, err = combineUnsafeDeepCopy(inherited, options, combined.Scheme)
268+
combined.View.ByObject.UnsafeDisableDeepCopy, err = combineUnsafeDeepCopy(inherited, options, combined.Scheme)
245269
if err != nil {
246270
return nil, err
247271
}
248-
combined.TransformByObject, combined.DefaultTransform, err = combineTransforms(inherited, options, combined.Scheme)
272+
combined.View.ByObject.Transform, combined.View.DefaultTransform, err = combineTransforms(inherited, options, combined.Scheme)
249273
if err != nil {
250274
return nil, err
251275
}
@@ -282,8 +306,8 @@ func selectResync(def, override *time.Duration) *time.Duration {
282306
return def
283307
}
284308

285-
func selectNamespace(def, override string) string {
286-
if override != "" {
309+
func selectNamespaces(def, override []string) []string {
310+
if len(override) > 0 {
287311
return override
288312
}
289313
return def
@@ -297,11 +321,11 @@ func combineSelectors(inherited, options Options, scheme *runtime.Scheme) (Selec
297321
//
298322
// There is a bunch of complexity here because we need to convert to SelectorsByGVK
299323
// to be able to match keys between options and inherited and then convert back to SelectorsByObject
300-
optionsSelectorsByGVK, err := convertToByGVK(options.SelectorsByObject, options.DefaultSelector, scheme)
324+
optionsSelectorsByGVK, err := convertToByGVK(options.View.ByObject.Selectors, options.View.DefaultSelector, scheme)
301325
if err != nil {
302326
return nil, ObjectSelector{}, err
303327
}
304-
inheritedSelectorsByGVK, err := convertToByGVK(inherited.SelectorsByObject, inherited.DefaultSelector, inherited.Scheme)
328+
inheritedSelectorsByGVK, err := convertToByGVK(inherited.View.ByObject.Selectors, inherited.View.DefaultSelector, inherited.Scheme)
305329
if err != nil {
306330
return nil, ObjectSelector{}, err
307331
}
@@ -360,11 +384,11 @@ func combineFieldSelectors(fs ...fields.Selector) fields.Selector {
360384
func combineUnsafeDeepCopy(inherited, options Options, scheme *runtime.Scheme) (DisableDeepCopyByObject, error) {
361385
// UnsafeDisableDeepCopyByObject is combined via precedence. Only if a value for a particular GVK is unset
362386
// in options will a value from inherited be used.
363-
optionsDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(options.UnsafeDisableDeepCopyByObject, options.Scheme)
387+
optionsDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(options.View.ByObject.UnsafeDisableDeepCopy, options.Scheme)
364388
if err != nil {
365389
return nil, err
366390
}
367-
inheritedDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(inherited.UnsafeDisableDeepCopyByObject, inherited.Scheme)
391+
inheritedDisableDeepCopyByGVK, err := convertToDisableDeepCopyByGVK(inherited.View.ByObject.UnsafeDisableDeepCopy, inherited.Scheme)
368392
if err != nil {
369393
return nil, err
370394
}
@@ -384,11 +408,11 @@ func combineTransforms(inherited, options Options, scheme *runtime.Scheme) (Tran
384408
// Transform functions are combined via chaining. If both inherited and options define a transform
385409
// function, the transform function from inherited will be called first, and the transform function from
386410
// options will be called second.
387-
optionsTransformByGVK, err := convertToByGVK(options.TransformByObject, options.DefaultTransform, options.Scheme)
411+
optionsTransformByGVK, err := convertToByGVK(options.View.ByObject.Transform, options.View.DefaultTransform, options.Scheme)
388412
if err != nil {
389413
return nil, nil, err
390414
}
391-
inheritedTransformByGVK, err := convertToByGVK(inherited.TransformByObject, inherited.DefaultTransform, inherited.Scheme)
415+
inheritedTransformByGVK, err := convertToByGVK(inherited.View.ByObject.Transform, inherited.View.DefaultTransform, inherited.Scheme)
392416
if err != nil {
393417
return nil, nil, err
394418
}
@@ -447,8 +471,8 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) {
447471
}
448472

449473
// Default the resync period to 10 hours if unset
450-
if opts.Resync == nil {
451-
opts.Resync = &defaultResyncTime
474+
if opts.ResyncEvery == nil {
475+
opts.ResyncEvery = &defaultResyncTime
452476
}
453477
return opts, nil
454478
}

‎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)