[release-0.19] ✨ Add EnableWatchBookmarks option to cache informers (#3018)

* 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 <[email protected]>

* Defaults to false for 0.19

Signed-off-by: Vince Prignano <[email protected]>

---------

Signed-off-by: Vince Prignano <[email protected]>
Co-authored-by: Vince Prignano <[email protected]>

Author: k8s-infra-cherrypick-robot

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
 }
 

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{

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 != "" {