Do not fail DV reconcile if storage class is not found

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.

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

pkg/controller/datavolume/controller-base.go

@@ -454,6 +454,10 @@ func (r *ReconcilerBase) syncDvPvcState(log logr.Logger, req reconcile.Request,
 			Event{corev1.EventTypeWarning, cc.ErrClaimNotValid, err.Error()}); syncErr != nil {
 			log.Error(syncErr, "failed to sync DataVolume status with event")
 		}
+		if errors.Is(err, ErrStorageClassNotFound) {
+			syncState.result = &reconcile.Result{}
+			return syncState, nil
+		}
 		return syncState, err
 	}
 

pkg/controller/datavolume/import-controller_test.go

@@ -39,6 +39,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/record"
+	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -85,6 +86,23 @@ var _ = Describe("All DataVolume Tests", func() {
 			}
 		})
 
+		DescribeTable("Should return nil when storage spec has no AccessModes and", func(scName *string) {
+			importDataVolume := newImportDataVolumeWithPvc("test-dv", nil)
+			importDataVolume.Spec.Storage = &cdiv1.StorageSpec{
+				StorageClassName: scName,
+			}
+
+			reconciler = createImportReconciler(importDataVolume)
+			_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
+			Expect(err).ToNot(HaveOccurred())
+
+			event := <-reconciler.recorder.(*record.FakeRecorder).Events
+			Expect(event).To(ContainSubstring(MessageErrStorageClassNotFound))
+		},
+			Entry("no StorageClassName, and no default storage class set", nil),
+			Entry("non-existing StorageClassName", pointer.String("nosuch")),
+		)
+
 		It("Should create volumeImportSource if should use cdi populator", func() {
 			scName := "testSC"
 			sc := CreateStorageClassWithProvisioner(scName, map[string]string{AnnDefaultStorageClass: "true"}, map[string]string{}, "csi-plugin")

pkg/controller/datavolume/util.go

@@ -43,6 +43,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
@@ -97,8 +105,8 @@ func renderPvcSpecVolumeModeAndAccessModes(client client.Client, recorder record
 		// 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 errors.Errorf("DataVolume spec is missing accessMode")
+			recorder.Eventf(dv, v1.EventTypeWarning, cc.ErrClaimNotValid, MessageErrStorageClassNotFound)
+			return ErrStorageClassNotFound
 		}
 		return nil
 	}

tests/datavolume_test.go

@@ -2036,7 +2036,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
@@ -2451,19 +2451,19 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
 			Expect(err).ToNot(HaveOccurred())
 
 			By("verifying event occurred")
-			f.ExpectEvent(dataVolume.Namespace).Should(And(ContainSubstring(controller.ErrClaimNotValid), ContainSubstring("DataVolume spec is missing accessMode")))
+			f.ExpectEvent(dataVolume.Namespace).Should(And(ContainSubstring(controller.ErrClaimNotValid), ContainSubstring(dvc.MessageErrStorageClassNotFound)))
 
 			By("verifying conditions")
 			boundCondition := &cdiv1.DataVolumeCondition{
 				Type:    cdiv1.DataVolumeBound,
 				Status:  v1.ConditionUnknown,
-				Message: "DataVolume spec is missing accessMode",
+				Message: dvc.MessageErrStorageClassNotFound,
 				Reason:  controller.ErrClaimNotValid,
 			}
 			readyCondition := &cdiv1.DataVolumeCondition{
 				Type:    cdiv1.DataVolumeReady,
 				Status:  v1.ConditionFalse,
-				Message: "DataVolume spec is missing accessMode",
+				Message: dvc.MessageErrStorageClassNotFound,
 				Reason:  controller.ErrClaimNotValid,
 			}
 			utils.WaitForConditions(f, dataVolume.Name, f.Namespace.Name, timeout, pollingInterval, boundCondition, readyCondition)