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

Now GetDevicePath from metadata doesn't return an error if something
bad happens. This makes it difficult to debug Cinder CSI driver.

This commit modifies the function to return an error value, and also
adds additional loging that should help with debugging.

pkg/csi/cinder/nodeserver.go

@@ -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()
@@ -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)
@@ -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 {
@@ -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

pkg/util/metadata/metadata.go

@@ -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 {
@@ -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.