Merge pull request #120304 from p0lyn0mial/upstream-storage-options-progress-notification

storage: document ProgressNotify from storage.ListOptions

staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_test.go

@@ -312,7 +312,7 @@ func TestNamespaceScopedWatch(t *testing.T) {
 func TestWatchDispatchBookmarkEvents(t *testing.T) {
 	ctx, cacher, terminate := testSetup(t)
 	t.Cleanup(terminate)
-	storagetesting.RunTestWatchDispatchBookmarkEvents(ctx, t, cacher)
+	storagetesting.RunTestWatchDispatchBookmarkEvents(ctx, t, cacher, true)
 }
 
 func TestWatchBookmarksWithCorrectResourceVersion(t *testing.T) {

staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher_test.go

@@ -104,6 +104,17 @@ func TestProgressNotify(t *testing.T) {
 	storagetesting.RunOptionalTestProgressNotify(ctx, t, store)
 }
 
+// TestWatchDispatchBookmarkEvents makes sure that
+// setting allowWatchBookmarks query param against
+// etcd implementation doesn't have any effect.
+func TestWatchDispatchBookmarkEvents(t *testing.T) {
+	clusterConfig := testserver.NewTestConfig(t)
+	clusterConfig.ExperimentalWatchProgressNotifyInterval = time.Second
+	ctx, store, _ := testSetup(t, withClientConfig(clusterConfig))
+
+	storagetesting.RunTestWatchDispatchBookmarkEvents(ctx, t, store, false)
+}
+
 func TestSendInitialEventsBackwardCompatibility(t *testing.T) {
 	ctx, store, _ := testSetup(t)
 	storagetesting.RunSendInitialEventsBackwardCompatibility(ctx, t, store)

staging/src/k8s.io/apiserver/pkg/storage/interfaces.go

@@ -282,6 +282,19 @@ type ListOptions struct {
 	Recursive bool
 	// ProgressNotify determines whether storage-originated bookmark (progress notify) events should
 	// be delivered to the users. The option is ignored for non-watch requests.
+	//
+	// Firstly, note that this field is different from the Predicate.AllowWatchBookmarks field.
+	// Secondly, this field is intended for internal clients only such as the watch cache.
+	//
+	// This means that external clients do not have the ability to set this field directly.
+	// For example by setting the allowWatchBookmarks query parameter.
+	//
+	// The motivation for this approach is the fact that the frequency
+	// of bookmark events from a storage like etcd might be very high.
+	// As the number of watch requests increases, the server load would also increase.
+	//
+	// Furthermore, the server is not obligated to provide bookmark events at all,
+	// as described in https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/956-watch-bookmark#proposal
 	ProgressNotify bool
 	// SendInitialEvents, when set together with Watch option,
 	// begin the watch stream with synthetic init events to build the

staging/src/k8s.io/apiserver/pkg/storage/testing/watcher_tests.go

@@ -1042,7 +1042,7 @@ func RunTestNamespaceScopedWatch(ctx context.Context, t *testing.T, store storag
 // TODO(#109831): ProgressNotify feature is effectively implementing the same
 //
 //	functionality, so we should refactor this functionality to share the same input.
-func RunTestWatchDispatchBookmarkEvents(ctx context.Context, t *testing.T, store storage.Interface) {
+func RunTestWatchDispatchBookmarkEvents(ctx context.Context, t *testing.T, store storage.Interface, expectedWatchBookmarks bool) {
 	key, storedObj := testPropagateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "test-ns"}})
 	startRV := storedObj.ResourceVersion
 
@@ -1061,7 +1061,7 @@ func RunTestWatchDispatchBookmarkEvents(ctx context.Context, t *testing.T, store
 		{
 			name:                "allowWatchBookmarks=true",
 			timeout:             3 * time.Second,
-			expected:            true,
+			expected:            expectedWatchBookmarks,
 			allowWatchBookmarks: true,
 		},
 	}