Skip to content

Commit

Permalink
fix: allow using symlinks for wiping
Browse files Browse the repository at this point in the history
Signed-off-by: Suleyman Akbas <sakbas@redhat.com>
  • Loading branch information
suleymanakbas91 authored and openshift-cherrypick-robot committed Jan 11, 2024
1 parent 717528e commit 2dae117
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 4 deletions.
18 changes: 18 additions & 0 deletions internal/controllers/vgmanager/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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"},
}
Expand Down
17 changes: 13 additions & 4 deletions internal/controllers/vgmanager/wipe_devices.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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))
}
Expand Down
7 changes: 7 additions & 0 deletions internal/controllers/vgmanager/wipe_devices_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package vgmanager

import (
"context"
"path/filepath"
"testing"

"github.com/openshift/lvm-operator/api/v1alpha1"
Expand Down Expand Up @@ -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}
Expand Down

0 comments on commit 2dae117

Please sign in to comment.