Skip to content

Commit

Permalink
[controller] Fix block device filtering and add relevant tests (#19)
Browse files Browse the repository at this point in the history
Signed-off-by: Aleksandr Zimin <[email protected]>
  • Loading branch information
AleksZimin authored Jan 31, 2024
1 parent 1f71a09 commit 88652f9
Show file tree
Hide file tree
Showing 4 changed files with 311 additions and 12 deletions.
15 changes: 9 additions & 6 deletions images/agent/pkg/controller/block_device.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func GetBlockDeviceCandidates(log logger.Logger, cfg config.Options, metrics mon
return nil, fmt.Errorf("unable to GetBlockDevices, err: %w", err)
}

filteredDevices, err := filterDevices(log, devices)
filteredDevices, err := FilterDevices(log, devices)
if err != nil {
log.Error(err, "[GetBlockDeviceCandidates] unable to filter devices")
return nil, err
Expand Down Expand Up @@ -327,12 +327,11 @@ func GetBlockDeviceCandidates(log logger.Logger, cfg config.Options, metrics mon
return candidates, nil
}

func filterDevices(log logger.Logger, devices []internal.Device) ([]internal.Device, error) {
func FilterDevices(log logger.Logger, devices []internal.Device) ([]internal.Device, error) {
log.Trace(fmt.Sprintf("[filterDevices] devices before type filtration: %+v", devices))

validTypes := make([]internal.Device, 0, len(devices))

// We do first filtering to avoid block of devices by "isParent" condition with FSType "LVM2_member".
for _, device := range devices {
if !strings.HasPrefix(device.Name, internal.DRBDName) &&
hasValidType(device.Type) &&
Expand All @@ -344,13 +343,17 @@ func filterDevices(log logger.Logger, devices []internal.Device) ([]internal.Dev
log.Trace(fmt.Sprintf("[filterDevices] devices after type filtration: %+v", validTypes))

pkNames := make(map[string]struct{}, len(validTypes))
for _, device := range validTypes {
pkNames[device.PkName] = struct{}{}
for _, device := range devices {
if device.PkName != "" {
log.Trace(fmt.Sprintf("[filterDevices] find parent %s for child : %+v.", device.PkName, device))
pkNames[device.PkName] = struct{}{}
}
}
log.Trace(fmt.Sprintf("[filterDevices] pkNames: %+v", pkNames))

filtered := make([]internal.Device, 0, len(validTypes))
for _, device := range validTypes {
if !isParent(device.KName, pkNames) {
if !isParent(device.KName, pkNames) || device.FSType == internal.LVMFSType {
validSize, err := hasValidSize(device.Size)
if err != nil {
return nil, err
Expand Down
297 changes: 296 additions & 1 deletion images/agent/pkg/controller/block_device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@ package controller

import (
"fmt"
"k8s.io/apimachinery/pkg/api/resource"
"sds-node-configurator/api/v1alpha1"
"sds-node-configurator/internal"
"sds-node-configurator/pkg/logger"
"sds-node-configurator/pkg/utils"
"testing"

"k8s.io/apimachinery/pkg/api/resource"

"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -200,4 +203,296 @@ func TestBlockDeviceCtrl(t *testing.T) {
assert.Equal(t, expected[i], actual)
}
})

t.Run("validateTestLSBLKOutput", func(t *testing.T) {
log, err := logger.NewLogger("1")
if err != nil {
t.Fatal(err)
}
testLsblkOutputBytes := []byte(testLsblkOutput)
devices, err := utils.UnmarshalDevices(testLsblkOutputBytes)
if assert.NoError(t, err) {
assert.Equal(t, 19, len(devices))
}
filteredDevices, err := FilterDevices(*log, devices)

for i, device := range filteredDevices {
println("Filtered device: ", device.Name)
switch i {
case 0:
assert.Equal(t, "/dev/md1", device.Name)
assert.False(t, CheckConsumable(device))
case 1:
assert.Equal(t, "/dev/md127", device.Name)
assert.False(t, CheckConsumable(device))
case 2:
assert.Equal(t, "/dev/nvme4n1", device.Name)
assert.True(t, CheckConsumable(device))
case 3:
assert.Equal(t, "/dev/nvme5n1", device.Name)
assert.True(t, CheckConsumable(device))
}

}
if assert.NoError(t, err) {
assert.Equal(t, 4, len(filteredDevices))
}
})

}

var (
testLsblkOutput = `
{
"blockdevices": [
{
"name": "/dev/md0",
"mountpoint": "/boot",
"partuuid": null,
"hotplug": false,
"model": null,
"serial": null,
"size": "1022M",
"fstype": "ext3",
"type": "raid1",
"wwn": null,
"kname": "/dev/md0",
"pkname": "/dev/nvme3n1p2"
},{
"name": "/dev/md1",
"mountpoint": null,
"partuuid": null,
"hotplug": false,
"model": null,
"serial": null,
"size": "892.9G",
"fstype": "LVM2_member",
"type": "raid1",
"wwn": null,
"kname": "/dev/md1",
"pkname": "/dev/nvme3n1p3"
},{
"name": "/dev/mapper/vg0-root",
"mountpoint": "/",
"partuuid": null,
"hotplug": false,
"model": null,
"serial": null,
"size": "150G",
"fstype": "ext4",
"type": "lvm",
"wwn": null,
"kname": "/dev/dm-0",
"pkname": "/dev/md1"
},{
"name": "/dev/md127",
"mountpoint": null,
"partuuid": null,
"hotplug": false,
"model": null,
"serial": null,
"size": "3.3T",
"fstype": "LVM2_member",
"type": "raid1",
"wwn": null,
"kname": "/dev/md127",
"pkname": null
},{
"name": "/dev/mapper/vg0-pvc--nnnn--nnnnn--nnnn--nnnn--nnnnn_00000",
"mountpoint": null,
"partuuid": null,
"hotplug": false,
"model": null,
"serial": null,
"size": "1G",
"fstype": "drbd",
"type": "lvm",
"wwn": null,
"kname": "/dev/dm-1",
"pkname": "/dev/md127"
},{
"name": "/dev/nvme1n1",
"mountpoint": null,
"partuuid": null,
"hotplug": false,
"model": "Micron",
"serial": "000000BBBBB",
"size": "1.7T",
"fstype": "ceph_bluestore",
"type": "disk",
"wwn": "eui.000000000000000100aaaaa",
"kname": "/dev/nvme1n1",
"pkname": null
},{
"name": "/dev/nvme4n1",
"mountpoint": null,
"partuuid": null,
"hotplug": false,
"model": "Micron",
"serial": "000000AAAA",
"size": "1.7T",
"fstype": null,
"type": "disk",
"wwn": "eui.000000000000000100aaaab",
"kname": "/dev/nvme4n1",
"pkname": null
},{
"name": "/dev/nvme5n1",
"mountpoint": null,
"partuuid": null,
"hotplug": false,
"model": "Micron",
"serial": "000000AAAAA",
"size": "1.7T",
"fstype": null,
"type": "disk",
"wwn": "eui.000000000000000100aaaaac",
"kname": "/dev/nvme5n1",
"pkname": null
},{
"name": "/dev/nvme0n1",
"mountpoint": null,
"partuuid": null,
"hotplug": false,
"model": "Micron",
"serial": "000000AAAAAB",
"size": "1.7T",
"fstype": "ceph_bluestore",
"type": "disk",
"wwn": "eui.000000000000000100aaaaab",
"kname": "/dev/nvme0n1",
"pkname": null
},{
"name": "/dev/nvme2n1",
"mountpoint": null,
"partuuid": null,
"hotplug": false,
"model": "SAMSUNG",
"serial": "000000AAAAAC",
"size": "894.3G",
"fstype": null,
"type": "disk",
"wwn": "eui.000000000000000100aaaaad",
"kname": "/dev/nvme2n1",
"pkname": null
},{
"name": "/dev/nvme3n1",
"mountpoint": null,
"partuuid": null,
"hotplug": false,
"model": "SAMSUNG",
"serial": "000000AAAAAD",
"size": "894.3G",
"fstype": null,
"type": "disk",
"wwn": "eui.000000000000000100aaaaad",
"kname": "/dev/nvme3n1",
"pkname": null
},{
"name": "/dev/nvme2n1p1",
"mountpoint": null,
"partuuid": "11111111-e2bb-47fb-8cc1-xxxxxxx",
"hotplug": false,
"model": null,
"serial": null,
"size": "256M",
"fstype": "vfat",
"type": "part",
"wwn": "eui.000000000000000100aaaaae",
"kname": "/dev/nvme2n1p1",
"pkname": "/dev/nvme2n1"
},{
"name": "/dev/nvme2n1p2",
"mountpoint": null,
"partuuid": "11111111-d3d4-416a-ac76-xxxxxxx",
"hotplug": false,
"model": null,
"serial": null,
"size": "1G",
"fstype": "linux_raid_member",
"type": "part",
"wwn": "eui.000000000000000100aaaaaf",
"kname": "/dev/nvme2n1p2",
"pkname": "/dev/nvme2n1"
},{
"name": "/dev/nvme2n1p3",
"mountpoint": null,
"partuuid": "11111111-3677-4eb2-9491-xxxxxxx",
"hotplug": false,
"model": null,
"serial": null,
"size": "893G",
"fstype": "linux_raid_member",
"type": "part",
"wwn": "eui.000000000000000100aaaaag",
"kname": "/dev/nvme2n1p3",
"pkname": "/dev/nvme2n1"
},{
"name": "/dev/nvme3n1p1",
"mountpoint": "/boot/efi",
"partuuid": "11111111-2965-47d3-8983-xxxxxxx",
"hotplug": false,
"model": null,
"serial": null,
"size": "256M",
"fstype": "vfat",
"type": "part",
"wwn": "eui.000000000000000100aaaaah",
"kname": "/dev/nvme3n1p1",
"pkname": "/dev/nvme3n1"
},{
"name": "/dev/nvme3n1p2",
"mountpoint": null,
"partuuid": "11111111-7fa2-4318-91c4-xxxxxxx",
"hotplug": false,
"model": null,
"serial": null,
"size": "1G",
"fstype": "linux_raid_member",
"type": "part",
"wwn": "eui.000000000000000100aaaaabs",
"kname": "/dev/nvme3n1p2",
"pkname": "/dev/nvme3n1"
},{
"name": "/dev/nvme3n1p3",
"mountpoint": null,
"partuuid": "11111111-734d-45f4-b60e-xxxxxxx",
"hotplug": false,
"model": null,
"serial": null,
"size": "893G",
"fstype": "linux_raid_member",
"type": "part",
"wwn": "eui.000000000000000100aaaaaccx",
"kname": "/dev/nvme3n1p3",
"pkname": "/dev/nvme3n1"
},{
"name": "/dev/sda",
"mountpoint": null,
"partuuid": null,
"hotplug": false,
"model": "Virtual_Disk",
"serial": "6006",
"size": "50G",
"fstype": null,
"type": "disk",
"wwn": "0x6006",
"kname": "/dev/sda",
"pkname": null
},{
"name": "/dev/sda1",
"mountpoint": "/data",
"partuuid": "11111-01",
"hotplug": false,
"model": null,
"serial": null,
"size": "50G",
"fstype": "ext4",
"type": "part",
"wwn": "0x6006",
"kname": "/dev/sda1",
"pkname": "/dev/sda"
}
]
}`
)
4 changes: 2 additions & 2 deletions images/agent/pkg/utils/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func GetBlockDevices() ([]internal.Device, string, error) {
return nil, cmd.String(), fmt.Errorf("unable to GetBlockDevices, err: %w", err)
}

devices, err := unmarshalDevices(outs.Bytes())
devices, err := UnmarshalDevices(outs.Bytes())
if err != nil {
return nil, cmd.String(), fmt.Errorf("unable to unmarshal devices, err: %w", err)
}
Expand Down Expand Up @@ -315,7 +315,7 @@ func RemovePV(pvNames []string) (string, error) {
return cmd.String(), nil
}

func unmarshalDevices(out []byte) ([]internal.Device, error) {
func UnmarshalDevices(out []byte) ([]internal.Device, error) {
var devices internal.Devices
if err := json.Unmarshal(out, &devices); err != nil {
return nil, err
Expand Down
Loading

0 comments on commit 88652f9

Please sign in to comment.