From e3c7f44c96a10699f482aa560af2a821fe9d470b Mon Sep 17 00:00:00 2001 From: Preslav Date: Tue, 28 May 2024 10:19:19 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=B9=20Split=20device=20discovery=20fro?= =?UTF-8?q?m=20partition=20discovery=20in=20blockdevices.go?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Preslav --- .../connection/device/linux/device_manager.go | 50 ++++--- providers/os/connection/device/linux/lun.go | 15 +- .../os/connection/device/linux/lun_test.go | 7 +- .../os/connection/snapshot/blockdevices.go | 60 +++++++- .../connection/snapshot/blockdevices_test.go | 141 ++++++++++++++++++ .../os/connection/snapshot/volumemounter.go | 7 +- 6 files changed, 248 insertions(+), 32 deletions(-) diff --git a/providers/os/connection/device/linux/device_manager.go b/providers/os/connection/device/linux/device_manager.go index 910f2d37ac..abdf101693 100644 --- a/providers/os/connection/device/linux/device_manager.go +++ b/providers/os/connection/device/linux/device_manager.go @@ -104,27 +104,43 @@ func (c *LinuxDeviceManager) identifyViaLun(lun int) (*snapshot.PartitionInfo, e if len(filteredScsiDevices) == 0 { return nil, errors.New("no matching scsi devices found") } - - var target string - // if we have exactly one device present at the LUN we can directly point the volume mounter towards it + blockDevices, err := c.volumeMounter.CmdRunner.GetBlockDevices() + if err != nil { + return nil, err + } + var device snapshot.BlockDevice + var deviceErr error + // if we have exactly one device present at the LUN we can directly search for it if len(filteredScsiDevices) == 1 { - target = filteredScsiDevices[0].VolumePath + devicePath := filteredScsiDevices[0].VolumePath + device, deviceErr = blockDevices.FindDevice(devicePath) } else { - // we have multiple devices at the same LUN. we find the first non-mounted block devices in that list - blockDevices, err := c.volumeMounter.CmdRunner.GetBlockDevices() - if err != nil { - return nil, err - } - target, err = findMatchingDeviceByBlock(filteredScsiDevices, blockDevices) - if err != nil { - return nil, err - } + device, deviceErr = findMatchingDeviceByBlock(filteredScsiDevices, blockDevices) } - - return c.volumeMounter.GetMountablePartition(target) + if deviceErr != nil { + return nil, deviceErr + } + return device.GetMountablePartition() } func (c *LinuxDeviceManager) identifyViaDeviceName(deviceName string) (*snapshot.PartitionInfo, error) { - // GetMountablePartition also supports passing in empty strings, in that case we do a best-effort guess - return c.volumeMounter.GetMountablePartition(deviceName) + blockDevices, err := c.volumeMounter.CmdRunner.GetBlockDevices() + if err != nil { + return nil, err + } + + // if we don't have a device name we can just return the first non-boot, non-mounted partition. + // this is a best-guess approach + if deviceName == "" { + // TODO: we should rename/simplify this method + return blockDevices.GetUnnamedBlockEntry() + } + + // if we have a specific device we're looking for we can just ask only for that + device, err := blockDevices.FindDevice(deviceName) + if err != nil { + return nil, err + } + + return device.GetMountablePartition() } diff --git a/providers/os/connection/device/linux/lun.go b/providers/os/connection/device/linux/lun.go index c77092c5a8..cfa37be279 100644 --- a/providers/os/connection/device/linux/lun.go +++ b/providers/os/connection/device/linux/lun.go @@ -57,7 +57,7 @@ func filterScsiDevices(scsiDevices scsiDevices, lun int) []scsiDeviceInfo { // there can be multiple devices mounted at the same LUN. // the LUN so we need to find all the blocks, mounted at that LUN. then we find the first one // that has no mounted partitions and use that as the target device. this is a best-effort approach -func findMatchingDeviceByBlock(scsiDevices scsiDevices, blockDevices *snapshot.BlockDevices) (string, error) { +func findMatchingDeviceByBlock(scsiDevices scsiDevices, blockDevices *snapshot.BlockDevices) (snapshot.BlockDevice, error) { matchingBlocks := []snapshot.BlockDevice{} for _, device := range scsiDevices { for _, block := range blockDevices.BlockDevices { @@ -69,25 +69,22 @@ func findMatchingDeviceByBlock(scsiDevices scsiDevices, blockDevices *snapshot.B } if len(matchingBlocks) == 0 { - return "", errors.New("no matching blocks found") + return snapshot.BlockDevice{}, errors.New("no matching blocks found") } - var target string for _, b := range matchingBlocks { log.Debug().Str("name", b.Name).Msg("device connection> checking block") - mounted := false for _, ch := range b.Children { if len(ch.MountPoint) > 0 && ch.MountPoint != "" { log.Debug().Str("name", ch.Name).Msg("device connection> has mounted partitons, skipping") - mounted = true - } - if !mounted { - target = "/dev/" + b.Name + } else { + // we found a block that has no mounted partitions + return b, nil } } } - return target, nil + return snapshot.BlockDevice{}, errors.New("no matching block found") } // parses the output from running 'lsscsi --brief' and gets the device info, the output looks like this: diff --git a/providers/os/connection/device/linux/lun_test.go b/providers/os/connection/device/linux/lun_test.go index 5e7f6cd564..b073f49645 100644 --- a/providers/os/connection/device/linux/lun_test.go +++ b/providers/os/connection/device/linux/lun_test.go @@ -79,7 +79,7 @@ func TestFindDeviceByBlock(t *testing.T) { } target, err := findMatchingDeviceByBlock(devices, blockDevices) assert.NoError(t, err) - expected := "/dev/sdb" + expected := blockDevices.BlockDevices[1] assert.Equal(t, expected, target) }) @@ -132,8 +132,7 @@ func TestFindDeviceByBlock(t *testing.T) { }, }, } - target, err := findMatchingDeviceByBlock(devices, blockDevices) - assert.NoError(t, err) - assert.Empty(t, target) + _, err := findMatchingDeviceByBlock(devices, blockDevices) + assert.Error(t, err) }) } diff --git a/providers/os/connection/snapshot/blockdevices.go b/providers/os/connection/snapshot/blockdevices.go index 4ca2fa4fbd..e646c7b797 100644 --- a/providers/os/connection/snapshot/blockdevices.go +++ b/providers/os/connection/snapshot/blockdevices.go @@ -74,6 +74,7 @@ func (blockEntries BlockDevices) GetRootBlockEntry() (*PartitionInfo, error) { // Searches all the partitions in the target device and finds one that can be mounted. It must be unmounted, non-boot partition // If multiple partitions meet this criteria, the largest one is returned. +// Deprecated: Use GetMountablePartition instead func (blockEntries BlockDevices) GetMountablePartitionByDevice(device string) (*PartitionInfo, error) { log.Debug().Str("device", device).Msg("get partitions for device") var block BlockDevice @@ -123,6 +124,55 @@ func (blockEntries BlockDevices) GetMountablePartitionByDevice(device string) (* return &PartitionInfo{Name: devFsName, FsType: partitions[0].FsType}, nil } +// Searches for a device by name +func (blockEntries BlockDevices) FindDevice(name string) (BlockDevice, error) { + log.Debug().Str("device", name).Msg("searching for device") + var secondName string + if strings.HasPrefix(name, "/dev/sd") { + // sdh and xvdh are interchangeable + end := strings.TrimPrefix(name, "/dev/sd") + secondName = "/dev/xvd" + end + } + for i := range blockEntries.BlockDevices { + d := blockEntries.BlockDevices[i] + log.Debug().Str("name", d.Name).Interface("children", d.Children).Interface("mountpoint", d.MountPoint).Msg("found block device") + fullDeviceName := "/dev/" + d.Name + if name != fullDeviceName { // check if the device name matches + if secondName == "" || secondName != fullDeviceName { + continue + } + } + log.Debug().Str("name", d.Name).Msg("found matching device") + return d, nil + } + + return BlockDevice{}, fmt.Errorf("no block device found with name %s", name) +} + +// Searches all the partitions in the device and finds one that can be mounted. It must be unmounted, non-boot partition +// If multiple partitions meet this criteria, the largest one is returned. +func (device BlockDevice) GetMountablePartition() (*PartitionInfo, error) { + log.Debug().Str("device", device.Name).Msg("get partitions for device") + partitions := []BlockDevice{} + for _, partition := range device.Children { + log.Debug().Str("name", partition.Name).Int("size", partition.Size).Msg("checking partition") + if partition.IsNoBootVolumeAndUnmounted() { + partitions = append(partitions, partition) + } + } + + if len(partitions) == 0 { + return nil, fmt.Errorf("no suitable partitions found on device %s", device.Name) + } + + // sort the candidates by size, so we can pick the largest one + sortPartitionsBySize(partitions) + + // return the largest partition. we can extend this to be a parameter in the future + devFsName := "/dev/" + partitions[0].Name + return &PartitionInfo{Name: devFsName, FsType: partitions[0].FsType}, nil +} + func sortPartitionsBySize(partitions []BlockDevice) { sort.Slice(partitions, func(i, j int) bool { return partitions[i].Size > partitions[j].Size @@ -177,10 +227,18 @@ func (entry BlockDevice) IsNoBootVolume() bool { return entry.Uuid != "" && entry.FsType != "" && entry.FsType != "vfat" && entry.Label != "EFI" && entry.Label != "boot" } +func (entry BlockDevice) IsNoBootVolumeAndUnmounted() bool { + return entry.IsNoBootVolume() && !entry.IsMounted() +} + func (entry BlockDevice) IsRootVolume() bool { return strings.Contains(entry.Name, "sda") || strings.Contains(entry.Name, "xvda") || strings.Contains(entry.Name, "nvme0n1") } func (entry BlockDevice) IsNotBootOrRootVolumeAndUnmounted() bool { - return entry.IsNoBootVolume() && entry.MountPoint == "" && !entry.IsRootVolume() + return entry.IsNoBootVolumeAndUnmounted() && !entry.IsRootVolume() +} + +func (entry BlockDevice) IsMounted() bool { + return entry.MountPoint != "" } diff --git a/providers/os/connection/snapshot/blockdevices_test.go b/providers/os/connection/snapshot/blockdevices_test.go index 53490f1f44..82cb598eac 100644 --- a/providers/os/connection/snapshot/blockdevices_test.go +++ b/providers/os/connection/snapshot/blockdevices_test.go @@ -118,6 +118,147 @@ func TestGetMountablePartitionByDevice(t *testing.T) { }) } +func TestFindDevice(t *testing.T) { + t.Run("match by exact name", func(t *testing.T) { + blockEntries := BlockDevices{ + BlockDevices: []BlockDevice{ + {Name: "sda", Children: []BlockDevice{{Uuid: "1234", FsType: "xfs", Label: "ROOT", Name: "sda1", MountPoint: "/"}}}, + {Name: "nvme0n1", Children: []BlockDevice{{Uuid: "12345", FsType: "xfs", Label: "ROOT", Name: "nvmd1n1"}, {Uuid: "12345", FsType: "", Label: "EFI"}}}, + {Name: "sdx", Children: []BlockDevice{{Uuid: "12346", FsType: "xfs", Label: "ROOT", Name: "sdh1"}, {Uuid: "12345", FsType: "", Label: "EFI"}}}, + }, + } + + res, err := blockEntries.FindDevice("/dev/sdx") + require.Nil(t, err) + require.Equal(t, res, blockEntries.BlockDevices[2]) + }) + + t.Run("match by interchangeable name", func(t *testing.T) { + blockEntries := BlockDevices{ + BlockDevices: []BlockDevice{ + {Name: "sda", Children: []BlockDevice{{Uuid: "1234", FsType: "xfs", Label: "ROOT", Name: "sda1", MountPoint: "/"}}}, + {Name: "nvme0n1", Children: []BlockDevice{{Uuid: "12345", FsType: "xfs", Label: "ROOT", Name: "nvmd1n1"}, {Uuid: "12345", FsType: "", Label: "EFI"}}}, + {Name: "xvdc", Children: []BlockDevice{{Uuid: "12346", FsType: "xfs", Label: "ROOT", Name: "sdh1"}, {Uuid: "12345", FsType: "", Label: "EFI"}}}, + }, + } + + res, err := blockEntries.FindDevice("/dev/sdc") + require.Nil(t, err) + require.Equal(t, res, blockEntries.BlockDevices[2]) + }) + + t.Run("no match", func(t *testing.T) { + blockEntries := BlockDevices{ + BlockDevices: []BlockDevice{ + {Name: "sda", Children: []BlockDevice{{Uuid: "1234", FsType: "xfs", Label: "ROOT", Name: "sda1", MountPoint: "/"}}}, + {Name: "nvme0n1", Children: []BlockDevice{{Uuid: "12345", FsType: "xfs", Label: "ROOT", Name: "nvmd1n1"}, {Uuid: "12345", FsType: "", Label: "EFI"}}}, + {Name: "xvdc", Children: []BlockDevice{{Uuid: "12346", FsType: "xfs", Label: "ROOT", Name: "sdh1"}, {Uuid: "12345", FsType: "", Label: "EFI"}}}, + }, + } + + _, err := blockEntries.FindDevice("/dev/sdd") + require.Error(t, err) + require.Contains(t, err.Error(), "no block device found with name") + }) +} + +func TestGetMountablePartition(t *testing.T) { + t.Run("no suitable partition (mounted)", func(t *testing.T) { + block := BlockDevice{ + Name: "sda", + Children: []BlockDevice{ + {Uuid: "1234", FsType: "xfs", Label: "ROOT", Name: "sda1", MountPoint: "/"}, + }, + } + _, err := block.GetMountablePartition() + require.Error(t, err) + require.Contains(t, err.Error(), "no suitable partitions found") + }) + + t.Run("no suitable partition (no fs type)", func(t *testing.T) { + block := BlockDevice{ + Name: "sda", + Children: []BlockDevice{ + {Uuid: "1234", FsType: "", Label: "ROOT", Name: "sda1", MountPoint: ""}, + }, + } + _, err := block.GetMountablePartition() + require.Error(t, err) + require.Contains(t, err.Error(), "no suitable partitions found") + }) + + t.Run("no suitable partition (EFI label)", func(t *testing.T) { + block := BlockDevice{ + Name: "sda", + Children: []BlockDevice{ + {Uuid: "1234", FsType: "xfs", Label: "EFI", Name: "sda1", MountPoint: ""}, + }, + } + _, err := block.GetMountablePartition() + require.Error(t, err) + require.Contains(t, err.Error(), "no suitable partitions found") + }) + + t.Run("no suitable partition (vfat fs type)", func(t *testing.T) { + block := BlockDevice{ + Name: "sda", + Children: []BlockDevice{ + {Uuid: "1234", FsType: "vfat", Label: "", Name: "sda1", MountPoint: ""}, + }, + } + _, err := block.GetMountablePartition() + require.Error(t, err) + require.Contains(t, err.Error(), "no suitable partitions found") + }) + + t.Run("no suitable partition (boot label)", func(t *testing.T) { + block := BlockDevice{ + Name: "sda", + Children: []BlockDevice{ + {Uuid: "1234", FsType: "xfs", Label: "boot", Name: "sda1", MountPoint: ""}, + }, + } + _, err := block.GetMountablePartition() + require.Error(t, err) + require.Contains(t, err.Error(), "no suitable partitions found") + }) + + t.Run("no suitable partition (empty)", func(t *testing.T) { + block := BlockDevice{ + Name: "sda", + Children: []BlockDevice{}, + } + _, err := block.GetMountablePartition() + require.Error(t, err) + require.Contains(t, err.Error(), "no suitable partitions found") + }) + + t.Run("suitable single partition", func(t *testing.T) { + block := BlockDevice{ + Name: "sde", + Children: []BlockDevice{ + {Uuid: "12346", FsType: "xfs", Size: 110, Label: "ROOT", Name: "sde1"}, + }, + } + partition, err := block.GetMountablePartition() + require.Nil(t, err) + require.Equal(t, &PartitionInfo{FsType: "xfs", Name: "/dev/sde1"}, partition) + }) + + t.Run("largest suitable partition", func(t *testing.T) { + block := BlockDevice{ + Name: "sda", + Children: []BlockDevice{ + {Uuid: "12346", FsType: "xfs", Size: 110, Label: "ROOT", Name: "sda1"}, + {Uuid: "12346", FsType: "xfs", Size: 120, Label: "ROOT", Name: "sda2"}, + }, + } + partition, err := block.GetMountablePartition() + require.Nil(t, err) + require.Equal(t, &PartitionInfo{FsType: "xfs", Name: "/dev/sda2"}, partition) + }) +} + func TestGetNonRootBlockEntry(t *testing.T) { blockEntries := BlockDevices{BlockDevices: []BlockDevice{ {Name: "sda", Children: []BlockDevice{{Uuid: "1234", FsType: "xfs", Label: "ROOT", Name: "sda1", MountPoint: "/"}}}, diff --git a/providers/os/connection/snapshot/volumemounter.go b/providers/os/connection/snapshot/volumemounter.go index 6024fbe789..adeac8992b 100644 --- a/providers/os/connection/snapshot/volumemounter.go +++ b/providers/os/connection/snapshot/volumemounter.go @@ -139,7 +139,12 @@ func (m *VolumeMounter) GetMountablePartition(device string) (*PartitionInfo, er // we need to simplify those return blockDevices.GetUnnamedBlockEntry() } - return blockDevices.GetMountablePartitionByDevice(device) + + d, err := blockDevices.FindDevice(device) + if err != nil { + return nil, err + } + return d.GetMountablePartition() } func (m *VolumeMounter) mountVolume(fsInfo *PartitionInfo) error {