fix: wiping should not error on optional paths

Signed-off-by: Suleyman Akbas <[email protected]>
openshift · Oct 2, 2023 · 0abd6de · 0abd6de
1 parent e7498c2
commit 0abd6de
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 2 deletions.
diff --git a/pkg/vgmanager/wipe_devices.go b/pkg/vgmanager/wipe_devices.go
@@ -12,13 +12,14 @@ import (
 )
 
 func (r *VGReconciler) wipeDevicesIfNecessary(ctx context.Context, volumeGroup *lvmv1alpha1.LVMVolumeGroup, nodeStatus *lvmv1alpha1.LVMVolumeGroupNodeStatus, blockDevices []lsblk.BlockDevice) (bool, error) {
+	logger := log.FromContext(ctx)
+
 	if volumeGroup.Spec.DeviceSelector == nil || volumeGroup.Spec.DeviceSelector.ForceWipeDevicesAndDestroyAllData == nil || !*volumeGroup.Spec.DeviceSelector.ForceWipeDevicesAndDestroyAllData {
 		return false, nil
 	}
 
-	allPaths := append(volumeGroup.Spec.DeviceSelector.Paths, volumeGroup.Spec.DeviceSelector.OptionalPaths...)
 	wiped := false
-	for _, path := range allPaths {
+	for _, path := range volumeGroup.Spec.DeviceSelector.Paths {
 		if isDeviceAlreadyPartOfVG(nodeStatus, path, volumeGroup) { // Do not only check the vg devices, but also node status device paths
 			continue
 		}
@@ -30,6 +31,18 @@ func (r *VGReconciler) wipeDevicesIfNecessary(ctx context.Context, volumeGroup *
 			wiped = true
 		}
 	}
+	for _, path := range volumeGroup.Spec.DeviceSelector.OptionalPaths {
+		if isDeviceAlreadyPartOfVG(nodeStatus, path, volumeGroup) {
+			continue
+		}
+		deviceWiped, err := r.wipeDevice(ctx, path, blockDevices)
+		if err != nil {
+			logger.Info(fmt.Sprintf("skipping wiping optional device %s: %v", path, err))
+		}
+		if deviceWiped {
+			wiped = true
+		}
+	}
 
 	return wiped, nil
 }

diff --git a/pkg/vgmanager/wipe_devices_test.go b/pkg/vgmanager/wipe_devices_test.go
@@ -20,6 +20,7 @@ func TestWipeDevices(t *testing.T) {
 		name                 string
 		forceWipeNotEnabled  bool
 		devicePaths          []string
+		optionalDevicePaths  []string
 		blockDevices         []lsblk.BlockDevice
 		nodeStatus           v1alpha1.LVMVolumeGroupNodeStatus
 		wipeCount            int
@@ -111,6 +112,30 @@ func TestWipeDevices(t *testing.T) {
 			wipeCount:            2,
 			removeReferenceCount: 1,
 		},
+		{
+			name:                "One required and one optional device exist in the device list",
+			devicePaths:         []string{"/dev/loop1"},
+			optionalDevicePaths: []string{"/dev/loop2"},
+			blockDevices:        []lsblk.BlockDevice{{KName: "/dev/sda"}, {KName: "/dev/loop1"}, {KName: "/dev/loop2", Children: []lsblk.BlockDevice{{KName: "/dev/loop2p1"}}}},
+			nodeStatus: v1alpha1.LVMVolumeGroupNodeStatus{Spec: v1alpha1.LVMVolumeGroupNodeStatusSpec{
+				LVMVGStatus: []v1alpha1.VGStatus{
+					{Name: "vg1", Devices: []string{"/dev/sda"}}},
+			}},
+			wipeCount:            2,
+			removeReferenceCount: 1,
+		},
+		{
+			name:                "Optional device does not exist in the device list",
+			devicePaths:         []string{"/dev/loop1"},
+			optionalDevicePaths: []string{"/dev/loop2"},
+			blockDevices:        []lsblk.BlockDevice{{KName: "/dev/sda"}, {KName: "/dev/loop1"}},
+			nodeStatus: v1alpha1.LVMVolumeGroupNodeStatus{Spec: v1alpha1.LVMVolumeGroupNodeStatusSpec{
+				LVMVGStatus: []v1alpha1.VGStatus{
+					{Name: "vg1", Devices: []string{"/dev/sda"}}},
+			}},
+			wipeCount:            1,
+			removeReferenceCount: 0,
+		},
 		{
 			name:         "Both devices, one of them is a child, exist in the device list",
 			devicePaths:  []string{"/dev/loop1", "/dev/loop2p1"},
@@ -160,6 +185,7 @@ func TestWipeDevices(t *testing.T) {
 				ObjectMeta: metav1.ObjectMeta{Name: "vg1"},
 				Spec: v1alpha1.LVMVolumeGroupSpec{DeviceSelector: &v1alpha1.DeviceSelector{
 					Paths:                             tt.devicePaths,
+					OptionalPaths:                     tt.optionalDevicePaths,
 					ForceWipeDevicesAndDestroyAllData: ptr.To[bool](!tt.forceWipeNotEnabled),
 				}},
 			}