Squashed commit of the following:

commit 1b3ae27e7af577372d5aaaf28ea401eb33d1c4df
Author: weizhichen <weizhichen@microsoft.com>
Date:   Thu Mar 9 08:39:04 2023 +0000

    fix

commit 566e139308e3cec4c9d4765eb4ccc3a735346c2e
Author: weizhichen <weizhichen@microsoft.com>
Date:   Thu Mar 9 08:36:32 2023 +0000

    fix unit test

commit 13a58ebd25b824dcf854a132e9ac474c8296f0bf
Author: weizhichen <weizhichen@microsoft.com>
Date:   Thu Mar 2 03:32:39 2023 +0000

    add unit test

commit c984e36e37c41bbef8aec46fe3fe81ab1c6a2521
Author: weizhichen <weizhichen@microsoft.com>
Date:   Tue Feb 28 15:25:56 2023 +0000

    fix imports

commit 58ec617e0ff1fbd209ca0af3237017679c3c0ad7
Author: weizhichen <weizhichen@microsoft.com>
Date:   Tue Feb 28 15:24:21 2023 +0000

    delete CheckVolumeExistenceOperation

commit 0d8cf0caa78bdf1f1f84ce011c4cc0e0de0e8707
Author: weizhichen <weizhichen@microsoft.com>
Date:   Tue Feb 28 14:29:37 2023 +0000

    fix 111933
This commit is contained in:
weizhichen 2023-03-09 08:42:27 +00:00
parent f5ddaa152e
commit a6ffbb41f8
4 changed files with 69 additions and 80 deletions

View File

