Fix DataImportCron reconciliation when first default StorageClass is set

The default SC change condition only handled changes from one SC to another, but did not handle the first default SC being set.

To get the DIC-created DV with the DIC-special RWO preference, it must be deleted (as DV is immutable) and recreated with the right settings from the new default StorageClass.

Signed-off-by: Noam Assouline <nassouli@redhat.com>

Author: noamasu

pkg/controller/dataimportcron-controller.go

@@ -359,19 +359,25 @@ func (r *DataImportCronReconciler) update(ctx context.Context, dataImportCron *c
 		return res, err
 	}
 	if desiredStorageClass != nil {
-		if deleted, err := r.deleteOutdatedPendingPvc(ctx, pvc, desiredStorageClass.Name, dataImportCron.Name); deleted || err != nil {
+		desiredSc := desiredStorageClass.Name
+		if deleted, err := r.deleteOutdatedPendingPvc(ctx, pvc, desiredSc, dataImportCron.Name); deleted || err != nil {
 			return res, err
 		}
 		currentSc, hasCurrent := dataImportCron.Annotations[AnnStorageClass]
-		desiredSc := desiredStorageClass.Name
-		if hasCurrent && currentSc != desiredSc {
-			r.log.Info("Storage class changed, delete most recent source on the old sc as it's no longer the desired", "currentSc", currentSc, "desiredSc", desiredSc)
+		var storageClassChanged bool
+		if hasCurrent {
+			storageClassChanged = currentSc != desiredSc
+		} else {
+			storageClassChanged = len(imports) > 0
+		}
+		if storageClassChanged {
+			r.log.Info("Storage class changed, deleting imports on old SC", "currentSc", currentSc, "desiredSc", desiredSc)
 			if err := r.handleStorageClassChange(ctx, dataImportCron, desiredSc); err != nil {
 				return res, err
 			}
 			return reconcile.Result{RequeueAfter: time.Second}, nil
 		}
-		cc.AddAnnotation(dataImportCron, AnnStorageClass, desiredStorageClass.Name)
+		cc.AddAnnotation(dataImportCron, AnnStorageClass, desiredSc)
 	}
 	format, err := r.getSourceFormat(ctx, desiredStorageClass)
 	if err != nil {

pkg/controller/dataimportcron-controller_test.go

@@ -1844,6 +1844,67 @@ var _ = Describe("All DataImportCron Tests", func() {
 				Expect(dv.Spec.Storage.AccessModes).To(BeEmpty())
 			})
 
+			It("Should delete old DV and requeue when first default StorageClass is set", func() {
+				reconciler = createDataImportCronReconciler()
+				cron = newDataImportCron(cronName)
+				err := reconciler.client.Create(context.TODO(), cron)
+				Expect(err).ToNot(HaveOccurred())
+
+				cc.AddAnnotation(cron, AnnSourceDesiredDigest, testDigest)
+				err = reconciler.client.Update(context.TODO(), cron)
+				Expect(err).ToNot(HaveOccurred())
+				dataSource = &cdiv1.DataSource{}
+
+				// Reconcile creates a DV without any SC
+				_, err = reconciler.Reconcile(context.TODO(), cronReq)
+				Expect(err).ToNot(HaveOccurred())
+				err = reconciler.client.Get(context.TODO(), cronKey, cron)
+				Expect(err).ToNot(HaveOccurred())
+
+				imports := cron.Status.CurrentImports
+				Expect(imports).ToNot(BeEmpty())
+				oldDvName := imports[0].DataVolumeName
+
+				oldDv := &cdiv1.DataVolume{}
+				err = reconciler.client.Get(context.TODO(), dvKey(oldDvName), oldDv)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(cron.Annotations).ToNot(HaveKey(AnnStorageClass))
+
+				err = reconciler.client.Create(context.TODO(), sc)
+				Expect(err).ToNot(HaveOccurred())
+				sp := &cdiv1.StorageProfile{
+					ObjectMeta: metav1.ObjectMeta{Name: storageClassName},
+				}
+				err = reconciler.client.Create(context.TODO(), sp)
+				Expect(err).ToNot(HaveOccurred())
+
+				// Reconcile detects nil-to-non-nil SC change, deletes old DV, requeues
+				res, err := reconciler.Reconcile(context.TODO(), cronReq)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(res.RequeueAfter).To(Equal(time.Second))
+
+				err = reconciler.client.Get(context.TODO(), dvKey(oldDvName), oldDv)
+				Expect(k8serrors.IsNotFound(err)).To(BeTrue())
+
+				err = reconciler.client.Get(context.TODO(), cronKey, cron)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(cron.Annotations[AnnStorageClass]).To(Equal(storageClassName))
+
+				// Reconcile again creates a new DV
+				_, err = reconciler.Reconcile(context.TODO(), cronReq)
+				Expect(err).ToNot(HaveOccurred())
+
+				err = reconciler.client.Get(context.TODO(), cronKey, cron)
+				Expect(err).ToNot(HaveOccurred())
+				imports = cron.Status.CurrentImports
+				Expect(imports).ToNot(BeEmpty())
+				newDvName := imports[0].DataVolumeName
+
+				newDv := &cdiv1.DataVolume{}
+				err = reconciler.client.Get(context.TODO(), dvKey(newDvName), newDv)
+				Expect(err).ToNot(HaveOccurred())
+			})
+
 		})
 	})
 })