diff --git a/registry/storage/driver/walk.go b/registry/storage/driver/walk.go index 09e2d613b..23fc8800c 100644 --- a/registry/storage/driver/walk.go +++ b/registry/storage/driver/walk.go @@ -22,6 +22,20 @@ type WalkFn func(fileInfo FileInfo) error // to a directory, the directory will not be entered and Walk // will continue the traversal. If fileInfo refers to a normal file, processing stops func WalkFallback(ctx context.Context, driver StorageDriver, from string, f WalkFn) error { + return doWalk(ctx, driver, from, true, f) +} + +// WalkFilesFallback traverses a filesystem defined within driver, starting +// from the given path, calling f on each file. It uses the List method and Stat to drive itself. +// If an error is returned from WalkFn, processing stops +func WalkFilesFallback(ctx context.Context, driver StorageDriver, from string, f WalkFn) error { + return doWalk(ctx, driver, from, false, f) +} + +// WalkFilesFallback traverses a filesystem defined within driver, starting +// from the given path, calling f on each file. It uses the List method and Stat to drive itself. +// If an error is returned from WalkFn, processing stops +func doWalk(ctx context.Context, driver StorageDriver, from string, walkDir bool, f WalkFn) error { children, err := driver.List(ctx, from) if err != nil { return err @@ -43,12 +57,15 @@ func WalkFallback(ctx context.Context, driver StorageDriver, from string, f Walk return err } } - err = f(fileInfo) + err = nil + if !fileInfo.IsDir() || walkDir { + err = f(fileInfo) + } if err == nil && fileInfo.IsDir() { - if err := WalkFallback(ctx, driver, child, f); err != nil { + if err := doWalk(ctx, driver, child, walkDir, f); err != nil { return err } - } else if err == ErrSkipDir { + } else if err == ErrSkipDir && walkDir { // Stop iteration if it's a file, otherwise noop if it's a directory if !fileInfo.IsDir() { return nil @@ -59,41 +76,3 @@ func WalkFallback(ctx context.Context, driver StorageDriver, from string, f Walk } return nil } - -// WalkFilesFallback traverses a filesystem defined within driver, starting -// from the given path, calling f on each file. It uses the List method and Stat to drive itself. -// If an error is returned from WalkFn, processing stops -func WalkFilesFallback(ctx context.Context, driver StorageDriver, from string, f WalkFn) error { - children, err := driver.List(ctx, from) - if err != nil { - return err - } - sort.Stable(sort.StringSlice(children)) - for _, child := range children { - // TODO(stevvooe): Calling driver.Stat for every entry is quite - // expensive when running against backends with a slow Stat - // implementation, such as s3. This is very likely a serious - // performance bottleneck. - fileInfo, err := driver.Stat(ctx, child) - if err != nil { - switch err.(type) { - case PathNotFoundError: - // repository was removed in between listing and enumeration. Ignore it. - logrus.WithField("path", child).Infof("ignoring deleted path") - continue - default: - return err - } - } - if !fileInfo.IsDir() { - err = f(fileInfo) - if err != nil { - return err - } - } - if err := WalkFallback(ctx, driver, child, f); err != nil { - return err - } - } - return nil -} diff --git a/registry/storage/driver/walk_test.go b/registry/storage/driver/walk_test.go index d6e9beb64..c2627be5d 100644 --- a/registry/storage/driver/walk_test.go +++ b/registry/storage/driver/walk_test.go @@ -45,3 +45,23 @@ func TestWalkFileRemoved(t *testing.T) { t.Fatalf(err.Error()) } } + +func TestWalkFilesFileRemoved(t *testing.T) { + d := &changingFileSystem{ + fileset: []string{"zoidberg", "bender"}, + keptFiles: map[string]bool{ + "zoidberg": true, + }, + } + infos := []FileInfo{} + err := WalkFilesFallback(context.Background(), d, "", func(fileInfo FileInfo) error { + infos = append(infos, fileInfo) + return nil + }) + if len(infos) != 1 || infos[0].Path() != "zoidberg" { + t.Errorf(fmt.Sprintf("unexpected path set during walk: %s", infos)) + } + if err != nil { + t.Fatalf(err.Error()) + } +}