@ -2341,9 +2341,9 @@ func TestSyncStates(t *testing.T) {
{
name: "when reconstruction fails for a volume, volumes should be cleaned up",
volumePaths: []string{
filepath.Join("pod1", "volumes", "fake-plugin", "pvc-abcdef"),
filepath.Join("pod1", "volumes", "fake-plugin", volumetesting.FailNewMounter),
},
createMountPoint: false,
createMountPoint: true,
podInfos: []podInfo{},
verifyFunc: func(rcInstance *reconciler, fakePlugin *volumetesting.FakeVolumePlugin) error {
return retryWithExponentialBackOff(reconcilerSyncWaitDuration, func() (bool, error) {
@ -2355,6 +2355,65 @@ func TestSyncStates(t *testing.T) {
})
},
},
{
name: "when mount point does not exist, reconstruction should not fail, volumes should be added in asw",
volumePaths: []string{
filepath.Join("pod1", "volumes", "fake-plugin", "pvc-abcdef"),
},
createMountPoint: false,
podInfos: []podInfo{},
verifyFunc: func(rcInstance *reconciler, fakePlugin *volumetesting.FakeVolumePlugin) error {
mountedPods := rcInstance.actualStateOfWorld.GetMountedVolumes()
if len(mountedPods) != 1 {
return fmt.Errorf("expected 1 pods to in asw got %d", len(mountedPods))
}
return nil
},
},
{
name: "when mount point does not exist, reconstruction should not fail, if volume exists in dsw, volume should be recorded in skipped during reconstruction",
volumePaths: []string{
filepath.Join("pod1uid", "volumes", "fake-plugin", "volume-name"),
},
createMountPoint: false,
podInfos: []podInfo{defaultPodInfo},
postSyncStatCallback: func(rcInstance *reconciler, fakePlugin *volumetesting.FakeVolumePlugin) error {
skippedVolumes := rcInstance.skippedDuringReconstruction
if len(skippedVolumes) != 1 {
return fmt.Errorf("expected 1 pods to in skippedDuringReconstruction got %d", len(skippedVolumes))
}
rcInstance.processReconstructedVolumes()
return nil
},
verifyFunc: func(rcInstance *reconciler, fakePlugin *volumetesting.FakeVolumePlugin) error {
mountedPods := rcInstance.actualStateOfWorld.GetAllMountedVolumes()
if len(mountedPods) != 1 {
return fmt.Errorf("expected 1 pods to in mounted volume list got %d", len(mountedPods))
}
mountedPodVolume := mountedPods[0]
addedViaReconstruction := rcInstance.actualStateOfWorld.IsVolumeReconstructed(mountedPodVolume.VolumeName, mountedPodVolume.PodName)
if !addedViaReconstruction {
return fmt.Errorf("expected volume %s to be marked as added via reconstruction", mountedPodVolume.VolumeName)
}
// check device mount state
attachedVolumes := rcInstance.actualStateOfWorld.GetAttachedVolumes()
if len(attachedVolumes) != 1 {
return fmt.Errorf("expected 1 volume to be unmounted, got %d", len(attachedVolumes))
}
firstAttachedVolume := attachedVolumes[0]
if !firstAttachedVolume.DeviceMayBeMounted() {
return fmt.Errorf("expected %s volume to be mounted in uncertain state", firstAttachedVolume.VolumeName)
}
// also skippedVolumes map should be empty
skippedVolumes := rcInstance.skippedDuringReconstruction
if len(skippedVolumes) > 0 {
return fmt.Errorf("expected 0 pods in skipped volumes found %d", len(skippedVolumes))
}
return nil
},
},
{
name: "when volume exists in dsow, volume should be recorded in skipped during reconstruction",
volumePaths: []string{

View File

@ -89,7 +89,9 @@ func (gvi *globalVolumeInfo) addPodVolume(rcv *reconstructedVolume) {
func (rc *reconciler) cleanupMounts(volume podVolume) {
klog.V(2).InfoS("Reconciler sync states: could not find volume information in desired state, clean up the mount points", "podName", volume.podName, "volumeSpecName", volume.volumeSpecName)
mountedVolume := operationexecutor.MountedVolume{
PodName: volume.podName,
PodName: volume.podName,
// VolumeName should be generated by `GetUniqueVolumeNameFromSpec` or `GetUniqueVolumeNameFromSpecWithPod`.
// However, since we don't have the volume information in asw when cleanup mounts, it doesn't matter what we put here.
VolumeName: v1.UniqueVolumeName(volume.volumeSpecName),
InnerVolumeSpecName: volume.volumeSpecName,
PluginName: volume.pluginName,
@ -252,8 +254,6 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (rvolume *reconstructe
var volumeMapper volumepkg.BlockVolumeMapper
var volumeMounter volumepkg.Mounter
var deviceMounter volumepkg.DeviceMounter
// Path to the mount or block device to check
var checkPath string
if volume.volumeMode == v1.PersistentVolumeBlock {
var newMapperErr error
@ -270,8 +270,6 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (rvolume *reconstructe
pod.UID,
newMapperErr)
}
mapDir, linkName := volumeMapper.GetPodDeviceMapPath()
checkPath = filepath.Join(mapDir, linkName)
} else {
var err error
volumeMounter, err = plugin.NewMounter(
@ -287,7 +285,6 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (rvolume *reconstructe
pod.UID,
err)
}
checkPath = volumeMounter.GetPath()
if deviceMountablePlugin != nil {
deviceMounter, err = deviceMountablePlugin.NewDeviceMounter()
if err != nil {
@ -301,16 +298,6 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (rvolume *reconstructe
}
}
// 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{
volumeName: uniqueVolumeName,
podName: volume.podName,

View File

@ -94,6 +94,8 @@ const (
volumeNotMounted = "volumeNotMounted"
volumeMountUncertain = "volumeMountUncertain"
volumeMounted = "volumeMounted"
FailNewMounter = "fail-new-mounter"
)
// CommandScript is used to pre-configure a command that will be executed and
@ -297,6 +299,9 @@ func (plugin *FakeVolumePlugin) SupportsSELinuxContextMount(spec *volume.Spec) (
func (plugin *FakeVolumePlugin) NewMounter(spec *volume.Spec, pod *v1.Pod, opts volume.VolumeOptions) (volume.Mounter, error) {
plugin.Lock()
defer plugin.Unlock()
if spec.Name() == FailNewMounter {
return nil, fmt.Errorf("AlwaysFailNewMounter")
}
fakeVolume := plugin.getFakeVolume(&plugin.Mounters)
fakeVolume.Lock()
defer fakeVolume.Unlock()

View File

@ -28,7 +28,6 @@ import (
"github.com/go-logr/logr"
"k8s.io/klog/v2"
"k8s.io/mount-utils"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
@ -38,7 +37,6 @@ import (
"k8s.io/kubernetes/pkg/volume/util/hostutil"
"k8s.io/kubernetes/pkg/volume/util/nestedpendingoperations"
volumetypes "k8s.io/kubernetes/pkg/volume/util/types"
"k8s.io/kubernetes/pkg/volume/util/volumepathhandler"
)
// OperationExecutor defines a set of operations for attaching, detaching,
@ -151,8 +149,6 @@ type OperationExecutor interface {
ExpandInUseVolume(volumeToMount VolumeToMount, actualStateOfWorld ActualStateOfWorldMounterUpdater, currentSize resource.Quantity) error
// ReconstructVolumeOperation construct a new volumeSpec and returns it created by plugin
ReconstructVolumeOperation(volumeMode v1.PersistentVolumeMode, plugin volume.VolumePlugin, mapperPlugin volume.BlockVolumePlugin, uid types.UID, podName volumetypes.UniquePodName, volumeSpecName string, volumePath string, pluginName string) (volume.ReconstructedVolume, error)
// CheckVolumeExistenceOperation checks volume existence
CheckVolumeExistenceOperation(volumeSpec *volume.Spec, mountPath, volumeName string, mounter mount.Interface, uniqueVolumeName v1.UniqueVolumeName, podName volumetypes.UniquePodName, podUID types.UID, attachable volume.AttachableVolumePlugin) (bool, error)
}
// NewOperationExecutor returns a new instance of OperationExecutor.
@ -1089,61 +1085,3 @@ func (oe *operationExecutor) ReconstructVolumeOperation(
Spec: volumeSpec,
}, nil
}
// CheckVolumeExistenceOperation checks mount path directory if volume still exists
func (oe *operationExecutor) CheckVolumeExistenceOperation(
volumeSpec *volume.Spec,
mountPath, volumeName string,
mounter mount.Interface,
uniqueVolumeName v1.UniqueVolumeName,
podName volumetypes.UniquePodName,
podUID types.UID,
attachable volume.AttachableVolumePlugin) (bool, error) {
fsVolume, err := util.CheckVolumeModeFilesystem(volumeSpec)
if err != nil {
return false, err
}
// Filesystem Volume case
// For attachable volume case, check mount path directory if volume is still existing and mounted.
// Return true if volume is mounted.
if fsVolume {
if attachable != nil {
var isNotMount bool
var mountCheckErr error
if mounter == nil {
return false, fmt.Errorf("mounter was not set for a filesystem volume")
}
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,
podName,
podUID,
mountCheckErr)
}
return !isNotMount, nil
}
// For non-attachable volume case, skip check and return true without mount point check
// since plugins may not have volume mount point.
return true, nil
}
// Block Volume case
// Check mount path directory if volume still exists, then return true if volume
// is there. Either plugin is attachable or non-attachable, the plugin should
// have symbolic link associated to raw block device under pod device map
// if volume exists.
blkutil := volumepathhandler.NewBlockVolumePathHandler()
var islinkExist bool
var checkErr error
if islinkExist, checkErr = blkutil.IsSymlinkExist(mountPath); checkErr != nil {
return false, fmt.Errorf("could not check whether the block volume %q (spec.Name: %q) pod %q (UID: %q) is mapped to: %v",
uniqueVolumeName,
volumeName,
podName,
podUID,
checkErr)
}
return islinkExist, nil
}