From 6b73a9ab8973394f096953a4e0b4cde778c42885 Mon Sep 17 00:00:00 2001 From: Ryan Abrams Date: Mon, 13 Aug 2018 13:51:27 -0700 Subject: [PATCH] Ignore missing paths during enumeration It's possible to run into a race condition in which the enumerator lists lots of repositories and then starts the long process of enumerating through them. In that time if someone deletes a repo, the enumerator may error out. Signed-off-by: Ryan Abrams --- registry/storage/driver/walk.go | 11 ++++++- registry/storage/driver/walk_test.go | 47 ++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 registry/storage/driver/walk_test.go diff --git a/registry/storage/driver/walk.go b/registry/storage/driver/walk.go index 2a5bc51f..8ded5b8b 100644 --- a/registry/storage/driver/walk.go +++ b/registry/storage/driver/walk.go @@ -4,6 +4,8 @@ import ( "context" "errors" "sort" + + "github.com/sirupsen/logrus" ) // ErrSkipDir is used as a return value from onFileFunc to indicate that @@ -32,7 +34,14 @@ func WalkFallback(ctx context.Context, driver StorageDriver, from string, f Walk // performance bottleneck. fileInfo, err := driver.Stat(ctx, child) if err != nil { - return err + 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 + } } err = f(fileInfo) if err == nil && fileInfo.IsDir() { diff --git a/registry/storage/driver/walk_test.go b/registry/storage/driver/walk_test.go new file mode 100644 index 00000000..d6e9beb6 --- /dev/null +++ b/registry/storage/driver/walk_test.go @@ -0,0 +1,47 @@ +package driver + +import ( + "context" + "fmt" + "testing" +) + +type changingFileSystem struct { + StorageDriver + fileset []string + keptFiles map[string]bool +} + +func (cfs *changingFileSystem) List(ctx context.Context, path string) ([]string, error) { + return cfs.fileset, nil +} +func (cfs *changingFileSystem) Stat(ctx context.Context, path string) (FileInfo, error) { + kept, ok := cfs.keptFiles[path] + if ok && kept { + return &FileInfoInternal{ + FileInfoFields: FileInfoFields{ + Path: path, + }, + }, nil + } + return nil, PathNotFoundError{} +} +func TestWalkFileRemoved(t *testing.T) { + d := &changingFileSystem{ + fileset: []string{"zoidberg", "bender"}, + keptFiles: map[string]bool{ + "zoidberg": true, + }, + } + infos := []FileInfo{} + err := WalkFallback(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()) + } +}