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.

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

fix GenerateUnmapVolumeFunc missing globalUnmapPath #123032

Merged
merged 1 commit into from Feb 2, 2024
Merged
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion pkg/volume/util/operationexecutor/operation_generator.go
Expand Up @@ -1295,7 +1295,12 @@ func (og *operationGenerator) GenerateUnmapVolumeFunc(
// pods/{podUid}/volumeDevices/{escapeQualifiedPluginName}/{volumeName}
podDeviceUnmapPath, volName := blockVolumeUnmapper.GetPodDeviceMapPath()
// plugins/kubernetes.io/{PluginName}/volumeDevices/{volumePluginDependentPath}/{podUID}
globalUnmapPath := volumeToUnmount.DeviceMountPath
globalUnmapPath, err := blockVolumeUnmapper.GetGlobalMapPath(volumeToUnmount.VolumeSpec)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an unit tests that could check globalUnmapPath is filed correctly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only csi plugin and local volume plugin have unit test for GetGlobalMapPath

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After I read the operation_generator_test.go, I cannot find an existing unit test for the GenerateUnmapVolumeFunc. And the other funcs are also lack of unit tests.

if err != nil {
// On failure, return error. Caller will log and retry.
eventErr, detailedErr := volumeToUnmount.GenerateError("UnmapVolume.GetGlobalMapPath failed", err)
return volumetypes.NewOperationContext(eventErr, detailedErr, migrated)
}

// Mark the device as uncertain to make sure kubelet calls UnmapDevice again in all the "return err"
// cases below. The volume is marked as fully un-mapped at the end of this function, when everything
Expand Down