diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go index 647383b5540..186537d19d7 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/dryrun.go @@ -64,10 +64,6 @@ func (s *DryRunnableStorage) Get(ctx context.Context, key string, opts storage.G return s.Storage.Get(ctx, key, opts, objPtr) } -func (s *DryRunnableStorage) GetToList(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error { - return s.Storage.GetToList(ctx, key, opts, listObj) -} - func (s *DryRunnableStorage) GetList(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error { return s.Storage.GetList(ctx, key, opts, listObj) } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go index 28e600175c2..6a090ae7b60 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go @@ -352,7 +352,7 @@ func (e *Store) ListPredicate(ctx context.Context, p storage.SelectionPredicate, if name, ok := p.MatchesSingle(); ok { if key, err := e.KeyFunc(ctx, name); err == nil { storageOpts.Recursive = false - err := e.Storage.GetToList(ctx, key, storageOpts, list) + err := e.Storage.GetList(ctx, key, storageOpts, list) return list, storeerr.InterpretListError(err, qualifiedResource) } // if we cannot extract a key based on the current context, the optimization is skipped 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 362a4a8f38f..231c999b4a4 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go @@ -604,23 +604,6 @@ func shouldDelegateList(opts storage.ListOptions) bool { return resourceVersion == "" || hasContinuation || hasLimit || opts.ResourceVersionMatch == metav1.ResourceVersionMatchExact } -// GetToList implements storage.Interface. -func (c *Cacher) GetToList(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error { - return c.list(ctx, key, opts, listObj, false) -} - -// GetList implements storage.Interface. -func (c *Cacher) GetList(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error { - return c.list(ctx, key, opts, listObj, true) -} - -func (c *Cacher) delegateList(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object, recursive bool) error { - if !recursive { - return c.storage.GetToList(ctx, key, opts, listObj) - } - return c.storage.List(ctx, key, opts, listObj) -} - func (c *Cacher) listItems(listRV uint64, key string, pred storage.SelectionPredicate, trace *utiltrace.Trace, recursive bool) ([]interface{}, uint64, string, error) { if !recursive { obj, exists, readResourceVersion, err := c.watchCache.WaitUntilFreshAndGet(listRV, key, trace) @@ -635,11 +618,13 @@ func (c *Cacher) listItems(listRV uint64, key string, pred storage.SelectionPred return c.watchCache.WaitUntilFreshAndList(listRV, pred.MatcherIndex(), trace) } -func (c *Cacher) list(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object, recursive bool) error { +// GetList implements storage.Interface +func (c *Cacher) GetList(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error { + recursive := opts.Recursive resourceVersion := opts.ResourceVersion pred := opts.Predicate if shouldDelegateList(opts) { - return c.delegateList(ctx, key, opts, listObj, recursive) + return c.storage.GetList(ctx, key, opts, listObj) } // If resourceVersion is specified, serve it from cache. @@ -653,7 +638,7 @@ func (c *Cacher) list(ctx context.Context, key string, opts storage.ListOptions, if listRV == 0 && !c.ready.check() { // If Cacher is not yet initialized and we don't require any specific // minimal resource version, simply forward the request to storage. - return c.delegateList(ctx, key, opts, listObj, recursive) + return c.storage.GetList(ctx, key, opts, listObj) } trace := utiltrace.New("cacher list", utiltrace.Field{Key: "type", Value: c.objectType.String()}) 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 58fcab02d61..a359e5befd7 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 @@ -311,9 +311,6 @@ func (d *dummyStorage) Watch(_ context.Context, _ string, _ storage.ListOptions) func (d *dummyStorage) Get(_ context.Context, _ string, _ storage.GetOptions, _ runtime.Object) error { return d.err } -func (d *dummyStorage) GetToList(_ context.Context, _ string, _ storage.ListOptions, _ runtime.Object) error { - return d.err -} func (d *dummyStorage) GetList(_ context.Context, _ string, _ storage.ListOptions, listObj runtime.Object) error { podList := listObj.(*example.PodList) podList.ListMeta = metav1.ListMeta{ResourceVersion: "100"} @@ -363,7 +360,7 @@ func TestGetListCacheBypass(t *testing.T) { } } -func TestGetToListCacheBypass(t *testing.T) { +func TestGetListNonRecursiveCacheBypass(t *testing.T) { backingStorage := &dummyStorage{} cacher, _, err := newTestCacher(backingStorage) if err != nil { @@ -381,20 +378,20 @@ func TestGetToListCacheBypass(t *testing.T) { // Inject error to underlying layer and check if cacher is not bypassed. backingStorage.err = errDummy - err = cacher.GetToList(context.TODO(), "pods/ns", storage.ListOptions{ + err = cacher.GetList(context.TODO(), "pods/ns", storage.ListOptions{ ResourceVersion: "0", Predicate: pred, }, result) if err != nil { - t.Errorf("GetToList with Limit and RV=0 should be served from cache: %v", err) + t.Errorf("GetList with Limit and RV=0 should be served from cache: %v", err) } - err = cacher.GetToList(context.TODO(), "pods/ns", storage.ListOptions{ + err = cacher.GetList(context.TODO(), "pods/ns", storage.ListOptions{ ResourceVersion: "", Predicate: pred, }, result) if err != errDummy { - t.Errorf("List with Limit without RV=0 should bypass cacher: %v", err) + t.Errorf("GetList with Limit without RV=0 should bypass cacher: %v", err) } } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go index 77d6cdcce84..c4e80aa8e4e 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store.go @@ -456,11 +456,6 @@ func (s *store) GuaranteedUpdate( } } -// GetToList implements storage.Interface.GetToList. -func (s *store) GetToList(ctx context.Context, key string, listOpts storage.ListOptions, listObj runtime.Object) error { - return s.list(ctx, key, listOpts, listObj, false) -} - func getNewItemFunc(listObj runtime.Object, v reflect.Value) func() runtime.Object { // For unstructured lists with a target group/version, preserve the group/version in the instantiated list items if unstructuredList, isUnstructured := listObj.(*unstructured.UnstructuredList); isUnstructured { @@ -559,10 +554,7 @@ func encodeContinue(key, keyPrefix string, resourceVersion int64) (string, error // GetList implements storage.Interface. func (s *store) GetList(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error { - return s.list(ctx, key, opts, listObj, true) -} - -func (s *store) list(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object, recursive bool) error { + recursive := opts.Recursive resourceVersion := opts.ResourceVersion match := opts.ResourceVersionMatch pred := opts.Predicate diff --git a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go index a47a1e84e0b..2bc616db2c3 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/etcd3/store_test.go @@ -541,7 +541,7 @@ func TestPreconditionalDeleteWithSuggestion(t *testing.T) { } } -func TestGetToList(t *testing.T) { +func TestGetListNonRecursive(t *testing.T) { ctx, store, _ := testSetup(t) prevKey, prevStoredObj := testPropogateStore(ctx, t, store, &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "prev"}}) @@ -640,7 +640,13 @@ func TestGetToList(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { out := &example.PodList{} - err := store.GetToList(ctx, tt.key, storage.ListOptions{ResourceVersion: tt.rv, ResourceVersionMatch: tt.rvMatch, Predicate: tt.pred}, out) + storageOpts := storage.ListOptions{ + ResourceVersion: tt.rv, + ResourceVersionMatch: tt.rvMatch, + Predicate: tt.pred, + Recursive: false, + } + err := store.GetList(ctx, tt.key, storageOpts, out) if tt.expectRVTooLarge { if err == nil || !storage.IsTooLargeResourceVersion(err) { @@ -650,7 +656,7 @@ func TestGetToList(t *testing.T) { } if err != nil { - t.Fatalf("%s: GetToList failed: %v", tt.name, err) + t.Fatalf("GetList failed: %v", err) } if len(out.ResourceVersion) == 0 { t.Errorf("%s: unset resourceVersion", tt.name) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go b/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go index d2bcf199740..6e77ea700db 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/interfaces.go @@ -190,12 +190,6 @@ type Interface interface { // match 'opts.ResourceVersion' according 'opts.ResourceVersionMatch'. Get(ctx context.Context, key string, opts GetOptions, objPtr runtime.Object) error - // GetToList unmarshals json found at key and opaque it into *List api object - // (an object that satisfies the runtime.IsList definition). - // The returned contents may be delayed, but it is guaranteed that they will - // match 'opts.ResourceVersion' according 'opts.ResourceVersionMatch'. - GetToList(ctx context.Context, key string, opts ListOptions, listObj runtime.Object) error - // GetList unmarshalls objects found at key into a *List api object (an object // that satisfies runtime.IsList definition). // If 'opts.Recursive' is false, 'key' is used as an exact match. If `opts.Recursive' @@ -267,8 +261,8 @@ type ListOptions struct { ResourceVersionMatch metav1.ResourceVersionMatch // Predicate provides the selection rules for the list operation. Predicate SelectionPredicate - // Recursive determines whether the watch is defined for a single object or for the whole set - // of objects. The option is ignored for non-watch requests. + // Recursive determines whether the list or watch is defined for a single object located at the + // given key, or for the whole set of objects with the given key as a prefix. 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. diff --git a/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go b/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go index 3d4511fe892..93bea21fc1a 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/tests/cacher_test.go @@ -196,7 +196,7 @@ func TestGet(t *testing.T) { } } -func TestGetToList(t *testing.T) { +func TestGetListNonRecursive(t *testing.T) { server, etcdStorage := newEtcdTestStorage(t, etcd3testing.PathPrefix()) defer server.Terminate(t) cacher, _, err := newTestCacher(etcdStorage) @@ -212,15 +212,15 @@ func TestGetToList(t *testing.T) { key string pred storage.SelectionPredicate expectedOut []*example.Pod - }{{ // test GetToList on existing key + }{{ // test non-recursive GetList on existing key key: key, pred: storage.Everything, expectedOut: []*example.Pod{storedObj}, - }, { // test GetToList on non-existing key + }, { // test non-recursive GetList on non-existing key key: "/non-existing", pred: storage.Everything, expectedOut: nil, - }, { // test GetToList with matching pod name + }, { // test non-recursive GetList with matching pod name key: "/non-existing", pred: storage.SelectionPredicate{ Label: labels.Everything(), @@ -235,9 +235,9 @@ func TestGetToList(t *testing.T) { for i, tt := range tests { out := &example.PodList{} - err := cacher.GetToList(context.TODO(), tt.key, storage.ListOptions{Predicate: tt.pred}, out) + err := cacher.GetList(context.TODO(), tt.key, storage.ListOptions{Predicate: tt.pred, Recursive: false}, out) if err != nil { - t.Fatalf("GetToList failed: %v", err) + t.Fatalf("GetList failed: %v", err) } if len(out.ResourceVersion) == 0 { t.Errorf("#%d: unset resourceVersion", i) diff --git a/test/integration/apiserver/apiserver_test.go b/test/integration/apiserver/apiserver_test.go index eeb67f900dd..02ca413203f 100644 --- a/test/integration/apiserver/apiserver_test.go +++ b/test/integration/apiserver/apiserver_test.go @@ -561,7 +561,7 @@ func testListOptionsCase(t *testing.T, rsClient appsv1.ReplicaSetInterface, watc } count := int64(len(items)) - // Cacher.GetToList defines this for logic to decide if the watch cache is skipped. We need to know it to know if + // Cacher.GetList defines this for logic to decide if the watch cache is skipped. We need to know it to know if // the limit is respected when testing here. pagingEnabled := utilfeature.DefaultFeatureGate.Enabled(features.APIListChunking) hasContinuation := pagingEnabled && len(opts.Continue) > 0