CR fixes

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

pkg/controller/datavolume/controller-base.go

@@ -192,6 +192,16 @@ func getIndexArgs() []indexArgs {
 				return nil
 			},
 		},
+		{
+			obj:   &corev1.PersistentVolume{},
+			field: claimStorageClassNameField,
+			extractValue: func(obj client.Object) []string {
+				if pv, ok := obj.(*corev1.PersistentVolume); ok && pv.Status.Phase == corev1.VolumeAvailable {
+					return []string{pv.Spec.StorageClassName}
+				}
+				return nil
+			},
+		},
 	}
 }
 
@@ -206,14 +216,6 @@ func CreateCommonIndexes(mgr manager.Manager) error {
 			return err
 		}
 	}
-	if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &corev1.PersistentVolume{}, claimStorageClassNameField, func(obj client.Object) []string {
-		if pv, ok := obj.(*corev1.PersistentVolume); ok {
-			return []string{pv.Spec.StorageClassName}
-		}
-		return nil
-	}); err != nil {
-		return err
-	}
 	return nil
 }
 
@@ -322,6 +324,7 @@ func addDataVolumeControllerCommonWatches(mgr manager.Manager, dataVolumeControl
 				if storage != nil &&
 					storage.StorageClassName != nil &&
 					*storage.StorageClassName == pv.Spec.StorageClassName &&
+					pv.Status.Phase == corev1.VolumeAvailable &&
 					getDataVolumeOp(mgr.GetLogger(), &dv, mgr.GetClient()) == op {
 					reqs = append(reqs, reconcile.Request{NamespacedName: types.NamespacedName{Name: dv.Name, Namespace: dv.Namespace}})
 				}
@@ -446,11 +449,17 @@ func (r *ReconcilerBase) syncDvPvcState(log logr.Logger, req reconcile.Request,
 		}
 	}
 
