feat: bring more visibility to dynamic device discovery by warnings a…

…nd a new status field

Signed-off-by: Suleyman Akbas <[email protected]>

README.md

@@ -48,6 +48,7 @@ end
 - [Cleanup](#cleanup)
 - [Metrics](#metrics)
 - [Known Limitations](#known-limitations)
+    * [Dynamic Device Discovery](#dynamic-device-discovery)
     * [Unsupported Device Types](#unsupported-device-types)
     * [Single LVMCluster support](#single-lvmcluster-support)
     * [Upgrades from v 4.10 and v4.11](#upgrades-from-v-410-and-v411)
@@ -369,6 +370,16 @@ LVMS provides [TopoLVM metrics](https://github.com/topolvm/topolvm/blob/v0.21.0/
 
 ## Known Limitations
 
+### Dynamic Device Discovery
+
+When a `DeviceSelector` isn't configured for a device class, LVMS operates dynamically, continuously monitoring attached devices on the node and adding them to the volume group if they're unused and supported. However, this approach presents several potential issues:
+
+- LVMS may inadvertently add a device to the volume group that wasn't intended for LVMS.
+- Removing devices could disrupt the volume group.
+- LVMS lacks awareness of volume group changes that could lead to data loss, potentially necessitating manual node remediation.
+
+Given these considerations, it's advised against using LVMS in dynamic discovery mode for production environments.
+
 ### Unsupported Device Types
 
 Here is a list of the types of devices that are excluded by LVMS. To get more information about the devices on your machine and to check if they fall under any of these filters, run:

api/v1alpha1/lvmcluster_webhook.go

@@ -93,7 +93,8 @@ func (v *lvmClusterValidator) ValidateCreate(ctx context.Context, obj runtime.Ob
 		return warnings, err
 	}
 
-	err = v.verifyPathsAreNotEmpty(l)
+	pathWarnings, err := v.verifyPathsAreNotEmpty(l)
+	warnings = append(warnings, pathWarnings...)
 	if err != nil {
 		return warnings, err
 	}
@@ -129,7 +130,8 @@ func (v *lvmClusterValidator) ValidateUpdate(_ context.Context, old, new runtime
 		return warnings, err
 	}
 
-	err = v.verifyPathsAreNotEmpty(l)
+	pathWarnings, err := v.verifyPathsAreNotEmpty(l)
+	warnings = append(warnings, pathWarnings...)
 	if err != nil {
 		return warnings, err
 	}
@@ -298,23 +300,25 @@ func (v *lvmClusterValidator) verifyDeviceClass(l *LVMCluster) (admission.Warnin
 	return nil, nil
 }
 
-func (v *lvmClusterValidator) verifyPathsAreNotEmpty(l *LVMCluster) error {
+func (v *lvmClusterValidator) verifyPathsAreNotEmpty(l *LVMCluster) (admission.Warnings, error) {
 
 	var deviceClassesWithoutPaths []string
 	for _, deviceClass := range l.Spec.Storage.DeviceClasses {
 		if deviceClass.DeviceSelector != nil {
 			if len(deviceClass.DeviceSelector.Paths) == 0 && len(deviceClass.DeviceSelector.OptionalPaths) == 0 {
-				return ErrPathsOrOptionalPathsMandatoryWithNonNilDeviceSelector
+				return nil, ErrPathsOrOptionalPathsMandatoryWithNonNilDeviceSelector
 			}
 		} else {
 			deviceClassesWithoutPaths = append(deviceClassesWithoutPaths, deviceClass.Name)
 		}
 	}
 	if len(l.Spec.Storage.DeviceClasses) > 1 && len(deviceClassesWithoutPaths) > 0 {
-		return fmt.Errorf("%w. Please specify device path(s) under deviceSelector.paths for %s deviceClass(es)", ErrEmptyPathsWithMultipleDeviceClasses, strings.Join(deviceClassesWithoutPaths, `,`))
+		return nil, fmt.Errorf("%w. Please specify device path(s) under deviceSelector.paths for %s deviceClass(es)", ErrEmptyPathsWithMultipleDeviceClasses, strings.Join(deviceClassesWithoutPaths, `,`))
+	} else if len(l.Spec.Storage.DeviceClasses) == 1 && len(deviceClassesWithoutPaths) == 1 {
+		return admission.Warnings{fmt.Sprintf("no device path(s) under deviceSelector.paths was specified for the %s deviceClass, LVMS will actively monitor and dynamically utilize any supported unused devices. This is not recommended for production environments. Please refer to the limitations outlined in the product documentation for further details.", deviceClassesWithoutPaths[0])}, nil
 	}
 
-	return nil
+	return nil, nil
 }
 
 func (v *lvmClusterValidator) verifyAbsolutePath(l *LVMCluster) error {

api/v1alpha1/lvmvolumegroupnodestatus_types.go

@@ -22,6 +22,13 @@ import (
 
 // NOTE: json tags are required.  Any new fields you add must have json tags for the fields to be serialized.
 
+type DeviceDiscoveryPolicy string
+
+const (
+	DeviceDiscoveryPolicyPreConfigured  DeviceDiscoveryPolicy = "PreConfigured"
+	DeviceDiscoveryPolicyRuntimeDynamic DeviceDiscoveryPolicy = "RuntimeDynamic"
+)
+
 // LVMVolumeGroupNodeStatusSpec defines the desired state of LVMVolumeGroupNodeStatus
 type LVMVolumeGroupNodeStatusSpec struct {
 	// NodeStatus contains the per node status of the VG
@@ -53,6 +60,15 @@ type VGStatus struct {
 	// Excluded contains the per node status of applied device exclusions that were picked up via selector,
 	// but were not used for other reasons.
 	Excluded []ExcludedDevice `json:"excluded,omitempty"`
+	// DeviceDiscoveryPolicy is a field to indicate whether the devices are discovered
+	// at runtime or preconfigured through a DeviceSelector
+	// If set to DeviceDiscoveryPolicyPreConfigured, the devices are preconfigured through a DeviceSelector.
+	// If set to DeviceDiscoveryPolicyRuntimeDynamic, the devices will be added to the VG if they are present at runtime.
+	// By default, the value is set to RuntimeDynamic.
+	// +kubebuilder:validation:Enum=None;RuntimeDynamic
+	// +kubebuilder:default=RuntimeDynamic
+	// +kubebuilder:validation:Required
+	DeviceDiscoveryPolicy DeviceDiscoveryPolicy `json:"deviceDiscoveryPolicy,omitempty"`
 }
 
 type ExcludedDevice struct {

bundle/manifests/lvm.topolvm.io_lvmclusters.yaml

@@ -301,6 +301,18 @@ spec:
                         description: NodeStatus defines the observed state of the
                           deviceclass on the node
                         properties:
+                          deviceDiscoveryPolicy:
+                            default: RuntimeDynamic
+                            description: |-
+                              DeviceDiscoveryPolicy is a field to indicate whether the devices are discovered
+                              at runtime or preconfigured through a DeviceSelector
+                              If set to DeviceDiscoveryPolicyPreConfigured, the devices are preconfigured through a DeviceSelector.
+                              If set to DeviceDiscoveryPolicyRuntimeDynamic, the devices will be added to the VG if they are present at runtime.
+                              By default, the value is set to RuntimeDynamic.
+                            enum:
+                            - None
+                            - RuntimeDynamic
+                            type: string
                           devices:
                             description: Devices is the list of devices used by the
                               volume group

bundle/manifests/lvm.topolvm.io_lvmvolumegroupnodestatuses.yaml

@@ -45,6 +45,18 @@ spec:
                 description: NodeStatus contains the per node status of the VG
                 items:
                   properties:
+                    deviceDiscoveryPolicy:
+                      default: RuntimeDynamic
+                      description: |-
+                        DeviceDiscoveryPolicy is a field to indicate whether the devices are discovered
+                        at runtime or preconfigured through a DeviceSelector
+                        If set to DeviceDiscoveryPolicyPreConfigured, the devices are preconfigured through a DeviceSelector.
+                        If set to DeviceDiscoveryPolicyRuntimeDynamic, the devices will be added to the VG if they are present at runtime.
+                        By default, the value is set to RuntimeDynamic.
+                      enum:
+                      - None
+                      - RuntimeDynamic
+                      type: string
                     devices:
                       description: Devices is the list of devices used by the volume
                         group

config/crd/bases/lvm.topolvm.io_lvmclusters.yaml

@@ -297,6 +297,18 @@ spec:
                         description: NodeStatus defines the observed state of the
                           deviceclass on the node
                         properties:
+                          deviceDiscoveryPolicy:
+                            default: RuntimeDynamic
+                            description: |-
+                              DeviceDiscoveryPolicy is a field to indicate whether the devices are discovered
+                              at runtime or preconfigured through a DeviceSelector
+                              If set to DeviceDiscoveryPolicyPreConfigured, the devices are preconfigured through a DeviceSelector.
+                              If set to DeviceDiscoveryPolicyRuntimeDynamic, the devices will be added to the VG if they are present at runtime.
+                              By default, the value is set to RuntimeDynamic.
+                            enum:
+                            - None
+                            - RuntimeDynamic
+                            type: string
                           devices:
                             description: Devices is the list of devices used by the
                               volume group

config/crd/bases/lvm.topolvm.io_lvmvolumegroupnodestatuses.yaml

@@ -45,6 +45,18 @@ spec:
                 description: NodeStatus contains the per node status of the VG
                 items:
                   properties:
+                    deviceDiscoveryPolicy:
+                      default: RuntimeDynamic
+                      description: |-
+                        DeviceDiscoveryPolicy is a field to indicate whether the devices are discovered
+                        at runtime or preconfigured through a DeviceSelector
+                        If set to DeviceDiscoveryPolicyPreConfigured, the devices are preconfigured through a DeviceSelector.
+                        If set to DeviceDiscoveryPolicyRuntimeDynamic, the devices will be added to the VG if they are present at runtime.
+                        By default, the value is set to RuntimeDynamic.
+                      enum:
+                      - None
+                      - RuntimeDynamic
+                      type: string
                     devices:
                       description: Devices is the list of devices used by the volume
                         group

hack/verify-catalog.sh

@@ -2,7 +2,7 @@
 set -euo pipefail
 
 function print_failure {
-    echo "There are unexpected changes to the tree when running 'make catalog-render'. Please"
+    echo "There are unexpected changes to the tree when running 'make catalog'. Please"
     echo "run these commands locally and double-check the Git repository for unexpected changes which may"
     echo "need to be committed."
     exit 1

internal/controllers/vgmanager/controller_test.go

@@ -266,8 +266,9 @@ func testVGWithLocalDevice(ctx context.Context, vgTemplate lvmv1alpha1.LVMVolume
 		Expect(instances.client.Get(ctx, client.ObjectKeyFromObject(nodeStatus), nodeStatus)).To(Succeed())
 		Expect(nodeStatus.Spec.LVMVGStatus).ToNot(BeEmpty())
 		Expect(nodeStatus.Spec.LVMVGStatus).To(ContainElement(lvmv1alpha1.VGStatus{
-			Name:   vg.GetName(),
-			Status: lvmv1alpha1.VGStatusProgressing,
+			Name:                  vg.GetName(),
+			Status:                lvmv1alpha1.VGStatusProgressing,
+			DeviceDiscoveryPolicy: lvmv1alpha1.DeviceDiscoveryPolicyPreConfigured,
 		}))
 	})
 
@@ -365,9 +366,10 @@ func testVGWithLocalDevice(ctx context.Context, vgTemplate lvmv1alpha1.LVMVolume
 		Expect(instances.client.Get(ctx, client.ObjectKeyFromObject(nodeStatus), nodeStatus)).To(Succeed())
 		Expect(nodeStatus.Spec.LVMVGStatus).ToNot(BeEmpty())
 		Expect(nodeStatus.Spec.LVMVGStatus).To(ContainElement(lvmv1alpha1.VGStatus{
-			Name:    vg.GetName(),
-			Status:  lvmv1alpha1.VGStatusReady,
-			Devices: []string{device},
+			Name:                  vg.GetName(),
+			Status:                lvmv1alpha1.VGStatusReady,
+			Devices:               []string{device},
+			DeviceDiscoveryPolicy: lvmv1alpha1.DeviceDiscoveryPolicyPreConfigured,
 		}))
 		oldReadyGeneration = nodeStatus.GetGeneration()
 	})
@@ -422,9 +424,10 @@ func testVGWithLocalDevice(ctx context.Context, vgTemplate lvmv1alpha1.LVMVolume
 		Expect(instances.client.Get(ctx, client.ObjectKeyFromObject(nodeStatus), nodeStatus)).To(Succeed())
 		Expect(nodeStatus.Spec.LVMVGStatus).ToNot(BeEmpty())
 		Expect(nodeStatus.Spec.LVMVGStatus).To(ContainElement(lvmv1alpha1.VGStatus{
-			Name:    vg.GetName(),
-			Status:  lvmv1alpha1.VGStatusReady,
-			Devices: []string{device},
+			Name:                  vg.GetName(),
+			Status:                lvmv1alpha1.VGStatusReady,
+			Devices:               []string{device},
+			DeviceDiscoveryPolicy: lvmv1alpha1.DeviceDiscoveryPolicyPreConfigured,
 		}))
 		Expect(oldReadyGeneration).To(Equal(nodeStatus.GetGeneration()))
 	})

internal/controllers/vgmanager/status.go

@@ -81,6 +81,12 @@ func (r *Reconciler) setVolumeGroupFailedStatus(ctx context.Context, vg *lvmv1al
 func (r *Reconciler) setVolumeGroupStatus(ctx context.Context, vg *lvmv1alpha1.LVMVolumeGroup, status *lvmv1alpha1.VGStatus) (bool, error) {
 	logger := log.FromContext(ctx).WithValues("VolumeGroup", client.ObjectKeyFromObject(vg))
 
+	if vg.Spec.DeviceSelector == nil {
+		status.DeviceDiscoveryPolicy = lvmv1alpha1.DeviceDiscoveryPolicyRuntimeDynamic
+	} else {
+		status.DeviceDiscoveryPolicy = lvmv1alpha1.DeviceDiscoveryPolicyPreConfigured
+	}
+
 	// Get LVMVolumeGroupNodeStatus and set the relevant VGStatus
 	nodeStatus := r.getLVMVolumeGroupNodeStatus()