diff --git a/pkg/storage/cacher/cacher.go b/pkg/storage/cacher/cacher.go index d15b15b44..bc6500909 100644 --- a/pkg/storage/cacher/cacher.go +++ b/pkg/storage/cacher/cacher.go @@ -621,9 +621,11 @@ func (c *Cacher) Get(ctx context.Context, key string, opts storage.GetOptions, o func shouldDelegateList(opts storage.ListOptions) bool { resourceVersion := opts.ResourceVersion pred := opts.Predicate + match := opts.ResourceVersionMatch pagingEnabled := utilfeature.DefaultFeatureGate.Enabled(features.APIListChunking) hasContinuation := pagingEnabled && len(pred.Continue) > 0 hasLimit := pagingEnabled && pred.Limit > 0 && resourceVersion != "0" + unsupportedMatch := match != "" && match != metav1.ResourceVersionMatchNotOlderThan // If resourceVersion is not specified, serve it from underlying // storage (for backward compatibility). If a continuation is @@ -631,7 +633,7 @@ func shouldDelegateList(opts storage.ListOptions) bool { // Limits are only sent to storage when resourceVersion is non-zero // since the watch cache isn't able to perform continuations, and // limits are ignored when resource version is zero - return resourceVersion == "" || hasContinuation || hasLimit || opts.ResourceVersionMatch == metav1.ResourceVersionMatchExact + return resourceVersion == "" || hasContinuation || hasLimit || unsupportedMatch } func (c *Cacher) listItems(ctx context.Context, listRV uint64, key string, pred storage.SelectionPredicate, recursive bool) ([]interface{}, uint64, string, error) { @@ -657,6 +659,11 @@ func (c *Cacher) GetList(ctx context.Context, key string, opts storage.ListOptio return c.storage.GetList(ctx, key, opts, listObj) } + match := opts.ResourceVersionMatch + if match != metav1.ResourceVersionMatchNotOlderThan && match != "" { + return fmt.Errorf("unknown ResourceVersionMatch value: %v", match) + } + // If resourceVersion is specified, serve it from cache. // It's guaranteed that the returned value is at least that // fresh as the given resourceVersion. diff --git a/pkg/storage/etcd3/store_test.go b/pkg/storage/etcd3/store_test.go index ef2c0b067..d9df9c3d3 100644 --- a/pkg/storage/etcd3/store_test.go +++ b/pkg/storage/etcd3/store_test.go @@ -189,7 +189,7 @@ func TestTransformationFailure(t *testing.T) { func TestList(t *testing.T) { ctx, store, _ := testSetup(t) - storagetesting.RunTestList(ctx, t, store) + storagetesting.RunTestList(ctx, t, store, false) } func TestListWithoutPaging(t *testing.T) { diff --git a/pkg/storage/testing/store_tests.go b/pkg/storage/testing/store_tests.go index 075bd0d67..c60265308 100644 --- a/pkg/storage/testing/store_tests.go +++ b/pkg/storage/testing/store_tests.go @@ -188,6 +188,13 @@ func RunTestGet(ctx context.Context, t *testing.T, store storage.Interface) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { + // For some asynchronous implementations of storage interface (in particular watchcache), + // certain requests may impact result of further requests. As an example, if we first + // ensure that watchcache is synchronized up to ResourceVersion X (using Get/List requests + // with NotOlderThan semantic), the further requests (even specifying earlier resource + // version) will also return the result synchronized to at least ResourceVersion X. + // By parallelizing test cases we ensure that the order in which test cases are defined + // doesn't automatically preclude some scenarios from happening. t.Parallel() out := &example.Pod{} @@ -468,7 +475,7 @@ func RunTestPreconditionalDeleteWithSuggestion(ctx context.Context, t *testing.T } } -func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { +func RunTestList(ctx context.Context, t *testing.T, store storage.Interface, ignoreWatchCacheTests bool) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RemainingItemCount, true)() initialRV, preset, err := seedMultiLevelData(ctx, store) @@ -478,7 +485,8 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { list := &example.PodList{} storageOpts := storage.ListOptions{ - ResourceVersion: "0", + // Ensure we're listing from "now". + ResourceVersion: "", Predicate: storage.Everything, Recursive: true, } @@ -502,7 +510,9 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { rvMatch metav1.ResourceVersionMatch prefix string pred storage.SelectionPredicate - expectedOut []*example.Pod + ignoreForWatchCache bool + expectedOut []example.Pod + expectedAlternatives [][]example.Pod expectContinue bool expectedRemainingItemCount *int64 expectError bool @@ -539,31 +549,31 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { name: "test List on existing key", prefix: "/pods/first/", pred: storage.Everything, - expectedOut: []*example.Pod{preset[0]}, + expectedOut: []example.Pod{*preset[0]}, }, { - name: "test List on existing key with resource version set to 0", - prefix: "/pods/first/", - pred: storage.Everything, - expectedOut: []*example.Pod{preset[0]}, - rv: "0", + name: "test List on existing key with resource version set to 0", + prefix: "/pods/first/", + pred: storage.Everything, + expectedAlternatives: [][]example.Pod{{}, {*preset[0]}}, + rv: "0", }, { name: "test List on existing key with resource version set before first write, match=Exact", prefix: "/pods/first/", pred: storage.Everything, - expectedOut: []*example.Pod{}, + expectedOut: []example.Pod{}, rv: initialRV, rvMatch: metav1.ResourceVersionMatchExact, expectRV: initialRV, }, { - name: "test List on existing key with resource version set to 0, match=NotOlderThan", - prefix: "/pods/first/", - pred: storage.Everything, - expectedOut: []*example.Pod{preset[0]}, - rv: "0", - rvMatch: metav1.ResourceVersionMatchNotOlderThan, + name: "test List on existing key with resource version set to 0, match=NotOlderThan", + prefix: "/pods/first/", + pred: storage.Everything, + expectedAlternatives: [][]example.Pod{{}, {*preset[0]}}, + rv: "0", + rvMatch: metav1.ResourceVersionMatchNotOlderThan, }, { name: "test List on existing key with resource version set to 0, match=Invalid", @@ -574,12 +584,12 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { expectError: true, }, { - name: "test List on existing key with resource version set before first write, match=NotOlderThan", - prefix: "/pods/first/", - pred: storage.Everything, - expectedOut: []*example.Pod{preset[0]}, - rv: initialRV, - rvMatch: metav1.ResourceVersionMatchNotOlderThan, + name: "test List on existing key with resource version set before first write, match=NotOlderThan", + prefix: "/pods/first/", + pred: storage.Everything, + expectedAlternatives: [][]example.Pod{{}, {*preset[0]}}, + rv: initialRV, + rvMatch: metav1.ResourceVersionMatchNotOlderThan, }, { name: "test List on existing key with resource version set before first write, match=Invalid", @@ -593,14 +603,14 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { name: "test List on existing key with resource version set to current resource version", prefix: "/pods/first/", pred: storage.Everything, - expectedOut: []*example.Pod{preset[0]}, + expectedOut: []example.Pod{*preset[0]}, rv: list.ResourceVersion, }, { name: "test List on existing key with resource version set to current resource version, match=Exact", prefix: "/pods/first/", pred: storage.Everything, - expectedOut: []*example.Pod{preset[0]}, + expectedOut: []example.Pod{*preset[0]}, rv: list.ResourceVersion, rvMatch: metav1.ResourceVersionMatchExact, expectRV: list.ResourceVersion, @@ -609,7 +619,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { name: "test List on existing key with resource version set to current resource version, match=NotOlderThan", prefix: "/pods/first/", pred: storage.Everything, - expectedOut: []*example.Pod{preset[0]}, + expectedOut: []example.Pod{*preset[0]}, rv: list.ResourceVersion, rvMatch: metav1.ResourceVersionMatchNotOlderThan, }, @@ -617,7 +627,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { name: "test List on non-existing key", prefix: "/pods/non-existing/", pred: storage.Everything, - expectedOut: nil, + expectedOut: []example.Pod{}, }, { name: "test List with pod name matching", @@ -626,7 +636,18 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Label: labels.Everything(), Field: fields.ParseSelectorOrDie("metadata.name!=bar"), }, - expectedOut: nil, + expectedOut: []example.Pod{}, + }, + { + name: "test List with pod name matching with resource version set to current resource version, match=NotOlderThan", + prefix: "/pods/first/", + pred: storage.SelectionPredicate{ + Label: labels.Everything(), + Field: fields.ParseSelectorOrDie("metadata.name!=bar"), + }, + expectedOut: []example.Pod{}, + rv: list.ResourceVersion, + rvMatch: metav1.ResourceVersionMatchNotOlderThan, }, { name: "test List with limit", @@ -636,7 +657,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Field: fields.Everything(), Limit: 1, }, - expectedOut: []*example.Pod{preset[1]}, + expectedOut: []example.Pod{*preset[1]}, expectContinue: true, expectedRemainingItemCount: utilpointer.Int64Ptr(1), }, @@ -648,7 +669,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Field: fields.Everything(), Limit: 1, }, - expectedOut: []*example.Pod{preset[1]}, + expectedOut: []example.Pod{*preset[1]}, expectContinue: true, expectedRemainingItemCount: utilpointer.Int64Ptr(1), rv: list.ResourceVersion, @@ -662,13 +683,28 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Field: fields.Everything(), Limit: 1, }, - expectedOut: []*example.Pod{preset[1]}, + expectedOut: []example.Pod{*preset[1]}, expectContinue: true, expectedRemainingItemCount: utilpointer.Int64Ptr(1), rv: list.ResourceVersion, rvMatch: metav1.ResourceVersionMatchExact, expectRV: list.ResourceVersion, }, + { + name: "test List with limit at current resource version and match=NotOlderThan", + prefix: "/pods/second/", + pred: storage.SelectionPredicate{ + Label: labels.Everything(), + Field: fields.Everything(), + Limit: 1, + }, + expectedOut: []example.Pod{*preset[1]}, + expectContinue: true, + expectedRemainingItemCount: utilpointer.Int64Ptr(1), + rv: list.ResourceVersion, + rvMatch: metav1.ResourceVersionMatchNotOlderThan, + expectRV: list.ResourceVersion, + }, { name: "test List with limit at resource version 0", prefix: "/pods/second/", @@ -677,7 +713,12 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Field: fields.Everything(), Limit: 1, }, - expectedOut: []*example.Pod{preset[1]}, + // TODO(#108003): As of now, watchcache is deliberately ignoring + // limit if RV=0 is specified, returning whole list of objects. + // While this should eventually get fixed, for now we're explicitly + // ignoring this testcase for watchcache. + ignoreForWatchCache: true, + expectedOut: []example.Pod{*preset[1]}, expectContinue: true, expectedRemainingItemCount: utilpointer.Int64Ptr(1), rv: "0", @@ -691,7 +732,12 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Field: fields.Everything(), Limit: 1, }, - expectedOut: []*example.Pod{preset[1]}, + // TODO(#108003): As of now, watchcache is deliberately ignoring + // limit if RV=0 is specified, returning whole list of objects. + // While this should eventually get fixed, for now we're explicitly + // ignoring this testcase for watchcache. + ignoreForWatchCache: true, + expectedOut: []example.Pod{*preset[1]}, expectContinue: true, expectedRemainingItemCount: utilpointer.Int64Ptr(1), rv: "0", @@ -706,7 +752,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Field: fields.Everything(), Limit: 1, }, - expectedOut: []*example.Pod{}, + expectedOut: []example.Pod{}, expectContinue: false, rv: initialRV, rvMatch: metav1.ResourceVersionMatchExact, @@ -721,7 +767,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Limit: 1, Continue: secondContinuation, }, - expectedOut: []*example.Pod{preset[2]}, + expectedOut: []example.Pod{*preset[2]}, }, { name: "ignores resource version 0 for List with pregenerated continue token", @@ -733,13 +779,21 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Continue: secondContinuation, }, rv: "0", - expectedOut: []*example.Pod{preset[2]}, + expectedOut: []example.Pod{*preset[2]}, }, { name: "test List with multiple levels of directories and expect flattened result", prefix: "/pods/second/", pred: storage.Everything, - expectedOut: []*example.Pod{preset[1], preset[2]}, + expectedOut: []example.Pod{*preset[1], *preset[2]}, + }, + { + name: "test List with multiple levels of directories and expect flattened result with current resource version and match=NotOlderThan", + prefix: "/pods/second/", + pred: storage.Everything, + rv: list.ResourceVersion, + rvMatch: metav1.ResourceVersionMatchNotOlderThan, + expectedOut: []example.Pod{*preset[1], *preset[2]}, }, { name: "test List with filter returning only one item, ensure only a single page returned", @@ -749,7 +803,20 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Label: labels.Everything(), Limit: 1, }, - expectedOut: []*example.Pod{preset[3]}, + expectedOut: []example.Pod{*preset[3]}, + expectContinue: true, + }, + { + name: "test List with filter returning only one item, ensure only a single page returned with current resource version and match=NotOlderThan", + prefix: "/pods", + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "barfoo"), + Label: labels.Everything(), + Limit: 1, + }, + rv: list.ResourceVersion, + rvMatch: metav1.ResourceVersionMatchNotOlderThan, + expectedOut: []example.Pod{*preset[3]}, expectContinue: true, }, { @@ -760,7 +827,20 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Label: labels.Everything(), Limit: 2, }, - expectedOut: []*example.Pod{preset[3]}, + expectedOut: []example.Pod{*preset[3]}, + expectContinue: false, + }, + { + name: "test List with filter returning only one item, covers the entire list with current resource version and match=NotOlderThan", + prefix: "/pods", + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "barfoo"), + Label: labels.Everything(), + Limit: 2, + }, + rv: list.ResourceVersion, + rvMatch: metav1.ResourceVersionMatchNotOlderThan, + expectedOut: []example.Pod{*preset[3]}, expectContinue: false, }, { @@ -771,9 +851,9 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Label: labels.Everything(), Limit: 2, }, - rv: "0", - expectedOut: []*example.Pod{preset[3]}, - expectContinue: false, + rv: "0", + expectedAlternatives: [][]example.Pod{{}, {*preset[3]}}, + expectContinue: false, }, { name: "test List with filter returning two items, more pages possible", @@ -784,7 +864,20 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Limit: 2, }, expectContinue: true, - expectedOut: []*example.Pod{preset[0], preset[1]}, + expectedOut: []example.Pod{*preset[0], *preset[1]}, + }, + { + name: "test List with filter returning two items, more pages possible with current resource version and match=NotOlderThan", + prefix: "/pods", + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "bar"), + Label: labels.Everything(), + Limit: 2, + }, + rv: list.ResourceVersion, + rvMatch: metav1.ResourceVersionMatchNotOlderThan, + expectContinue: true, + expectedOut: []example.Pod{*preset[0], *preset[1]}, }, { name: "filter returns two items split across multiple pages", @@ -794,7 +887,19 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Label: labels.Everything(), Limit: 2, }, - expectedOut: []*example.Pod{preset[2], preset[4]}, + expectedOut: []example.Pod{*preset[2], *preset[4]}, + }, + { + name: "filter returns two items split across multiple pages with current resource version and match=NotOlderThan", + prefix: "/pods", + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "foo"), + Label: labels.Everything(), + Limit: 2, + }, + rv: list.ResourceVersion, + rvMatch: metav1.ResourceVersionMatchNotOlderThan, + expectedOut: []example.Pod{*preset[2], *preset[4]}, }, { name: "filter returns one item for last page, ends on last item, not full", @@ -805,7 +910,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Limit: 2, Continue: encodeContinueOrDie("third/barfoo", int64(continueRV)), }, - expectedOut: []*example.Pod{preset[4]}, + expectedOut: []example.Pod{*preset[4]}, }, { name: "filter returns one item for last page, starts on last item, full", @@ -816,7 +921,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Limit: 1, Continue: encodeContinueOrDie("third/barfoo", int64(continueRV)), }, - expectedOut: []*example.Pod{preset[4]}, + expectedOut: []example.Pod{*preset[4]}, }, { name: "filter returns one item for last page, starts on last item, partial page", @@ -827,7 +932,7 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Limit: 2, Continue: encodeContinueOrDie("third/barfoo", int64(continueRV)), }, - expectedOut: []*example.Pod{preset[4]}, + expectedOut: []example.Pod{*preset[4]}, }, { name: "filter returns two items, page size equal to total list size", @@ -837,7 +942,19 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Label: labels.Everything(), Limit: 5, }, - expectedOut: []*example.Pod{preset[2], preset[4]}, + expectedOut: []example.Pod{*preset[2], *preset[4]}, + }, + { + name: "filter returns two items, page size equal to total list size with current resource version and match=NotOlderThan", + prefix: "/pods", + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "foo"), + Label: labels.Everything(), + Limit: 5, + }, + rv: list.ResourceVersion, + rvMatch: metav1.ResourceVersionMatchNotOlderThan, + expectedOut: []example.Pod{*preset[2], *preset[4]}, }, { name: "filter returns one item, page size equal to total list size", @@ -847,12 +964,52 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Label: labels.Everything(), Limit: 5, }, - expectedOut: []*example.Pod{preset[3]}, + expectedOut: []example.Pod{*preset[3]}, + }, + { + name: "filter returns one item, page size equal to total list size with current resource version and match=NotOlderThan", + prefix: "/pods", + pred: storage.SelectionPredicate{ + Field: fields.OneTermEqualSelector("metadata.name", "barfoo"), + Label: labels.Everything(), + Limit: 5, + }, + rv: list.ResourceVersion, + rvMatch: metav1.ResourceVersionMatchNotOlderThan, + expectedOut: []example.Pod{*preset[3]}, + }, + { + name: "list all items", + prefix: "/pods", + pred: storage.Everything, + expectedOut: []example.Pod{*preset[0], *preset[1], *preset[2], *preset[3], *preset[4]}, + }, + { + name: "list all items with current resource version and match=NotOlderThan", + prefix: "/pods", + pred: storage.Everything, + rv: list.ResourceVersion, + rvMatch: metav1.ResourceVersionMatchNotOlderThan, + expectedOut: []example.Pod{*preset[0], *preset[1], *preset[2], *preset[3], *preset[4]}, }, } for _, tt := range tests { + tt := tt t.Run(tt.name, func(t *testing.T) { + // For some asynchronous implementations of storage interface (in particular watchcache), + // certain requests may impact result of further requests. As an example, if we first + // ensure that watchcache is synchronized up to ResourceVersion X (using Get/List requests + // with NotOlderThan semantic), the further requests (even specifying earlier resource + // version) will also return the result synchronized to at least ResourceVersion X. + // By parallelizing test cases we ensure that the order in which test cases are defined + // doesn't automatically preclude some scenarios from happening. + t.Parallel() + + if ignoreWatchCacheTests && tt.ignoreForWatchCache { + t.Skip() + } + if tt.pred.GetAttrs == nil { tt.pred.GetAttrs = getAttrs } @@ -864,9 +1021,9 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { Predicate: tt.pred, Recursive: true, } - err = store.GetList(ctx, tt.prefix, storageOpts, out) + err := store.GetList(ctx, tt.prefix, storageOpts, out) if tt.expectRVTooLarge { - if err == nil || !storage.IsTooLargeResourceVersion(err) { + if err == nil || !apierrors.IsTimeout(err) || !storage.IsTooLargeResourceVersion(err) { t.Fatalf("expecting resource version too high error, but get: %s", err) } return @@ -896,15 +1053,18 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface) { t.Errorf("resourceVersion in list response invalid: %v", err) } } - if len(tt.expectedOut) != len(out.Items) { - t.Fatalf("length of list want=%d, got=%d", len(tt.expectedOut), len(out.Items)) - } - if diff := cmp.Diff(tt.expectedRemainingItemCount, out.ListMeta.GetRemainingItemCount()); diff != "" { - t.Errorf("incorrect remainingItemCount: %s", diff) - } - for j, wantPod := range tt.expectedOut { - getPod := &out.Items[j] - ExpectNoDiff(t, fmt.Sprintf("%s: incorrect pod", tt.name), wantPod, getPod) + + if tt.expectedAlternatives == nil { + ExpectNoDiff(t, "incorrect list pods", tt.expectedOut, out.Items) + } else { + toInterfaceSlice := func(podLists [][]example.Pod) []interface{} { + result := make([]interface{}, 0, len(podLists)) + for i := range podLists { + result = append(result, podLists[i]) + } + return result + } + ExpectContains(t, "incorrect list pods", toInterfaceSlice(tt.expectedAlternatives), out.Items) } }) } @@ -1160,6 +1320,13 @@ func RunTestGetListNonRecursive(ctx context.Context, t *testing.T, store storage for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { + // For some asynchronous implementations of storage interface (in particular watchcache), + // certain requests may impact result of further requests. As an example, if we first + // ensure that watchcache is synchronized up to ResourceVersion X (using Get/List requests + // with NotOlderThan semantic), the further requests (even specifying earlier resource + // version) will also return the result synchronized to at least ResourceVersion X. + // By parallelizing test cases we ensure that the order in which test cases are defined + // doesn't automatically preclude some scenarios from happening. t.Parallel() out := &example.PodList{} diff --git a/pkg/storage/tests/cacher_test.go b/pkg/storage/tests/cacher_test.go index a1437403d..b731d2c53 100644 --- a/pkg/storage/tests/cacher_test.go +++ b/pkg/storage/tests/cacher_test.go @@ -176,6 +176,14 @@ func TestGetListNonRecursive(t *testing.T) { } func TestList(t *testing.T) { + ctx, cacher, terminate := testSetup(t) + t.Cleanup(terminate) + storagetesting.RunTestList(ctx, t, cacher, true) +} + +// TODO(wojtek-t): We should extend the generic RunTestList test to cover the +// scenarios that are not yet covered by it and get rid of this test. +func TestListDeprecated(t *testing.T) { server, etcdStorage := newEtcdTestStorage(t, etcd3testing.PathPrefix()) defer server.Terminate(t) cacher, _, err := newTestCacher(etcdStorage) @@ -268,42 +276,6 @@ func TestList(t *testing.T) { } } -// TestTooLargeResourceVersionList ensures that a list request for a resource version higher than available -// in the watch cache completes (does not wait indefinitely) and results in a ResourceVersionTooLarge error. -func TestTooLargeResourceVersionList(t *testing.T) { - server, etcdStorage := newEtcdTestStorage(t, etcd3testing.PathPrefix()) - defer server.Terminate(t) - cacher, v, err := newTestCacher(etcdStorage) - if err != nil { - t.Fatalf("Couldn't create cacher: %v", err) - } - defer cacher.Stop() - - podFoo := makeTestPod("foo") - fooCreated := updatePod(t, etcdStorage, podFoo, nil) - - // Set up List at fooCreated.ResourceVersion + 10 - rv, err := v.ParseResourceVersion(fooCreated.ResourceVersion) - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } - listRV := strconv.Itoa(int(rv + 10)) - - result := &example.PodList{} - options := storage.ListOptions{ - ResourceVersion: listRV, - Predicate: storage.Everything, - Recursive: true, - } - err = cacher.GetList(context.TODO(), "pods/ns", options, result) - if !errors.IsTimeout(err) { - t.Errorf("Unexpected error: %v", err) - } - if !storage.IsTooLargeResourceVersion(err) { - t.Errorf("expected 'Too large resource version' cause in error but got: %v", err) - } -} - func verifyWatchEvent(t *testing.T, w watch.Interface, eventType watch.EventType, eventObject runtime.Object) { _, _, line, _ := goruntime.Caller(1) select { diff --git a/pkg/util/flowcontrol/request/list_work_estimator.go b/pkg/util/flowcontrol/request/list_work_estimator.go index 75d70a0ad..130746a41 100644 --- a/pkg/util/flowcontrol/request/list_work_estimator.go +++ b/pkg/util/flowcontrol/request/list_work_estimator.go @@ -147,8 +147,11 @@ func key(requestInfo *apirequest.RequestInfo) string { // staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go func shouldListFromStorage(query url.Values, opts *metav1.ListOptions) bool { resourceVersion := opts.ResourceVersion + match := opts.ResourceVersionMatch pagingEnabled := utilfeature.DefaultFeatureGate.Enabled(features.APIListChunking) hasContinuation := pagingEnabled && len(opts.Continue) > 0 hasLimit := pagingEnabled && opts.Limit > 0 && resourceVersion != "0" - return resourceVersion == "" || hasContinuation || hasLimit || opts.ResourceVersionMatch == metav1.ResourceVersionMatchExact + unsupportedMatch := match != "" && match != metav1.ResourceVersionMatchNotOlderThan + + return resourceVersion == "" || hasContinuation || hasLimit || unsupportedMatch }