From 63db74d30528279c13f61b7e9e4df8f152a436f5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakob=20M=C3=B6ller?= <jmoller@redhat.com>
Date: Tue, 10 Sep 2024 16:59:15 +0200
Subject: [PATCH] fix: ensure uniq available devices
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Jakob Möller <jmoller@redhat.com>
---
 internal/controllers/vgmanager/devices.go     | 18 +++++--
 .../controllers/vgmanager/devices_test.go     | 53 +++++++++++++++++++
 2 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/internal/controllers/vgmanager/devices.go b/internal/controllers/vgmanager/devices.go
index af6b6b4e5..0fe63b310 100644
--- a/internal/controllers/vgmanager/devices.go
+++ b/internal/controllers/vgmanager/devices.go
@@ -141,14 +141,21 @@ func (f FilteredBlockDevices) FilterErrors(dev string) []error {
 func filterDevices(ctx context.Context, devices []lsblk.BlockDevice, resolver *symlinkResolver.Resolver, filters filter.Filters) FilteredBlockDevices {
 	logger := log.FromContext(ctx)
 
-	var availableDevices []lsblk.BlockDevice
+	availableByKName := make(map[string]lsblk.BlockDevice)
 	excludedByKName := make(map[string]FilteredBlockDevice)
 
 	for _, device := range devices {
 		// check for partitions recursively
 		if device.HasChildren() {
 			filteredChildDevices := filterDevices(ctx, device.Children, resolver, filters)
-			availableDevices = append(availableDevices, filteredChildDevices.Available...)
+			for _, available := range filteredChildDevices.Available {
+				if _, exists := availableByKName[available.KName]; exists {
+					logger.WithValues("device.KName", available.KName).
+						V(3).Info("skipping duplicate available device, this can happen for multipath / dmsetup devices")
+				} else {
+					availableByKName[available.KName] = available
+				}
+			}
 			for _, excludedChildDevice := range filteredChildDevices.Excluded {
 				if excluded, ok := excludedByKName[excludedChildDevice.KName]; ok {
 					excluded.FilterErrors = append(excluded.FilterErrors, excludedChildDevice.FilterErrors...)
@@ -167,7 +174,7 @@ func filterDevices(ctx context.Context, devices []lsblk.BlockDevice, resolver *s
 			}
 		}
 		if len(filterErrs) == 0 {
-			availableDevices = append(availableDevices, device)
+			availableByKName[device.KName] = device
 			continue
 		}
 
@@ -184,6 +191,11 @@ func filterDevices(ctx context.Context, devices []lsblk.BlockDevice, resolver *s
 		excluded = append(excluded, device)
 	}
 
+	availableDevices := make([]lsblk.BlockDevice, 0, len(availableByKName))
+	for _, device := range availableByKName {
+		availableDevices = append(availableDevices, device)
+	}
+
 	return FilteredBlockDevices{
 		Available: availableDevices,
 		Excluded:  excluded,
diff --git a/internal/controllers/vgmanager/devices_test.go b/internal/controllers/vgmanager/devices_test.go
index 32d021a95..1957525c2 100644
--- a/internal/controllers/vgmanager/devices_test.go
+++ b/internal/controllers/vgmanager/devices_test.go
@@ -26,6 +26,7 @@ func Test_getNewDevicesToBeAdded(t *testing.T) {
 	devicePaths = make(map[string]v1alpha1.DevicePath)
 	devicePaths["nvme1n1p1"] = v1alpha1.DevicePath(fmt.Sprintf("%s/%s", tmpDir, "nvme1n1p1"))
 	devicePaths["nvme1n1p2"] = v1alpha1.DevicePath(fmt.Sprintf("%s/%s", tmpDir, "nvme1n1p2"))
+	devicePaths["md1"] = v1alpha1.DevicePath(fmt.Sprintf("%s/%s", tmpDir, "md1"))
 	for _, path := range devicePaths {
 		err := os.Mkdir(path.Unresolved(), 0755)
 		if err != nil {
@@ -588,6 +589,58 @@ func Test_getNewDevicesToBeAdded(t *testing.T) {
 			},
 			numOfAvailableDevices: 0,
 		},
+		{
+			description: "vg RAID with multiple paths under different devices",
+			volumeGroup: v1alpha1.LVMVolumeGroup{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "vg1",
+				},
+				Spec: v1alpha1.LVMVolumeGroupSpec{
+					DeviceSelector: &v1alpha1.DeviceSelector{
+						Paths: []v1alpha1.DevicePath{
+							devicePaths["md1"],
+						},
+					},
+				},
+			},
+			existingBlockDevices: []lsblk.BlockDevice{
+				{
+					Name:     "nvme1n1p1",
+					Type:     "disk",
+					Size:     "50G",
+					ReadOnly: false,
+					State:    "live",
+					KName:    calculateDevicePath(t, "nvme1n1p1"),
+					Serial:   "vol071204a42d92d52a2",
+					FSType:   "linux_raid_member",
+					Children: []lsblk.BlockDevice{{
+						Name:     "/dev/md1",
+						Type:     "raid1",
+						Size:     "50G",
+						ReadOnly: false,
+						KName:    calculateDevicePath(t, "md1"),
+					}},
+				},
+				{
+					Name:     "nvme1n1p2",
+					Type:     "disk",
+					Size:     "50G",
+					ReadOnly: false,
+					State:    "live",
+					KName:    calculateDevicePath(t, "nvme1n1p2"),
+					Serial:   "vol07993aa64fe3c5316",
+					FSType:   "linux_raid_member",
+					Children: []lsblk.BlockDevice{{
+						Name:     "/dev/md1",
+						Type:     "raid1",
+						Size:     "50G",
+						ReadOnly: false,
+						KName:    calculateDevicePath(t, "md1"),
+					}},
+				},
+			},
+			numOfAvailableDevices: 1,
+		},
 	}
 
 	for _, tc := range testCases {