diff --git a/changelogs/unreleased/516-akhilerm b/changelogs/unreleased/516-akhilerm new file mode 100644 index 000000000..6b1c5ccd6 --- /dev/null +++ b/changelogs/unreleased/516-akhilerm @@ -0,0 +1 @@ +fix a bug in parsing /proc/cmdline where the root partition identifier is not present \ No newline at end of file diff --git a/changelogs/unreleased/518-akhilerm b/changelogs/unreleased/518-akhilerm new file mode 100644 index 000000000..eab555814 --- /dev/null +++ b/changelogs/unreleased/518-akhilerm @@ -0,0 +1 @@ +add check for device mapper path in path filter \ No newline at end of file diff --git a/cmd/ndm_daemonset/filter/pathfilter.go b/cmd/ndm_daemonset/filter/pathfilter.go index 9b058a33b..27677ee03 100644 --- a/cmd/ndm_daemonset/filter/pathfilter.go +++ b/cmd/ndm_daemonset/filter/pathfilter.go @@ -94,6 +94,12 @@ func (pf *pathFilter) Include(blockDevice *blockdevice.BlockDevice) bool { if len(pf.includePaths) == 0 { return true } + if util.Contains(blockdevice.DeviceMapperDeviceTypes, + blockDevice.DeviceAttributes.DeviceType) { + if util.MatchIgnoredCase(pf.includePaths, blockDevice.DMInfo.DevMapperPath) { + return true + } + } return util.MatchIgnoredCase(pf.includePaths, blockDevice.DevPath) } @@ -103,5 +109,12 @@ func (pf *pathFilter) Exclude(blockDevice *blockdevice.BlockDevice) bool { if len(pf.excludePaths) == 0 { return true } + // for DM devices, need to check both the devpath and device mapper path + if util.Contains(blockdevice.DeviceMapperDeviceTypes, + blockDevice.DeviceAttributes.DeviceType) { + if util.MatchIgnoredCase(pf.excludePaths, blockDevice.DMInfo.DevMapperPath) { + return false + } + } return !util.MatchIgnoredCase(pf.excludePaths, blockDevice.DevPath) } diff --git a/cmd/ndm_daemonset/filter/pathfilter_test.go b/cmd/ndm_daemonset/filter/pathfilter_test.go index 6a439a8e7..7f2664970 100644 --- a/cmd/ndm_daemonset/filter/pathfilter_test.go +++ b/cmd/ndm_daemonset/filter/pathfilter_test.go @@ -91,53 +91,211 @@ func TestPathStart(t *testing.T) { } func TestPathFilterExclude(t *testing.T) { - fakePathFilter1 := pathFilter{} - fakePathFilter2 := pathFilter{} - fakePathFilter3 := pathFilter{} tests := map[string]struct { - filter pathFilter excludePath string - path string + bd blockdevice.BlockDevice expected bool }{ - "excludePath is empty": {filter: fakePathFilter1, excludePath: "", path: "/dev/loop0", expected: true}, - "excludePath and path is same": {filter: fakePathFilter2, excludePath: "loop", path: "/dev/loop0", expected: false}, - "excludePath and path is not same": {filter: fakePathFilter3, excludePath: "loop", path: "/dev/sdb", expected: true}, + "excludePath is empty": { + excludePath: "", + bd: blockdevice.BlockDevice{ + Identifier: blockdevice.Identifier{ + DevPath: "/dev/loop0", + }, + }, + expected: true, + }, + "excludePath and path is same": { + excludePath: "loop", + bd: blockdevice.BlockDevice{ + Identifier: blockdevice.Identifier{ + DevPath: "/dev/loop0", + }, + }, + expected: false, + }, + "excludePath and path is not same": { + excludePath: "loop", + bd: blockdevice.BlockDevice{ + Identifier: blockdevice.Identifier{ + DevPath: "/dev/sdb", + }, + }, + expected: true, + }, + "dm device with only mapper path in exclude list": { + excludePath: "/dev/mapper/vg0-lv0", + bd: blockdevice.BlockDevice{ + Identifier: blockdevice.Identifier{ + DevPath: "/dev/dm-0", + }, + DeviceAttributes: blockdevice.DeviceAttribute{ + DeviceType: blockdevice.BlockDeviceTypeLVM, + }, + DMInfo: blockdevice.DeviceMapperInformation{ + DevMapperPath: "/dev/mapper/vg0-lv0", + }, + }, + expected: false, + }, + "dm device with mapper path not in exclude list, but /dev/dm-x path in list": { + excludePath: "/dev/dm-0", + bd: blockdevice.BlockDevice{ + Identifier: blockdevice.Identifier{ + DevPath: "/dev/dm-0", + }, + DeviceAttributes: blockdevice.DeviceAttribute{ + DeviceType: blockdevice.BlockDeviceTypeLVM, + }, + DMInfo: blockdevice.DeviceMapperInformation{ + DevMapperPath: "/dev/mapper/vg0-lv0", + }, + }, + expected: false, + }, + "dm device with both mapper and /dev/dm-x path in exclude list": { + excludePath: "/dev/mapper/vg0-lv0,/dev/dm-0", + bd: blockdevice.BlockDevice{ + Identifier: blockdevice.Identifier{ + DevPath: "/dev/dm-0", + }, + DeviceAttributes: blockdevice.DeviceAttribute{ + DeviceType: blockdevice.BlockDeviceTypeLVM, + }, + DMInfo: blockdevice.DeviceMapperInformation{ + DevMapperPath: "/dev/mapper/vg0-lv0", + }, + }, + expected: false, + }, + "dm device with no dm paths in exclude list": { + excludePath: "/dev/sdb", + bd: blockdevice.BlockDevice{ + Identifier: blockdevice.Identifier{ + DevPath: "/dev/dm-0", + }, + DeviceAttributes: blockdevice.DeviceAttribute{ + DeviceType: blockdevice.BlockDeviceTypeLVM, + }, + DMInfo: blockdevice.DeviceMapperInformation{ + DevMapperPath: "/dev/mapper/vg0-lv0", + }, + }, + expected: true, + }, } for name, test := range tests { t.Run(name, func(t *testing.T) { - bd := &blockdevice.BlockDevice{} - bd.DevPath = test.path + filter := pathFilter{} if test.excludePath != "" { - test.filter.excludePaths = strings.Split(test.excludePath, ",") + filter.excludePaths = strings.Split(test.excludePath, ",") } - assert.Equal(t, test.expected, test.filter.Exclude(bd)) + assert.Equal(t, test.expected, filter.Exclude(&test.bd)) }) } } func TestPathFilterInclude(t *testing.T) { - fakePathFilter1 := pathFilter{} - fakePathFilter2 := pathFilter{} - fakePathFilter3 := pathFilter{} tests := map[string]struct { - filter pathFilter includePath string - path string + bd blockdevice.BlockDevice expected bool }{ - "includePath is empty": {filter: fakePathFilter1, includePath: "", path: "/dev/loop0", expected: true}, - "includePath and path is same": {filter: fakePathFilter2, includePath: "loop", path: "/dev/loop0", expected: true}, - "includePath and path is not same": {filter: fakePathFilter3, includePath: "loop", path: "/dev/sdb", expected: false}, + "includePath is empty": { + includePath: "", + bd: blockdevice.BlockDevice{ + Identifier: blockdevice.Identifier{ + DevPath: "/dev/loop0", + }, + }, + expected: true, + }, + "includePath and path is same": { + includePath: "loop", + bd: blockdevice.BlockDevice{ + Identifier: blockdevice.Identifier{ + DevPath: "/dev/loop0", + }, + }, + expected: true, + }, + "includePath and path is not same": { + includePath: "loop", + bd: blockdevice.BlockDevice{ + Identifier: blockdevice.Identifier{ + DevPath: "/dev/sdb", + }, + }, + expected: false, + }, + "dm device with only mapper path in exclude list": { + includePath: "/dev/mapper/vg0-lv0", + bd: blockdevice.BlockDevice{ + Identifier: blockdevice.Identifier{ + DevPath: "/dev/dm-0", + }, + DeviceAttributes: blockdevice.DeviceAttribute{ + DeviceType: blockdevice.BlockDeviceTypeLVM, + }, + DMInfo: blockdevice.DeviceMapperInformation{ + DevMapperPath: "/dev/mapper/vg0-lv0", + }, + }, + expected: true, + }, + "dm device with mapper path not in exclude list, but /dev/dm-x path in list": { + includePath: "/dev/dm-0", + bd: blockdevice.BlockDevice{ + Identifier: blockdevice.Identifier{ + DevPath: "/dev/dm-0", + }, + DeviceAttributes: blockdevice.DeviceAttribute{ + DeviceType: blockdevice.BlockDeviceTypeLVM, + }, + DMInfo: blockdevice.DeviceMapperInformation{ + DevMapperPath: "/dev/mapper/vg0-lv0", + }, + }, + expected: true, + }, + "dm device with both mapper and /dev/dm-x path in exclude list": { + includePath: "/dev/mapper/vg0-lv0,/dev/dm-0", + bd: blockdevice.BlockDevice{ + Identifier: blockdevice.Identifier{ + DevPath: "/dev/dm-0", + }, + DeviceAttributes: blockdevice.DeviceAttribute{ + DeviceType: blockdevice.BlockDeviceTypeLVM, + }, + DMInfo: blockdevice.DeviceMapperInformation{ + DevMapperPath: "/dev/mapper/vg0-lv0", + }, + }, + expected: true, + }, + "dm device with no dm paths in exclude list": { + includePath: "/dev/sdb", + bd: blockdevice.BlockDevice{ + Identifier: blockdevice.Identifier{ + DevPath: "/dev/dm-0", + }, + DeviceAttributes: blockdevice.DeviceAttribute{ + DeviceType: blockdevice.BlockDeviceTypeLVM, + }, + DMInfo: blockdevice.DeviceMapperInformation{ + DevMapperPath: "/dev/mapper/vg0-lv0", + }, + }, + expected: false, + }, } for name, test := range tests { t.Run(name, func(t *testing.T) { - bd := &blockdevice.BlockDevice{} - bd.DevPath = test.path + filter := pathFilter{} if test.includePath != "" { - test.filter.includePaths = strings.Split(test.includePath, ",") + filter.includePaths = strings.Split(test.includePath, ",") } - assert.Equal(t, test.expected, test.filter.Include(bd)) + assert.Equal(t, test.expected, filter.Include(&test.bd)) }) } } diff --git a/cmd/ndm_daemonset/probe/udevprobe.go b/cmd/ndm_daemonset/probe/udevprobe.go index d532d203a..dddcd88ee 100644 --- a/cmd/ndm_daemonset/probe/udevprobe.go +++ b/cmd/ndm_daemonset/probe/udevprobe.go @@ -270,6 +270,8 @@ func (up *udevProbe) FillBlockDeviceDetails(blockDevice *blockdevice.BlockDevice blockDevice.DeviceAttributes.Vendor = udevDiskDetails.Vendor blockDevice.DeviceAttributes.IDType = udevDiskDetails.IDType + blockDevice.DMInfo.DevMapperPath = udevDiskDetails.DMPath + // log only if details are present to prevent log flooding if blockDevice.DeviceAttributes.Model != "" { klog.V(4).Infof("device: %s, Model: %s filled by udev probe", diff --git a/pkg/udev/common.go b/pkg/udev/common.go index ced2a5bdf..7df89f8a5 100644 --- a/pkg/udev/common.go +++ b/pkg/udev/common.go @@ -68,6 +68,8 @@ const ( UDEV_PARTITION_UUID = "ID_PART_ENTRY_UUID" // udev attribute to get partition uuid UDEV_PARTITION_TYPE = "ID_PART_ENTRY_TYPE" // udev attribute to get partition type UDEV_DM_UUID = "DM_UUID" // udev attribute to get the device mapper uuid + // UDEV_DM_NAME is udev attribute to get the name of the dm device. This is used to generate the device mapper path + UDEV_DM_NAME = "DM_NAME" ) // UdevDiskDetails struct contain different attribute of disk. @@ -89,6 +91,8 @@ type UdevDiskDetails struct { PartitionNumber uint8 // PartitionTableType is the type of the partition table (dos/gpt) PartitionTableType string + // DMPath is the /dev/mapper path if this is a dm device + DMPath string } // freeCharPtr frees c pointer @@ -114,6 +118,11 @@ func (device *UdevDevice) DiskInfoFromLibudev() UdevDiskDetails { PartitionNumber: device.GetPartitionNumber(), PartitionTableType: device.GetPropertyValue(UDEV_PARTITION_TABLE_TYPE), } + // get the devicemapper path from the dm name + dmName := device.GetPropertyValue(UDEV_DM_NAME) + if len(dmName) != 0 { + diskDetails.DMPath = "/dev/mapper/" + dmName + } return diskDetails }