fix(dataset): report NotBound before recreating the ThinRuntime of a reference dataset

pkg/controllers/v1alpha1/dataset/dataset_controller.go

@@ -143,22 +143,19 @@ func (r *DatasetReconciler) reconcileDataset(ctx reconcileRequestContext, needRe
 		return r.addFinalizerAndRequeue(ctx)
 	}
 
-	// 3. Create Runtime if it's reference dataset
+	// 3. Validate the dataset before anything is created for it
 	checkReferenceDataset, err := base.CheckReferenceDataset(&ctx.Dataset)
 	if err != nil {
 		ctx.Log.Error(err, "Failed to validate dataset", "ctx", ctx)
 		r.Recorder.Eventf(&ctx.Dataset, v1.EventTypeWarning, common.ErrorCreateDataset, "Failed to validate dataset because err: %v", err)
 		return utils.RequeueIfError(err)
 	}
-	if checkReferenceDataset {
-		err := utils.CreateRuntimeForReferenceDatasetIfNotExist(r.Client, &ctx.Dataset)
-		if err != nil {
-			ctx.Log.Error(err, "Failed to create thinRuntime", "ctx", ctx)
-			return utils.RequeueIfError(err)
-		}
-	}
 
-	// 4. Update the phase to NotBoundDatasetPhase
+	// 4. Update the phase to NotBoundDatasetPhase.
+	// This happens before the runtime of a reference dataset is created on purpose: that creation can keep
+	// failing for a while, for example while a previously deleted ThinRuntime is still terminating, and the
+	// dataset should already report NotBound rather than staying in the empty phase until the recreation
+	// finally succeeds.
 	if ctx.Dataset.Status.Phase == datav1alpha1.NoneDatasetPhase {
 		dataset := ctx.Dataset.DeepCopy()
 		dataset.Status.Phase = datav1alpha1.NotBoundDatasetPhase
@@ -173,7 +170,16 @@ func (r *DatasetReconciler) reconcileDataset(ctx reconcileRequestContext, needRe
 		}
 	}
 
-	// 5. Check if needRequeue
+	// 5. Create Runtime if it's reference dataset
+	if checkReferenceDataset {
+		err := utils.CreateRuntimeForReferenceDatasetIfNotExist(r.Client, &ctx.Dataset)
+		if err != nil {
+			ctx.Log.Error(err, "Failed to create thinRuntime", "ctx", ctx)
+			return utils.RequeueIfError(err)
+		}
+	}
+
+	// 6. Check if needRequeue
 	if needRequeue {
 		return utils.RequeueAfterInterval(r.ResyncPeriod)
 	}

pkg/controllers/v1alpha1/dataset/dataset_reconciler_test.go

@@ -326,6 +326,47 @@ var _ = Describe("DatasetReconciler (fake client)", func() {
 			Expect(stored.OwnerReferences).To(BeEmpty())
 		})
 
+		It("advances phase from None to NotBound even while the ThinRuntime is still terminating", func() {
+			// Recreating the runtime can keep failing for a while, but the dataset is already known not to be
+			// bound to any runtime, so its phase must not stay empty until the terminating object is gone.
+			now := metav1.Now()
+			ds := datav1alpha1.Dataset{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "ref-ds-none-terminating",
+					Namespace:  "default",
+					UID:        types.UID("ref-ds-none-terminating-uid"),
+					Finalizers: []string{finalizer},
+				},
+				Spec: datav1alpha1.DatasetSpec{
+					Mounts: []datav1alpha1.Mount{
+						{Name: "m1", MountPoint: "dataset://default/physical-ds"},
+					},
+				},
+				Status: datav1alpha1.DatasetStatus{Phase: datav1alpha1.NoneDatasetPhase},
+			}
+			// The finalizer keeps the terminating runtime in the fake client's tracker.
+			terminatingRuntime := datav1alpha1.ThinRuntime{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              "ref-ds-none-terminating",
+					Namespace:         "default",
+					DeletionTimestamp: &now,
+					Finalizers:        []string{"thin-runtime-controller-finalizer"},
+				},
+			}
+			r := newTestReconciler(&ds, &terminatingRuntime)
+			ctx := makeReconcileCtx(r, ds)
+
+			// The reconcile still fails, so that it is retried once the runtime is really gone.
+			_, err := r.reconcileDataset(ctx, false)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("terminating"))
+
+			// The phase has been advanced regardless of that failure.
+			stored := &datav1alpha1.Dataset{}
+			Expect(r.Get(ctx, types.NamespacedName{Namespace: "default", Name: "ref-ds-none-terminating"}, stored)).To(Succeed())
+			Expect(stored.Status.Phase).To(Equal(datav1alpha1.NotBoundDatasetPhase))
+		})
+
 		It("returns error when CreateRuntimeForReferenceDatasetIfNotExist fails", func() {
 			ds := datav1alpha1.Dataset{
 				ObjectMeta: metav1.ObjectMeta{