Merge pull request #536 from openshift-cherrypick-robot/cherry-pick-5…

…35-to-release-4.15 [release-4.15] OCPBUGS-27003: Allow using symlinks for wiping a device
openshift · Jan 11, 2024 · b50bf96 · b50bf96
2 parents 717528e + 2dae117
commit b50bf96
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 4 deletions.
diff --git a/internal/controllers/vgmanager/controller_test.go b/internal/controllers/vgmanager/controller_test.go
@@ -717,6 +717,12 @@ func testReconcileFailure(ctx context.Context) {
 	})
 
 	By("triggering wipefs failure", func() {
+		evalSymlinks = func(path string) (string, error) {
+			return path, nil
+		}
+		defer func() {
+			evalSymlinks = filepath.EvalSymlinks
+		}()
 		instances.LSBLK.EXPECT().ListBlockDevices().Once().Return([]lsblk.BlockDevice{
 			{Name: "/dev/sda", KName: "/dev/sda", FSType: "ext4"},
 		}, nil)
@@ -727,6 +733,12 @@ func testReconcileFailure(ctx context.Context) {
 	})
 
 	By("triggering lsblk failure after wipefs", func() {
+		evalSymlinks = func(path string) (string, error) {
+			return path, nil
+		}
+		defer func() {
+			evalSymlinks = filepath.EvalSymlinks
+		}()
 		instances.LSBLK.EXPECT().ListBlockDevices().Once().Return([]lsblk.BlockDevice{
 			{Name: "/dev/sda", KName: "/dev/sda", FSType: "ext4"},
 		}, nil)
@@ -738,6 +750,12 @@ func testReconcileFailure(ctx context.Context) {
 	})
 
 	By("triggering failure on fetching new devices to add to volume group", func() {
+		evalSymlinks = func(path string) (string, error) {
+			return path, nil
+		}
+		defer func() {
+			evalSymlinks = filepath.EvalSymlinks
+		}()
 		blockDevices := []lsblk.BlockDevice{
 			{Name: "/dev/xxx", KName: "/dev/xxx", FSType: "ext4"},
 		}

diff --git a/internal/controllers/vgmanager/wipe_devices.go b/internal/controllers/vgmanager/wipe_devices.go
@@ -20,10 +20,14 @@ func (r *Reconciler) wipeDevicesIfNecessary(ctx context.Context, volumeGroup *lv
 
 	wiped := false
 	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
+		diskName, err := evalSymlinks(path)
+		if err != nil {
+			return false, fmt.Errorf("unable to find symlink for disk path %s: %v", path, err)
+		}
+		if isDeviceAlreadyPartOfVG(nodeStatus, diskName, volumeGroup) {
 			continue
 		}
-		deviceWiped, err := r.wipeDevice(ctx, path, blockDevices)
+		deviceWiped, err := r.wipeDevice(ctx, diskName, blockDevices)
 		if err != nil {
 			return false, fmt.Errorf("failed to wipe device %s: %w", path, err)
 		}
@@ -32,10 +36,15 @@ func (r *Reconciler) wipeDevicesIfNecessary(ctx context.Context, volumeGroup *lv
 		}
 	}
 	for _, path := range volumeGroup.Spec.DeviceSelector.OptionalPaths {
-		if isDeviceAlreadyPartOfVG(nodeStatus, path, volumeGroup) {
+		diskName, err := evalSymlinks(path)
+		if err != nil {
+			logger.Info(fmt.Sprintf("skipping wiping optional device %s as unable to find symlink: %v", path, err))
+			continue
+		}
+		if isDeviceAlreadyPartOfVG(nodeStatus, diskName, volumeGroup) {
 			continue
 		}
-		deviceWiped, err := r.wipeDevice(ctx, path, blockDevices)
+		deviceWiped, err := r.wipeDevice(ctx, diskName, blockDevices)
 		if err != nil {
 			logger.Info(fmt.Sprintf("skipping wiping optional device %s: %v", path, err))
 		}

diff --git a/internal/controllers/vgmanager/wipe_devices_test.go b/internal/controllers/vgmanager/wipe_devices_test.go
@@ -2,6 +2,7 @@ package vgmanager
 
 import (
 	"context"
+	"path/filepath"
 	"testing"
 
 	"github.com/openshift/lvm-operator/api/v1alpha1"
@@ -172,6 +173,12 @@ func TestWipeDevices(t *testing.T) {
 	}
 	mockWipefs := wipefsmocks.NewMockWipefs(t)
 	mockDmsetup := dmsetupmocks.NewMockDmsetup(t)
+	evalSymlinks = func(path string) (string, error) {
+		return path, nil
+	}
+	defer func() {
+		evalSymlinks = filepath.EvalSymlinks
+	}()
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			r := &Reconciler{Wipefs: mockWipefs, Dmsetup: mockDmsetup}