Reconcile DIC only if DataSource is not managed by another DIC (#2295)

* Reconcile DIC only if DataSource is not managed by another DIC Signed-off-by: Arnon Gilboa <agilboa@redhat.com> * CR fixes Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
kubevirt · Jun 2, 2022 · ed4c5bf · ed4c5bf
1 parent ca9a05f
commit ed4c5bf
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 0 deletions.
diff --git a/pkg/controller/dataimportcron-controller.go b/pkg/controller/dataimportcron-controller.go
@@ -60,6 +60,11 @@ import (
 )
 
 const (
+	// ErrDataSourceAlreadyManaged provides a const to indicate DataSource already managed error
+	ErrDataSourceAlreadyManaged = "ErrDataSourceAlreadyManaged"
+	// MessageDataSourceAlreadyManaged provides a const to form DataSource already managed error message
+	MessageDataSourceAlreadyManaged = "DataSource %s is already managed by DataImportCron %s"
+
 	prometheusNsLabel       = "ns"
 	prometheusCronNameLabel = "cron_name"
 )
@@ -114,12 +119,44 @@ func (r *DataImportCronReconciler) Reconcile(ctx context.Context, req reconcile.
 		err := r.cleanup(ctx, req.NamespacedName)
 		return reconcile.Result{}, err
 	}
+	shouldReconcile, err := r.shouldReconcileCron(ctx, dataImportCron)
+	if !shouldReconcile || err != nil {
+		return reconcile.Result{}, err
+	}
 	if err := r.initCron(ctx, dataImportCron); err != nil {
 		return reconcile.Result{}, err
 	}
 	return r.update(ctx, dataImportCron)
 }
 
+func (r *DataImportCronReconciler) shouldReconcileCron(ctx context.Context, cron *cdiv1.DataImportCron) (bool, error) {
+	dataSource := &cdiv1.DataSource{}
+	if err := r.client.Get(ctx, types.NamespacedName{Namespace: cron.Namespace, Name: cron.Spec.ManagedDataSource}, dataSource); err != nil {
+		if k8serrors.IsNotFound(err) {
+			return true, nil
+		}
+		return false, err
+	}
+	dataSourceCronLabel := dataSource.Labels[common.DataImportCronLabel]
+	if dataSourceCronLabel == cron.Name || dataSourceCronLabel == "" {
+		return true, nil
+	}
+	otherCron := &cdiv1.DataImportCron{}
+	if err := r.client.Get(ctx, types.NamespacedName{Namespace: cron.Namespace, Name: dataSourceCronLabel}, otherCron); err != nil {
+		if k8serrors.IsNotFound(err) {
+			return true, nil
+		}
+		return false, err
+	}
+	if otherCron.Spec.ManagedDataSource == dataSource.Name {
+		msg := fmt.Sprintf(MessageDataSourceAlreadyManaged, dataSource.Name, otherCron.Name)
+		r.recorder.Event(cron, corev1.EventTypeWarning, ErrDataSourceAlreadyManaged, msg)
+		r.log.V(3).Info(msg)
+		return false, nil
+	}
+	return true, nil
+}
+
 func (r *DataImportCronReconciler) initCron(ctx context.Context, dataImportCron *cdiv1.DataImportCron) error {
 	if isURLSource(dataImportCron) && !r.cronJobExists(ctx, dataImportCron) {
 		cronJob, err := r.newCronJob(dataImportCron)

diff --git a/pkg/controller/dataimportcron-controller_test.go b/pkg/controller/dataimportcron-controller_test.go
@@ -273,6 +273,44 @@ var _ = Describe("All DataImportCron Tests", func() {
 			Expect(len(dvList.Items)).To(Equal(0))
 		})
 
+		It("Should reconcile only if DataSource is not labeled by another existing DIC", func() {
+			cron = newDataImportCron(cronName)
+			reconciler = createDataImportCronReconciler(cron)
+
+			shouldReconcile, err := reconciler.shouldReconcileCron(context.TODO(), cron)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(shouldReconcile).To(Equal(true))
+
+			_, err = reconciler.Reconcile(context.TODO(), cronReq)
+			Expect(err).ToNot(HaveOccurred())
+
+			dataSource = &cdiv1.DataSource{}
+			err = reconciler.client.Get(context.TODO(), dataSourceKey(cron), dataSource)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(dataSource.Labels[common.DataImportCronLabel]).To(Equal(cron.Name))
+
+			cron1 := newDataImportCron(cronName + "1")
+			shouldReconcile, err = reconciler.shouldReconcileCron(context.TODO(), cron1)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(shouldReconcile).To(Equal(false))
+			event := <-reconciler.recorder.(*record.FakeRecorder).Events
+			Expect(event).To(ContainSubstring(fmt.Sprintf(MessageDataSourceAlreadyManaged, dataSource.Name, cron.Name)))
+
+			dataSource.Labels[common.DataImportCronLabel] = "nosuchdic"
+			err = reconciler.client.Update(context.TODO(), dataSource)
+			Expect(err).ToNot(HaveOccurred())
+			shouldReconcile, err = reconciler.shouldReconcileCron(context.TODO(), cron1)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(shouldReconcile).To(Equal(true))
+
+			dataSource.Labels[common.DataImportCronLabel] = ""
+			err = reconciler.client.Update(context.TODO(), dataSource)
+			Expect(err).ToNot(HaveOccurred())
+			shouldReconcile, err = reconciler.shouldReconcileCron(context.TODO(), cron1)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(shouldReconcile).To(Equal(true))
+		})
+
 		DescribeTable("Should fail when digest", func(digest, errorString string) {
 			cron = newDataImportCron(cronName)
 			cron.Annotations[AnnSourceDesiredDigest] = digest