feat: implement thick provisioning

Signed-off-by: Jakob Möller <[email protected]>
openshift · Apr 5, 2024 · 9dd88ea · 9dd88ea
1 parent d7b09d0
commit 9dd88ea
Show file tree

Hide file tree

Showing 17 changed files with 311 additions and 157 deletions.
diff --git a/api/v1alpha1/lvmcluster_test.go b/api/v1alpha1/lvmcluster_test.go
@@ -63,6 +63,13 @@ var _ = Describe("webhook acceptance tests", func() {
 		Expect(k8sClient.Delete(ctx, resource)).To(Succeed())
 	})
 
+	It("minimum viable create for ReadWriteMany configuration", func(ctx SpecContext) {
+		resource := defaultLVMClusterInUniqueNamespace(ctx)
+		resource.Spec.Storage.DeviceClasses[0].ThinPoolConfig = nil
+		Expect(k8sClient.Create(ctx, resource)).To(Succeed())
+		Expect(k8sClient.Delete(ctx, resource)).To(Succeed())
+	})
+
 	It("duplicate LVMClusters get rejected", func(ctx SpecContext) {
 		generatedName := generateUniqueNameForTestCase(ctx)
 		GinkgoT().Setenv(cluster.OperatorNamespaceEnvVar, generatedName)

diff --git a/api/v1alpha1/lvmcluster_types.go b/api/v1alpha1/lvmcluster_types.go
@@ -79,9 +79,8 @@ type DeviceClass struct {
 	NodeSelector *corev1.NodeSelector `json:"nodeSelector,omitempty"`
 
 	// ThinPoolConfig contains configurations for the thin-pool
-	// +kubebuilder:validation:Required
-	// +required
-	ThinPoolConfig *ThinPoolConfig `json:"thinPoolConfig"`
+	// +optional
+	ThinPoolConfig *ThinPoolConfig `json:"thinPoolConfig,omitempty"`
 
 	// Default is a flag to indicate whether the device-class is the default
 	// +optional

diff --git a/api/v1alpha1/lvmvolumegroup_types.go b/api/v1alpha1/lvmvolumegroup_types.go
@@ -34,9 +34,8 @@ type LVMVolumeGroupSpec struct {
 	NodeSelector *corev1.NodeSelector `json:"nodeSelector,omitempty"`
 
 	// ThinPoolConfig contains configurations for the thin-pool
-	// +kubebuilder:validation:Required
-	// +required
-	ThinPoolConfig *ThinPoolConfig `json:"thinPoolConfig"`
+	// +optional
+	ThinPoolConfig *ThinPoolConfig `json:"thinPoolConfig,omitempty"`
 
 	// Default is a flag to indicate whether the device-class is the default
 	// +optional

diff --git a/bundle/manifests/lvm.topolvm.io_lvmclusters.yaml b/bundle/manifests/lvm.topolvm.io_lvmclusters.yaml
@@ -211,8 +211,6 @@ spec:
                           - name
                           - overprovisionRatio
                           type: object
-                      required:
-                      - thinPoolConfig
                       type: object
                     type: array
                 type: object

diff --git a/bundle/manifests/lvm.topolvm.io_lvmvolumegroups.yaml b/bundle/manifests/lvm.topolvm.io_lvmvolumegroups.yaml
@@ -177,8 +177,6 @@ spec:
                 - name
                 - overprovisionRatio
                 type: object
-            required:
-            - thinPoolConfig
             type: object
           status:
             description: LVMVolumeGroupStatus defines the observed state of LVMVolumeGroup

diff --git a/config/crd/bases/lvm.topolvm.io_lvmclusters.yaml b/config/crd/bases/lvm.topolvm.io_lvmclusters.yaml
@@ -207,8 +207,6 @@ spec:
                           - name
                           - overprovisionRatio
                           type: object
-                      required:
-                      - thinPoolConfig
                       type: object
                     type: array
                 type: object

diff --git a/config/crd/bases/lvm.topolvm.io_lvmvolumegroups.yaml b/config/crd/bases/lvm.topolvm.io_lvmvolumegroups.yaml
@@ -177,8 +177,6 @@ spec:
                 - name
                 - overprovisionRatio
                 type: object
-            required:
-            - thinPoolConfig
             type: object
           status:
             description: LVMVolumeGroupStatus defines the observed state of LVMVolumeGroup

diff --git a/internal/controllers/lvmcluster/resource/topolvm_snapshotclass.go b/internal/controllers/lvmcluster/resource/topolvm_snapshotclass.go
@@ -104,6 +104,9 @@ func getTopolvmSnapshotClasses(lvmCluster *lvmv1alpha1.LVMCluster) []*snapapi.Vo
 	var vsc []*snapapi.VolumeSnapshotClass
 
 	for _, deviceClass := range lvmCluster.Spec.Storage.DeviceClasses {
+		if deviceClass.ThinPoolConfig == nil {
+			continue
+		}
 		snapshotClass := &snapapi.VolumeSnapshotClass{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: GetVolumeSnapshotClassName(deviceClass.Name),

diff --git a/internal/controllers/lvmcluster/resource/vgmanager.go b/internal/controllers/lvmcluster/resource/vgmanager.go
@@ -55,6 +55,9 @@ func (v vgManager) EnsureCreated(r Reconciler, ctx context.Context, lvmCluster *
 		r.GetVGManagerCommand(),
 		r.GetLogPassthroughOptions().VGManager.AsArgs(),
 	)
+	if err := ctrl.SetControllerReference(lvmCluster, &dsTemplate, r.Scheme()); err != nil {
+		return fmt.Errorf("failed to set controller reference on vgManager daemonset %q. %v", dsTemplate.Name, err)
+	}
 
 	// create desired daemonset or update mutable fields on existing one
 	ds := &appsv1.DaemonSet{
@@ -66,14 +69,16 @@ func (v vgManager) EnsureCreated(r Reconciler, ctx context.Context, lvmCluster *
 	// the anonymous mutate function modifies the daemonset object after fetching it.
 	// if the daemonset does not already exist, it creates it, otherwise, it updates it
 	result, err := ctrl.CreateOrUpdate(ctx, r, ds, func() error {
-		if err := ctrl.SetControllerReference(lvmCluster, ds, r.Scheme()); err != nil {
-			return fmt.Errorf("failed to set controller reference on vgManager daemonset %q. %v", dsTemplate.Name, err)
-		}
 		// at creation, deep copy the whole daemonset
 		if ds.CreationTimestamp.IsZero() {
 			dsTemplate.DeepCopyInto(ds)
 			return nil
 		}
+
+		if err := ctrl.SetControllerReference(lvmCluster, ds, r.Scheme()); err != nil {
+			return fmt.Errorf("failed to set controller reference on vgManager daemonset %q. %v", dsTemplate.Name, err)
+		}
+
 		// if update, update only mutable fields
 		initMapIfNil(&ds.ObjectMeta.Labels)
 		initMapIfNil(&ds.Spec.Template.ObjectMeta.Labels)

diff --git a/internal/controllers/lvmcluster/resource/vgmanager_daemonset.go b/internal/controllers/lvmcluster/resource/vgmanager_daemonset.go
@@ -329,12 +329,13 @@ func newVGManagerDaemonset(lvmCluster *lvmv1alpha1.LVMCluster, namespace, vgImag
 				},
 
 				Spec: corev1.PodSpec{
-					PriorityClassName:  constants.PriorityClassNameUserCritical,
-					Volumes:            volumes,
-					Containers:         containers,
-					HostPID:            true,
-					Tolerations:        tolerations,
-					ServiceAccountName: constants.VGManagerServiceAccount,
+					TerminationGracePeriodSeconds: ptr.To(int64(30)),
+					PriorityClassName:             constants.PriorityClassNameUserCritical,
+					Volumes:                       volumes,
+					Containers:                    containers,
+					HostPID:                       true,
+					Tolerations:                   tolerations,
+					ServiceAccountName:            constants.VGManagerServiceAccount,
 				},
 			},
 		},

diff --git a/internal/controllers/vgmanager/controller.go b/internal/controllers/vgmanager/controller.go
@@ -30,6 +30,7 @@ import (
 	"github.com/openshift/lvm-operator/internal/controllers/vgmanager/lvm"
 	"github.com/openshift/lvm-operator/internal/controllers/vgmanager/lvmd"
 	"github.com/openshift/lvm-operator/internal/controllers/vgmanager/wipefs"
+	"k8s.io/utils/ptr"
 
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
@@ -227,14 +228,17 @@ func (r *Reconciler) reconcile(
 
 		logger.V(1).Info("no new available devices discovered, verifying existing setup")
 
-		// since the last reconciliation there could have been corruption on the LVs, so we need to verify them again
-		if err := r.validateLVs(ctx, volumeGroup); err != nil {
-			err := fmt.Errorf("error while validating logical volumes in existing volume group: %w", err)
-			r.WarningEvent(ctx, volumeGroup, EventReasonErrorInconsistentLVs, err)
-			if _, err := r.setVolumeGroupFailedStatus(ctx, volumeGroup, vgs, devices, err); err != nil {
-				logger.Error(err, "failed to set status to failed")
+		// If we are provisioning a thin pool, we need to verify that the thin pool and its LVs are in a consistent state
+		if volumeGroup.Spec.ThinPoolConfig != nil {
+			// since the last reconciliation there could have been corruption on the LVs, so we need to verify them again
+			if err := r.validateLVs(ctx, volumeGroup); err != nil {
+				err := fmt.Errorf("error while validating logical volumes in existing volume group: %w", err)
+				r.WarningEvent(ctx, volumeGroup, EventReasonErrorInconsistentLVs, err)
+				if _, err := r.setVolumeGroupFailedStatus(ctx, volumeGroup, vgs, devices, err); err != nil {
+					logger.Error(err, "failed to set status to failed")
+				}
+				return ctrl.Result{}, err
 			}
-			return ctrl.Result{}, err
 		}
 
 		if err := r.applyLVMDConfig(ctx, volumeGroup, vgs, devices); err != nil {
@@ -272,23 +276,24 @@ func (r *Reconciler) reconcile(
 	}
 
 	// Create thin pool
-	if err = r.addThinPoolToVG(ctx, volumeGroup.Name, volumeGroup.Spec.ThinPoolConfig); err != nil {
-		err := fmt.Errorf("failed to create thin pool %s for volume group %s: %w", volumeGroup.Spec.ThinPoolConfig.Name, volumeGroup.Name, err)
-		r.WarningEvent(ctx, volumeGroup, EventReasonErrorThinPoolCreateOrExtendFailed, err)
-		if _, err := r.setVolumeGroupFailedStatus(ctx, volumeGroup, vgs, devices, err); err != nil {
-			logger.Error(err, "failed to set status to failed")
+	if volumeGroup.Spec.ThinPoolConfig != nil {
+		if err = r.addThinPoolToVG(ctx, volumeGroup.Name, volumeGroup.Spec.ThinPoolConfig); err != nil {
+			err := fmt.Errorf("failed to create thin pool %s for volume group %s: %w", volumeGroup.Spec.ThinPoolConfig.Name, volumeGroup.Name, err)
+			r.WarningEvent(ctx, volumeGroup, EventReasonErrorThinPoolCreateOrExtendFailed, err)
+			if _, err := r.setVolumeGroupFailedStatus(ctx, volumeGroup, vgs, devices, err); err != nil {
+				logger.Error(err, "failed to set status to failed")
+			}
+			return ctrl.Result{}, err
 		}
-		return ctrl.Result{}, err
-	}
-
-	// Validate the LVs created from the Thin-Pool to make sure the adding went as planned.
-	if err := r.validateLVs(ctx, volumeGroup); err != nil {
-		err := fmt.Errorf("error while validating logical volumes in existing volume group: %w", err)
-		r.WarningEvent(ctx, volumeGroup, EventReasonErrorInconsistentLVs, err)
-		if _, err := r.setVolumeGroupFailedStatus(ctx, volumeGroup, vgs, devices, err); err != nil {
-			logger.Error(err, "failed to set status to failed")
+		// Validate the LVs created from the Thin-Pool to make sure the adding went as planned.
+		if err := r.validateLVs(ctx, volumeGroup); err != nil {
+			err := fmt.Errorf("error while validating logical volumes in existing volume group: %w", err)
+			r.WarningEvent(ctx, volumeGroup, EventReasonErrorInconsistentLVs, err)
+			if _, err := r.setVolumeGroupFailedStatus(ctx, volumeGroup, vgs, devices, err); err != nil {
+				logger.Error(err, "failed to set status to failed")
+			}
+			return ctrl.Result{}, err
 		}
-		return ctrl.Result{}, err
 	}
 
 	// refresh list of vgs to be used in status
@@ -347,16 +352,21 @@ func (r *Reconciler) applyLVMDConfig(ctx context.Context, volumeGroup *lvmv1alph
 	}
 	if !found {
 		dc := &lvmd.DeviceClass{
-			Name:           volumeGroup.Name,
-			VolumeGroup:    volumeGroup.Name,
-			Default:        volumeGroup.Spec.Default,
-			ThinPoolConfig: &lvmd.ThinPoolConfig{},
+			Name:        volumeGroup.Name,
+			VolumeGroup: volumeGroup.Name,
+			Default:     volumeGroup.Spec.Default,
 		}
 
 		if volumeGroup.Spec.ThinPoolConfig != nil {
 			dc.Type = lvmd.TypeThin
-			dc.ThinPoolConfig.Name = volumeGroup.Spec.ThinPoolConfig.Name
-			dc.ThinPoolConfig.OverprovisionRatio = float64(volumeGroup.Spec.ThinPoolConfig.OverprovisionRatio)
+			dc.ThinPoolConfig = &lvmd.ThinPoolConfig{
+				Name:               volumeGroup.Spec.ThinPoolConfig.Name,
+				OverprovisionRatio: float64(volumeGroup.Spec.ThinPoolConfig.OverprovisionRatio),
+			}
+		} else {
+			dc.Type = lvmd.TypeThick
+			// set SpareGB to 0 to avoid automatic default to 10GiB
+			dc.SpareGB = ptr.To(uint64(0))
 		}
 
 		lvmdConfig.DeviceClasses = append(lvmdConfig.DeviceClasses, dc)