Skip to content

Commit

Permalink
fix: don't fail if devices already in vg
Browse files Browse the repository at this point in the history
  • Loading branch information
jeff-roche authored and openshift-cherrypick-robot committed Aug 24, 2023
1 parent 00ae496 commit 05e262e
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 3 deletions.
9 changes: 7 additions & 2 deletions pkg/vgmanager/devices.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ DeviceLoop:
// filterMatchingDevices filters devices based on DeviceSelector.Paths if specified.
func (r *VGReconciler) filterMatchingDevices(blockDevices []internal.BlockDevice, vgs []VolumeGroup, volumeGroup *lvmv1alpha1.LVMVolumeGroup) ([]internal.BlockDevice, error) {
var filteredBlockDevices []internal.BlockDevice
devicesAlreadyInVG := false

if volumeGroup.Spec.DeviceSelector != nil {

Expand All @@ -146,6 +147,7 @@ func (r *VGReconciler) filterMatchingDevices(blockDevices []internal.BlockDevice
// Check if we should skip this device
if blockDevice.DevicePath == "" {
r.Log.Info(fmt.Sprintf("skipping required device that is already part of volume group %s: %s", volumeGroup.Name, path))
devicesAlreadyInVG = true
continue
}

Expand All @@ -160,13 +162,14 @@ func (r *VGReconciler) filterMatchingDevices(blockDevices []internal.BlockDevice

// Check if we should skip this device
if err != nil {
r.Log.Info(fmt.Sprintf("skipping optional device path due to error: %v", err))
r.Log.Info(fmt.Sprintf("skipping optional device path: %v", err))
continue
}

// Check if we should skip this device
if blockDevice.DevicePath == "" {
r.Log.Info(fmt.Sprintf("skipping optional device path that is already part of volume group %s: %s", volumeGroup.Name, path))
devicesAlreadyInVG = true
continue
}

Expand All @@ -176,8 +179,10 @@ func (r *VGReconciler) filterMatchingDevices(blockDevices []internal.BlockDevice
// At least 1 of the optional paths are required if:
// - OptionalPaths was specified AND
// - There were no required paths
// - Devices were not already part of the volume group (meaning this was run after vg creation)
// This guarantees at least 1 device could be found between optionalPaths and paths
if len(filteredBlockDevices) == 0 {
//if len(filteredBlockDevices) == 0 && !devicesAlreadyInVG {
if len(filteredBlockDevices) == 0 && !devicesAlreadyInVG {
return nil, errors.New("at least 1 valid device is required if DeviceSelector paths or optionalPaths are specified")
}
}
Expand Down
14 changes: 13 additions & 1 deletion pkg/vgmanager/devices_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ func TestAvailableDevicesForVG(t *testing.T) {
numOfAvailableDevices: 0,
},
{
description: "vg has device path that is already a part of the existing vg",
description: "vg has device paths that are already a part of the existing vg",
volumeGroup: v1alpha1.LVMVolumeGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "vg1",
Expand All @@ -303,6 +303,9 @@ func TestAvailableDevicesForVG(t *testing.T) {
Paths: []string{
devicePaths["nvme1n1p1"],
},
OptionalPaths: []string{
devicePaths["nvme1n1p2"],
},
},
},
},
Expand All @@ -311,6 +314,7 @@ func TestAvailableDevicesForVG(t *testing.T) {
Name: "vg1",
PVs: []string{
calculateDevicePath(t, "nvme1n1p1"),
calculateDevicePath(t, "nvme1n1p2"),
},
},
},
Expand All @@ -323,6 +327,14 @@ func TestAvailableDevicesForVG(t *testing.T) {
ReadOnly: false,
State: "live",
},
{
Name: "nvme1n1p2",
KName: calculateDevicePath(t, "nvme1n1p2"),
Type: "disk",
Size: "279.4G",
ReadOnly: false,
State: "live",
},
},
numOfAvailableDevices: 0,
expectError: false,
Expand Down

0 comments on commit 05e262e

Please sign in to comment.