Don't create mounter when reconstructing block volume

CSI mounter will create a new directory + json for a filesystem volume,
leading to even more orphaned files/directories.
This commit is contained in:
Jan Safranek 2019-07-09 15:34:16 +02:00 committed by Masaki Kimura
parent f1d2d9d670
commit bab81b809b
2 changed files with 36 additions and 24 deletions

View File

@ -493,32 +493,12 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume,
uniqueVolumeName = util.GetUniqueVolumeNameFromSpecWithPod(volume.podName, plugin, volumeSpec) uniqueVolumeName = util.GetUniqueVolumeNameFromSpecWithPod(volume.podName, plugin, volumeSpec)
} }
volumeMounter, newMounterErr := plugin.NewMounter( var volumeMapper volumepkg.BlockVolumeMapper
volumeSpec, var volumeMounter volumepkg.Mounter
pod, // Path to the mount or block device to check
volumepkg.VolumeOptions{}) var checkPath string
if newMounterErr != nil {
return nil, fmt.Errorf(
"reconstructVolume.NewMounter failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v",
uniqueVolumeName,
volumeSpec.Name(),
volume.podName,
pod.UID,
newMounterErr)
}
// Check existence of mount point for filesystem volume or symbolic link for block volume
isExist, checkErr := rc.operationExecutor.CheckVolumeExistenceOperation(volumeSpec, volumeMounter.GetPath(), volumeSpec.Name(), rc.mounter, uniqueVolumeName, volume.podName, pod.UID, attachablePlugin)
if checkErr != nil {
return nil, checkErr
}
// If mount or symlink doesn't exist, volume reconstruction should be failed
if !isExist {
return nil, fmt.Errorf("Volume: %q is not mounted", uniqueVolumeName)
}
// TODO: remove feature gate check after no longer needed // TODO: remove feature gate check after no longer needed
var volumeMapper volumepkg.BlockVolumeMapper
if utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) && volume.volumeMode == v1.PersistentVolumeBlock { if utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) && volume.volumeMode == v1.PersistentVolumeBlock {
var newMapperErr error var newMapperErr error
if mapperPlugin != nil { if mapperPlugin != nil {
@ -535,7 +515,34 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume,
pod.UID, pod.UID,
newMapperErr) newMapperErr)
} }
checkPath, _ = volumeMapper.GetPodDeviceMapPath()
} }
} else {
var err error
volumeMounter, err = plugin.NewMounter(
volumeSpec,
pod,
volumepkg.VolumeOptions{})
if err != nil {
return nil, fmt.Errorf(
"reconstructVolume.NewMounter failed for volume %q (spec.Name: %q) pod %q (UID: %q) with: %v",
uniqueVolumeName,
volumeSpec.Name(),
volume.podName,
pod.UID,
err)
}
checkPath = volumeMounter.GetPath()
}
// Check existence of mount point for filesystem volume or symbolic link for block volume
isExist, checkErr := rc.operationExecutor.CheckVolumeExistenceOperation(volumeSpec, checkPath, volumeSpec.Name(), rc.mounter, uniqueVolumeName, volume.podName, pod.UID, attachablePlugin)
if checkErr != nil {
return nil, checkErr
}
// If mount or symlink doesn't exist, volume reconstruction should be failed
if !isExist {
return nil, fmt.Errorf("Volume: %q is not mounted", uniqueVolumeName)
} }
reconstructedVolume := &reconstructedVolume{ reconstructedVolume := &reconstructedVolume{

View File

@ -543,10 +543,12 @@ type MountedVolume struct {
// Mounter is the volume mounter used to mount this volume. It is required // Mounter is the volume mounter used to mount this volume. It is required
// by kubelet to create container.VolumeMap. // by kubelet to create container.VolumeMap.
// Mounter is only required for file system volumes and not required for block volumes.
Mounter volume.Mounter Mounter volume.Mounter
// BlockVolumeMapper is the volume mapper used to map this volume. It is required // BlockVolumeMapper is the volume mapper used to map this volume. It is required
// by kubelet to create container.VolumeMap. // by kubelet to create container.VolumeMap.
// BlockVolumeMapper is only required for block volumes and not required for file system volumes.
BlockVolumeMapper volume.BlockVolumeMapper BlockVolumeMapper volume.BlockVolumeMapper
// VolumeGidValue contains the value of the GID annotation, if present. // VolumeGidValue contains the value of the GID annotation, if present.
@ -935,6 +937,9 @@ func (oe *operationExecutor) CheckVolumeExistenceOperation(
if attachable != nil { if attachable != nil {
var isNotMount bool var isNotMount bool
var mountCheckErr error var mountCheckErr error
if mounter == nil {
return false, fmt.Errorf("mounter was not set for a filesystem volume")
}
if isNotMount, mountCheckErr = mounter.IsLikelyNotMountPoint(mountPath); mountCheckErr != nil { if isNotMount, mountCheckErr = mounter.IsLikelyNotMountPoint(mountPath); mountCheckErr != nil {
return false, fmt.Errorf("Could not check whether the volume %q (spec.Name: %q) pod %q (UID: %q) is mounted with: %v", return false, fmt.Errorf("Could not check whether the volume %q (spec.Name: %q) pod %q (UID: %q) is mounted with: %v",
uniqueVolumeName, uniqueVolumeName,