-
Notifications
You must be signed in to change notification settings - Fork 787
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
[release-1.3] Fix windows NodePublish failing because mount target doesn't exist #1083
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -589,7 +589,7 @@ func (d *nodeService) nodePublishVolumeForBlock(req *csi.NodePublishVolumeReques | |
//Checking if the target file is already mounted with a device. | ||
mounted, err := d.isMounted(source, target) | ||
if err != nil { | ||
return status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) | ||
return status.Errorf(codes.Internal, "Could not check if %q is mounted: %v", target, err) | ||
} | ||
|
||
if !mounted { | ||
|
@@ -626,15 +626,20 @@ func (d *nodeService) isMounted(source string, target string) (bool, error) { | |
//After successful unmount, the device is ready to be mounted. | ||
return !notMnt, nil | ||
} | ||
return !notMnt, status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) | ||
return !notMnt, status.Errorf(codes.Internal, "Could not check if %q is a mount point: %v, %v", target, err, pathErr) | ||
} | ||
|
||
if !notMnt { | ||
klog.V(4).Infof("NodePublishVolume: Target path %q is already mounted", target) | ||
return !notMnt, nil | ||
} | ||
|
||
return !notMnt, err | ||
// Do not return os.IsNotExist error. Other errors were handled above. It is | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why isn't an error desirable here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the idea is to ignore these because the caller should check that independently of whether There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I edited PR description to say this is acherry-pick of #1081. https://github.com/kubernetes-sigs/aws-ebs-csi-driver/pull/1087/files is PR to master trying to make it more understandable . |
||
// the responsibility of the caller to check whether the given target path | ||
// exists (in Linux, the target mount directory must exist before mount is | ||
// called on it) or not (in Windows, the target must NOT exist before a | ||
// symlink is created at it) | ||
return !notMnt, nil | ||
} | ||
|
||
func (d *nodeService) nodePublishVolumeForFileSystem(req *csi.NodePublishVolumeRequest, mountOptions []string, mode *csi.VolumeCapability_Mount) error { | ||
|
@@ -665,7 +670,7 @@ func (d *nodeService) nodePublishVolumeForFileSystem(req *csi.NodePublishVolumeR | |
mounted, err := d.isMounted(source, target) | ||
|
||
if err != nil { | ||
return status.Errorf(codes.Internal, "Could not mount %q at %q: %v", source, target, err) | ||
return status.Errorf(codes.Internal, "Could not check if %q is mounted: %v", target, err) | ||
} | ||
|
||
if !mounted { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.