diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 83125da9a9c8..87aeca3fd2fb 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -1307,6 +1307,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS genericfeatures.WatchBookmark: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, + genericfeatures.WatchFromStorageWithoutResourceVersion: {Default: false, PreRelease: featuregate.Beta}, + genericfeatures.WatchList: {Default: false, PreRelease: featuregate.Alpha}, genericfeatures.ZeroLimitedNominalConcurrencyShares: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.32 diff --git a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go index 3789a371697e..dbd41b8c5f0c 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go @@ -258,6 +258,12 @@ const ( // Enables support for watch bookmark events. WatchBookmark featuregate.Feature = "WatchBookmark" + // owner: @serathius + // beta: 1.30 + // Enables watches without resourceVersion to be served from storage. + // Used to prevent https://github.com/kubernetes/kubernetes/issues/123072 until etcd fixes the issue. + WatchFromStorageWithoutResourceVersion featuregate.Feature = "WatchFromStorageWithoutResourceVersion" + // owner: @vinaykul // kep: http://kep.k8s.io/1287 // alpha: v1.27 @@ -349,6 +355,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS WatchBookmark: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, + WatchFromStorageWithoutResourceVersion: {Default: false, PreRelease: featuregate.Beta}, + InPlacePodVerticalScaling: {Default: false, PreRelease: featuregate.Alpha}, WatchList: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go index f94feb7c4fec..1e6cc4d021a7 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go @@ -523,7 +523,7 @@ func (c *Cacher) Watch(ctx context.Context, key string, opts storage.ListOptions opts.SendInitialEvents = nil } // TODO: we should eventually get rid of this legacy case - if opts.SendInitialEvents == nil && opts.ResourceVersion == "" { + if utilfeature.DefaultFeatureGate.Enabled(features.WatchFromStorageWithoutResourceVersion) && opts.SendInitialEvents == nil && opts.ResourceVersion == "" { return c.storage.Watch(ctx, key, opts) } requestedWatchRV, err := c.versioner.ParseResourceVersion(opts.ResourceVersion) @@ -1282,12 +1282,14 @@ func (c *Cacher) getBookmarkAfterResourceVersionLockedFunc(parsedResourceVersion // // if SendInitiaEvents != nil => ResourceVersionMatch = NotOlderThan // if ResourceVersionmatch != nil => ResourceVersionMatch = NotOlderThan & SendInitialEvents != nil -// -// to satisfy the legacy case (SendInitialEvents = true, RV="") we skip checking opts.Predicate.AllowWatchBookmarks func (c *Cacher) getWatchCacheResourceVersion(ctx context.Context, parsedWatchResourceVersion uint64, opts storage.ListOptions) (uint64, error) { if len(opts.ResourceVersion) != 0 { return parsedWatchResourceVersion, nil } + // legacy case + if !utilfeature.DefaultFeatureGate.Enabled(features.WatchFromStorageWithoutResourceVersion) && opts.SendInitialEvents == nil && opts.ResourceVersion == "" { + return 0, nil + } rv, err := storage.GetCurrentResourceVersionFromStorage(ctx, c.storage, c.newListFunc, c.resourcePrefix, c.objectType.String()) return rv, err } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_test.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_test.go index 01995b1b1b65..8c057da40e4c 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_test.go @@ -381,9 +381,18 @@ func TestSendInitialEventsBackwardCompatibility(t *testing.T) { } func TestWatchSemantics(t *testing.T) { - store, terminate := testSetupWithEtcdAndCreateWrapper(t) - t.Cleanup(terminate) - storagetesting.RunWatchSemantics(context.TODO(), t, store) + t.Run("WatchFromStorageWithoutResourceVersion=true", func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WatchFromStorageWithoutResourceVersion, true)() + store, terminate := testSetupWithEtcdAndCreateWrapper(t) + t.Cleanup(terminate) + storagetesting.RunWatchSemantics(context.TODO(), t, store) + }) + t.Run("WatchFromStorageWithoutResourceVersion=false", func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WatchFromStorageWithoutResourceVersion, false)() + store, terminate := testSetupWithEtcdAndCreateWrapper(t) + t.Cleanup(terminate) + storagetesting.RunWatchSemantics(context.TODO(), t, store) + }) } func TestWatchSemanticInitialEventsExtended(t *testing.T) { diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go index 25f11e495c51..8caa3551a774 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go @@ -338,8 +338,6 @@ func TestWatchCacheBypass(t *testing.T) { t.Fatalf("unexpected error waiting for the cache to be ready") } - // Inject error to underlying layer and check if cacher is not bypassed. - backingStorage.injectError(errDummy) _, err = cacher.Watch(context.TODO(), "pod/ns", storage.ListOptions{ ResourceVersion: "0", Predicate: storage.Everything, @@ -348,12 +346,32 @@ func TestWatchCacheBypass(t *testing.T) { t.Errorf("Watch with RV=0 should be served from cache: %v", err) } - // With unset RV, check if cacher is bypassed. _, err = cacher.Watch(context.TODO(), "pod/ns", storage.ListOptions{ ResourceVersion: "", + Predicate: storage.Everything, }) - if err != errDummy { - t.Errorf("Watch with unset RV should bypass cacher: %v", err) + if err != nil { + t.Errorf("Watch with RV=0 should be served from cache: %v", err) + } + + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WatchFromStorageWithoutResourceVersion, false)() + _, err = cacher.Watch(context.TODO(), "pod/ns", storage.ListOptions{ + ResourceVersion: "", + Predicate: storage.Everything, + }) + if err != nil { + t.Errorf("With WatchFromStorageWithoutResourceVersion disabled, watch with unset RV should be served from cache: %v", err) + } + + // Inject error to underlying layer and check if cacher is not bypassed. + backingStorage.injectError(errDummy) + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WatchFromStorageWithoutResourceVersion, true)() + _, err = cacher.Watch(context.TODO(), "pod/ns", storage.ListOptions{ + ResourceVersion: "", + Predicate: storage.Everything, + }) + if !errors.Is(err, errDummy) { + t.Errorf("With WatchFromStorageWithoutResourceVersion enabled, watch with unset RV should be served from storage: %v", err) } } @@ -2032,9 +2050,11 @@ func TestGetWatchCacheResourceVersion(t *testing.T) { // | Unset | true/false | nil/true/false | // +-----------------+---------------------+-----------------------+ { - name: "RV=unset, allowWatchBookmarks=true, sendInitialEvents=nil", - opts: listOptions(true, nil, ""), - expectedWatchResourceVersion: 100, + name: "RV=unset, allowWatchBookmarks=true, sendInitialEvents=nil", + opts: listOptions(true, nil, ""), + // Expecting RV 0, due to https://github.com/kubernetes/kubernetes/pull/123935 reverted to serving those requests from watch cache. + // Set to 100, when WatchFromStorageWithoutResourceVersion is set to true. + expectedWatchResourceVersion: 0, }, { name: "RV=unset, allowWatchBookmarks=true, sendInitialEvents=true", @@ -2047,9 +2067,11 @@ func TestGetWatchCacheResourceVersion(t *testing.T) { expectedWatchResourceVersion: 100, }, { - name: "RV=unset, allowWatchBookmarks=false, sendInitialEvents=nil", - opts: listOptions(false, nil, ""), - expectedWatchResourceVersion: 100, + name: "RV=unset, allowWatchBookmarks=false, sendInitialEvents=nil", + opts: listOptions(false, nil, ""), + // Expecting RV 0, due to https://github.com/kubernetes/kubernetes/pull/123935 reverted to serving those requests from watch cache. + // Set to 100, when WatchFromStorageWithoutResourceVersion is set to true. + expectedWatchResourceVersion: 0, }, { name: "RV=unset, allowWatchBookmarks=false, sendInitialEvents=true, legacy",