diff --git a/build/test.sh b/build/test.sh index 6c5f3f323..f3963e80c 100755 --- a/build/test.sh +++ b/build/test.sh @@ -30,12 +30,17 @@ if [ "$ARCH" != "amd64" ]; then exit 0 fi +exit_val=0 echo "" > coverage.txt PACKAGES=$(go list ./... | grep -v '/vendor/\|/pkg/apis/\|/pkg/client/\|integration_test') for d in $PACKAGES; do - go test -coverprofile=profile.out -covermode=atomic "$d" + if ! go test -coverprofile=profile.out -covermode=atomic "$d"; then + exit_val=1 + fi if [ -f profile.out ]; then cat profile.out >> coverage.txt rm profile.out fi done + +exit ${exit_val} diff --git a/cmd/ndm_daemonset/filter/osdiskexcludefilter.go b/cmd/ndm_daemonset/filter/osdiskexcludefilter.go index c31c14253..7c4ba19b3 100644 --- a/cmd/ndm_daemonset/filter/osdiskexcludefilter.go +++ b/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 + + // Check for mountpoints in both: + // the host's /proc/1/mounts file + // the /proc/self/mounts file + // If it is found in either one and we are able to get the + // disk's devpath, add it to the Controller struct. Otherwise + // log an error. + 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 { + + mountPointUtil = mount.NewMountUtil(defaultMountFilePath, "", mountPoint) + if devPath, err = mountPointUtil.GetDiskPath(); err == nil { odf.excludeDevPaths = append(odf.excludeDevPaths, devPath) + continue } - } + klog.Errorf("unable to configure os disk filter for mountpoint: %s, error: %v", mountPoint, err) + } } // Include contains nothing by default it returns false diff --git a/pkg/mount/mountutil.go b/pkg/mount/mountutil.go index e86eb158a..e7cd7ba50 100644 --- a/pkg/mount/mountutil.go +++ b/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" + nvmeSubsysClass := "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 == nvmeSubsysClass) && len(parts)-1 >= i+2 { return parts[i+2], true } diff --git a/pkg/mount/mountutil_test.go b/pkg/mount/mountutil_test.go index 80f11287e..7c0d64086 100644 --- a/pkg/mount/mountutil_test.go +++ b/pkg/mount/mountutil_test.go @@ -291,6 +291,16 @@ func TestGetParentBlockDevice(t *testing.T) { expectedParentBlockDevice: "nvme0n1", expectedOk: true, }, + "getting parent of main virtual NVMe blockdevice": { + syspath: "/sys/devices/virtual/nvme-subsystem/nvme-subsys0/nvme0n1", + expectedParentBlockDevice: "nvme0n1", + expectedOk: true, + }, + "getting parent of partitioned virtual NVMe blockdevice": { + syspath: "/sys/devices/virtual/nvme-subsystem/nvme-subsys0/nvme0n1/nvme0n1p1", + expectedParentBlockDevice: "nvme0n1", + expectedOk: true, + }, "getting parent of wrong disk": { syspath: "/sys/devices/pci0000:00/0000:00:0e.0/nvme/nvme0", expectedParentBlockDevice: "", diff --git a/pkg/sysfs/syspath.go b/pkg/sysfs/syspath.go index 73555ce61..8f12a6f9c 100644 --- a/pkg/sysfs/syspath.go +++ b/pkg/sysfs/syspath.go @@ -31,6 +31,7 @@ const ( BlockSubSystem = "block" // NVMeSubSystem is the key used to represent nvme subsystem in sysfs NVMeSubSystem = "nvme" + NVMeSubSysClass = "nvme-subsystem" // sectorSize is the sector size as understood by the unix systems. // It is kept as 512 bytes. all entries in /sys/class/block/sda/size // are in 512 byte blocks @@ -86,7 +87,7 @@ func (s Device) getParent() (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 == NVMeSubSysClass { // check if the length is greater to avoid panic. Also need to make sure that // the same device is not returned if the given device is a parent. if len(parts)-1 >= i+2 && s.deviceName != parts[i+2] { diff --git a/pkg/sysfs/syspath_test.go b/pkg/sysfs/syspath_test.go index 478d173af..7f4341dfe 100644 --- a/pkg/sysfs/syspath_test.go +++ b/pkg/sysfs/syspath_test.go @@ -68,6 +68,24 @@ func TestGetParent(t *testing.T) { wantedDeviceName: "nvme0n1", wantOk: true, }, + "[nvme-subsystem] given blockdevice is a parent": { + sysfsDevice: &Device{ + deviceName: "nvme0n1", + path: "/dev/nvme0n1", + sysPath: "/sys/devices/virtual/nvme-subsystem/nvme-subsys0/nvme0n1/", + }, + wantedDeviceName: "", + wantOk: false, + }, + "[nvme-subsystem] given blockdevice is a partition": { + sysfsDevice: &Device{ + deviceName: "nvme0n1p1", + path: "/dev/nvme0n1p1", + sysPath: "/sys/devices/virtual/nvme-subsystem/nvme-subsys0/nvme0n1/nvme0n1p1/", + }, + wantedDeviceName: "nvme0n1", + wantOk: true, + }, } for name, test := range tests { t.Run(name, func(t *testing.T) {