From bfd1cf925238323fd53b372780a3d0a1a750a85c Mon Sep 17 00:00:00 2001 From: k8s-infra-cherrypick-robot <90416843+k8s-infra-cherrypick-robot@users.noreply.github.com> Date: Thu, 21 Nov 2024 08:32:55 -0800 Subject: [PATCH] =?UTF-8?q?[release-0.19]=20=E2=9C=A8=20Add=20EnableWatchB?= =?UTF-8?q?ookmarks=20option=20to=20cache=20informers=20(#3018)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Ensure all WatchFunc enable watch and boomarks AllowWatchBookmarks is generally pretty safe to enable as it has been available in Kuberentes for a long while, and the server ignores the flag if it doesn't implement it (per docs). Signed-off-by: Vince Prignano * Defaults to false for 0.19 Signed-off-by: Vince Prignano --------- Signed-off-by: Vince Prignano Co-authored-by: Vince Prignano --- pkg/cache/cache.go | 38 ++++++++++++++++++++++++++++++++- pkg/cache/defaulting_test.go | 24 +++++++++++++++++++++ pkg/cache/internal/informers.go | 16 +++++++++++++- 3 files changed, 76 insertions(+), 2 deletions(-) diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index 706f9c6cdd..406380d329 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -222,6 +222,18 @@ type Options struct { // DefaultNamespaces. DefaultUnsafeDisableDeepCopy *bool + // DefaultEnableWatchBookmarks requests watch events with type "BOOKMARK". + // Servers that do not implement bookmarks may ignore this flag and + // bookmarks are sent at the server's discretion. Clients should not + // assume bookmarks are returned at any specific interval, nor may they + // assume the server will send any BOOKMARK event during a session. + // + // This will be used for all object types, unless it is set in ByObject or + // DefaultNamespaces. + // + // Defaults to false. + DefaultEnableWatchBookmarks *bool + // ByObject restricts the cache's ListWatch to the desired fields per GVK at the specified object. // If unset, this will fall through to the Default* settings. ByObject map[client.Object]ByObject @@ -272,6 +284,15 @@ type ByObject struct { // Be very careful with this, when enabled you must DeepCopy any object before mutating it, // otherwise you will mutate the object in the cache. UnsafeDisableDeepCopy *bool + + // EnableWatchBookmarks requests watch events with type "BOOKMARK". + // Servers that do not implement bookmarks may ignore this flag and + // bookmarks are sent at the server's discretion. Clients should not + // assume bookmarks are returned at any specific interval, nor may they + // assume the server will send any BOOKMARK event during a session. + // + // Defaults to false. + EnableWatchBookmarks *bool } // Config describes all potential options for a given watch. @@ -298,6 +319,15 @@ type Config struct { // UnsafeDisableDeepCopy specifies if List and Get requests against the // cache should not DeepCopy. A nil value allows to default this. UnsafeDisableDeepCopy *bool + + // EnableWatchBookmarks requests watch events with type "BOOKMARK". + // Servers that do not implement bookmarks may ignore this flag and + // bookmarks are sent at the server's discretion. Clients should not + // assume bookmarks are returned at any specific interval, nor may they + // assume the server will send any BOOKMARK event during a session. + // + // Defaults to false. + EnableWatchBookmarks *bool } // NewCacheFunc - Function for creating a new cache from the options and a rest config. @@ -367,6 +397,7 @@ func optionDefaultsToConfig(opts *Options) Config { FieldSelector: opts.DefaultFieldSelector, Transform: opts.DefaultTransform, UnsafeDisableDeepCopy: opts.DefaultUnsafeDisableDeepCopy, + EnableWatchBookmarks: opts.DefaultEnableWatchBookmarks, } } @@ -376,6 +407,7 @@ func byObjectToConfig(byObject ByObject) Config { FieldSelector: byObject.Field, Transform: byObject.Transform, UnsafeDisableDeepCopy: byObject.UnsafeDisableDeepCopy, + EnableWatchBookmarks: byObject.EnableWatchBookmarks, } } @@ -398,6 +430,7 @@ func newCache(restConfig *rest.Config, opts Options) newCacheFunc { Transform: config.Transform, WatchErrorHandler: opts.DefaultWatchErrorHandler, UnsafeDisableDeepCopy: ptr.Deref(config.UnsafeDisableDeepCopy, false), + EnableWatchBookmarks: ptr.Deref(config.EnableWatchBookmarks, false), NewInformer: opts.newInformer, }), readerFailOnMissingInformer: opts.ReaderFailOnMissingInformer, @@ -482,6 +515,7 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) { byObject.Field = defaultedConfig.FieldSelector byObject.Transform = defaultedConfig.Transform byObject.UnsafeDisableDeepCopy = defaultedConfig.UnsafeDisableDeepCopy + byObject.EnableWatchBookmarks = defaultedConfig.EnableWatchBookmarks } opts.ByObject[obj] = byObject @@ -523,7 +557,9 @@ func defaultConfig(toDefault, defaultFrom Config) Config { if toDefault.UnsafeDisableDeepCopy == nil { toDefault.UnsafeDisableDeepCopy = defaultFrom.UnsafeDisableDeepCopy } - + if toDefault.EnableWatchBookmarks == nil { + toDefault.EnableWatchBookmarks = defaultFrom.EnableWatchBookmarks + } return toDefault } diff --git a/pkg/cache/defaulting_test.go b/pkg/cache/defaulting_test.go index 3c01bf8404..8e3033eb47 100644 --- a/pkg/cache/defaulting_test.go +++ b/pkg/cache/defaulting_test.go @@ -224,6 +224,30 @@ func TestDefaultOpts(t *testing.T) { return cmp.Diff(expected, o.ByObject[pod].UnsafeDisableDeepCopy) }, }, + { + name: "ByObject.EnableWatchBookmarks gets defaulted from DefaultEnableWatchBookmarks", + in: Options{ + ByObject: map[client.Object]ByObject{pod: {}}, + DefaultEnableWatchBookmarks: ptr.To(true), + }, + + verification: func(o Options) string { + expected := ptr.To(true) + return cmp.Diff(expected, o.ByObject[pod].EnableWatchBookmarks) + }, + }, + { + name: "ByObject.EnableWatchBookmarks doesn't get defaulted when set", + in: Options{ + ByObject: map[client.Object]ByObject{pod: {EnableWatchBookmarks: ptr.To(false)}}, + DefaultEnableWatchBookmarks: ptr.To(true), + }, + + verification: func(o Options) string { + expected := ptr.To(false) + return cmp.Diff(expected, o.ByObject[pod].EnableWatchBookmarks) + }, + }, { name: "DefaultNamespace label selector gets defaulted from DefaultLabelSelector", in: Options{ diff --git a/pkg/cache/internal/informers.go b/pkg/cache/internal/informers.go index cd8c6774ca..a40382d6f3 100644 --- a/pkg/cache/internal/informers.go +++ b/pkg/cache/internal/informers.go @@ -51,6 +51,7 @@ type InformersOpts struct { Selector Selector Transform cache.TransformFunc UnsafeDisableDeepCopy bool + EnableWatchBookmarks bool WatchErrorHandler cache.WatchErrorHandler } @@ -78,6 +79,7 @@ func NewInformers(config *rest.Config, options *InformersOpts) *Informers { selector: options.Selector, transform: options.Transform, unsafeDisableDeepCopy: options.UnsafeDisableDeepCopy, + enableWatchBookmarks: options.EnableWatchBookmarks, newInformer: newInformer, watchErrorHandler: options.WatchErrorHandler, } @@ -174,6 +176,7 @@ type Informers struct { selector Selector transform cache.TransformFunc unsafeDisableDeepCopy bool + enableWatchBookmarks bool // NewInformer allows overriding of the shared index informer constructor for testing. newInformer func(cache.ListerWatcher, runtime.Object, time.Duration, cache.Indexers) cache.SharedIndexInformer @@ -361,8 +364,10 @@ func (ip *Informers) addInformerToMap(gvk schema.GroupVersionKind, obj runtime.O return listWatcher.ListFunc(opts) }, WatchFunc: func(opts metav1.ListOptions) (watch.Interface, error) { - ip.selector.ApplyToList(&opts) opts.Watch = true // Watch needs to be set to true separately + opts.AllowWatchBookmarks = ip.enableWatchBookmarks + + ip.selector.ApplyToList(&opts) return listWatcher.WatchFunc(opts) }, }, obj, calculateResyncPeriod(ip.resync), cache.Indexers{ @@ -444,6 +449,9 @@ func (ip *Informers) makeListWatcher(gvk schema.GroupVersionKind, obj runtime.Ob }, // Setup the watch function WatchFunc: func(opts metav1.ListOptions) (watch.Interface, error) { + opts.Watch = true // Watch needs to be set to true separately + opts.AllowWatchBookmarks = ip.enableWatchBookmarks + if namespace != "" { return resources.Namespace(namespace).Watch(ip.ctx, opts) } @@ -486,6 +494,9 @@ func (ip *Informers) makeListWatcher(gvk schema.GroupVersionKind, obj runtime.Ob }, // Setup the watch function WatchFunc: func(opts metav1.ListOptions) (watcher watch.Interface, err error) { + opts.Watch = true // Watch needs to be set to true separately + opts.AllowWatchBookmarks = ip.enableWatchBookmarks + if namespace != "" { watcher, err = resources.Namespace(namespace).Watch(ip.ctx, opts) } else { @@ -527,6 +538,9 @@ func (ip *Informers) makeListWatcher(gvk schema.GroupVersionKind, obj runtime.Ob }, // Setup the watch function WatchFunc: func(opts metav1.ListOptions) (watch.Interface, error) { + opts.Watch = true // Watch needs to be set to true separately + opts.AllowWatchBookmarks = ip.enableWatchBookmarks + // Build the request. req := client.Get().Resource(mapping.Resource.Resource).VersionedParams(&opts, ip.paramCodec) if namespace != "" {