Merge pull request #1633 from pwschuurman/fix-nvme-pattern

Fix nvme path filtering logic for udevadm trigger

pkg/deviceutils/device-utils.go

@@ -36,9 +36,10 @@ const (
 	diskScsiGooglePrefix = "scsi-0Google_PersistentDisk_"
 	diskPartitionSuffix  = "-part"
 	diskSDPath           = "/dev/sd"
-	diskSDPattern        = "/dev/sd*"
+	diskSDGlob           = "/dev/sd*"
 	diskNvmePath         = "/dev/nvme"
 	diskNvmePattern      = "^/dev/nvme[0-9]+n[0-9]+$"
+	diskNvmeGlob         = "/dev/nvme*"
 	// How many times to retry for a consistent read of /proc/mounts.
 	maxListTries = 3
 	// Number of fields per line in /proc/mounts as per the fstab man page.
@@ -69,6 +70,8 @@ var (
 	scsiRegex = regexp.MustCompile(scsiPattern)
 	// regex to parse google_nvme_id output and extract the serial
 	nvmeRegex = regexp.MustCompile(nvmePattern)
+	// regex to filter for disk drive paths from filepath.Glob output of diskNvmeGlob
+	diskNvmeRegex = regexp.MustCompile(diskNvmePattern)
 )
 
 // DeviceUtils are a collection of methods that act on the devices attached
@@ -306,15 +309,28 @@ func getDevFsSerial(devFsPath string) (string, error) {
 	}
 }
 
+func filterAvailableNvmeDevFsPaths(devNvmePaths []string) []string {
+	// Devices under /dev/nvme need to be filtered for disk drive paths only.
+	diskNvmePaths := []string{}
+	for _, devNvmePath := range devNvmePaths {
+		if diskNvmeRegex.MatchString(devNvmePath) {
+			diskNvmePaths = append(diskNvmePaths, devNvmePath)
+		}
+	}
+	return diskNvmePaths
+}
+
 func findAvailableDevFsPaths() ([]string, error) {
-	diskSDPaths, err := filepath.Glob(diskSDPattern)
+	diskSDPaths, err := filepath.Glob(diskSDGlob)
 	if err != nil {
-		return nil, fmt.Errorf("failed to filepath.Glob(\"%s\"): %w", diskSDPattern, err)
+		return nil, fmt.Errorf("failed to filepath.Glob(\"%s\"): %w", diskSDGlob, err)
 	}
-	diskNvmePaths, err := filepath.Glob(diskNvmePattern)
+	devNvmePaths, err := filepath.Glob(diskNvmeGlob)
 	if err != nil {
-		return nil, fmt.Errorf("failed to filepath.Glob(\"%s\"): %w", diskNvmePattern, err)
+		return nil, fmt.Errorf("failed to filepath.Glob(\"%s\"): %w", diskNvmeGlob, err)
 	}
+	// Devices under /dev/nvme need to be filtered for disk drive paths only.
+	diskNvmePaths := filterAvailableNvmeDevFsPaths(devNvmePaths)
 	return append(diskSDPaths, diskNvmePaths...), nil
 }
 

pkg/deviceutils/device-utils_test.go

@@ -2,7 +2,7 @@ package deviceutils
 
 import (
 	"fmt"
-	"regexp"
+	"path/filepath"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -64,8 +64,6 @@ func less(a, b fmt.Stringer) bool {
 // and may prevent our tests from running on all platforms. See the following test for an example:
 // https://github.com/golang/go/blob/d33548d178016122726342911f8e15016a691472/src/syscall/exec_linux_test.go#L250
 func TestDiskNvmePattern(t *testing.T) {
-	nvmeDiskRegex := regexp.MustCompile(diskNvmePattern)
-
 	testCases := []struct {
 		paths     []string
 		wantPaths []string
@@ -97,12 +95,19 @@ func TestDiskNvmePattern(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		gotPaths := []string{}
+		devPaths := []string{}
+
 		for _, path := range tc.paths {
-			if nvmeDiskRegex.MatchString(path) {
-				gotPaths = append(gotPaths, path)
+			ok, err := filepath.Match(diskNvmeGlob, path)
+			if err != nil {
+				t.Errorf("Error encountered matching path: %v", path)
+			}
+
+			if ok {
+				devPaths = append(devPaths, path)
 			}
 		}
+		gotPaths := filterAvailableNvmeDevFsPaths(devPaths)
 		if diff := cmp.Diff(gotPaths, tc.wantPaths, cmpopts.SortSlices(less)); diff != "" {
 			t.Errorf("Unexpected NVMe device paths (-got, +want):\n%s", diff)
 		}