diff --git a/pkg/volume/util/subpath/subpath_linux.go b/pkg/volume/util/subpath/subpath_linux.go index 84661ac1c59..7da8ca10a95 100644 --- a/pkg/volume/util/subpath/subpath_linux.go +++ b/pkg/volume/util/subpath/subpath_linux.go @@ -30,7 +30,6 @@ import ( "golang.org/x/sys/unix" "k8s.io/klog/v2" - "k8s.io/kubernetes/pkg/volume/util/hostutil" "k8s.io/mount-utils" ) @@ -110,12 +109,12 @@ func prepareSubpathTarget(mounter mount.Interface, subpath Subpath) (bool, strin notMount = true } if !notMount { - linuxHostUtil := hostutil.NewHostUtil() - mntInfo, err := linuxHostUtil.FindMountInfo(bindPathTarget) + // It's already mounted, so check if it's bind-mounted to the same path + samePath, err := checkSubPathFileEqual(subpath, bindPathTarget) if err != nil { - return false, "", fmt.Errorf("error calling findMountInfo for %s: %s", bindPathTarget, err) + return false, "", fmt.Errorf("error checking subpath mount info for %s: %s", bindPathTarget, err) } - if mntInfo.Root != subpath.Path { + if !samePath { // It's already mounted but not what we want, unmount it if err = mounter.Unmount(bindPathTarget); err != nil { return false, "", fmt.Errorf("error ummounting %s: %s", bindPathTarget, err) @@ -156,6 +155,23 @@ func prepareSubpathTarget(mounter mount.Interface, subpath Subpath) (bool, strin return false, bindPathTarget, nil } +func checkSubPathFileEqual(subpath Subpath, bindMountTarget string) (bool, error) { + s, err := os.Lstat(subpath.Path) + if err != nil { + return false, fmt.Errorf("stat %s failed: %s", subpath.Path, err) + } + + t, err := os.Lstat(bindMountTarget) + if err != nil { + return false, fmt.Errorf("lstat %s failed: %s", bindMountTarget, err) + } + + if !os.SameFile(s, t) { + return false, nil + } + return true, nil +} + func getSubpathBindTarget(subpath Subpath) string { // containerName is DNS label, i.e. safe as a directory name. return filepath.Join(subpath.PodDir, containerSubPathDirectoryName, subpath.VolumeName, subpath.ContainerName, strconv.Itoa(subpath.VolumeMountIndex)) diff --git a/pkg/volume/util/subpath/subpath_linux_test.go b/pkg/volume/util/subpath/subpath_linux_test.go index c1dbdf24681..86f24da11ff 100644 --- a/pkg/volume/util/subpath/subpath_linux_test.go +++ b/pkg/volume/util/subpath/subpath_linux_test.go @@ -921,6 +921,143 @@ func TestBindSubPath(t *testing.T) { } } +func TestSubpath_PrepareSafeSubpath(t *testing.T) { + //complete code + defaultPerm := os.FileMode(0750) + + tests := []struct { + name string + // Function that prepares directory structure for the test under given + // base. + prepare func(base string) ([]string, string, string, error) + expectError bool + expectAction []mount.FakeAction + mountExists bool + }{ + { + name: "subpath-mount-already-exists-with-mismatching-mount", + prepare: func(base string) ([]string, string, string, error) { + volpath, subpathMount := getTestPaths(base) + mounts := []string{subpathMount} + if err := os.MkdirAll(subpathMount, defaultPerm); err != nil { + return nil, "", "", err + } + + subpath := filepath.Join(volpath, "dir0") + return mounts, volpath, subpath, os.MkdirAll(subpath, defaultPerm) + }, + expectError: false, + expectAction: []mount.FakeAction{{Action: "unmount"}}, + mountExists: false, + }, + { + name: "subpath-mount-already-exists-with-samefile", + prepare: func(base string) ([]string, string, string, error) { + volpath, subpathMount := getTestPaths(base) + mounts := []string{subpathMount} + subpathMountRoot := filepath.Dir(subpathMount) + + if err := os.MkdirAll(subpathMountRoot, defaultPerm); err != nil { + return nil, "", "", err + } + targetFile, err := os.Create(subpathMount) + if err != nil { + return nil, "", "", err + } + defer targetFile.Close() + + if err := os.MkdirAll(volpath, defaultPerm); err != nil { + return nil, "", "", err + } + subpath := filepath.Join(volpath, "file0") + // using hard link to simulate bind mounts + err = os.Link(subpathMount, subpath) + if err != nil { + return nil, "", "", err + } + return mounts, volpath, subpath, nil + }, + expectError: false, + expectAction: []mount.FakeAction{}, + mountExists: true, + }, + } + for _, test := range tests { + klog.V(4).Infof("test %q", test.name) + base, err := ioutil.TempDir("", "bind-subpath-"+test.name+"-") + if err != nil { + t.Fatalf(err.Error()) + } + defer os.RemoveAll(base) + + mounts, volPath, subPath, err := test.prepare(base) + if err != nil { + os.RemoveAll(base) + t.Fatalf("failed to prepare test %q: %v", test.name, err.Error()) + } + + fm := setupFakeMounter(mounts) + + subpath := Subpath{ + VolumeMountIndex: testSubpath, + Path: subPath, + VolumeName: testVol, + VolumePath: volPath, + PodDir: filepath.Join(base, "pod0"), + ContainerName: testContainer, + } + + _, subpathMount := getTestPaths(base) + bindMountExists, bindPathTarget, err := prepareSubpathTarget(fm, subpath) + + if bindMountExists != test.mountExists { + t.Errorf("test %q failed: expected bindMountExists %v, got %v", test.name, test.mountExists, bindMountExists) + } + + logActions := fm.GetLog() + if len(test.expectAction) == 0 && len(logActions) > 0 { + t.Errorf("test %q failed: expected no actions, got %v", test.name, logActions) + } + + if len(test.expectAction) > 0 { + foundMatchingAction := false + testAction := test.expectAction[0] + for _, action := range logActions { + if action.Action == testAction.Action { + foundMatchingAction = true + break + } + } + if !foundMatchingAction { + t.Errorf("test %q failed: expected action %q, got %v", test.name, testAction.Action, logActions) + } + } + + if test.expectError { + if err == nil { + t.Errorf("test %q failed: expected error, got success", test.name) + } + if bindPathTarget != "" { + t.Errorf("test %q failed: expected empty bindPathTarget, got %v", test.name, bindPathTarget) + } + if err = validateDirNotExists(subpathMount); err != nil { + t.Errorf("test %q failed: %v", test.name, err) + } + } + if !test.expectError { + if err != nil { + t.Errorf("test %q failed: %v", test.name, err) + } + if bindPathTarget != subpathMount { + t.Errorf("test %q failed: expected bindPathTarget %v, got %v", test.name, subpathMount, bindPathTarget) + } + if err = validateFileExists(subpathMount); err != nil { + t.Errorf("test %q failed: %v", test.name, err) + } + } + } +} + func TestSafeOpen(t *testing.T) { defaultPerm := os.FileMode(0750)