enable empty schedule in dataimportcron

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

Hide file tree

Showing 4 changed files with 64 additions and 12 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 {
@@ -503,6 +505,37 @@ var _ = Describe("All DataImportCron Tests", func() {
 			Expect(shouldReconcile).To(BeTrue())
 		})
 
+		It("Should allow an empty schedule to trigger an external update to the source", func() {
+			cron = newDataImportCron(cronName, emptySchedule)
+			dataSource = nil
+			reconciler = createDataImportCronReconciler(cron)
+			verifyConditions("Before DesiredDigest is set", false, false, false, noImport, noDigest, "")
+
+			cc.AddAnnotation(cron, AnnSourceDesiredDigest, testDigest)
+			err := reconciler.client.Update(context.TODO(), cron)
+			Expect(err).ToNot(HaveOccurred())
+			dataSource = &cdiv1.DataSource{}
+			verifyConditions("After DesiredDigest is set", false, false, false, noImport, outdated, noSource)
+
+			imports := cron.Status.CurrentImports
+			Expect(imports).ToNot(BeNil())
+			Expect(imports).ToNot(BeEmpty())
+			dvName := imports[0].DataVolumeName
+			Expect(dvName).ToNot(BeEmpty())
+			digest := imports[0].Digest
+			Expect(digest).To(Equal(testDigest))
+
+			dv := &cdiv1.DataVolume{}
+			err = reconciler.client.Get(context.TODO(), dvKey(dvName), dv)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(*dv.Spec.Source.Registry.URL).To(Equal(testRegistryURL + "@" + testDigest))
+			Expect(dv.Annotations[cc.AnnImmediateBinding]).To(Equal("true"))
+
+			exists, err := reconciler.cronJobExistsAndUpdated(context.TODO(), cron)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(exists).To(BeFalse())
+		})
+
 		DescribeTable("Should fail when digest", func(digest, errorString string) {
 			cron = newDataImportCron(cronName)
 			cron.Annotations[AnnSourceDesiredDigest] = digest
@@ -744,7 +777,14 @@ func newImageStream(name string) *imagev1.ImageStream {
 	}
 }
 
-func newDataImportCron(name string) *cdiv1.DataImportCron {
+func newDataImportCron(name string, specificSchedule ...string) *cdiv1.DataImportCron {
+	var schedule string
+	if len(specificSchedule) == 0 {
+		schedule = defaultSchedule
+	} else {
+		schedule = specificSchedule[0]
+	}
+
 	garbageCollect := cdiv1.DataImportCronGarbageCollectOutdated
 	registryPullNodesource := cdiv1.RegistryPullNode
 	importsToKeep := int32(2)
@@ -772,7 +812,7 @@ func newDataImportCron(name string) *cdiv1.DataImportCron {
 					},
 				},
 			},
-			Schedule:          "* * * * *",
+			Schedule:          schedule,
 			ManagedDataSource: dataSourceName,
 			GarbageCollect:    &garbageCollect,
 			ImportsToKeep:     &importsToKeep,