fix(node-disk-manager): support nvme virtual paths

Enhance getParentBlockDevice() by looking not only
for /nvme/ in the path, but also /nvme-subystem/,
which is the name under /sys/devices/virtual on
OpenSUSE 15.3.

Also clean up the osdiskexcludefilter.go Start()
routine so that for each mount point it will first
check /proc/1/mounts and if that fails, then check
/proc/self/mounts, and then if that also fails, finally
log an error message.  The observation is that things
will be found in one or the other, but not both, so
the old code would print out an error message in once
case, even when the mountpoint had been added to the
filter list by the other case.

Signed-off-by: David Borman <[email protected]>

cmd/ndm_daemonset/filter/osdiskexcludefilter.go

@@ -80,33 +80,31 @@ func newNonOsDiskFilter(ctrl *controller.Controller) *oSDiskExcludeFilter {
 
 // Start set os disk devPath in nonOsDiskFilter pointer
 func (odf *oSDiskExcludeFilter) Start() {
-	/*
-		process1 check for mentioned mountpoint in host's /proc/1/mounts file
-		host's /proc/1/mounts file mounted inside container it checks for
-		every mentioned mountpoints if it is able to get parent disk's devpath
-		it adds it in Controller struct and make isOsDiskFilterSet true
-	*/
 	for _, mountPoint := range mountPoints {
+		var err error
+		var devPath string
+
+		// process1 check for mentioned mountpoint in host's /proc/1/mounts file
+		// host's /proc/1/mounts file mounted inside container it checks for
+		// every mentioned mountpoints if it is able to get parent disk's devpath
+		// it adds it in Controller struct and make isOsDiskFilterSet true
 		mountPointUtil := mount.NewMountUtil(hostMountFilePath, "", mountPoint)
-		if devPath, err := mountPointUtil.GetDiskPath(); err != nil {
-			klog.Errorf("unable to configure os disk filter for mountpoint: %s, error: %v", mountPoint, err)
-		} else {
+		if devPath, err = mountPointUtil.GetDiskPath(); err == nil {
 			odf.excludeDevPaths = append(odf.excludeDevPaths, devPath)
+			continue
 		}
-	}
-	/*
-		process2 check for mountpoints in /proc/self/mounts file if it is able to get
-		disk's path of it adds it in Controller struct and make isOsDiskFilterSet true
-	*/
-	for _, mountPoint := range mountPoints {
-		mountPointUtil := mount.NewMountUtil(defaultMountFilePath, "", mountPoint)
-		if devPath, err := mountPointUtil.GetDiskPath(); err != nil {
-			klog.Errorf("unable to configure os disk filter for mountpoint: %s, error: %v", mountPoint, err)
-		} else {
+
+		// process2 check for mountpoints in /proc/self/mounts file if it is able to get
+		// disk's path of it adds it in Controller struct and make isOsDiskFilterSet true
+		mountPointUtil = mount.NewMountUtil(defaultMountFilePath, "", mountPoint)
+		if devPath, err = mountPointUtil.GetDiskPath(); err == nil {
 			odf.excludeDevPaths = append(odf.excludeDevPaths, devPath)
+			continue
 		}
-	}
 
+		// Neither one succeeded, log the error
+		klog.Errorf("unable to configure os disk filter for mountpoint: %s, error: %v", mountPoint, err)
+	}
 }
 
 // Include contains nothing by default it returns false

pkg/mount/mountutil.go

@@ -240,13 +240,18 @@ func parseRootDeviceLink(file io.Reader) (string, error) {
 // syspath looks like - /sys/devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda4
 // parent disk is present after block then partition of that disk
 //
-// for blockdevices that belong to the nvme subsystem, the symlink has a different format,
-// /sys/devices/pci0000:00/0000:00:0e.0/nvme/nvme0/nvme0n1/nvme0n1p1. So we search for the nvme subsystem
-// instead of `block`. The blockdevice will be available after the NVMe instance, nvme/instance/namespace.
+// for blockdevices that belong to the nvme subsystem, the symlink has different formats
+//     /sys/devices/pci0000:00/0000:00:0e.0/nvme/nvme0/nvme0n1/nvme0n1p1.
+//     /sys/devices/virtual/nvme-subsystem/nvme-subsys0/nvme0n1/nvme0n1p1
+// So we search for the "nvme" or "nvme-subsystem" subsystem instead of `block`. The
+// blockdevice will be available after the NVMe instance,
+//     nvme/instance/namespace
+//     nvme-subsystem/instance/namespace
 // The namespace will be the blockdevice.
 func getParentBlockDevice(sysPath string) (string, bool) {
 	blockSubsystem := "block"
 	nvmeSubsystem := "nvme"
+	altNvmeSubsystem := "nvme-subsystem"
 	parts := strings.Split(sysPath, "/")
 
 	// checking for block subsystem, return the next part after subsystem only
@@ -262,7 +267,7 @@ func getParentBlockDevice(sysPath string) (string, bool) {
 	// nvme namespace. Length checking is to avoid index out of range in case of malformed
 	// links (extremely rare case)
 	for i, part := range parts {
-		if part == nvmeSubsystem &&
+		if (part == nvmeSubsystem || part == altNvmeSubsystem) &&
 			len(parts)-1 >= i+2 {
 			return parts[i+2], true
 		}