fix the localdisk.partitionInfo.path not display correctly #1199

hikariwo · 2023-10-26T05:04:45Z

What this PR does / why we need it:

fix #1198

Special notes for your reviewer:

Does this PR introduce a user-facing change?

…getStorageNodeMigrateOperations Signed-off-by: lancerxiu <1377004871@qq.com>

codecov-commenter · 2023-10-26T05:08:24Z

Codecov Report

Merging #1199 (97724ac) into main (997ca1b) will decrease coverage by 2.32%.
The diff coverage is 83.33%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             main    #1199      +/-   ##
==========================================
- Coverage   38.13%   35.81%   -2.32%     
==========================================
  Files          25       28       +3     
  Lines        1980     2178     +198     
==========================================
+ Hits          755      780      +25     
- Misses       1125     1295     +170     
- Partials      100      103       +3

Files	Coverage Δ
.../local-disk-manager/builder/localdisk/localdisk.go	`64.38% <100.00%> (+0.49%)`	⬆️
pkg/local-disk-manager/udev/device.go	`36.66% <80.00%> (ø)`

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

SSmallMonster

overall lgtm, just some nits

SSmallMonster · 2023-10-26T07:14:08Z

pkg/local-disk-manager/udev/device_test.go

+	"testing"
+)
+
+func TestDevice_ParseDeviceInfo_UnPartition(t *testing.T) {


these 2 test cases may be merged into one TestFunc, that may be better.

ok, I try to modify it

SSmallMonster · 2023-10-26T07:20:01Z

pkg/local-disk-manager/disk/manager/partition.go

@@ -29,4 +29,7 @@ type PartitionInfo struct {

 	// Filesystem
 	Filesystem string
+
+	// Path
+	Path string


does this path represent the partition-path like /dev/sda1,/dev/sda2?

i'm wondering if it's a mount-path, but I think the partition-path should be more meaningful.

Yes,its format turly like /dev/sda1. It comes from the field DEVNAME of udevadm info command.

SSmallMonster · 2023-10-26T07:58:48Z

pkg/local-disk-manager/udev/device_test.go

+	udevInfo_unPartition := "P: /devices/pci0000:00/0000:00:07.1/ata1/host1/target1:0:0/1:0:0:0/block/sdb\nN: sdb\nS: disk/by-id/ata-VMware_Virtual_IDE_Hard_Drive_00000000000000000001\nS: disk/by-id/wwn-0x5000c298825951d9\nS: disk/by-path/pci-0000:00:07.1-ata-1.0\nE: DEVLINKS=/dev/disk/by-id/ata-VMware_Virtual_IDE_Hard_Drive_00000000000000000001 /dev/disk/by-id/wwn-0x5000c298825951d9 /dev/disk/by-path/pci-0000:00:07.1-ata-1.0\nE: DEVNAME=/dev/sdb\nE: DEVPATH=/devices/pci0000:00/0000:00:07.1/ata1/host1/target1:0:0/1:0:0:0/block/sdb\nE: DEVTYPE=disk\nE: ID_ATA=1\nE: ID_ATA_FEATURE_SET_APM=1\nE: ID_ATA_FEATURE_SET_APM_ENABLED=0\nE: ID_ATA_FEATURE_SET_PM=1\nE: ID_ATA_FEATURE_SET_PM_ENABLED=1\nE: ID_BUS=ata\nE: ID_MODEL=VMware_Virtual_IDE_Hard_Drive\nE: ID_MODEL_ENC=VMware\\x20Virtual\\x20IDE\\x20Hard\\x20Drive\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\nE: ID_PART_TABLE_TYPE=dos\nE: ID_PATH=pci-0000:00:07.1-ata-1.0\nE: ID_PATH_TAG=pci-0000_00_07_1-ata-1_0\nE: ID_REVISION=00000001\nE: ID_SERIAL=VMware_Virtual_IDE_Hard_Drive_00000000000000000001\nE: ID_SERIAL_SHORT=00000000000000000001\nE: ID_TYPE=disk\nE: ID_WWN=0x5000c298825951d9\nE: ID_WWN_WITH_EXTENSION=0x5000c298825951d9\nE: MAJOR=8\nE: MINOR=16\nE: SUBSYSTEM=block\nE: TAGS=:systemd:\nE: USEC_INITIALIZED=645583\n"
+	udevInfo_partition := "P: /devices/pci0000:00/0000:00:07.1/ata1/host1/target1:0:0/1:0:0:0/block/sdb/sdb1\nN: sdb1\nS: disk/by-id/ata-VMware_Virtual_IDE_Hard_Drive_00000000000000000001-part1\nS: disk/by-id/lvm-pv-uuid-QKLBih-32Tt-0JP7-uQKr-GzjD-cx0C-I8Vnno\nS: disk/by-id/wwn-0x5000c298825951d9-part1\nS: disk/by-path/pci-0000:00:07.1-ata-1.0-part1\nE: DEVLINKS=/dev/disk/by-id/ata-VMware_Virtual_IDE_Hard_Drive_00000000000000000001-part1 /dev/disk/by-id/lvm-pv-uuid-QKLBih-32Tt-0JP7-uQKr-GzjD-cx0C-I8Vnno /dev/disk/by-id/wwn-0x5000c298825951d9-part1 /dev/disk/by-path/pci-0000:00:07.1-ata-1.0-part1\nE: DEVNAME=/dev/sdb1\nE: DEVPATH=/devices/pci0000:00/0000:00:07.1/ata1/host1/target1:0:0/1:0:0:0/block/sdb/sdb1\nE: DEVTYPE=partition\nE: ID_ATA=1\nE: ID_ATA_FEATURE_SET_APM=1\nE: ID_ATA_FEATURE_SET_APM_ENABLED=0\nE: ID_ATA_FEATURE_SET_PM=1\nE: ID_ATA_FEATURE_SET_PM_ENABLED=1\nE: ID_BUS=ata\nE: ID_FS_TYPE=LVM2_member\nE: ID_FS_USAGE=raid\nE: ID_FS_UUID=QKLBih-32Tt-0JP7-uQKr-GzjD-cx0C-I8Vnno\nE: ID_FS_UUID_ENC=QKLBih-32Tt-0JP7-uQKr-GzjD-cx0C-I8Vnno\nE: ID_FS_VERSION=LVM2 001\nE: ID_MODEL=VMware_Virtual_IDE_Hard_Drive\nE: ID_MODEL_ENC=VMware\\x20Virtual\\x20IDE\\x20Hard\\x20Drive\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\nE: ID_PART_ENTRY_DISK=8:16\nE: ID_PART_ENTRY_NUMBER=1\nE: ID_PART_ENTRY_OFFSET=2048\nE: ID_PART_ENTRY_SCHEME=dos\nE: ID_PART_ENTRY_SIZE=31455232\nE: ID_PART_ENTRY_TYPE=0x83\nE: ID_PART_TABLE_TYPE=dos\nE: ID_PATH=pci-0000:00:07.1-ata-1.0\nE: ID_PATH_TAG=pci-0000_00_07_1-ata-1_0\nE: ID_REVISION=00000001\nE: ID_SERIAL=VMware_Virtual_IDE_Hard_Drive_00000000000000000001\nE: ID_SERIAL_SHORT=00000000000000000001\nE: ID_TYPE=disk\nE: ID_WWN=0x5000c298825951d9\nE: ID_WWN_WITH_EXTENSION=0x5000c298825951d9\nE: MAJOR=8\nE: MINOR=17\nE: SUBSYSTEM=block\nE: SYSTEMD_ALIAS=/dev/block/8:17\nE: SYSTEMD_READY=1\nE: SYSTEMD_WANTS=lvm2-pvscan@8:17.service\nE: TAGS=:systemd:"


you can use a struct to represent all test cases like

hwameistor/pkg/local-disk-manager/handler/localdiskclaim/localdiskclaim_test.go

Line 49 in 997ca1b

testCases := []struct {

SSmallMonster · 2023-10-26T08:41:56Z

pkg/local-disk-manager/udev/device_test.go

+			info := parseUdevInfo(testCase.UdevInfo)
+			d := &Device{}
+			d.ParseDiskAttribute(info)
+			fmt.Printf("PartName: %v\n", d.PartName)


this print log is unnecessry here

ok,I remove it!

SSmallMonster · 2023-10-26T08:42:58Z

@hikariwo lgtm, can you rebase the commit and repush

hikariwo · 2023-10-26T08:50:37Z

I try to clean it!

@hikariwo lgtm, can you rebase the commit and repush

… the bug about device.PartName and PartitionInfo.Path not to be assigned a value Signed-off-by: lancerxiu <1377004871@qq.com> add udev_test file to test parsing the UdevInfo work properly and fix the bug about device.PartName and PartitionInfo.Path not to be assigned a value Signed-off-by: lancerxiu <1377004871@qq.com> combine two TestFunc into one TestFunc Signed-off-by: lancerxiu <1377004871@qq.com> modify test file Signed-off-by: lancerxiu <1377004871@qq.com> modify test file Signed-off-by: lancerxiu <1377004871@qq.com>

hikariwo · 2023-10-26T09:40:01Z

@SSmallMonster done, commit-tree has been cleaned!

hikariwo and others added 4 commits October 16, 2023 10:24

simplified and fixed the bug code in localstoragenode_controller.go::…

2046fe6

…getStorageNodeMigrateOperations Signed-off-by: lancerxiu <1377004871@qq.com>

simplified and fixed the bug code in localstoragenode_controller.go::…

e0bc386

…getStorageNodeMigrateOperations Signed-off-by: lancerxiu <1377004871@qq.com>

Merge branch 'hwameistor:main' into main

32889da

Merge branch 'hwameistor:main' into main

d1fede1

SSmallMonster reviewed Oct 26, 2023

View reviewed changes

hikariwo force-pushed the main branch 2 times, most recently from bb3d3d9 to 5b3f000 Compare October 26, 2023 08:41

SSmallMonster reviewed Oct 26, 2023

View reviewed changes

hikariwo force-pushed the main branch from 299ed1a to a1305f1 Compare October 26, 2023 08:58

hikariwo force-pushed the main branch from a1305f1 to 9a5f001 Compare October 26, 2023 09:36

SSmallMonster merged commit 80e8eba into hwameistor:main Oct 26, 2023
3 checks passed

FloatXD mentioned this pull request Oct 30, 2023

Release Proposal v0.13.1 #1207

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix the localdisk.partitionInfo.path not display correctly #1199

fix the localdisk.partitionInfo.path not display correctly #1199

hikariwo commented Oct 26, 2023

codecov-commenter commented Oct 26, 2023

SSmallMonster left a comment

SSmallMonster Oct 26, 2023

hikariwo Oct 26, 2023

SSmallMonster Oct 26, 2023

hikariwo Oct 26, 2023

SSmallMonster Oct 26, 2023

SSmallMonster Oct 26, 2023

hikariwo Oct 26, 2023

SSmallMonster commented Oct 26, 2023

hikariwo commented Oct 26, 2023

hikariwo commented Oct 26, 2023

		udevInfo_unPartition := "P: /devices/pci0000:00/0000:00:07.1/ata1/host1/target1:0:0/1:0:0:0/block/sdb\nN: sdb\nS: disk/by-id/ata-VMware_Virtual_IDE_Hard_Drive_00000000000000000001\nS: disk/by-id/wwn-0x5000c298825951d9\nS: disk/by-path/pci-0000:00:07.1-ata-1.0\nE: DEVLINKS=/dev/disk/by-id/ata-VMware_Virtual_IDE_Hard_Drive_00000000000000000001 /dev/disk/by-id/wwn-0x5000c298825951d9 /dev/disk/by-path/pci-0000:00:07.1-ata-1.0\nE: DEVNAME=/dev/sdb\nE: DEVPATH=/devices/pci0000:00/0000:00:07.1/ata1/host1/target1:0:0/1:0:0:0/block/sdb\nE: DEVTYPE=disk\nE: ID_ATA=1\nE: ID_ATA_FEATURE_SET_APM=1\nE: ID_ATA_FEATURE_SET_APM_ENABLED=0\nE: ID_ATA_FEATURE_SET_PM=1\nE: ID_ATA_FEATURE_SET_PM_ENABLED=1\nE: ID_BUS=ata\nE: ID_MODEL=VMware_Virtual_IDE_Hard_Drive\nE: ID_MODEL_ENC=VMware\\x20Virtual\\x20IDE\\x20Hard\\x20Drive\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\nE: ID_PART_TABLE_TYPE=dos\nE: ID_PATH=pci-0000:00:07.1-ata-1.0\nE: ID_PATH_TAG=pci-0000_00_07_1-ata-1_0\nE: ID_REVISION=00000001\nE: ID_SERIAL=VMware_Virtual_IDE_Hard_Drive_00000000000000000001\nE: ID_SERIAL_SHORT=00000000000000000001\nE: ID_TYPE=disk\nE: ID_WWN=0x5000c298825951d9\nE: ID_WWN_WITH_EXTENSION=0x5000c298825951d9\nE: MAJOR=8\nE: MINOR=16\nE: SUBSYSTEM=block\nE: TAGS=:systemd:\nE: USEC_INITIALIZED=645583\n"
		udevInfo_partition := "P: /devices/pci0000:00/0000:00:07.1/ata1/host1/target1:0:0/1:0:0:0/block/sdb/sdb1\nN: sdb1\nS: disk/by-id/ata-VMware_Virtual_IDE_Hard_Drive_00000000000000000001-part1\nS: disk/by-id/lvm-pv-uuid-QKLBih-32Tt-0JP7-uQKr-GzjD-cx0C-I8Vnno\nS: disk/by-id/wwn-0x5000c298825951d9-part1\nS: disk/by-path/pci-0000:00:07.1-ata-1.0-part1\nE: DEVLINKS=/dev/disk/by-id/ata-VMware_Virtual_IDE_Hard_Drive_00000000000000000001-part1 /dev/disk/by-id/lvm-pv-uuid-QKLBih-32Tt-0JP7-uQKr-GzjD-cx0C-I8Vnno /dev/disk/by-id/wwn-0x5000c298825951d9-part1 /dev/disk/by-path/pci-0000:00:07.1-ata-1.0-part1\nE: DEVNAME=/dev/sdb1\nE: DEVPATH=/devices/pci0000:00/0000:00:07.1/ata1/host1/target1:0:0/1:0:0:0/block/sdb/sdb1\nE: DEVTYPE=partition\nE: ID_ATA=1\nE: ID_ATA_FEATURE_SET_APM=1\nE: ID_ATA_FEATURE_SET_APM_ENABLED=0\nE: ID_ATA_FEATURE_SET_PM=1\nE: ID_ATA_FEATURE_SET_PM_ENABLED=1\nE: ID_BUS=ata\nE: ID_FS_TYPE=LVM2_member\nE: ID_FS_USAGE=raid\nE: ID_FS_UUID=QKLBih-32Tt-0JP7-uQKr-GzjD-cx0C-I8Vnno\nE: ID_FS_UUID_ENC=QKLBih-32Tt-0JP7-uQKr-GzjD-cx0C-I8Vnno\nE: ID_FS_VERSION=LVM2 001\nE: ID_MODEL=VMware_Virtual_IDE_Hard_Drive\nE: ID_MODEL_ENC=VMware\\x20Virtual\\x20IDE\\x20Hard\\x20Drive\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\\x20\nE: ID_PART_ENTRY_DISK=8:16\nE: ID_PART_ENTRY_NUMBER=1\nE: ID_PART_ENTRY_OFFSET=2048\nE: ID_PART_ENTRY_SCHEME=dos\nE: ID_PART_ENTRY_SIZE=31455232\nE: ID_PART_ENTRY_TYPE=0x83\nE: ID_PART_TABLE_TYPE=dos\nE: ID_PATH=pci-0000:00:07.1-ata-1.0\nE: ID_PATH_TAG=pci-0000_00_07_1-ata-1_0\nE: ID_REVISION=00000001\nE: ID_SERIAL=VMware_Virtual_IDE_Hard_Drive_00000000000000000001\nE: ID_SERIAL_SHORT=00000000000000000001\nE: ID_TYPE=disk\nE: ID_WWN=0x5000c298825951d9\nE: ID_WWN_WITH_EXTENSION=0x5000c298825951d9\nE: MAJOR=8\nE: MINOR=17\nE: SUBSYSTEM=block\nE: SYSTEMD_ALIAS=/dev/block/8:17\nE: SYSTEMD_READY=1\nE: SYSTEMD_WANTS=lvm2-pvscan@8:17.service\nE: TAGS=:systemd:"

fix the localdisk.partitionInfo.path not display correctly #1199

fix the localdisk.partitionInfo.path not display correctly #1199

Conversation

hikariwo commented Oct 26, 2023

What this PR does / why we need it:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

codecov-commenter commented Oct 26, 2023

Codecov Report

SSmallMonster left a comment

Choose a reason for hiding this comment

SSmallMonster Oct 26, 2023

Choose a reason for hiding this comment

hikariwo Oct 26, 2023

Choose a reason for hiding this comment

SSmallMonster Oct 26, 2023

Choose a reason for hiding this comment

hikariwo Oct 26, 2023

Choose a reason for hiding this comment

SSmallMonster Oct 26, 2023

Choose a reason for hiding this comment

SSmallMonster Oct 26, 2023

Choose a reason for hiding this comment

hikariwo Oct 26, 2023

Choose a reason for hiding this comment

SSmallMonster commented Oct 26, 2023

hikariwo commented Oct 26, 2023

hikariwo commented Oct 26, 2023