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 staging / unmounting volume operations on Windows #1526

Merged
merged 1 commit into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions pkg/driver/mount_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (m NodeMounter) GetDeviceNameFromMount(mountPath string) (string, int, erro
// internal Get-Item cmdlet didn't fail but no item/device was found at the
// path so we should return empty string and nil error just like the Linux
// implementation would.
pattern := `(Get-Item -Path \S+).Target, output: , error: <nil>`
pattern := `(Get-Item -Path \S+).Target, output: , error: <nil>|because it does not exist`
matched, matchErr := regexp.MatchString(pattern, err.Error())
if matched {
return "", 0, nil
Expand Down Expand Up @@ -128,8 +128,6 @@ func (m *NodeMounter) Unpublish(target string) error {
if !ok {
return fmt.Errorf("failed to cast mounter to csi proxy mounter")
}
// WriteVolumeCache before unmount
proxyMounter.WriteVolumeCache(target)
ConnorJC3 marked this conversation as resolved.
Show resolved Hide resolved
// Remove symlink
err := proxyMounter.Rmdir(target)
if err != nil {
Expand All @@ -150,11 +148,6 @@ func (m *NodeMounter) Unstage(target string) error {
if err != nil {
return err
}
// Cleanup stage path
err = proxyMounter.Rmdir(target)
if err != nil {
return err
}
ConnorJC3 marked this conversation as resolved.
Show resolved Hide resolved
return nil
}

Expand Down
8 changes: 5 additions & 3 deletions pkg/driver/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,13 +220,14 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
// This operation (NodeStageVolume) MUST be idempotent.
// If the volume corresponding to the volume_id is already staged to the staging_target_path,
// and is identical to the specified volume_capability the Plugin MUST reply 0 OK.
klog.V(4).InfoS("NodeStageVolume: checking if volume is already staged", "device", device, "source", source, "target", target)
if device == source {
klog.V(4).InfoS("NodeStageVolume: volume already staged", "volumeID", volumeID)
return &csi.NodeStageVolumeResponse{}, nil
}

// FormatAndMount will format only if needed
klog.V(4).InfoS("NodeStageVolume: formatting and mounting with fstype", "source", source, "target", target, "fstype", fsType)
klog.V(4).InfoS("NodeStageVolume: formatting and mounting with fstype", "source", source, "volumeID", volumeID, "target", target, "fstype", fsType)
formatOptions := []string{}
if len(blockSize) > 0 {
if fsType == FSTypeXfs {
Expand Down Expand Up @@ -255,6 +256,7 @@ func (d *nodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
return nil, status.Errorf(codes.Internal, "Could not resize volume %q (%q): %v", volumeID, source, err)
}
}
klog.V(4).InfoS("NodeStageVolume: successfully formatted and mounted volume", "source", source, "volumeID", volumeID, "target", target, "fstype", fsType)
return &csi.NodeStageVolumeResponse{}, nil
}

Expand Down Expand Up @@ -283,7 +285,7 @@ func (d *nodeService) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag
// returns the device name, reference count, and error code
dev, refCount, err := d.mounter.GetDeviceNameFromMount(target)
if err != nil {
msg := fmt.Sprintf("failed to check if volume is mounted: %v", err)
msg := fmt.Sprintf("failed to check if target %q is a mount point: %v", target, err)
return nil, status.Error(codes.Internal, msg)
ConnorJC3 marked this conversation as resolved.
Show resolved Hide resolved
}

Expand All @@ -304,7 +306,7 @@ func (d *nodeService) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not unmount target %q: %v", target, err)
}

klog.V(4).InfoS("NodeUnStageVolume: successfully unstaged volume", "volumeID", volumeID, "target", target)
return &csi.NodeUnstageVolumeResponse{}, nil
}

Expand Down
34 changes: 25 additions & 9 deletions pkg/mounter/safe_mounter_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,37 +97,45 @@ func (mounter *CSIProxyMounter) Unmount(target string) error {
TargetPath: normalizeWindowsPath(target),
}
volumeIdResponse, err := mounter.VolumeClient.GetVolumeIDFromTargetPath(context.Background(), getVolumeIdRequest)
volumeId := volumeIdResponse.GetVolumeId()
if err != nil {
return err
}

// Call UnmountVolume CSI proxy function which flushes data cache to disk and removes the global staging path
unmountVolumeRequest := &volume.UnmountVolumeRequest{
VolumeId: volumeId,
VolumeId: volumeIdResponse.VolumeId,
TargetPath: normalizeWindowsPath(target),
}
_, err = mounter.VolumeClient.UnmountVolume(context.Background(), unmountVolumeRequest)
if err != nil {
return err
}

// Cleanup stage path
err = mounter.Rmdir(target)
if err != nil {
return err
}

ConnorJC3 marked this conversation as resolved.
Show resolved Hide resolved
// Get disk number
getDiskNumberRequest := &volume.GetDiskNumberFromVolumeIDRequest{
VolumeId: volumeId,
VolumeId: volumeIdResponse.VolumeId,
}
getDiskNumberResponse, err := mounter.VolumeClient.GetDiskNumberFromVolumeID(context.Background(), getDiskNumberRequest)
diskNumber := getDiskNumberResponse.GetDiskNumber()
if err != nil {
return err
}

// Offline the disk
setDiskStateRequest := &disk.SetDiskStateRequest{
DiskNumber: diskNumber,
DiskNumber: getDiskNumberResponse.DiskNumber,
IsOnline: false,
}
_, err = mounter.DiskClient.SetDiskState(context.Background(), setDiskStateRequest)
if err != nil {
return err
}
klog.V(4).InfoS("Successfully unmounted volume", "diskNumber", getDiskNumberResponse.DiskNumber, "volumeId", volumeIdResponse.VolumeId, "target", target)
return nil
}

Expand Down Expand Up @@ -208,15 +216,23 @@ func (mounter *CSIProxyMounter) DeviceOpened(pathname string) (bool, error) {
return false, fmt.Errorf("DeviceOpened not implemented for CSIProxyMounter")
}

// GetDeviceNameFromMount returns the volume ID for a mount path.
func (mounter *CSIProxyMounter) GetDeviceNameFromMount(mountPath, pluginMountDir string) (string, error) {
// GetDeviceNameFromMount returns the disk number for a mount path.
func (mounter *CSIProxyMounter) GetDeviceNameFromMount(mountPath, _ string) (string, error) {
req := &volume.GetVolumeIDFromTargetPathRequest{TargetPath: normalizeWindowsPath(mountPath)}
resp, err := mounter.VolumeClient.GetVolumeIDFromTargetPath(context.Background(), req)
if err != nil {
return "", err
}

return resp.VolumeId, nil
// Get disk number
getDiskNumberRequest := &volume.GetDiskNumberFromVolumeIDRequest{
VolumeId: resp.VolumeId,
}
getDiskNumberResponse, err := mounter.VolumeClient.GetDiskNumberFromVolumeID(context.Background(), getDiskNumberRequest)
if err != nil {
return "", err
}
klog.V(4).InfoS("GetDeviceNameFromMount called", "diskNumber", getDiskNumberResponse.DiskNumber, "volumeID", resp.VolumeId, "mountPath", mountPath)
return fmt.Sprint(getDiskNumberResponse.DiskNumber), nil
}

func (mounter *CSIProxyMounter) MakeRShared(path string) error {
Expand Down