[release-v1.56] Do not fail DV reconcile if storage class is not found (

#2861)

Manual backport of #2860, fixing bz #2232347

Currently there is a repeating reconcile error due to exponential
backoff: "DataVolume.storage spec is missing accessMode and no
storageClass to choose profile", however the error is not needed as we
already have SC watch for this case, e.g. when the DV is using the
default SC but no default SC is configured yet.

Note the SC Watch() was mistakenly looking for DVs with unset phase
(like in the main/1.57 branch), but should look for Pending DVs instead.

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

pkg/controller/datavolume/controller-base.go

@@ -265,7 +265,7 @@ func addDataVolumeControllerCommonWatches(mgr manager.Manager, dataVolumeControl
 	if err := dataVolumeController.Watch(&source.Kind{Type: &storagev1.StorageClass{}}, handler.EnqueueRequestsFromMapFunc(
 		func(obj client.Object) (reqs []reconcile.Request) {
 			dvList := &cdiv1.DataVolumeList{}
-			if err := mgr.GetClient().List(context.TODO(), dvList, client.MatchingFields{dvPhaseField: ""}); err != nil {
+			if err := mgr.GetClient().List(context.TODO(), dvList, client.MatchingFields{dvPhaseField: string(cdiv1.Pending)}); err != nil {
 				return
 			}
 			for _, dv := range dvList.Items {
@@ -394,6 +394,10 @@ func (r *ReconcilerBase) syncDvPvcState(log logr.Logger, req reconcile.Request,
 
 	syncState.pvcSpec, err = renderPvcSpec(r.client, r.recorder, log, dv)
 	if err != nil {
+		if errors.Is(err, ErrStorageClassNotFound) {
+			syncState.result = &reconcile.Result{}
+			return syncState, nil
+		}
 		return syncState, err
 	}
 

pkg/controller/datavolume/import-controller_test.go

@@ -89,6 +89,19 @@ var _ = Describe("All DataVolume Tests", func() {
 			}
 		})
 
+		It("Should return nil when storage spec has no AccessModes, no StorageClassName, and no default storage class set", func() {
+			importDataVolume := newImportDataVolumeWithPvc("test-dv", nil)
+			importDataVolume.Spec.Storage = &cdiv1.StorageSpec{}
+
+			reconciler = createImportReconciler(importDataVolume)
+			res, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(res).To(Equal(reconcile.Result{}))
+
+			event := <-reconciler.recorder.(*record.FakeRecorder).Events
+			Expect(event).To(ContainSubstring(MessageErrStorageClassNotFound))
+		})
+
 		It("Should create a PVC on a valid import DV", func() {
 			reconciler = createImportReconciler(NewImportDataVolume("test-dv"))
 			_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})

pkg/controller/datavolume/util.go

@@ -44,6 +44,14 @@ import (
 const (
 	// AnnOwnedByDataVolume annotation has the owner DataVolume name
 	AnnOwnedByDataVolume = "cdi.kubevirt.io/ownedByDataVolume"
+
+	// MessageErrStorageClassNotFound provides a const to indicate the DV storage spec is missing accessMode and no storageClass to choose profile
+	MessageErrStorageClassNotFound = "DataVolume.storage spec is missing accessMode and no storageClass to choose profile"
+)
+
+var (
+	// ErrStorageClassNotFound indicates the DV storage spec is missing accessMode and no storageClass to choose profile
+	ErrStorageClassNotFound = errors.New(MessageErrStorageClassNotFound)
 )
 
 // renderPvcSpec creates a new PVC Spec based on either the dv.spec.pvc or dv.spec.storage section
@@ -82,8 +90,8 @@ func pvcFromStorage(client client.Client, recorder record.EventRecorder, log log
 		// 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)
-			recorder.Eventf(dv, v1.EventTypeWarning, cc.ErrClaimNotValid, "DataVolume.storage spec is missing accessMode and no storageClass to choose profile")
-			return nil, errors.Errorf("DataVolume spec is missing accessMode")
+			recorder.Eventf(dv, v1.EventTypeWarning, cc.ErrClaimNotValid, MessageErrStorageClassNotFound)
+			return nil, ErrStorageClassNotFound
 		}
 	} else {
 		pvcSpec.StorageClassName = &storageClass.Name

tests/datavolume_test.go

@@ -1895,7 +1895,7 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
 				events, err := f.RunKubectlCommand("get", "events", "-n", dataVolume.Namespace, "--field-selector=involvedObject.kind=DataVolume")
 				if err == nil {
 					fmt.Fprintf(GinkgoWriter, "%s", events)
-					return strings.Contains(events, controller.ErrClaimNotValid) && strings.Contains(events, "DataVolume.storage spec is missing accessMode and no storageClass to choose profile")
+					return strings.Contains(events, controller.ErrClaimNotValid) && strings.Contains(events, dvc.MessageErrStorageClassNotFound)
 				}
 				fmt.Fprintf(GinkgoWriter, "ERROR: %s\n", err.Error())
 				return false