From fe2c39b42234a6b39cb8dfa803f86a9f7780c1ae Mon Sep 17 00:00:00 2001 From: Aleksandr Zimin Date: Tue, 21 May 2024 15:15:13 +0300 Subject: [PATCH] [controller] Fix multipath device serial discovery (#52) Signed-off-by: Aleksandr Zimin --- images/agent/config/config.go | 4 +- images/agent/internal/const.go | 7 +- images/agent/pkg/controller/block_device.go | 79 ++++++++++++++----- .../agent/pkg/controller/block_device_test.go | 62 +++++++++++++-- openapi/config-values.yaml | 3 - templates/agent/daemonset.yaml | 6 ++ 6 files changed, 130 insertions(+), 31 deletions(-) diff --git a/images/agent/config/config.go b/images/agent/config/config.go index 1b61cebe..9d417df1 100644 --- a/images/agent/config/config.go +++ b/images/agent/config/config.go @@ -23,6 +23,7 @@ import ( "os/exec" "sds-node-configurator/internal" "sds-node-configurator/pkg/logger" + "strings" "time" ) @@ -90,9 +91,8 @@ func getMachineId() (string, error) { return "", err } - id = stdout.String() + id = strings.TrimSpace(stdout.String()) fmt.Println("MACHINE ID " + id) - } return id, nil diff --git a/images/agent/internal/const.go b/images/agent/internal/const.go index 8288b87f..1c4090d4 100644 --- a/images/agent/internal/const.go +++ b/images/agent/internal/const.go @@ -17,11 +17,14 @@ limitations under the License. package internal const ( - TypePart = "part" + PartType = "part" + MultiPathType = "mpath" + CDROMDeviceType = "rom" DRBDName = "/dev/drbd" LoopDeviceType = "loop" LVMDeviceType = "lvm" LVMFSType = "LVM2_member" + MultiPathMemberFSType = "mpath_member" SdsNodeConfiguratorFinalizer = "storage.deckhouse.io/sds-node-configurator" LVMVGHealthOperational = "Operational" LVMVGHealthNonOperational = "NonOperational" @@ -36,7 +39,7 @@ const ( var ( AllowedFSTypes = [...]string{LVMFSType} - InvalidDeviceTypes = [...]string{LoopDeviceType, LVMDeviceType} + InvalidDeviceTypes = [...]string{LoopDeviceType, LVMDeviceType, CDROMDeviceType} Finalizers = []string{SdsNodeConfiguratorFinalizer} LVMTags = []string{"storage.deckhouse.io/enabled=true", "linstor-"} ) diff --git a/images/agent/pkg/controller/block_device.go b/images/agent/pkg/controller/block_device.go index e65869fb..b6ba5a23 100644 --- a/images/agent/pkg/controller/block_device.go +++ b/images/agent/pkg/controller/block_device.go @@ -272,7 +272,7 @@ func GetBlockDeviceCandidates(log logger.Logger, cfg config.Options, metrics mon } log.Trace(fmt.Sprintf("[GetBlockDeviceCandidates] Get following candidate: %+v", candidate)) - candidateName := CreateCandidateName(log, candidate) + candidateName := CreateCandidateName(log, candidate, devices) if candidateName == "" { log.Trace("[GetBlockDeviceCandidates] candidateName is empty. Skipping device") @@ -428,10 +428,10 @@ func CheckTag(tags string) (bool, string) { return true, "" } -func CreateCandidateName(log logger.Logger, candidate internal.BlockDeviceCandidate) string { +func CreateCandidateName(log logger.Logger, candidate internal.BlockDeviceCandidate, devices []internal.Device) 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 candidate.Type == internal.PartType { if len(candidate.PartUUID) == 0 { log.Warning(fmt.Sprintf("[CreateCandidateName] Type = part and cannot get PartUUID; skipping this device, path: %s", candidate.Path)) return "" @@ -440,25 +440,38 @@ func CreateCandidateName(log logger.Logger, candidate internal.BlockDeviceCandid } 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)) - 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 "" + switch candidate.Type { + case internal.MultiPathType: + log.Debug(fmt.Sprintf("[CreateCandidateName] device %s type = %s; get serial number from parent device.", candidate.Path, candidate.Type)) + log.Trace(fmt.Sprintf("[CreateCandidateName] device: %+v. Device list: %+v", candidate, devices)) + serial, err := getSerialForMultipathDevice(candidate, devices) + 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 "" + } + candidate.Serial = serial + log.Info(fmt.Sprintf("[CreateCandidateName] Successfully obtained serial number or its equivalent: %s for device: %s", candidate.Serial, candidate.Path)) + default: + 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.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)) } } + log.Trace(fmt.Sprintf("[CreateCandidateName] Serial number is now: %s. Creating candidate name", candidate.Serial)) return CreateUniqDeviceName(candidate) } @@ -622,6 +635,7 @@ func ReTag(log logger.Logger, metrics monitoring.Metrics) error { cmdStr, err = utils.LVChangeDelTag(lv, tag) metrics.UtilsCommandsDuration(blockDeviceCtrlName, "lvchange").Observe(metrics.GetEstimatedTimeInSeconds(start)) metrics.UtilsCommandsExecutionCount(blockDeviceCtrlName, "lvchange").Inc() + log.Debug(fmt.Sprintf("[ReTag] exec cmd: %s", cmdStr)) if err != nil { metrics.UtilsCommandsErrorsCount(blockDeviceCtrlName, "lvchange").Inc() log.Error(err, "[ReTag] unable to LVChangeDelTag") @@ -691,3 +705,32 @@ func ReTag(log logger.Logger, metrics monitoring.Metrics) error { return nil } + +func getSerialForMultipathDevice(candidate internal.BlockDeviceCandidate, devices []internal.Device) (string, error) { + parentDevice := getParentDevice(candidate.PkName, devices) + if parentDevice.Name == "" { + err := fmt.Errorf("parent device %s not found for multipath device: %s in device list", candidate.PkName, candidate.Path) + return "", err + } + + if parentDevice.FSType != internal.MultiPathMemberFSType { + err := fmt.Errorf("parent device %s for multipath device %s is not a multipath member (fstype != %s)", parentDevice.Name, candidate.Path, internal.MultiPathMemberFSType) + return "", err + } + + if parentDevice.Serial == "" { + err := fmt.Errorf("serial number is empty for parent device %s", parentDevice.Name) + return "", err + } + + return parentDevice.Serial, nil +} + +func getParentDevice(pkName string, devices []internal.Device) internal.Device { + for _, device := range devices { + if device.Name == pkName { + return device + } + } + return internal.Device{} +} diff --git a/images/agent/pkg/controller/block_device_test.go b/images/agent/pkg/controller/block_device_test.go index c58773ab..bb98cb9c 100644 --- a/images/agent/pkg/controller/block_device_test.go +++ b/images/agent/pkg/controller/block_device_test.go @@ -212,7 +212,7 @@ func TestBlockDeviceCtrl(t *testing.T) { testLsblkOutputBytes := []byte(testLsblkOutput) devices, err := utils.UnmarshalDevices(testLsblkOutputBytes) if assert.NoError(t, err) { - assert.Equal(t, 28, len(devices)) + assert.Equal(t, 31, len(devices)) } filteredDevices, err := FilterDevices(*log, devices) @@ -244,25 +244,33 @@ func TestBlockDeviceCtrl(t *testing.T) { case 2: assert.Equal(t, "/dev/nvme4n1", device.Name) assert.True(t, candidate.Consumable) + candidateName := CreateCandidateName(*log, candidate, devices) + assert.Equal(t, "dev-794d93d177d16bc9a85e2dd2ccbdc7325c287374", candidateName, "device name generated incorrectly") case 3: assert.Equal(t, "/dev/nvme5n1", device.Name) assert.True(t, candidate.Consumable) + candidateName := CreateCandidateName(*log, candidate, devices) + assert.Equal(t, "dev-3306e773ab3cde6d519ce8d7c3686bf17a124dcb", candidateName, "device name generated incorrectly") case 4: assert.Equal(t, "/dev/sda4", device.Name) assert.False(t, candidate.Consumable) - candidateName := CreateCandidateName(*log, candidate) + candidateName := CreateCandidateName(*log, candidate, devices) assert.Equal(t, "dev-377bc6adf33d84eb5932f5c89798bb6c5949ae2d", candidateName, "device name generated incorrectly") case 5: assert.Equal(t, "/dev/vdc1", device.Name) assert.True(t, candidate.Consumable) - candidateName := CreateCandidateName(*log, candidate) + candidateName := CreateCandidateName(*log, candidate, devices) assert.Equal(t, "dev-a9d768213aaead8b42465ec859189de8779f96b7", candidateName, "device name generated incorrectly") - + case 6: + assert.Equal(t, "/dev/mapper/mpatha", device.Name) + assert.True(t, candidate.Consumable) + candidateName := CreateCandidateName(*log, candidate, devices) + assert.Equal(t, "dev-6367f0efb2dfa697c90aaf1fad24ad9cc87a76cd", candidateName, "device name generated incorrectly") } } if assert.NoError(t, err) { - assert.Equal(t, 6, len(filteredDevices)) + assert.Equal(t, 7, len(filteredDevices)) } }) @@ -636,7 +644,49 @@ var ( "wwn": null, "kname": "/dev/vdc1", "pkname": "/dev/vdc" - } + },{ + "name": "/dev/mapper/mpatha", + "mountpoint": null, + "partuuid": null, + "hotplug": false, + "model": null, + "serial": null, + "size": 3650722201600, + "fstype": null, + "type": "mpath", + "wwn": null, + "kname": "/dev/dm-6", + "pkname": "/dev/sdf", + "rota": false + },{ + "name": "/dev/sdf", + "mountpoint": null, + "partuuid": null, + "hotplug": false, + "model": "test-model", + "serial": "22222222xxxxx", + "size": 3650722201600, + "fstype": "mpath_member", + "type": "disk", + "wwn": "2222xxxxxx", + "kname": "/dev/sdf", + "pkname": null, + "rota": false + },{ + "name": "/dev/sdh", + "mountpoint": null, + "partuuid": null, + "hotplug": false, + "model": "test-model", + "serial": "22222222xxxxx", + "size": 3650722201600, + "fstype": "mpath_member", + "type": "disk", + "wwn": "2222xxxxxx", + "kname": "/dev/sdh", + "pkname": null, + "rota": false + } ] }` ) diff --git a/openapi/config-values.yaml b/openapi/config-values.yaml index 3dd9a2cf..6da1fec3 100644 --- a/openapi/config-values.yaml +++ b/openapi/config-values.yaml @@ -2,9 +2,6 @@ x-config-version: 2 type: object required: [] properties: - enableDevMode: - type: boolean - default: false disableDs: type: boolean default: false diff --git a/templates/agent/daemonset.yaml b/templates/agent/daemonset.yaml index 970ddeb8..b98b6373 100644 --- a/templates/agent/daemonset.yaml +++ b/templates/agent/daemonset.yaml @@ -91,6 +91,8 @@ spec: value: "4" {{- end }} volumeMounts: + - mountPath: /dev/ + name: host-device-dir - mountPath: /sys/ name: host-sys-dir - mountPath: /run/udev/ @@ -106,6 +108,10 @@ spec: path: /opt/deckhouse/sds type: DirectoryOrCreate name: opt-deckhouse-sds + - hostPath: + path: /dev/ + type: "" + name: host-device-dir - hostPath: path: /sys/ type: Directory