-	syncState.pvcSpec, err = renderPvcSpec(r.client, r.recorder, log, dv)
-	if err != nil {
-		r.syncDataVolumeStatusPhaseWithEvent(&syncState, cdiv1.PhaseUnset, nil,
-			Event{corev1.EventTypeWarning, cc.ErrClaimNotValid, err.Error()})
-		return syncState, err
+	if syncState.pvc == nil || syncState.dv.Spec.PVC != nil {
+		syncState.pvcSpec, err = renderPvcSpec(r.client, r.recorder, log, dv)
+		if err != nil {
+			if syncErr := r.syncDataVolumeStatusPhaseWithEvent(&syncState, cdiv1.PhaseUnset, nil,
+				Event{corev1.EventTypeWarning, cc.ErrClaimNotValid, err.Error()}); syncErr != nil {
+				log.Error(syncErr, "failed to sync DataVolume status with event")
+			}
+			return syncState, err
+		}
+	} else {
+		syncState.pvcSpec = syncState.pvc.Spec.DeepCopy()
 	}
 
 	if err := r.handleStaticVolume(&syncState, log); err != nil || syncState.result != nil {

pkg/controller/datavolume/pvc-clone-controller.go

@@ -694,7 +694,7 @@ func (r *PvcCloneReconciler) isSourcePVCPopulated(dv *cdiv1.DataVolume) (bool, e
 }
 
 func (r *PvcCloneReconciler) sourceInUse(dv *cdiv1.DataVolume, eventReason string) (bool, error) {
-	pods, err := cc.GetPodsUsingPVCs(r.client, dv.Spec.Source.PVC.Namespace, sets.New[string](dv.Spec.Source.PVC.Name), false)
+	pods, err := cc.GetPodsUsingPVCs(r.client, dv.Spec.Source.PVC.Namespace, sets.New(dv.Spec.Source.PVC.Name), false)
 	if err != nil {
 		return false, err
 	}

pkg/controller/datavolume/util.go

@@ -25,7 +25,6 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 
-	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
 	storagev1 "k8s.io/api/storage/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -77,17 +76,9 @@ func pvcFromStorage(client client.Client, recorder record.EventRecorder, log log
 		return nil, err
 	}
 	if storageClass == nil {
-		// If SC was not found, look for PV with this SC name
-		pv, err := getPV(client, pvcSpec, dv.Name, dv.Namespace)
-		if err != nil {
+		if err := renderPvcSpecFromAvailablePv(client, pvcSpec); err != nil {
 			return nil, err
 		}
-		if pv != nil {
-			pvcSpec.VolumeMode = pv.Spec.VolumeMode
-			if len(pvcSpec.AccessModes) == 0 {
-				pvcSpec.AccessModes = pv.Spec.AccessModes
-			}
-		}
 		// Not even default storageClass on the cluster, cannot apply the defaults, verify spec is ok
 		if len(pvcSpec.AccessModes) == 0 {
 			log.V(1).Info("Cannot set accessMode for new pvc", "namespace", dv.Namespace, "name", dv.Name)
@@ -160,35 +151,31 @@ func copyStorageAsPvc(log logr.Logger, storage *cdiv1.StorageSpec) *v1.Persisten
 	return pvcSpec
 }
 
-// Gets either the bound PV or an available satisfying PV
-func getPV(c client.Client, pvcSpec *v1.PersistentVolumeClaimSpec, pvcName, pvcNamespace string) (*corev1.PersistentVolume, error) {
+// Renders the PVC spec VolumeMode and AccessModes from an available satisfying PV
+func renderPvcSpecFromAvailablePv(c client.Client, pvcSpec *v1.PersistentVolumeClaimSpec) error {
 	if pvcSpec.StorageClassName == nil {
-		return nil, nil
+		return nil
 	}
 
 	pvList := &v1.PersistentVolumeList{}
 	fields := client.MatchingFields{claimStorageClassNameField: *pvcSpec.StorageClassName}
 	if err := c.List(context.TODO(), pvList, fields); err != nil {
-		return nil, err
-	}
-	for _, pv := range pvList.Items {
-		if pv.Status.Phase == corev1.VolumeBound &&
-			pv.Spec.ClaimRef != nil &&
-			pv.Spec.ClaimRef.Name == pvcName &&
-			pv.Spec.ClaimRef.Namespace == pvcNamespace {
-			return &pv, nil
-		}
+		return err
 	}
 	for _, pv := range pvList.Items {
-		if pv.Status.Phase == corev1.VolumeAvailable {
-			pvc := &corev1.PersistentVolumeClaim{Spec: *pvcSpec}
+		if pv.Status.Phase == v1.VolumeAvailable {
+			pvc := &v1.PersistentVolumeClaim{Spec: *pvcSpec}
 			if err := checkVolumeSatisfyClaim(&pv, pvc); err == nil {
-				return &pv, nil
+				pvcSpec.VolumeMode = pv.Spec.VolumeMode
+				if len(pvcSpec.AccessModes) == 0 {
+					pvcSpec.AccessModes = pv.Spec.AccessModes
+				}
+				return nil
 			}
 		}
 	}
 
-	return nil, nil
+	return nil
 }
 
 func getDefaultVolumeAndAccessMode(c client.Client, storageClass *storagev1.StorageClass) ([]v1.PersistentVolumeAccessMode, *v1.PersistentVolumeMode, error) {

tests/datavolume_test.go

@@ -2421,7 +2421,7 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
 			err = utils.WaitForPersistentVolumeClaimPhase(f.K8sClient, dataVolume.Namespace, v1.ClaimBound, dataVolume.Name)
 			Expect(err).ToNot(HaveOccurred())
 
-			By("waiting for dv succeeeded")
+			By("waiting for dv succeeded")
 			err = utils.WaitForDataVolumePhase(f, dataVolume.Namespace, cdiv1.Succeeded, dataVolume.Name)
 			Expect(err).ToNot(HaveOccurred())
 		},
@@ -2479,7 +2479,7 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
 			err = utils.WaitForPersistentVolumeClaimPhase(f.K8sClient, dataVolume.Namespace, v1.ClaimBound, dataVolume.Name)
 			Expect(err).ToNot(HaveOccurred())
 
-			By("waiting for dv succeeeded")
+			By("waiting for dv succeeded")
 			err = utils.WaitForDataVolumePhase(f, dataVolume.Namespace, cdiv1.Succeeded, dataVolume.Name)
 			Expect(err).ToNot(HaveOccurred())
 		},