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

[cinder-csi-plugin] Cleanup: handle errors from GetDevicePath #1522

Merged
merged 1 commit into from
May 13, 2021
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
25 changes: 17 additions & 8 deletions pkg/csi/cinder/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ func nodePublishEphemeral(req *csi.NodePublishVolumeRequest, ns *nodeServer) (*c
m := ns.Mount

devicePath, err := getDevicePath(evol.ID, m)
if devicePath == "" {
return nil, status.Error(codes.Internal, "Unable to find Device path for volume")
if err != nil {
return nil, status.Error(codes.Internal, fmt.Sprintf("Unable to find Device path for volume: %v", err))
}

targetPath := req.GetTargetPath()
Expand Down Expand Up @@ -237,8 +237,8 @@ func nodePublishVolumeForBlock(req *csi.NodePublishVolumeRequest, ns *nodeServer

// Do not trust the path provided by cinder, get the real path on node
source, err := getDevicePath(volumeID, m)
if source == "" {
return nil, status.Error(codes.Internal, "Unable to find Device path for volume")
if err != nil {
return nil, status.Error(codes.Internal, fmt.Sprintf("Unable to find Device path for volume: %v", err))
}

exists, err := utilpath.Exists(utilpath.CheckFollowSymlink, podVolumePath)
Expand Down Expand Up @@ -377,8 +377,8 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
m := ns.Mount
// Do not trust the path provided by cinder, get the real path on node
devicePath, err := getDevicePath(volumeID, m)
if devicePath == "" {
return nil, status.Error(codes.Internal, "Unable to find Device path for volume")
if err != nil {
return nil, status.Error(codes.Internal, fmt.Sprintf("Unable to find Device path for volume: %v", err))
}

if blk := volumeCapability.GetBlock(); blk != nil {
Expand Down Expand Up @@ -555,10 +555,19 @@ func (ns *nodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandV

func getDevicePath(volumeID string, m mount.IMount) (string, error) {
var devicePath string
devicePath, _ = m.GetDevicePath(volumeID)
devicePath, err := m.GetDevicePath(volumeID)
if err != nil {
klog.Warningf("Couldn't get device path from mount: %v", err)
}

if devicePath == "" {
// try to get from metadata service
devicePath = metadata.GetDevicePath(volumeID)
klog.Info("Trying to get device path from metadata service")
devicePath, err = metadata.GetDevicePath(volumeID)
if err != nil {
klog.Errorf("Couldn't get device path from metadata service: %v", err)
return "", fmt.Errorf("couldn't get device path from metadata service: %v", err)
}
}

return devicePath, nil
Expand Down
26 changes: 14 additions & 12 deletions pkg/util/metadata/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,18 +207,17 @@ func getFromMetadataService(metadataVersion string) (*Metadata, error) {
}

// GetDevicePath retrieves device path from metadata service
func GetDevicePath(volumeID string) string {
func GetDevicePath(volumeID string) (string, error) {
// Nova Hyper-V hosts cannot override disk SCSI IDs. In order to locate
// volumes, we're querying the metadata service. Note that the Hyper-V
// driver will include device metadata for untagged volumes as well.
//
// We're avoiding using cached metadata (or the configdrive),
// relying on the metadata service.
instanceMetadata, err := getFromMetadataService(defaultMetadataVersion)

if err != nil {
klog.V(4).Infof("Could not retrieve instance metadata. Error: %v", err)
return ""
klog.Errorf("Could not retrieve instance metadata: %v", err)
return "", fmt.Errorf("could not retrieve instance metadata: %v", err)
}

for _, device := range instanceMetadata.Devices {
Expand All @@ -230,24 +229,27 @@ func GetDevicePath(volumeID string) string {
diskPattern := fmt.Sprintf("/dev/disk/by-path/*-%s-%s", device.Bus, device.Address)
diskPaths, err := filepath.Glob(diskPattern)
if err != nil {
klog.Errorf(
"could not retrieve disk path for volumeID: %q. Error filepath.Glob(%q): %v",
klog.Errorf("Could not retrieve disk path for volumeID: %q. Error filepath.Glob(%q): %v",
volumeID, diskPattern, err)
return "", fmt.Errorf("could not retrieve disk path for volumeID: %q. Error filepath.Glob(%q): %v",
volumeID, diskPattern, err)
return ""
}

if len(diskPaths) == 1 {
return diskPaths[0]
if diskPaths[0] == "" {
klog.Errorf("Disk path for volumeID %q is empty", volumeID)
return "", fmt.Errorf("disk path for volumeID %q is empty", volumeID)
}
return diskPaths[0], nil
}

klog.V(4).Infof("expecting to find one disk path for volumeID %q, found %d: %v",
klog.Warningf("Expecting to find one disk path for volumeID %q, found %d: %v",
volumeID, len(diskPaths), diskPaths)

}
}

klog.V(4).Infof("Could not retrieve device metadata for volumeID: %q", volumeID)
return ""
klog.Errorf("Could not retrieve device metadata for volumeID: %q", volumeID)
return "", fmt.Errorf("could not retrieve device metadata for volumeID: %q", volumeID)
}

// Get retrieves metadata from either config drive or metadata service.
Expand Down