Merge pull request #82698 from janario/fix/umount-subpath-warns

Unmount subpath should only scan the first level of files/directories

pkg/util/mount/fake.go

@@ -32,9 +32,12 @@ type FakeMounter struct {
 	MountCheckErrors map[string]error
 	// Some tests run things in parallel, make sure the mounter does not produce
 	// any golang's DATA RACE warnings.
-	mutex sync.Mutex
+	mutex       sync.Mutex
+	UnmountFunc UnmountFunc
 }
 
+type UnmountFunc func(path string) error
+
 var _ Interface = &FakeMounter{}
 
 const (
@@ -117,6 +120,12 @@ func (f *FakeMounter) Unmount(target string) error {
 	newMountpoints := []MountPoint{}
 	for _, mp := range f.MountPoints {
 		if mp.Path == absTarget {
+			if f.UnmountFunc != nil {
+				err := f.UnmountFunc(absTarget)
+				if err != nil {
+					return err
+				}
+			}
 			klog.V(5).Infof("Fake mounter: unmounted %s from %s", mp.Device, absTarget)
 			// Don't copy it to newMountpoints
 			continue

pkg/volume/util/subpath/subpath_linux.go

@@ -241,6 +241,12 @@ func doCleanSubPaths(mounter mount.Interface, podDir string, volumeName string)
 			if err = doCleanSubPath(mounter, fullContainerDirPath, filepath.Base(path)); err != nil {
 				return err
 			}
+
+			if info.IsDir() {
+				// skip subdirs of the volume: it only matters the first level to unmount, otherwise it would try to unmount subdir of the volume
+				return filepath.SkipDir
+			}
+
 			return nil
 		})
 		if err != nil {

pkg/volume/util/subpath/subpath_linux_test.go

@@ -409,6 +409,7 @@ func TestCleanSubPaths(t *testing.T) {
 		// Function that validates directory structure after the test
 		validate    func(base string) error
 		expectError bool
+		unmount     func(path string) error
 	}{
 		{
 			name: "not-exists",
@@ -539,6 +540,64 @@ func TestCleanSubPaths(t *testing.T) {
 				return validateDirExists(baseSubdir)
 			},
 		},
+		{
+			name: "subpath-with-files",
+			prepare: func(base string) ([]mount.MountPoint, error) {
+				containerPath := filepath.Join(base, containerSubPathDirectoryName, testVol, "container1")
+				if err := os.MkdirAll(containerPath, defaultPerm); err != nil {
+					return nil, err
+				}
+
+				file0 := filepath.Join(containerPath, "0")
+				if err := ioutil.WriteFile(file0, []byte{}, defaultPerm); err != nil {
+					return nil, err
+				}
+
+				dir1 := filepath.Join(containerPath, "1")
+				if err := os.MkdirAll(filepath.Join(dir1, "my-dir-1"), defaultPerm); err != nil {
+					return nil, err
+				}
+
+				dir2 := filepath.Join(containerPath, "2")
+				if err := os.MkdirAll(filepath.Join(dir2, "my-dir-2"), defaultPerm); err != nil {
+					return nil, err
+				}
+
+				file3 := filepath.Join(containerPath, "3")
+				if err := ioutil.WriteFile(file3, []byte{}, defaultPerm); err != nil {
+					return nil, err
+				}
+
+				mounts := []mount.MountPoint{
+					{Device: "/dev/sdb", Path: file0},
+					{Device: "/dev/sdc", Path: dir1},
+					{Device: "/dev/sdd", Path: dir2},
+					{Device: "/dev/sde", Path: file3},
+				}
+				return mounts, nil
+			},
+			unmount: func(mountpath string) error {
+				err := filepath.Walk(mountpath, func(path string, info os.FileInfo, err error) error {
+					if path == mountpath {
+						// Skip top level directory
+						return nil
+					}
+
+					if err = os.Remove(path); err != nil {
+						return err
+					}
+					return filepath.SkipDir
+				})
+				if err != nil {
+					return fmt.Errorf("error processing %s: %s", mountpath, err)
+				}
+
+				return nil
+			},
+			validate: func(base string) error {
+				return validateDirNotExists(filepath.Join(base, containerSubPathDirectoryName))
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -553,7 +612,7 @@ func TestCleanSubPaths(t *testing.T) {
 			t.Fatalf("failed to prepare test %q: %v", test.name, err.Error())
 		}
 
-		fm := &mount.FakeMounter{MountPoints: mounts}
+		fm := &mount.FakeMounter{MountPoints: mounts, UnmountFunc: test.unmount}
 
 		err = doCleanSubPaths(fm, base, testVol)
 		if err != nil && !test.expectError {