Merge pull request #59 from munnerz/cleanup-corrupt-dirs

if a volume containing incomplete metadata is found during a ListVolumes call, clean up and continue

Author: jetstack-bot

storage/filesystem.go

@@ -47,7 +47,7 @@ type Filesystem struct {
 	baseDir string
 
 	// used by the 'read only' methods
-	fs fs.FS
+	fs fs.StatFS
 
 	// FixedFSGroup is an optional field which will set the gid ownership of all
 	// volume's data directories to this value.
@@ -71,7 +71,7 @@ func NewFilesystem(log logr.Logger, baseDir string) (*Filesystem, error) {
 		baseDir: baseDir,
 		// Use the rootfs as the DirFS so that paths passed to both read &
 		// write methods on this struct use a consistent root.
-		fs: os.DirFS("/"),
+		fs: os.DirFS("/").(fs.StatFS),
 	}
 
 	notMnt, err := mount.IsNotMountPoint(mount.New(""), f.tempfsPath())
@@ -110,14 +110,19 @@ func (f *Filesystem) ListVolumes() ([]string, error) {
 
 	var vols []string
 	for _, dir := range dirs {
-		file, err := f.fs.Open(f.metadataPathForVolumeID(dir.Name()))
-		if err != nil {
+		_, err := f.fs.Stat(f.metadataPathForVolumeID(dir.Name()))
+		switch {
+		case errors.Is(err, fs.ErrNotExist):
+			f.log.Info("Directory exists but does not contain a metadata file - deleting directory and its contents", "volume_id", dir.Name())
+			if err := f.RemoveVolume(dir.Name()); err != nil {
+				return nil, fmt.Errorf("deleting stale volume: %v", err)
+			}
+			// continue to skip this loop iteration
+			continue
+		case err != nil:
 			// discovered a volume/directory that does not contain a metadata file
-			// TODO: log this error to allow startup to continue
 			return nil, err
 		}
-		// immediately close the file as we just need to verify it exists
-		file.Close()
 		vols = append(vols, dir.Name())
 	}
 

storage/filesystem_test.go

@@ -118,6 +118,25 @@ func TestFilesystem_ListVolumes(t *testing.T) {
 	}
 }
 
+func TestFilesystem_ListVolumes_CleansUpCorruptVolumes(t *testing.T) {
+	backend := &Filesystem{
+		fs: fstest.MapFS{
+			"inmemfs/fake-volume/metadata.json": &fstest.MapFile{Data: []byte{}},
+			"inmemfs/fake-emptyvolume/nothing":  &fstest.MapFile{Data: []byte{}},
+		},
+	}
+
+	vols, err := backend.ListVolumes()
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if len(vols) != 1 {
+		t.Errorf("expected 1 volume to be returned but got: %+v", vols)
+	}
+	if vols[0] != "fake-volume" {
+		t.Errorf("expected only entry to be 'fake-volume' but got: %s", vols[0])
+	}
+}
 func Test_fsGroupForMetadata(t *testing.T) {
 	intPtr := func(i int64) *int64 {
 		return &i