Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-4.14] OCPBUGS-17865: fix: improve logging around device path validation #390

Merged
merged 1 commit into from
Aug 21, 2023
Merged

[release-4.14] OCPBUGS-17865: fix: improve logging around device path validation #390

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions pkg/vgmanager/devices.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package vgmanager

import (
"errors"
"fmt"
"path/filepath"
"strings"
Expand Down Expand Up @@ -126,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

if volumeGroup.Spec.DeviceSelector != nil {

if err := checkDuplicateDeviceSelectorPaths(volumeGroup.Spec.DeviceSelector); err != nil {
Expand All @@ -143,6 +145,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))
continue
}

Expand All @@ -156,7 +159,14 @@ func (r *VGReconciler) filterMatchingDevices(blockDevices []internal.BlockDevice
blockDevice, err := getValidDevice(path, blockDevices, vgs, volumeGroup)

// Check if we should skip this device
if err != nil || blockDevice.DevicePath == "" {
if err != nil {
r.Log.Info(fmt.Sprintf("skipping optional device path due to error: %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))
continue
}

Expand All @@ -166,9 +176,9 @@ 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
// This guarantees at least 1 device could be found
// This guarantees at least 1 device could be found between optionalPaths and paths
if len(filteredBlockDevices) == 0 {
return nil, fmt.Errorf("at least 1 valid optional device is required if DeviceSelector.OptionalPaths is specified")
return nil, errors.New("at least 1 valid device is required if DeviceSelector paths or optionalPaths are specified")
}
}

Expand Down Expand Up @@ -252,7 +262,7 @@ func getValidDevice(devicePath string, blockDevices []internal.BlockDevice, vgs
// Make sure the symlink exists
diskName, err := filepath.EvalSymlinks(devicePath)
if err != nil {
return internal.BlockDevice{}, fmt.Errorf("unable to find symlink for required disk path %s: %v", devicePath, err)
return internal.BlockDevice{}, fmt.Errorf("unable to find symlink for disk path %s: %v", devicePath, err)
}

// Make sure this isn't a duplicate in the VG
Expand Down