fix: only cleanup empty parent directories during DeleteVolume

fix

fix

Author: andyzhangx

pkg/nfs/controllerserver.go

@@ -292,17 +292,16 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
 			}
 			klog.V(2).Infof("archived subdirectory %s --> %s", internalVolumePath, archivedInternalVolumePath)
 		} else {
-			rootDir := getRootDir(nfsVol.subDir)
-			if rootDir != "" {
-				rootDir = filepath.Join(getInternalMountPath(cs.Driver.workingMountDir, nfsVol), rootDir)
-			} else {
-				rootDir = internalVolumePath
-			}
-			// delete subdirectory under base-dir
-			klog.V(2).Infof("removing subdirectory at %v on internalVolumePath %s", rootDir, internalVolumePath)
-			if err = os.RemoveAll(rootDir); err != nil {
+			klog.V(2).Infof("removing subdirectory at %v", internalVolumePath)
+			if err = os.RemoveAll(internalVolumePath); err != nil {
 				return nil, status.Errorf(codes.Internal, "delete subdirectory(%s) failed with %v", internalVolumePath, err)
 			}
+
+			parentDir := filepath.Dir(internalVolumePath)
+			klog.V(2).Infof("DeleteVolume: removing empty directories in %s", parentDir)
+			if err = removeEmptyDirs(getInternalMountPath(cs.Driver.workingMountDir, nfsVol), parentDir); err != nil {
+				return nil, status.Errorf(codes.Internal, "failed to remove empty directories: %v", err)
+			}
 		}
 	} else {
 		klog.V(2).Infof("DeleteVolume: volume(%s) is set to retain, not deleting/archiving subdirectory", volumeID)

pkg/nfs/utils.go

@@ -19,6 +19,7 @@ package nfs
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 	"sync"
 	"time"
@@ -222,6 +223,52 @@ func getRootDir(path string) string {
 	return parts[0]
 }
 
+// removeEmptyDirs removes empty directories in the given directory dir until the parent directory parentDir
+// It will remove all empty directories in the path from the given directory to the parent directory
+// It will not remove the parent directory parentDir
+func removeEmptyDirs(parentDir, dir string) error {
+	if parentDir == "" || dir == "" {
+		return nil
+	}
+
+	absParentDir, err := filepath.Abs(parentDir)
+	if err != nil {
+		return err
+	}
+
+	absDir, err := filepath.Abs(dir)
+	if err != nil {
+		return err
+	}
+
+	if !strings.HasPrefix(absDir, absParentDir) {
+		return fmt.Errorf("dir %s is not a subdirectory of parentDir %s", dir, parentDir)
+	}
+
+	var depth int
+	for absDir != absParentDir {
+		entries, err := os.ReadDir(absDir)
+		if err != nil {
+			return err
+		}
+		if len(entries) == 0 {
+			klog.V(2).Infof("Removing empty directory %s", absDir)
+			if err := os.Remove(absDir); err != nil {
+				return err
+			}
+		} else {
+			klog.V(2).Infof("Directory %s is not empty", absDir)
+			break
+		}
+		if depth++; depth > 10 {
+			return fmt.Errorf("depth of directory %s is too deep", dir)
+		}
+		absDir = filepath.Dir(absDir)
+	}
+
+	return nil
+}
+
 // ExecFunc returns a exec function's output and error
 type ExecFunc func() (err error)
 

pkg/nfs/utils_test.go

@@ -429,6 +429,67 @@ func TestGetRootPath(t *testing.T) {
 	}
 }
 
+func TestRemoveEmptyDirs(t *testing.T) {
+	parentDir, _ := os.Getwd()
+	emptyDirOneLevel, _ := getWorkDirPath("emptyDir1")
+	emptyDirTwoLevels, _ := getWorkDirPath("emptyDir2/emptyDir2")
+	emptyDirThreeLevels, _ := getWorkDirPath("emptyDir3/emptyDir2/emptyDir3")
+
+	tests := []struct {
+		desc      string
+		parentDir string
+		dir       string
+		expected  error
+	}{
+		{
+			desc:      "empty path",
+			parentDir: parentDir,
+			expected:  nil,
+		},
+		{
+			desc:      "empty dir with one level",
+			parentDir: parentDir,
+			dir:       emptyDirOneLevel,
+			expected:  nil,
+		},
+		{
+			desc:      "dir is not a subdirectory of parentDir",
+			parentDir: "/dir1",
+			dir:       "/dir2",
+			expected:  fmt.Errorf("dir /dir2 is not a subdirectory of parentDir /dir1"),
+		},
+		{
+			desc:      "empty dir with two levels",
+			parentDir: parentDir,
+			dir:       emptyDirTwoLevels,
+			expected:  nil,
+		},
+		{
+			desc:      "empty dir with three levels",
+			parentDir: parentDir,
+			dir:       emptyDirThreeLevels,
+			expected:  nil,
+		},
+	}
+
+	for _, test := range tests {
+		if strings.Contains(test.dir, "emptyDir") {
+			_ = makeDir(test.dir)
+			defer os.RemoveAll(test.dir)
+		}
+		err := removeEmptyDirs(test.parentDir, test.dir)
+		if !reflect.DeepEqual(err, test.expected) {
+			t.Errorf("test[%s]: unexpected output: %v, expected result: %v", test.desc, err, test.expected)
+		}
+		if strings.Contains(test.dir, "emptyDir") {
+			// directory should be removed
+			if _, err := os.Stat(emptyDirOneLevel); !os.IsNotExist(err) {
+				t.Errorf("test[%s]: directory %s should be removed", test.desc, emptyDirOneLevel)
+			}
+		}
+	}
+}
+
 func TestWaitUntilTimeout(t *testing.T) {
 	tests := []struct {
 		desc        string