Merge pull request #95301 from lorenz/pr-remove-dangling-volumes

Automatically remove orphaned pod's dangling volumes

pkg/kubelet/kubelet_getters.go

@@ -368,17 +368,46 @@ func (kl *Kubelet) getMountedVolumePathListFromDisk(podUID types.UID) ([]string,
 	return mountedVolumes, nil
 }
 
-// podVolumesSubpathsDirExists returns true if the pod volume-subpaths directory for
-// a given pod exists
-func (kl *Kubelet) podVolumeSubpathsDirExists(podUID types.UID) (bool, error) {
-	podVolDir := kl.getPodVolumeSubpathsDir(podUID)
+// getPodVolumeSubpathListFromDisk returns a list of the volume-subpath paths by reading the
+// subpath directories for the given pod from the disk.
+func (kl *Kubelet) getPodVolumeSubpathListFromDisk(podUID types.UID) ([]string, error) {
+	volumes := []string{}
+	podSubpathsDir := kl.getPodVolumeSubpathsDir(podUID)
 
-	if pathExists, pathErr := mount.PathExists(podVolDir); pathErr != nil {
-		return true, fmt.Errorf("error checking if path %q exists: %v", podVolDir, pathErr)
+	if pathExists, pathErr := mount.PathExists(podSubpathsDir); pathErr != nil {
+		return nil, fmt.Errorf("error checking if path %q exists: %v", podSubpathsDir, pathErr)
 	} else if !pathExists {
-		return false, nil
+		return volumes, nil
 	}
-	return true, nil
+
+	// Explicitly walks /<volume>/<container name>/<subPathIndex>
+	volumePluginDirs, err := ioutil.ReadDir(podSubpathsDir)
+	if err != nil {
+		klog.Errorf("Could not read directory %s: %v", podSubpathsDir, err)
+		return volumes, err
+	}
+	for _, volumePluginDir := range volumePluginDirs {
+		volumePluginName := volumePluginDir.Name()
+		volumePluginPath := filepath.Join(podSubpathsDir, volumePluginName)
+		containerDirs, err := ioutil.ReadDir(volumePluginPath)
+		if err != nil {
+			return volumes, fmt.Errorf("could not read directory %s: %v", volumePluginPath, err)
+		}
+		for _, containerDir := range containerDirs {
+			containerName := containerDir.Name()
+			containerPath := filepath.Join(volumePluginPath, containerName)
+			// Switch to ReadDirNoStat at the subPathIndex level to prevent issues with stat'ing
+			// mount points that may not be responsive
+			subPaths, err := utilpath.ReadDirNoStat(containerPath)
+			if err != nil {
+				return volumes, fmt.Errorf("could not read directory %s: %v", containerPath, err)
+			}
+			for _, subPathDir := range subPaths {
+				volumes = append(volumes, filepath.Join(containerPath, subPathDir))
+			}
+		}
+	}
+	return volumes, nil
 }
 
 // GetRequestedContainersInfo returns container info.

pkg/kubelet/kubelet_volumes.go

@@ -18,6 +18,7 @@ package kubelet
 
 import (
 	"fmt"
+	"syscall"
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -120,25 +121,49 @@ func (kl *Kubelet) cleanupOrphanedPodDirs(pods []*v1.Pod, runningPods []*kubecon
 			klog.V(3).Infof("Orphaned pod %q found, but volumes are not cleaned up", uid)
 			continue
 		}
-		// If there are still volume directories, do not delete directory
+
+		allVolumesCleanedUp := true
+
+		// If there are still volume directories, attempt to rmdir them
 		volumePaths, err := kl.getPodVolumePathListFromDisk(uid)
 		if err != nil {
 			orphanVolumeErrors = append(orphanVolumeErrors, fmt.Errorf("orphaned pod %q found, but error %v occurred during reading volume dir from disk", uid, err))
 			continue
 		}
 		if len(volumePaths) > 0 {
-			orphanVolumeErrors = append(orphanVolumeErrors, fmt.Errorf("orphaned pod %q found, but volume paths are still present on disk", uid))
-			continue
+			for _, volumePath := range volumePaths {
+				if err := syscall.Rmdir(volumePath); err != nil {
+					orphanVolumeErrors = append(orphanVolumeErrors, fmt.Errorf("orphaned pod %q found, but failed to rmdir() volume at path %v: %v", uid, volumePath, err))
+					allVolumesCleanedUp = false
+				} else {
+					klog.Warningf("Cleaned up orphaned volume from pod %q at %s", uid, volumePath)
+				}
+			}
 		}
 
-		// If there are any volume-subpaths, do not cleanup directories
-		volumeSubpathExists, err := kl.podVolumeSubpathsDirExists(uid)
+		// If there are any volume-subpaths, attempt to rmdir them
+		subpathVolumePaths, err := kl.getPodVolumeSubpathListFromDisk(uid)
 		if err != nil {
 			orphanVolumeErrors = append(orphanVolumeErrors, fmt.Errorf("orphaned pod %q found, but error %v occurred during reading of volume-subpaths dir from disk", uid, err))
 			continue
 		}
-		if volumeSubpathExists {
-			orphanVolumeErrors = append(orphanVolumeErrors, fmt.Errorf("orphaned pod %q found, but volume subpaths are still present on disk", uid))
+		if len(subpathVolumePaths) > 0 {
+			for _, subpathVolumePath := range subpathVolumePaths {
+				if err := syscall.Rmdir(subpathVolumePath); err != nil {
+					orphanVolumeErrors = append(orphanVolumeErrors, fmt.Errorf("orphaned pod %q found, but failed to rmdir() subpath at path %v: %v", uid, subpathVolumePath, err))
+					allVolumesCleanedUp = false
+				} else {
+					klog.Warningf("Cleaned up orphaned volume subpath from pod %q at %s", uid, subpathVolumePath)
+				}
+			}
+		}
+
+		if !allVolumesCleanedUp {
+			// Not all volumes were removed, so don't clean up the pod directory yet. It is likely
+			// that there are still mountpoints left which could stall RemoveAllOneFilesystem which
+			// would otherwise be called below.
+			// Errors for all removal operations have already been recorded, so don't add another
+			// one here.
 			continue
 		}
 

pkg/kubelet/kubelet_volumes_linux_test.go

@@ -101,7 +101,7 @@ func TestCleanupOrphanedPodDirs(t *testing.T) {
 			},
 			validateFunc: func(kubelet *Kubelet) error {
 				podDir := kubelet.getPodDir("pod1uid")
-				return validateDirExists(filepath.Join(podDir, "volumes/plugin/name"))
+				return validateDirNotExists(podDir)
 			},
 		},
 		"pod-doesnot-exist-with-subpath": {
@@ -111,7 +111,7 @@ func TestCleanupOrphanedPodDirs(t *testing.T) {
 			},
 			validateFunc: func(kubelet *Kubelet) error {
 				podDir := kubelet.getPodDir("pod1uid")
-				return validateDirExists(filepath.Join(podDir, "volume-subpaths/volume/container/index"))
+				return validateDirNotExists(podDir)
 			},
 		},
 		"pod-doesnot-exist-with-subpath-top": {
@@ -121,7 +121,35 @@ func TestCleanupOrphanedPodDirs(t *testing.T) {
 			},
 			validateFunc: func(kubelet *Kubelet) error {
 				podDir := kubelet.getPodDir("pod1uid")
-				return validateDirExists(filepath.Join(podDir, "volume-subpaths"))
+				return validateDirNotExists(podDir)
+			},
+		},
+		"pod-doesnot-exists-with-populated-volume": {
+			prepareFunc: func(kubelet *Kubelet) error {
+				podDir := kubelet.getPodDir("pod1uid")
+				volumePath := filepath.Join(podDir, "volumes/plugin/name")
+				if err := os.MkdirAll(volumePath, 0750); err != nil {
+					return err
+				}
+				return ioutil.WriteFile(filepath.Join(volumePath, "test.txt"), []byte("test1"), 0640)
+			},
+			validateFunc: func(kubelet *Kubelet) error {
+				podDir := kubelet.getPodDir("pod1uid")
+				return validateDirExists(filepath.Join(podDir, "volumes/plugin/name"))
+			},
+		},
+		"pod-doesnot-exists-with-populated-subpath": {
+			prepareFunc: func(kubelet *Kubelet) error {
+				podDir := kubelet.getPodDir("pod1uid")
+				subPath := filepath.Join(podDir, "volume-subpaths/volume/container/index")
+				if err := os.MkdirAll(subPath, 0750); err != nil {
+					return err
+				}
+				return ioutil.WriteFile(filepath.Join(subPath, "test.txt"), []byte("test1"), 0640)
+			},
+			validateFunc: func(kubelet *Kubelet) error {
+				podDir := kubelet.getPodDir("pod1uid")
+				return validateDirExists(filepath.Join(podDir, "volume-subpaths/volume/container/index"))
 			},
 		},
 		// TODO: test volume in volume-manager