Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make mount errors uncertain #110650

Closed
wants to merge 2 commits into from
Closed

Make mount errors uncertain #110650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f6e6870
Mark all mount errors as uncertain
gnufied Jun 18, 2022
f27c245
Make every failed mount operation in uncertain mount
gnufied Jun 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 0 additions & 14 deletions pkg/kubelet/volumemanager/reconciler/reconciler_test.go
View file
Open in desktop
Expand Up @@ -1641,20 +1641,6 @@ func Test_UncertainVolumeMountState(t *testing.T) {
unmountVolumeCount: 1,
volumeName: volumetesting.TimeoutOnSetupVolumeName,
},
{
name: "failed operation should result in not-mounted volume",
volumeState: operationexecutor.VolumeNotMounted,
unmountDeviceCallCount: 1,
unmountVolumeCount: 0,
volumeName: volumetesting.FailOnSetupVolumeName,
},
{
name: "timeout followed by failed operation should result in non-mounted volume",
volumeState: operationexecutor.VolumeNotMounted,
unmountDeviceCallCount: 1,
unmountVolumeCount: 0,
volumeName: volumetesting.TimeoutAndFailOnSetupVolumeName,
},
{
name: "success followed by timeout operation should result in mounted volume",
volumeState: operationexecutor.VolumeMounted,
Expand Down
28 changes: 8 additions & 20 deletions pkg/volume/util/operationexecutor/operation_generator.go
View file
Open in desktop
Expand Up @@ -648,7 +648,7 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
volume.DeviceMounterArgs{FsGroup: fsGroup},
)
if err != nil {
og.checkForFailedMount(volumeToMount, err)
og.addFailedMountEvent(volumeToMount, err)
og.markDeviceErrorState(volumeToMount, devicePath, deviceMountPath, err, actualStateOfWorld)
// On failure, return error. Caller will log and retry.
eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.MountDevice failed", err)
Expand Down Expand Up @@ -688,8 +688,8 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
VolumeMountState: VolumeMounted,
}
if mountErr != nil {
og.checkForFailedMount(volumeToMount, mountErr)
og.markVolumeErrorState(volumeToMount, markOpts, mountErr, actualStateOfWorld)
og.addFailedMountEvent(volumeToMount, mountErr)
og.markVolumeErrorState(volumeToMount, markOpts, actualStateOfWorld)
// On failure, return error. Caller will log and retry.
eventErr, detailedErr := volumeToMount.GenerateError("MountVolume.SetUp failed", mountErr)
return volumetypes.NewOperationContext(eventErr, detailedErr, migrated)
Expand Down Expand Up @@ -747,7 +747,7 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
}
}

func (og *operationGenerator) checkForFailedMount(volumeToMount VolumeToMount, mountError error) {
func (og *operationGenerator) addFailedMountEvent(volumeToMount VolumeToMount, mountError error) {
pv := volumeToMount.VolumeSpec.PersistentVolume
if pv == nil {
return
Expand Down Expand Up @@ -782,24 +782,13 @@ func (og *operationGenerator) markDeviceErrorState(volumeToMount VolumeToMount,

}

func (og *operationGenerator) markVolumeErrorState(volumeToMount VolumeToMount, markOpts MarkVolumeOpts, mountError error, actualStateOfWorld ActualStateOfWorldMounterUpdater) {
if volumetypes.IsOperationFinishedError(mountError) &&
actualStateOfWorld.GetVolumeMountState(volumeToMount.VolumeName, markOpts.PodName) == VolumeMountUncertain {
t := actualStateOfWorld.MarkVolumeAsUnmounted(volumeToMount.PodName, volumeToMount.VolumeName)
if t != nil {
klog.Errorf(volumeToMount.GenerateErrorDetailed("MountVolume.MarkVolumeAsUnmounted failed", t).Error())
}
return
}

if volumetypes.IsUncertainProgressError(mountError) &&
actualStateOfWorld.GetVolumeMountState(volumeToMount.VolumeName, markOpts.PodName) == VolumeNotMounted {
func (og *operationGenerator) markVolumeErrorState(volumeToMount VolumeToMount, markOpts MarkVolumeOpts, actualStateOfWorld ActualStateOfWorldMounterUpdater) {
if actualStateOfWorld.GetVolumeMountState(volumeToMount.VolumeName, markOpts.PodName) == VolumeNotMounted {
t := actualStateOfWorld.MarkVolumeMountAsUncertain(markOpts)
if t != nil {
klog.Errorf(volumeToMount.GenerateErrorDetailed("MountVolume.MarkVolumeMountAsUncertain failed", t).Error())
}
}

}

func (og *operationGenerator) GenerateUnmountVolumeFunc(
Expand Down Expand Up @@ -1118,7 +1107,7 @@ func (og *operationGenerator) GenerateMapVolumeFunc(
pluginDevicePath, mapErr := customBlockVolumeMapper.MapPodDevice()
if mapErr != nil {
// On failure, return error. Caller will log and retry.
og.markVolumeErrorState(volumeToMount, markVolumeOpts, mapErr, actualStateOfWorld)
og.markVolumeErrorState(volumeToMount, markVolumeOpts, actualStateOfWorld)
eventErr, detailedErr := volumeToMount.GenerateError("MapVolume.MapPodDevice failed", mapErr)
return volumetypes.NewOperationContext(eventErr, detailedErr, migrated)
}
Expand All @@ -1127,8 +1116,7 @@ func (og *operationGenerator) GenerateMapVolumeFunc(
// so it is is unmapped when corresponding pod is deleted.
defer func() {
if operationContext.EventErr != nil {
errText := operationContext.EventErr.Error()
og.markVolumeErrorState(volumeToMount, markVolumeOpts, volumetypes.NewUncertainProgressError(errText), actualStateOfWorld)
og.markVolumeErrorState(volumeToMount, markVolumeOpts, actualStateOfWorld)
}
}()

Expand Down