From 8f3357d9c803eaf92a68bc1c029647468bde5f96 Mon Sep 17 00:00:00 2001 From: Aleksandr Zimin Date: Fri, 2 Feb 2024 12:33:21 +0300 Subject: [PATCH] [controller] Fix naming bug by removing previous partUUID fix. Refactor name generation. Add test for device name. (#21) Signed-off-by: Aleksandr Zimin --- images/agent/pkg/controller/block_device.go | 104 ++++++++---------- .../agent/pkg/controller/block_device_test.go | 53 ++++++--- 2 files changed, 81 insertions(+), 76 deletions(-) diff --git a/images/agent/pkg/controller/block_device.go b/images/agent/pkg/controller/block_device.go index 9322ffd6..a0f26c9b 100644 --- a/images/agent/pkg/controller/block_device.go +++ b/images/agent/pkg/controller/block_device.go @@ -273,29 +273,14 @@ func GetBlockDeviceCandidates(log logger.Logger, cfg config.Options, metrics mon } log.Trace(fmt.Sprintf("[GetBlockDeviceCandidates] Get following candidate: %+v", candidate)) - if len(candidate.Serial) == 0 { - log.Info(fmt.Sprintf("[GetBlockDeviceCandidates] Serial number is empty; attempting to retrieve it from an alternate source: %s", candidate.Path)) + candidateName := CreateCandidateName(log, candidate) - if candidate.Type == internal.TypePart { - if len(candidate.PartUUID) == 0 { - log.Warning(fmt.Sprintf("[GetBlockDeviceCandidates] Type = part and cannot get PartUUID; skipping this device, path: %s", candidate.Path)) - continue - } - log.Info(fmt.Sprintf("[GetBlockDeviceCandidates] Type = part and PartUUID is not empty; using PartUUID as serial number, path: %s", candidate.Path)) - candidate.Serial = candidate.PartUUID - } else { - serial, err := GetSerial(log, candidate) - if err != nil { - log.Warning(fmt.Sprintf("[GetBlockDeviceCandidates] Unable to obtain serial number or its equivalent; skipping device: %s. Error: %s", candidate.Path, err)) - continue - } - log.Info(fmt.Sprintf("[GetBlockDeviceCandidates] Successfully obtained serial number or its equivalent: %s for device: %s", serial, candidate.Path)) - candidate.Serial = serial - } - log.Trace(fmt.Sprintf("[GetBlockDeviceCandidates] Serial number is now: %s", candidate.Serial)) + if candidateName == "" { + log.Trace("[GetBlockDeviceCandidates] candidateName is empty. Skipping device") + continue } - candidate.Name = CreateUniqDeviceName(candidate) + candidate.Name = candidateName log.Trace(fmt.Sprintf("[GetBlockDeviceCandidates] Generated a unique candidate name: %s", candidate.Name)) delFlag = false @@ -444,39 +429,56 @@ func CheckTag(tags string) (bool, string) { return true, "" } -func CreateUniqDeviceName(can internal.BlockDeviceCandidate) string { - temp := fmt.Sprintf("%s%s%s%s%s", can.NodeName, can.Wwn, can.Model, can.Serial, can.PartUUID) - s := fmt.Sprintf("dev-%x", sha1.Sum([]byte(temp))) - return s -} +func CreateCandidateName(log logger.Logger, candidate internal.BlockDeviceCandidate) string { + if len(candidate.Serial) == 0 { + log.Trace(fmt.Sprintf("[CreateCandidateName] Serial number is empty for device: %s", candidate.Path)) + if candidate.Type == internal.TypePart { + if len(candidate.PartUUID) == 0 { + log.Warning(fmt.Sprintf("[CreateCandidateName] Type = part and cannot get PartUUID; skipping this device, path: %s", candidate.Path)) + return "" + } + log.Trace(fmt.Sprintf("[CreateCandidateName] Type = part and PartUUID is not empty; skiping getting serial number for device: %s", candidate.Path)) + } else { + log.Debug(fmt.Sprintf("[CreateCandidateName] Serial number is empty and device type is not part; trying to obtain serial number or its equivalent for device: %s, with type: %s", candidate.Path, candidate.Type)) -func GetSerial(log logger.Logger, candidate internal.BlockDeviceCandidate) (string, error) { - var serial string - matched, err := regexp.MatchString(`raid.*`, candidate.Type) - if err != nil { - log.Error(err, "[GetSerial] unable to regexp.MatchString. Trying to get serial from device") - serial, err = readSerialBlockDevice(candidate.Path) - } else if matched { - log.Trace("[GetSerial] device is mdraid") - serial, err = readUUIDmdRaidBlockDevice(candidate.Path) - } else { - log.Trace("[GetSerial] device is not mdraid") - serial, err = readSerialBlockDevice(candidate.Path) + isMdRaid := false + matched, err := regexp.MatchString(`raid.*`, candidate.Type) + if err != nil { + log.Error(err, "[CreateCandidateName] failed to match regex - unable to determine if the device is an mdraid. Attempting to retrieve serial number directly from the device") + } else if matched { + log.Trace("[CreateCandidateName] device is mdraid") + isMdRaid = true + } + serial, err := readSerialBlockDevice(candidate.Path, isMdRaid) + if err != nil { + log.Warning(fmt.Sprintf("[CreateCandidateName] Unable to obtain serial number or its equivalent; skipping device: %s. Error: %s", candidate.Path, err)) + return "" + } + log.Info(fmt.Sprintf("[CreateCandidateName] Successfully obtained serial number or its equivalent: %s for device: %s", serial, candidate.Path)) + candidate.Serial = serial + log.Trace(fmt.Sprintf("[CreateCandidateName] Serial number is now: %s. Creating candidate name", candidate.Serial)) + } } - if err != nil { - return "", err - } + return CreateUniqDeviceName(candidate) +} - return serial, nil +func CreateUniqDeviceName(can internal.BlockDeviceCandidate) string { + temp := fmt.Sprintf("%s%s%s%s%s", can.NodeName, can.Wwn, can.Model, can.Serial, can.PartUUID) + s := fmt.Sprintf("dev-%x", sha1.Sum([]byte(temp))) + return s } -func readSerialBlockDevice(deviceName string) (string, error) { +func readSerialBlockDevice(deviceName string, isMdRaid bool) (string, error) { if len(deviceName) < 6 { return "", fmt.Errorf("device name is too short") } - strPath := fmt.Sprintf("/sys/block/%s/serial", deviceName[5:]) + + if isMdRaid { + strPath = fmt.Sprintf("/sys/block/%s/md/uuid", deviceName[5:]) + } + serial, err := os.ReadFile(strPath) if err != nil { return "", fmt.Errorf("unable to read serial from block device: %s, error: %s", deviceName, err) @@ -487,22 +489,6 @@ func readSerialBlockDevice(deviceName string) (string, error) { return string(serial), nil } -func readUUIDmdRaidBlockDevice(deviceName string) (string, error) { - if len(deviceName) < 6 { - return "", fmt.Errorf("device name is too short") - } - - strPath := fmt.Sprintf("/sys/block/%s/md/uuid", deviceName[5:]) - uuid, err := os.ReadFile(strPath) - if err != nil { - return "", fmt.Errorf("unable to read uuid from mdraid block device: %s, error: %s", deviceName, err) - } - if len(uuid) == 0 { - return "", fmt.Errorf("uuid of mdraid block device is empty") - } - return string(uuid), nil -} - func UpdateAPIBlockDevice(ctx context.Context, kc kclient.Client, metrics monitoring.Metrics, res v1alpha1.BlockDevice, candidate internal.BlockDeviceCandidate) error { candidateSizeTmp := resource.NewQuantity(candidate.Size.Value(), resource.BinarySI) device := &v1alpha1.BlockDevice{ diff --git a/images/agent/pkg/controller/block_device_test.go b/images/agent/pkg/controller/block_device_test.go index c11d7dd9..a7f5cdb1 100644 --- a/images/agent/pkg/controller/block_device_test.go +++ b/images/agent/pkg/controller/block_device_test.go @@ -218,22 +218,41 @@ func TestBlockDeviceCtrl(t *testing.T) { for i, device := range filteredDevices { println("Filtered device: ", device.Name) + candidate := internal.BlockDeviceCandidate{ + NodeName: "test-node", + Consumable: CheckConsumable(device), + Wwn: device.Wwn, + Serial: device.Serial, + Path: device.Name, + Size: device.Size, + Rota: device.Rota, + Model: device.Model, + HotPlug: device.HotPlug, + KName: device.KName, + PkName: device.PkName, + Type: device.Type, + FSType: device.FSType, + PartUUID: device.PartUUID, + } switch i { case 0: assert.Equal(t, "/dev/md1", device.Name) - assert.False(t, CheckConsumable(device)) + assert.False(t, candidate.Consumable) case 1: assert.Equal(t, "/dev/md127", device.Name) - assert.False(t, CheckConsumable(device)) + assert.False(t, candidate.Consumable) case 2: assert.Equal(t, "/dev/nvme4n1", device.Name) - assert.True(t, CheckConsumable(device)) + assert.True(t, candidate.Consumable) case 3: assert.Equal(t, "/dev/nvme5n1", device.Name) - assert.True(t, CheckConsumable(device)) + assert.True(t, candidate.Consumable) case 4: assert.Equal(t, "/dev/sda4", device.Name) - assert.False(t, CheckConsumable(device)) + assert.False(t, candidate.Consumable) + candidateName := CreateCandidateName(*log, candidate) + assert.Equal(t, "dev-377bc6adf33d84eb5932f5c89798bb6c5949ae2d", candidateName, "device name generated incorrectly") + } } @@ -500,68 +519,68 @@ var ( "mountpoint": null, "partuuid": null, "hotplug": false, - "model": "INTEL SSDSC2KB48", - "serial": "PHYS729000AS480BGN", + "model": "INTEL", + "serial": "PHYS729000AAAA", "size": "447.1G", "fstype": null, "type": "disk", - "wwn": "0x55cd2e414e193b8c", + "wwn": "0x5555555", "kname": "/dev/sda", "pkname": null },{ "name": "/dev/sda1", "mountpoint": "/boot/efi", - "partuuid": "e2ac92cc-6a34-4402-a253-40533a3fc877", + "partuuid": "xxxxx-6a34-4402-a253-nnnnn", "hotplug": false, "model": null, "serial": null, "size": "1G", "fstype": "vfat", "type": "part", - "wwn": "0x55cd2e414e193b8c", + "wwn": "0x5555555", "kname": "/dev/sda1", "pkname": "/dev/sda" },{ "name": "/dev/sda2", "mountpoint": null, - "partuuid": "e95db4fa-99b4-42c4-9dc4-ff5459557801", + "partuuid": "xxxxx-99b4-42c4-9dc4-nnnnnnn", "hotplug": false, "model": null, "serial": null, "size": "1G", "fstype": "linux_raid_member", "type": "part", - "wwn": "0x55cd2e414e193b8c", + "wwn": "0x5555555", "kname": "/dev/sda2", "pkname": "/dev/sda" },{ "name": "/dev/sda3", "mountpoint": null, - "partuuid": "ed7633b6-f3ef-4b4a-86f8-48c0180de78f", + "partuuid": "xxxxx-f3ef-4b4a-86f8-nnnnnn", "hotplug": false, "model": null, "serial": null, "size": "55G", "fstype": "linux_raid_member", "type": "part", - "wwn": "0x55cd2e414e193b8c", + "wwn": "0x5555555", "kname": "/dev/sda3", "pkname": "/dev/sda" },{ "name": "/dev/sda4", "mountpoint": null, - "partuuid": "6f1d599d-9f91-41c5-9616-69709cf30b9d", + "partuuid": "xxxxx-9f91-41c5-9616-nnnnnn", "hotplug": false, "model": null, "serial": null, "size": "390.1G", "fstype": "LVM2_member", "type": "part", - "wwn": "0x55cd2e414e193b8c", + "wwn": "0x55cddd", "kname": "/dev/sda4", "pkname": "/dev/sda" },{ - "name": "/dev/mapper/data--linstor-pvc--9671a7f4--8997--4630--a728--2cd6f915c19b_00000", + "name": "/dev/mapper/data--linstor-pvc--xxxx--8997--4630--a728--nnnnnn_00000", "mountpoint": null, "partuuid": null, "hotplug": false,