diff --git a/pkg/kubelet/BUILD b/pkg/kubelet/BUILD index d25639a975e..a8d46311ea9 100644 --- a/pkg/kubelet/BUILD +++ b/pkg/kubelet/BUILD @@ -152,6 +152,7 @@ go_library( "//vendor/k8s.io/utils/mount:go_default_library", "//vendor/k8s.io/utils/net:go_default_library", "//vendor/k8s.io/utils/path:go_default_library", + "//vendor/k8s.io/utils/strings:go_default_library", ] + select({ "@io_bazel_rules_go//go/platform:windows": [ "//pkg/kubelet/winstats:go_default_library", diff --git a/pkg/kubelet/kubelet_getters.go b/pkg/kubelet/kubelet_getters.go index a2ace13dd90..4467032522b 100644 --- a/pkg/kubelet/kubelet_getters.go +++ b/pkg/kubelet/kubelet_getters.go @@ -27,6 +27,7 @@ import ( "k8s.io/klog/v2" "k8s.io/utils/mount" utilpath "k8s.io/utils/path" + utilstrings "k8s.io/utils/strings" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -35,6 +36,7 @@ import ( kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" kubelettypes "k8s.io/kubernetes/pkg/kubelet/types" utilnode "k8s.io/kubernetes/pkg/util/node" + "k8s.io/kubernetes/pkg/volume/csi" ) // getRootDir returns the full path to the directory under which kubelet can @@ -310,8 +312,22 @@ func (kl *Kubelet) getPodVolumePathListFromDisk(podUID types.UID) ([]string, err if err != nil { return volumes, fmt.Errorf("could not read directory %s: %v", volumePluginPath, err) } - for _, volumeDir := range volumeDirs { - volumes = append(volumes, filepath.Join(volumePluginPath, volumeDir)) + unescapePluginName := utilstrings.UnescapeQualifiedName(volumePluginName) + + if unescapePluginName != csi.CSIPluginName { + for _, volumeDir := range volumeDirs { + volumes = append(volumes, filepath.Join(volumePluginPath, volumeDir)) + } + } else { + // For CSI volumes, the mounted volume path has an extra sub path "/mount", so also add it + // to the list if the mounted path exists. + for _, volumeDir := range volumeDirs { + path := filepath.Join(volumePluginPath, volumeDir) + csimountpath := csi.GetCSIMounterPath(path) + if pathExists, _ := mount.PathExists(csimountpath); pathExists { + volumes = append(volumes, csimountpath) + } + } } } return volumes, nil @@ -323,10 +339,15 @@ func (kl *Kubelet) getMountedVolumePathListFromDisk(podUID types.UID) ([]string, if err != nil { return mountedVolumes, err } + // Only use IsLikelyNotMountPoint to check might not cover all cases. For CSI volumes that + // either: 1) don't mount or 2) bind mount in the rootfs, the mount check will not work as expected. + // We plan to remove this mountpoint check as a condition before deleting pods since it is + // not reliable and the condition might be different for different types of volumes. But it requires + // a reliable way to clean up unused volume dir to avoid problems during pod deletion. See discussion in issue #74650 for _, volumePath := range volumePaths { isNotMount, err := kl.mounter.IsLikelyNotMountPoint(volumePath) if err != nil { - return mountedVolumes, err + return mountedVolumes, fmt.Errorf("fail to check mount point %q: %v", volumePath, err) } if !isNotMount { mountedVolumes = append(mountedVolumes, volumePath) diff --git a/pkg/kubelet/kubelet_volumes_linux_test.go b/pkg/kubelet/kubelet_volumes_linux_test.go index 7f08ff4d5cc..bd1c073b11f 100644 --- a/pkg/kubelet/kubelet_volumes_linux_test.go +++ b/pkg/kubelet/kubelet_volumes_linux_test.go @@ -28,7 +28,9 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" _ "k8s.io/kubernetes/pkg/apis/core/install" + "k8s.io/utils/mount" ) func validateDirExists(dir string) error { @@ -154,3 +156,115 @@ func TestCleanupOrphanedPodDirs(t *testing.T) { }) } } + +func TestPodVolumesExistWithMount(t *testing.T) { + poduid := types.UID("poduid") + testCases := map[string]struct { + prepareFunc func(kubelet *Kubelet) error + expected bool + }{ + "noncsivolume-dir-not-exist": { + prepareFunc: func(kubelet *Kubelet) error { + return nil + }, + expected: false, + }, + "noncsivolume-dir-exist-noplugins": { + prepareFunc: func(kubelet *Kubelet) error { + podDir := kubelet.getPodDir(poduid) + return os.MkdirAll(filepath.Join(podDir, "volumes/"), 0750) + }, + expected: false, + }, + "noncsivolume-dir-exist-nomount": { + prepareFunc: func(kubelet *Kubelet) error { + podDir := kubelet.getPodDir(poduid) + return os.MkdirAll(filepath.Join(podDir, "volumes/plugin/name"), 0750) + }, + expected: false, + }, + "noncsivolume-dir-exist-with-mount": { + prepareFunc: func(kubelet *Kubelet) error { + podDir := kubelet.getPodDir(poduid) + volumePath := filepath.Join(podDir, "volumes/plugin/name") + if err := os.MkdirAll(volumePath, 0750); err != nil { + return err + } + fm := mount.NewFakeMounter( + []mount.MountPoint{ + {Device: "/dev/sdb", Path: volumePath}, + }) + kubelet.mounter = fm + return nil + }, + expected: true, + }, + "noncsivolume-dir-exist-nomount-withcsimountpath": { + prepareFunc: func(kubelet *Kubelet) error { + podDir := kubelet.getPodDir(poduid) + volumePath := filepath.Join(podDir, "volumes/plugin/name/mount") + if err := os.MkdirAll(volumePath, 0750); err != nil { + return err + } + fm := mount.NewFakeMounter( + []mount.MountPoint{ + {Device: "/dev/sdb", Path: volumePath}, + }) + kubelet.mounter = fm + return nil + }, + expected: false, + }, + "csivolume-dir-exist-nomount": { + prepareFunc: func(kubelet *Kubelet) error { + podDir := kubelet.getPodDir(poduid) + volumePath := filepath.Join(podDir, "volumes/kubernetes.io~csi/name") + return os.MkdirAll(volumePath, 0750) + }, + expected: false, + }, + "csivolume-dir-exist-mount-nocsimountpath": { + prepareFunc: func(kubelet *Kubelet) error { + podDir := kubelet.getPodDir(poduid) + volumePath := filepath.Join(podDir, "volumes/kubernetes.io~csi/name/mount") + return os.MkdirAll(volumePath, 0750) + }, + expected: false, + }, + "csivolume-dir-exist-withcsimountpath": { + prepareFunc: func(kubelet *Kubelet) error { + podDir := kubelet.getPodDir(poduid) + volumePath := filepath.Join(podDir, "volumes/kubernetes.io~csi/name/mount") + if err := os.MkdirAll(volumePath, 0750); err != nil { + return err + } + fm := mount.NewFakeMounter( + []mount.MountPoint{ + {Device: "/dev/sdb", Path: volumePath}, + }) + kubelet.mounter = fm + return nil + }, + expected: true, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) + defer testKubelet.Cleanup() + kubelet := testKubelet.kubelet + + if tc.prepareFunc != nil { + if err := tc.prepareFunc(kubelet); err != nil { + t.Fatalf("%s failed preparation: %v", name, err) + } + } + + exist := kubelet.podVolumesExist(poduid) + if tc.expected != exist { + t.Errorf("%s failed: expected %t, got %t", name, tc.expected, exist) + } + }) + } +} diff --git a/pkg/volume/csi/csi_mounter.go b/pkg/volume/csi/csi_mounter.go index befb327a9b1..590f74a6282 100644 --- a/pkg/volume/csi/csi_mounter.go +++ b/pkg/volume/csi/csi_mounter.go @@ -82,7 +82,7 @@ type csiMountMgr struct { var _ volume.Volume = &csiMountMgr{} func (c *csiMountMgr) GetPath() string { - dir := filepath.Join(getTargetPath(c.podUID, c.specVolumeID, c.plugin.host), "/mount") + dir := GetCSIMounterPath(filepath.Join(getTargetPath(c.podUID, c.specVolumeID, c.plugin.host))) klog.V(4).Info(log("mounter.GetPath generated [%s]", dir)) return dir } diff --git a/pkg/volume/csi/csi_util.go b/pkg/volume/csi/csi_util.go index 72691bf26f5..e15addfd1e9 100644 --- a/pkg/volume/csi/csi_util.go +++ b/pkg/volume/csi/csi_util.go @@ -171,3 +171,8 @@ func getPVSourceFromSpec(spec *volume.Spec) (*api.CSIPersistentVolumeSource, err } return pvSrc, nil } + +// GetCSIMounterPath returns the mounter path given the base path. +func GetCSIMounterPath(path string) string { + return filepath.Join(path, "/mount") +}