Change storage.Interface to utilize recurisve parameter

This commit is contained in:
Wojciech Tyczyński 2022-02-10 09:58:31 +01:00
parent f0d5ea1e1d
commit 3016b3d8f8
9 changed files with 71 additions and 45 deletions

View File

@ -67,8 +67,9 @@ func (s *storageLeases) ListLeases() ([]string, error) {
ResourceVersion: "0", ResourceVersion: "0",
ResourceVersionMatch: metav1.ResourceVersionMatchNotOlderThan, ResourceVersionMatch: metav1.ResourceVersionMatchNotOlderThan,
Predicate: storage.Everything, Predicate: storage.Everything,
Recursive: true,
} }
if err := s.storage.List(apirequest.NewDefaultContext(), s.baseKey, storageOpts, ipInfoList); err != nil { if err := s.storage.GetList(apirequest.NewDefaultContext(), s.baseKey, storageOpts, ipInfoList); err != nil {
return nil, err return nil, err
} }

View File

@ -68,8 +68,8 @@ func (s *DryRunnableStorage) GetToList(ctx context.Context, key string, opts sto
return s.Storage.GetToList(ctx, key, opts, listObj) return s.Storage.GetToList(ctx, key, opts, listObj)
} }
func (s *DryRunnableStorage) List(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error { func (s *DryRunnableStorage) GetList(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error {
return s.Storage.List(ctx, key, opts, listObj) return s.Storage.GetList(ctx, key, opts, listObj)
} }
func (s *DryRunnableStorage) GuaranteedUpdate( func (s *DryRunnableStorage) GuaranteedUpdate(

View File

@ -347,16 +347,18 @@ func (e *Store) ListPredicate(ctx context.Context, p storage.SelectionPredicate,
ResourceVersion: options.ResourceVersion, ResourceVersion: options.ResourceVersion,
ResourceVersionMatch: options.ResourceVersionMatch, ResourceVersionMatch: options.ResourceVersionMatch,
Predicate: p, Predicate: p,
Recursive: true,
} }
if name, ok := p.MatchesSingle(); ok { if name, ok := p.MatchesSingle(); ok {
if key, err := e.KeyFunc(ctx, name); err == nil { if key, err := e.KeyFunc(ctx, name); err == nil {
storageOpts.Recursive = false
err := e.Storage.GetToList(ctx, key, storageOpts, list) err := e.Storage.GetToList(ctx, key, storageOpts, list)
return list, storeerr.InterpretListError(err, qualifiedResource) return list, storeerr.InterpretListError(err, qualifiedResource)
} }
// if we cannot extract a key based on the current context, the optimization is skipped // if we cannot extract a key based on the current context, the optimization is skipped
} }
err := e.Storage.List(ctx, e.KeyRootFunc(ctx), storageOpts, list) err := e.Storage.GetList(ctx, e.KeyRootFunc(ctx), storageOpts, list)
return list, storeerr.InterpretListError(err, qualifiedResource) return list, storeerr.InterpretListError(err, qualifiedResource)
} }

View File

@ -609,8 +609,8 @@ func (c *Cacher) GetToList(ctx context.Context, key string, opts storage.ListOpt
return c.list(ctx, key, opts, listObj, false) return c.list(ctx, key, opts, listObj, false)
} }
// List implements storage.Interface. // GetList implements storage.Interface.
func (c *Cacher) List(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error { func (c *Cacher) GetList(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error {
return c.list(ctx, key, opts, listObj, true) return c.list(ctx, key, opts, listObj, true)
} }
@ -1090,8 +1090,9 @@ func (lw *cacherListerWatcher) List(options metav1.ListOptions) (runtime.Object,
storageOpts := storage.ListOptions{ storageOpts := storage.ListOptions{
ResourceVersionMatch: options.ResourceVersionMatch, ResourceVersionMatch: options.ResourceVersionMatch,
Predicate: pred, Predicate: pred,
Recursive: true,
} }
if err := lw.storage.List(context.TODO(), lw.resourcePrefix, storageOpts, list); err != nil { if err := lw.storage.GetList(context.TODO(), lw.resourcePrefix, storageOpts, list); err != nil {
return nil, err return nil, err
} }
return list, nil return list, nil

View File

@ -314,7 +314,7 @@ func (d *dummyStorage) Get(_ context.Context, _ string, _ storage.GetOptions, _
func (d *dummyStorage) GetToList(_ context.Context, _ string, _ storage.ListOptions, _ runtime.Object) error { func (d *dummyStorage) GetToList(_ context.Context, _ string, _ storage.ListOptions, _ runtime.Object) error {
return d.err return d.err
} }
func (d *dummyStorage) List(_ context.Context, _ string, _ storage.ListOptions, listObj runtime.Object) error { func (d *dummyStorage) GetList(_ context.Context, _ string, _ storage.ListOptions, listObj runtime.Object) error {
podList := listObj.(*example.PodList) podList := listObj.(*example.PodList)
podList.ListMeta = metav1.ListMeta{ResourceVersion: "100"} podList.ListMeta = metav1.ListMeta{ResourceVersion: "100"}
return d.err return d.err
@ -326,7 +326,7 @@ func (d *dummyStorage) Count(_ string) (int64, error) {
return 0, fmt.Errorf("unimplemented") return 0, fmt.Errorf("unimplemented")
} }
func TestListCacheBypass(t *testing.T) { func TestGetListCacheBypass(t *testing.T) {
backingStorage := &dummyStorage{} backingStorage := &dummyStorage{}
cacher, _, err := newTestCacher(backingStorage) cacher, _, err := newTestCacher(backingStorage)
if err != nil { if err != nil {
@ -344,20 +344,22 @@ func TestListCacheBypass(t *testing.T) {
// Inject error to underlying layer and check if cacher is not bypassed. // Inject error to underlying layer and check if cacher is not bypassed.
backingStorage.err = errDummy backingStorage.err = errDummy
err = cacher.List(context.TODO(), "pods/ns", storage.ListOptions{ err = cacher.GetList(context.TODO(), "pods/ns", storage.ListOptions{
ResourceVersion: "0", ResourceVersion: "0",
Predicate: pred, Predicate: pred,
Recursive: true,
}, result) }, result)
if err != nil { if err != nil {
t.Errorf("List 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.List(context.TODO(), "pods/ns", storage.ListOptions{ err = cacher.GetList(context.TODO(), "pods/ns", storage.ListOptions{
ResourceVersion: "", ResourceVersion: "",
Predicate: pred, Predicate: pred,
Recursive: true,
}, result) }, result)
if err != errDummy { 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)
} }
} }

View File

@ -557,8 +557,8 @@ func encodeContinue(key, keyPrefix string, resourceVersion int64) (string, error
return base64.RawURLEncoding.EncodeToString(out), nil return base64.RawURLEncoding.EncodeToString(out), nil
} }
// List implements storage.Interface.List. // GetList implements storage.Interface.
func (s *store) List(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error { func (s *store) GetList(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error {
return s.list(ctx, key, opts, listObj, true) return s.list(ctx, key, opts, listObj, true)
} }

View File

@ -1080,8 +1080,9 @@ func TestTransformationFailure(t *testing.T) {
var got example.PodList var got example.PodList
storageOpts := storage.ListOptions{ storageOpts := storage.ListOptions{
Predicate: storage.Everything, Predicate: storage.Everything,
Recursive: true,
} }
if err := store.List(ctx, "/", storageOpts, &got); !storage.IsInternalError(err) { if err := store.GetList(ctx, "/", storageOpts, &got); !storage.IsInternalError(err) {
t.Errorf("Unexpected error %v", err) t.Errorf("Unexpected error %v", err)
} }
@ -1166,7 +1167,7 @@ func TestList(t *testing.T) {
// we want to figure out the resourceVersion before we create anything // we want to figure out the resourceVersion before we create anything
initialList := &example.PodList{} initialList := &example.PodList{}
if err := store.List(ctx, "/", storage.ListOptions{Predicate: storage.Everything}, initialList); err != nil { if err := store.GetList(ctx, "/", storage.ListOptions{Predicate: storage.Everything, Recursive: true}, initialList); err != nil {
t.Errorf("Unexpected List error: %v", err) t.Errorf("Unexpected List error: %v", err)
} }
initialRV := initialList.ResourceVersion initialRV := initialList.ResourceVersion
@ -1183,9 +1184,10 @@ func TestList(t *testing.T) {
storageOpts := storage.ListOptions{ storageOpts := storage.ListOptions{
ResourceVersion: "0", ResourceVersion: "0",
Predicate: storage.Everything, Predicate: storage.Everything,
Recursive: true,
} }
if err := store.List(ctx, "/two-level", storageOpts, list); err != nil { if err := store.GetList(ctx, "/two-level", storageOpts, list); err != nil {
t.Errorf("Unexpected List error: %v", err) t.Errorf("Unexpected error: %v", err)
} }
continueRV, _ := strconv.Atoi(list.ResourceVersion) continueRV, _ := strconv.Atoi(list.ResourceVersion)
secondContinuation, err := encodeContinue("/two-level/2", "/two-level/", int64(continueRV)) secondContinuation, err := encodeContinue("/two-level/2", "/two-level/", int64(continueRV))
@ -1576,12 +1578,13 @@ func TestList(t *testing.T) {
ResourceVersion: tt.rv, ResourceVersion: tt.rv,
ResourceVersionMatch: tt.rvMatch, ResourceVersionMatch: tt.rvMatch,
Predicate: tt.pred, Predicate: tt.pred,
Recursive: true,
} }
var err error var err error
if tt.disablePaging { if tt.disablePaging {
err = disablePagingStore.List(ctx, tt.prefix, storageOpts, out) err = disablePagingStore.GetList(ctx, tt.prefix, storageOpts, out)
} else { } else {
err = store.List(ctx, tt.prefix, storageOpts, out) err = store.GetList(ctx, tt.prefix, storageOpts, out)
} }
if tt.expectRVTooLarge { if tt.expectRVTooLarge {
if err == nil || !storage.IsTooLargeResourceVersion(err) { if err == nil || !storage.IsTooLargeResourceVersion(err) {
@ -1592,7 +1595,7 @@ func TestList(t *testing.T) {
if err != nil { if err != nil {
if !tt.expectError { if !tt.expectError {
t.Fatalf("List failed: %v", err) t.Fatalf("GetList failed: %v", err)
} }
return return
} }
@ -1690,8 +1693,9 @@ func TestListContinuation(t *testing.T) {
options := storage.ListOptions{ options := storage.ListOptions{
ResourceVersion: "0", ResourceVersion: "0",
Predicate: pred(1, ""), Predicate: pred(1, ""),
Recursive: true,
} }
if err := store.List(ctx, "/", options, out); err != nil { if err := store.GetList(ctx, "/", options, out); err != nil {
t.Fatalf("Unable to get initial list: %v", err) t.Fatalf("Unable to get initial list: %v", err)
} }
if len(out.Continue) == 0 { if len(out.Continue) == 0 {
@ -1716,8 +1720,9 @@ func TestListContinuation(t *testing.T) {
options = storage.ListOptions{ options = storage.ListOptions{
ResourceVersion: "0", ResourceVersion: "0",
Predicate: pred(0, continueFromSecondItem), Predicate: pred(0, continueFromSecondItem),
Recursive: true,
} }
if err := store.List(ctx, "/", options, out); err != nil { if err := store.GetList(ctx, "/", options, out); err != nil {
t.Fatalf("Unable to get second page: %v", err) t.Fatalf("Unable to get second page: %v", err)
} }
if len(out.Continue) != 0 { if len(out.Continue) != 0 {
@ -1742,8 +1747,9 @@ func TestListContinuation(t *testing.T) {
options = storage.ListOptions{ options = storage.ListOptions{
ResourceVersion: "0", ResourceVersion: "0",
Predicate: pred(1, continueFromSecondItem), Predicate: pred(1, continueFromSecondItem),
Recursive: true,
} }
if err := store.List(ctx, "/", options, out); err != nil { if err := store.GetList(ctx, "/", options, out); err != nil {
t.Fatalf("Unable to get second page: %v", err) t.Fatalf("Unable to get second page: %v", err)
} }
if len(out.Continue) == 0 { if len(out.Continue) == 0 {
@ -1767,8 +1773,9 @@ func TestListContinuation(t *testing.T) {
options = storage.ListOptions{ options = storage.ListOptions{
ResourceVersion: "0", ResourceVersion: "0",
Predicate: pred(1, continueFromThirdItem), Predicate: pred(1, continueFromThirdItem),
Recursive: true,
} }
if err := store.List(ctx, "/", options, out); err != nil { if err := store.GetList(ctx, "/", options, out); err != nil {
t.Fatalf("Unable to get second page: %v", err) t.Fatalf("Unable to get second page: %v", err)
} }
if len(out.Continue) != 0 { if len(out.Continue) != 0 {
@ -1861,8 +1868,9 @@ func TestListContinuationWithFilter(t *testing.T) {
options := storage.ListOptions{ options := storage.ListOptions{
ResourceVersion: "0", ResourceVersion: "0",
Predicate: pred(2, ""), Predicate: pred(2, ""),
Recursive: true,
} }
if err := store.List(ctx, "/", options, out); err != nil { if err := store.GetList(ctx, "/", options, out); err != nil {
t.Errorf("Unable to get initial list: %v", err) t.Errorf("Unable to get initial list: %v", err)
} }
if len(out.Continue) == 0 { if len(out.Continue) == 0 {
@ -1894,8 +1902,9 @@ func TestListContinuationWithFilter(t *testing.T) {
options = storage.ListOptions{ options = storage.ListOptions{
ResourceVersion: "0", ResourceVersion: "0",
Predicate: pred(2, cont), Predicate: pred(2, cont),
Recursive: true,
} }
if err := store.List(ctx, "/", options, out); err != nil { if err := store.GetList(ctx, "/", options, out); err != nil {
t.Errorf("Unable to get second page: %v", err) t.Errorf("Unable to get second page: %v", err)
} }
if len(out.Continue) != 0 { if len(out.Continue) != 0 {
@ -1976,8 +1985,9 @@ func TestListInconsistentContinuation(t *testing.T) {
options := storage.ListOptions{ options := storage.ListOptions{
ResourceVersion: "0", ResourceVersion: "0",
Predicate: pred(1, ""), Predicate: pred(1, ""),
Recursive: true,
} }
if err := store.List(ctx, "/", options, out); err != nil { if err := store.GetList(ctx, "/", options, out); err != nil {
t.Fatalf("Unable to get initial list: %v", err) t.Fatalf("Unable to get initial list: %v", err)
} }
if len(out.Continue) == 0 { if len(out.Continue) == 0 {
@ -2021,8 +2031,9 @@ func TestListInconsistentContinuation(t *testing.T) {
options = storage.ListOptions{ options = storage.ListOptions{
ResourceVersion: "0", ResourceVersion: "0",
Predicate: pred(0, continueFromSecondItem), Predicate: pred(0, continueFromSecondItem),
Recursive: true,
} }
err = store.List(ctx, "/", options, out) err = store.GetList(ctx, "/", options, out)
if err == nil { if err == nil {
t.Fatalf("unexpected no error") t.Fatalf("unexpected no error")
} }
@ -2042,8 +2053,9 @@ func TestListInconsistentContinuation(t *testing.T) {
options = storage.ListOptions{ options = storage.ListOptions{
ResourceVersion: "0", ResourceVersion: "0",
Predicate: pred(1, inconsistentContinueFromSecondItem), Predicate: pred(1, inconsistentContinueFromSecondItem),
Recursive: true,
} }
if err := store.List(ctx, "/", options, out); err != nil { if err := store.GetList(ctx, "/", options, out); err != nil {
t.Fatalf("Unable to get second page: %v", err) t.Fatalf("Unable to get second page: %v", err)
} }
if len(out.Continue) == 0 { if len(out.Continue) == 0 {
@ -2060,8 +2072,9 @@ func TestListInconsistentContinuation(t *testing.T) {
options = storage.ListOptions{ options = storage.ListOptions{
ResourceVersion: "0", ResourceVersion: "0",
Predicate: pred(1, continueFromThirdItem), Predicate: pred(1, continueFromThirdItem),
Recursive: true,
} }
if err := store.List(ctx, "/", options, out); err != nil { if err := store.GetList(ctx, "/", options, out); err != nil {
t.Fatalf("Unable to get second page: %v", err) t.Fatalf("Unable to get second page: %v", err)
} }
if len(out.Continue) != 0 { if len(out.Continue) != 0 {
@ -2316,8 +2329,9 @@ func TestConsistentList(t *testing.T) {
result1 := example.PodList{} result1 := example.PodList{}
options := storage.ListOptions{ options := storage.ListOptions{
Predicate: predicate, Predicate: predicate,
Recursive: true,
} }
if err := store.List(ctx, "/", options, &result1); err != nil { if err := store.GetList(ctx, "/", options, &result1); err != nil {
t.Fatalf("failed to list objects: %v", err) t.Fatalf("failed to list objects: %v", err)
} }
@ -2326,10 +2340,11 @@ func TestConsistentList(t *testing.T) {
Predicate: predicate, Predicate: predicate,
ResourceVersion: result1.ResourceVersion, ResourceVersion: result1.ResourceVersion,
ResourceVersionMatch: metav1.ResourceVersionMatchExact, ResourceVersionMatch: metav1.ResourceVersionMatchExact,
Recursive: true,
} }
result2 := example.PodList{} result2 := example.PodList{}
if err := store.List(ctx, "/", options, &result2); err != nil { if err := store.GetList(ctx, "/", options, &result2); err != nil {
t.Fatalf("failed to list objects: %v", err) t.Fatalf("failed to list objects: %v", err)
} }
@ -2341,7 +2356,7 @@ func TestConsistentList(t *testing.T) {
options.ResourceVersionMatch = metav1.ResourceVersionMatchNotOlderThan options.ResourceVersionMatch = metav1.ResourceVersionMatchNotOlderThan
result3 := example.PodList{} result3 := example.PodList{}
if err := store.List(ctx, "/", options, &result3); err != nil { if err := store.GetList(ctx, "/", options, &result3); err != nil {
t.Fatalf("failed to list objects: %v", err) t.Fatalf("failed to list objects: %v", err)
} }
@ -2349,7 +2364,7 @@ func TestConsistentList(t *testing.T) {
options.ResourceVersionMatch = metav1.ResourceVersionMatchExact options.ResourceVersionMatch = metav1.ResourceVersionMatchExact
result4 := example.PodList{} result4 := example.PodList{}
if err := store.List(ctx, "/", options, &result4); err != nil { if err := store.GetList(ctx, "/", options, &result4); err != nil {
t.Fatalf("failed to list objects: %v", err) t.Fatalf("failed to list objects: %v", err)
} }

View File

@ -183,7 +183,7 @@ type Interface interface {
// and send it in an "ADDED" event, before watch starts. // and send it in an "ADDED" event, before watch starts.
Watch(ctx context.Context, key string, opts ListOptions) (watch.Interface, error) Watch(ctx context.Context, key string, opts ListOptions) (watch.Interface, error)
// Get unmarshals json found at key into objPtr. On a not found error, will either // Get unmarshals object found at key into objPtr. On a not found error, will either
// return a zero object of the requested type, or an error, depending on 'opts.ignoreNotFound'. // return a zero object of the requested type, or an error, depending on 'opts.ignoreNotFound'.
// Treats empty responses and nil response nodes exactly like a not found error. // Treats empty responses and nil response nodes exactly like a not found error.
// The returned contents may be delayed, but it is guaranteed that they will // The returned contents may be delayed, but it is guaranteed that they will
@ -196,11 +196,13 @@ type Interface interface {
// match 'opts.ResourceVersion' according 'opts.ResourceVersionMatch'. // match 'opts.ResourceVersion' according 'opts.ResourceVersionMatch'.
GetToList(ctx context.Context, key string, opts ListOptions, listObj runtime.Object) error GetToList(ctx context.Context, key string, opts ListOptions, listObj runtime.Object) error
// List unmarshalls jsons found at directory defined by key and opaque them // GetList unmarshalls objects found at key into a *List api object (an object
// into *List api object (an object that satisfies runtime.IsList definition). // that satisfies runtime.IsList definition).
// If 'opts.Recursive' is false, 'key' is used as an exact match. If `opts.Recursive'
// is true, 'key' is used as a prefix.
// The returned contents may be delayed, but it is guaranteed that they will // The returned contents may be delayed, but it is guaranteed that they will
// match 'opts.ResourceVersion' according 'opts.ResourceVersionMatch'. // match 'opts.ResourceVersion' according 'opts.ResourceVersionMatch'.
List(ctx context.Context, key string, opts ListOptions, listObj runtime.Object) error GetList(ctx context.Context, key string, opts ListOptions, listObj runtime.Object) error
// GuaranteedUpdate keeps calling 'tryUpdate()' to update key 'key' (of type 'ptrToType') // GuaranteedUpdate keeps calling 'tryUpdate()' to update key 'key' (of type 'ptrToType')
// retrying the update until success if there is index conflict. // retrying the update until success if there is index conflict.

View File

@ -293,8 +293,9 @@ func TestList(t *testing.T) {
rvResult := &example.PodList{} rvResult := &example.PodList{}
options := storage.ListOptions{ options := storage.ListOptions{
Predicate: storage.Everything, Predicate: storage.Everything,
Recursive: true,
} }
if err := cacher.List(context.TODO(), "pods/ns", options, rvResult); err != nil { if err := cacher.GetList(context.TODO(), "pods/ns", options, rvResult); err != nil {
t.Errorf("Unexpected error: %v", err) t.Errorf("Unexpected error: %v", err)
} }
deletedPodRV := rvResult.ListMeta.ResourceVersion deletedPodRV := rvResult.ListMeta.ResourceVersion
@ -305,8 +306,9 @@ func TestList(t *testing.T) {
options = storage.ListOptions{ options = storage.ListOptions{
ResourceVersion: deletedPodRV, ResourceVersion: deletedPodRV,
Predicate: storage.Everything, Predicate: storage.Everything,
Recursive: true,
} }
if err := cacher.List(context.TODO(), "pods/ns", options, result); err != nil { if err := cacher.GetList(context.TODO(), "pods/ns", options, result); err != nil {
t.Errorf("Unexpected error: %v", err) t.Errorf("Unexpected error: %v", err)
} }
if result.ListMeta.ResourceVersion != deletedPodRV { if result.ListMeta.ResourceVersion != deletedPodRV {
@ -371,8 +373,9 @@ func TestTooLargeResourceVersionList(t *testing.T) {
options := storage.ListOptions{ options := storage.ListOptions{
ResourceVersion: listRV, ResourceVersion: listRV,
Predicate: storage.Everything, Predicate: storage.Everything,
Recursive: true,
} }
err = cacher.List(context.TODO(), "pods/ns", options, result) err = cacher.GetList(context.TODO(), "pods/ns", options, result)
if !errors.IsTimeout(err) { if !errors.IsTimeout(err) {
t.Errorf("Unexpected error: %v", err) t.Errorf("Unexpected error: %v", err)
} }
@ -408,12 +411,12 @@ type injectListError struct {
storage.Interface storage.Interface
} }
func (self *injectListError) List(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error { func (self *injectListError) GetList(ctx context.Context, key string, opts storage.ListOptions, listObj runtime.Object) error {
if self.errors > 0 { if self.errors > 0 {
self.errors-- self.errors--
return fmt.Errorf("injected error") return fmt.Errorf("injected error")
} }
return self.Interface.List(ctx, key, opts, listObj) return self.Interface.GetList(ctx, key, opts, listObj)
} }
func TestWatch(t *testing.T) { func TestWatch(t *testing.T) {