From a78bcfbdbc484584768e26b6dced00b4b4f858e1 Mon Sep 17 00:00:00 2001
From: Peter Schuurman <psch@google.com>
Date: Wed, 6 Mar 2024 16:07:47 -0800
Subject: [PATCH] Fix nvme path filtering logic for udevadm trigger

---
 pkg/deviceutils/device-utils.go      | 20 ++++++++++++++++++--
 pkg/deviceutils/device-utils_test.go | 17 +++++++++++------
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/pkg/deviceutils/device-utils.go b/pkg/deviceutils/device-utils.go
index bdae2e2f79..3037ae52c6 100644
--- a/pkg/deviceutils/device-utils.go
+++ b/pkg/deviceutils/device-utils.go
@@ -39,6 +39,7 @@ const (
 	diskSDPattern        = "/dev/sd*"
 	diskNvmePath         = "/dev/nvme"
 	diskNvmePattern      = "^/dev/nvme[0-9]+n[0-9]+$"
+	devNvmePattern       = "/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 only disk drive paths from filepath.Glob output of devNvmePattern
+	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)
 	if err != nil {
 		return nil, fmt.Errorf("failed to filepath.Glob(\"%s\"): %w", diskSDPattern, err)
 	}
-	diskNvmePaths, err := filepath.Glob(diskNvmePattern)
+	devNvmePaths, err := filepath.Glob(devNvmePattern)
 	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", devNvmePattern, err)
 	}
+	// Devices under /dev/nvme need to be filtered for disk drive paths only.
+	diskNvmePaths := filterAvailableNvmeDevFsPaths(devNvmePaths)
 	return append(diskSDPaths, diskNvmePaths...), nil
 }
 
diff --git a/pkg/deviceutils/device-utils_test.go b/pkg/deviceutils/device-utils_test.go
index bf686af332..60301de6d1 100644
--- a/pkg/deviceutils/device-utils_test.go
+++ b/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(devNvmePattern, 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)
 		}