Skip to content

Commit

Permalink
fix: unmount volume issue on Windows node
Browse files Browse the repository at this point in the history
  • Loading branch information
andyzhangx committed Dec 5, 2024
1 parent dec89a7 commit 87311ca
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 11 deletions.
7 changes: 5 additions & 2 deletions pkg/azuredisk/azure_common_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,12 @@ func preparePublishPath(path string, m *mount.SafeFormatAndMount) error {
return fmt.Errorf("could not cast to csi proxy class")
}

func CleanupMountPoint(path string, m *mount.SafeFormatAndMount, extensiveCheck bool) error {
func CleanupMountPoint(path string, m *mount.SafeFormatAndMount, unmountVolume bool) error {
if proxy, ok := m.Interface.(mounter.CSIProxyMounter); ok {
return proxy.Unmount(path)
if unmountVolume {
return proxy.Unmount(path)
}
return proxy.Rmdir(path)
}
return fmt.Errorf("could not cast to csi proxy class")
}
Expand Down
11 changes: 7 additions & 4 deletions pkg/azuredisk/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,7 @@ func (d *Driver) NodeUnstageVolume(_ context.Context, req *csi.NodeUnstageVolume
defer d.volumeLocks.Release(volumeID)

klog.V(2).Infof("NodeUnstageVolume: unmounting %s", stagingTargetPath)
err := CleanupMountPoint(stagingTargetPath, d.mounter, true /*extensiveMountPointCheck*/)
if err != nil {
if err := CleanupMountPoint(stagingTargetPath, d.mounter, true /*extensiveMountPointCheck*/); err != nil {
return nil, status.Errorf(codes.Internal, "failed to unmount staging target %q: %v", stagingTargetPath, err)
}
klog.V(2).Infof("NodeUnstageVolume: unmount %s successfully", stagingTargetPath)
Expand Down Expand Up @@ -299,8 +298,12 @@ func (d *Driver) NodeUnpublishVolume(_ context.Context, req *csi.NodeUnpublishVo
}

klog.V(2).Infof("NodeUnpublishVolume: unmounting volume %s on %s", volumeID, targetPath)
err := CleanupMountPoint(targetPath, d.mounter, true /*extensiveMountPointCheck*/)
if err != nil {
extensiveMountPointCheck := true
if runtime.GOOS == "windows" {
// on Windows, this parameter indicates whether to unmount volume, not necessary in NodeUnpublishVolume
extensiveMountPointCheck = false
}
if err := CleanupMountPoint(targetPath, d.mounter, extensiveMountPointCheck); err != nil {
return nil, status.Errorf(codes.Internal, "failed to unmount target %q: %v", targetPath, err)
}

Expand Down
7 changes: 4 additions & 3 deletions pkg/azuredisk/nodeserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -717,9 +717,10 @@ func TestNodeUnstageVolume(t *testing.T) {
},
},
{
desc: "[Success] Valid request",
req: csi.NodeUnstageVolumeRequest{StagingTargetPath: targetFile, VolumeId: "vol_1"},
expectedErr: testutil.TestError{},
desc: "[Success] Valid request",
req: csi.NodeUnstageVolumeRequest{StagingTargetPath: targetFile, VolumeId: "vol_1"},
skipOnWindows: true, // error on Windows
expectedErr: testutil.TestError{},
},
}

Expand Down
9 changes: 8 additions & 1 deletion pkg/mounter/safe_mounter_host_process_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,14 @@ func (mounter *winMounter) Rmdir(path string) error {

// Unmount - Removes the directory - equivalent to unmount on Linux.
func (mounter *winMounter) Unmount(target string) error {
klog.V(4).Infof("Unmount: %s", target)
volumeID, err := mounter.GetDeviceNameFromMount(target, "")
if err != nil {
return err
}
klog.V(2).Infof("Unmounting volume %s from %s", volumeID, target)
if err = volume.UnmountVolume(volumeID, normalizeWindowsPath(target)); err != nil {
return err
}
return mounter.Rmdir(target)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/os/volume/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func UnmountVolume(volumeID, path string) error {
cmd := "Get-Volume -UniqueId \"$Env:volumeID\" | Get-Partition | Remove-PartitionAccessPath -AccessPath $Env:path"
out, err := azureutils.RunPowershellCmd(cmd, fmt.Sprintf("volumeID=%s", volumeID), fmt.Sprintf("path=%s", path))
if err != nil {
return fmt.Errorf("error getting driver letter to mount volume. cmd: %s, output: %s,error: %v", cmd, string(out), err)
return fmt.Errorf("failed to mount volume. cmd: %s, output: %s,error: %v", cmd, string(out), err)
}
return nil
}
Expand Down

0 comments on commit 87311ca

Please sign in to comment.