Skip to content

Commit

Permalink
kubelet: fix volume reconstruction for CSI ephemeral volumes
Browse files Browse the repository at this point in the history
This resolves a couple of issues for CSI volume reconstruction.
1. IsLikelyNotMountPoint is known not to work for bind mounts and was
   causing problems for subpaths and hostpath volumes.
2. Inline volumes were failing reconstruction due to calling
   GetVolumeName, which only works when there is a PV spec.
  • Loading branch information
dobsonj committed Oct 25, 2022
1 parent efe0af9 commit 4edf677
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 11 deletions.
22 changes: 14 additions & 8 deletions pkg/kubelet/volumemanager/reconciler/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,14 +442,6 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume,
if err != nil {
return nil, err
}
attachablePlugin, err := rc.volumePluginMgr.FindAttachablePluginByName(volume.pluginName)
if err != nil {
return nil, err
}
deviceMountablePlugin, err := rc.volumePluginMgr.FindDeviceMountablePluginByName(volume.pluginName)
if err != nil {
return nil, err
}

// Create pod object
pod := &v1.Pod{
Expand Down Expand Up @@ -478,6 +470,20 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume,
return nil, err
}

// We have to find the plugins by volume spec (NOT by plugin name) here
// in order to correctly reconstruct ephemeral volume types.
// Searching by spec checks whether the volume is actually attachable
// (i.e. has a PV) whereas searching by plugin name can only tell whether
// the plugin supports attachable volumes.
attachablePlugin, err := rc.volumePluginMgr.FindAttachablePluginBySpec(volumeSpec)
if err != nil {
return nil, err
}
deviceMountablePlugin, err := rc.volumePluginMgr.FindDeviceMountablePluginBySpec(volumeSpec)
if err != nil {
return nil, err
}

var uniqueVolumeName v1.UniqueVolumeName
if attachablePlugin != nil || deviceMountablePlugin != nil {
uniqueVolumeName, err = util.GetUniqueVolumeNameFromSpec(plugin, volumeSpec)
Expand Down
6 changes: 3 additions & 3 deletions pkg/volume/util/operationexecutor/operation_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -971,7 +971,7 @@ func (oe *operationExecutor) ReconstructVolumeOperation(
// Filesystem Volume case
if volumeMode == v1.PersistentVolumeFilesystem {
// Create volumeSpec from mount path
klog.V(5).Infof("Starting operationExecutor.ReconstructVolumepodName")
klog.V(5).Infof("Starting operationExecutor.ReconstructVolume for file volume on pod %q", podName)
volumeSpec, err := plugin.ConstructVolumeSpec(volumeSpecName, volumePath)
if err != nil {
return nil, err
Expand All @@ -981,7 +981,7 @@ func (oe *operationExecutor) ReconstructVolumeOperation(

// Block Volume case
// Create volumeSpec from mount path
klog.V(5).Infof("Starting operationExecutor.ReconstructVolume")
klog.V(5).Infof("Starting operationExecutor.ReconstructVolume for block volume on pod %q", podName)

// volumePath contains volumeName on the path. In the case of block volume, {volumeName} is symbolic link
// corresponding to raw block device.
Expand Down Expand Up @@ -1017,7 +1017,7 @@ func (oe *operationExecutor) CheckVolumeExistenceOperation(
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 = mount.IsNotMountPoint(mounter, 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",
uniqueVolumeName,
volumeName,
Expand Down

0 comments on commit 4edf677

Please sign in to comment.