enable empty schedule in dataimportcron

Signed-off-by: Ido Aharon <iaharon@redhat.com>
kubevirt · May 16, 2023 · c8549f3 · c8549f3
1 parent 4b2d633
commit c8549f3
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 13 deletions.
diff --git a/pkg/apiserver/webhooks/dataimportcron-validate.go b/pkg/apiserver/webhooks/dataimportcron-validate.go
@@ -116,13 +116,15 @@ func (wh *dataImportCronValidatingWebhook) validateDataImportCronSpec(request *a
 		return causes
 	}
 
-	if _, err := cronexpr.Parse(spec.Schedule); err != nil {
-		causes = append(causes, metav1.StatusCause{
-			Type:    metav1.CauseTypeFieldValueInvalid,
-			Message: "Illegal cron schedule",
-			Field:   field.Child("Schedule").String(),
-		})
-		return causes
+	if spec.Schedule != "" {
+		if _, err := cronexpr.Parse(spec.Schedule); err != nil {
+			causes = append(causes, metav1.StatusCause{
+				Type:    metav1.CauseTypeFieldValueInvalid,
+				Message: "Illegal cron schedule",
+				Field:   field.Child("Schedule").String(),
+			})
+			return causes
+		}
 	}
 
 	if spec.ImportsToKeep != nil && *spec.ImportsToKeep < 0 {

diff --git a/pkg/apiserver/webhooks/dataimportcron-validate_test.go b/pkg/apiserver/webhooks/dataimportcron-validate_test.go
@@ -121,6 +121,12 @@ var _ = Describe("Validating Webhook", func() {
 			resp := validateDataImportCronCreate(cron)
 			Expect(resp.Allowed).To(BeFalse())
 		})
+		It("should allow DataImportCron with empty cron schedule", func() {
+			cron := newDataImportCron(cdiv1.DataVolumeSourceRegistry{URL: &testRegistryURL})
+			cron.Spec.Schedule = ""
+			resp := validateDataImportCronCreate(cron)
+			Expect(resp.Allowed).To(BeTrue())
+		})
 		It("should reject DataImportCron with illegal ManagedDataSource on create", func() {
 			cron := newDataImportCron(cdiv1.DataVolumeSourceRegistry{URL: &testRegistryURL})
 			cron.Spec.ManagedDataSource = ""

diff --git a/pkg/controller/dataimportcron-controller.go b/pkg/controller/dataimportcron-controller.go
@@ -132,8 +132,11 @@ func (r *DataImportCronReconciler) Reconcile(ctx context.Context, req reconcile.
 	if !shouldReconcile || err != nil {
 		return reconcile.Result{}, err
 	}
-	if err := r.initCron(ctx, dataImportCron); err != nil {
-		return reconcile.Result{}, err
+	// Cron job and initial job are not initialized if no schedule, allowing external source update trigger
+	if dataImportCron.Spec.Schedule != "" {
+		if err := r.initCron(ctx, dataImportCron); err != nil {
+			return reconcile.Result{}, err
+		}
 	}
 	return r.update(ctx, dataImportCron)
 }
@@ -353,7 +356,8 @@ func (r *DataImportCronReconciler) update(ctx context.Context, dataImportCron *c
 	}
 
 	// We use the poller returned reconcile.Result for RequeueAfter if needed
-	if isImageStreamSource(dataImportCron) {
+	// skip if we disabled schedule
+	if isImageStreamSource(dataImportCron) && dataImportCron.Spec.Schedule != "" {
 		res, err = r.pollImageStreamDigest(ctx, dataImportCron)
 		if err != nil {
 			return res, err

diff --git a/pkg/controller/dataimportcron-controller_test.go b/pkg/controller/dataimportcron-controller_test.go
@@ -70,6 +70,8 @@ const (
 	imageStreamName = "test-imagestream"
 	imageStreamTag  = "test-imagestream-tag"
 	tagWithNoItems  = "tag-with-no-items"
+	defaultSchedule = "* * * * *"
+	emptySchedule   = ""
 )
 
 type possiblyErroringFakeCtrlRuntimeClient struct {
@@ -312,8 +314,9 @@ var _ = Describe("All DataImportCron Tests", func() {
 			verifyCronJobContainerImage("new-image")
 		})
 
-		It("Should create DataVolume on AnnSourceDesiredDigest annotation update, and update DataImportCron and DataSource on DataVolume Succeeded", func() {
+		DescribeTable("Should create DataVolume on AnnSourceDesiredDigest annotation update, and update DataImportCron and DataSource on DataVolume Succeeded", func(schedule, errorString string) {
 			cron = newDataImportCron(cronName)
+			cron.Spec.Schedule = schedule
 			dataSource = nil
 			retentionPolicy := cdiv1.DataImportCronRetainNone
 			cron.Spec.RetentionPolicy = &retentionPolicy
@@ -340,6 +343,12 @@ var _ = Describe("All DataImportCron Tests", func() {
 			Expect(*dv.Spec.Source.Registry.URL).To(Equal(testRegistryURL + "@" + testDigest))
 			Expect(dv.Annotations[cc.AnnImmediateBinding]).To(Equal("true"))
 
+			if cron.Spec.Schedule == emptySchedule {
+				exists, err := reconciler.cronJobExistsAndUpdated(context.TODO(), cron)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(exists).To(BeFalse())
+			}
+
 			dv.Status.Phase = cdiv1.ImportScheduled
 			err = reconciler.client.Update(context.TODO(), dv)
 			Expect(err).ToNot(HaveOccurred())
@@ -384,7 +393,10 @@ var _ = Describe("All DataImportCron Tests", func() {
 			err = reconciler.client.List(context.TODO(), dvList, &client.ListOptions{})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(dvList.Items).To(BeEmpty())
-		})
+		},
+			Entry("default schedule", defaultSchedule, "should succeed with a default schedule"),
+			Entry("empty schedule", emptySchedule, "should succeed with an empty schedule"),
+		)
 
 		It("Should not create DV if PVC exists on DesiredDigest update; Should update DIC and DAS, and GC LRU PVCs", func() {
 			const nPVCs = 3
@@ -772,7 +784,7 @@ func newDataImportCron(name string) *cdiv1.DataImportCron {
 					},
 				},
 			},
-			Schedule:          "* * * * *",
+			Schedule:          defaultSchedule,
 			ManagedDataSource: dataSourceName,
 			GarbageCollect:    &garbageCollect,
 			ImportsToKeep:     &importsToKeep,