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

[release-1.10] Fix nvme path filtering logic for udevadm trigger #1731

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
26 changes: 21 additions & 5 deletions pkg/deviceutils/device-utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
17 changes: 11 additions & 6 deletions pkg/deviceutils/device-utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package deviceutils

import (
"fmt"
"regexp"
"path/filepath"
"testing"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down