Bugfix: Reconcile pointer datasource on reference status update (#3870)

* bugfix: update datasource when datasource reference status updates

Previously when a datasource reference status would change pointers to
said reference wouldn't reconcile to reflect the latest status.

This commit takes care to update them accordingly.

Signed-off-by: Adi Aloni <[email protected]>

* datasource-controller: include source.datasource in sameSourceSpec

Previously only considered PVC and Snpashot sources.

Signed-off-by: Adi Aloni <[email protected]>

---------

Signed-off-by: Adi Aloni <[email protected]>

Author: Acedus

pkg/controller/datasource-controller.go

@@ -237,20 +237,9 @@ func NewDataSourceController(mgr manager.Manager, log logr.Logger, installerLabe
 }
 
 func addDataSourceControllerWatches(mgr manager.Manager, c controller.Controller, log logr.Logger) error {
-	if err := c.Watch(source.Kind(mgr.GetCache(), &cdiv1.DataSource{}, &handler.TypedEnqueueRequestForObject[*cdiv1.DataSource]{},
-		predicate.TypedFuncs[*cdiv1.DataSource]{
-			CreateFunc: func(e event.TypedCreateEvent[*cdiv1.DataSource]) bool { return true },
-			DeleteFunc: func(e event.TypedDeleteEvent[*cdiv1.DataSource]) bool { return true },
-			UpdateFunc: func(e event.TypedUpdateEvent[*cdiv1.DataSource]) bool {
-				return !sameSourceSpec(e.ObjectOld, e.ObjectNew)
-			},
-		},
-	)); err != nil {
-		return err
-	}
-
 	const dataSourcePvcField = "spec.source.pvc"
 	const dataSourceSnapshotField = "spec.source.snapshot"
+	const dataSourceDataSourceField = "spec.source.dataSource"
 
 	getKey := func(namespace, name string) string {
 		return namespace + "/" + name
@@ -269,6 +258,30 @@ func addDataSourceControllerWatches(mgr manager.Manager, c controller.Controller
 		return reqs
 	}
 
+	if err := c.Watch(source.Kind(mgr.GetCache(), &cdiv1.DataSource{},
+		handler.TypedEnqueueRequestsFromMapFunc[*cdiv1.DataSource](func(ctx context.Context, obj *cdiv1.DataSource) []reconcile.Request {
+			reqs := []reconcile.Request{
+				{
+					NamespacedName: types.NamespacedName{
+						Name:      obj.Name,
+						Namespace: obj.Namespace,
+					},
+				},
+			}
+			return appendMatchingDataSourceRequests(ctx, dataSourceDataSourceField, obj, reqs)
+		}),
+		predicate.TypedFuncs[*cdiv1.DataSource]{
+			CreateFunc: func(e event.TypedCreateEvent[*cdiv1.DataSource]) bool { return true },
+			DeleteFunc: func(e event.TypedDeleteEvent[*cdiv1.DataSource]) bool { return true },
+			UpdateFunc: func(e event.TypedUpdateEvent[*cdiv1.DataSource]) bool {
+				return !sameSourceSpec(e.ObjectOld, e.ObjectNew) ||
+					!sameConditions(e.ObjectOld, e.ObjectNew)
+			},
+		},
+	)); err != nil {
+		return err
+	}
+
 	if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &cdiv1.DataSource{}, dataSourcePvcField, func(obj client.Object) []string {
 		if pvc := obj.(*cdiv1.DataSource).Spec.Source.PVC; pvc != nil {
 			ns := cc.GetNamespace(pvc.Namespace, obj.GetNamespace())
@@ -278,6 +291,7 @@ func addDataSourceControllerWatches(mgr manager.Manager, c controller.Controller
 	}); err != nil {
 		return err
 	}
+
 	if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &cdiv1.DataSource{}, dataSourceSnapshotField, func(obj client.Object) []string {
 		if snapshot := obj.(*cdiv1.DataSource).Spec.Source.Snapshot; snapshot != nil {
 			ns := cc.GetNamespace(snapshot.Namespace, obj.GetNamespace())
@@ -288,6 +302,17 @@ func addDataSourceControllerWatches(mgr manager.Manager, c controller.Controller
 		return err
 	}
 
+	if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &cdiv1.DataSource{}, dataSourceDataSourceField, func(obj client.Object) []string {
+		ds := obj.(*cdiv1.DataSource)
+		if sourceDS := ds.Spec.Source.DataSource; sourceDS != nil {
+			ns := cc.GetNamespace(sourceDS.Namespace, ds.GetNamespace())
+			return []string{getKey(ns, sourceDS.Name)}
+		}
+		return nil
+	}); err != nil {
+		return err
+	}
+
 	mapToDataSource := func(ctx context.Context, obj client.Object) []reconcile.Request {
 		reqs := appendMatchingDataSourceRequests(ctx, dataSourcePvcField, obj, nil)
 		return appendMatchingDataSourceRequests(ctx, dataSourceSnapshotField, obj, reqs)
@@ -367,6 +392,41 @@ func sameSourceSpec(objOld, objNew client.Object) bool {
 	if dsOld.Spec.Source.Snapshot != nil {
 		return reflect.DeepEqual(dsOld.Spec.Source.Snapshot, dsNew.Spec.Source.Snapshot)
 	}
+	if dsOld.Spec.Source.DataSource != nil {
+		return reflect.DeepEqual(dsOld.Spec.Source.DataSource, dsNew.Spec.Source.DataSource)
+	}
 
 	return false
 }
+
+func sameConditions(objOld, objNew client.Object) bool {
+	dsOld, okOld := objOld.(*cdiv1.DataSource)
+	dsNew, okNew := objNew.(*cdiv1.DataSource)
+
+	if !okOld || !okNew {
+		return false
+	}
+
+	oldConditions := dsOld.Status.Conditions
+	newConditions := dsNew.Status.Conditions
+
+	if len(oldConditions) != len(newConditions) {
+		return false
+	}
+
+	condMap := make(map[cdiv1.DataSourceConditionType]cdiv1.DataSourceCondition, len(oldConditions))
+	for _, c := range oldConditions {
+		condMap[c.Type] = c
+	}
+
+	for _, c := range newConditions {
+		if oldC, ok := condMap[c.Type]; !ok ||
+			oldC.Reason != c.Reason ||
+			oldC.Message != c.Message ||
+			oldC.Status != c.Status {
+			return false
+		}
+	}
+
+	return true
+}

tests/datasource_test.go

@@ -358,6 +358,38 @@ var _ = Describe("DataSource", func() {
 			_ = waitForReadyCondition(ds2, corev1.ConditionFalse, "NotFound")
 		})
 	})
+
+	Context("datasource source", func() {
+		createDsNoVerify := func(dsName, pvcName string) *cdiv1.DataSource {
+			By(fmt.Sprintf("creating DataSource %s -> %s", dsName, pvcName))
+			ds := newDataSource(dsName)
+			ds.Spec.Source.PVC = &cdiv1.DataVolumeSourcePVC{Namespace: f.Namespace.Name, Name: pvcName}
+			ds, err := f.CdiClient.CdiV1beta1().DataSources(ds.Namespace).Create(context.TODO(), ds, metav1.CreateOptions{})
+			Expect(err).ToNot(HaveOccurred())
+			return ds
+		}
+
+		createDsPointer := func(dsName, dsRefName string) *cdiv1.DataSource {
+			By(fmt.Sprintf("creating DataSource %s -> DataSource %s", dsName, dsRefName))
+			ds := newDataSource(dsName)
+			ds.Spec.Source.DataSource = &cdiv1.DataSourceRefSourceDataSource{Namespace: f.Namespace.Name, Name: dsRefName}
+			ds, err := f.CdiClient.CdiV1beta1().DataSources(ds.Namespace).Create(context.TODO(), ds, metav1.CreateOptions{})
+			Expect(err).ToNot(HaveOccurred())
+			return ds
+		}
+
+		It("should update datasource status when referenced datasource updates", func() {
+			ds := createDsNoVerify(ds1Name, pvc1Name)
+			_ = waitForReadyCondition(ds, corev1.ConditionFalse, "NotFound")
+
+			dsPointer := createDsPointer(ds2Name, ds.Name)
+			_ = waitForReadyCondition(dsPointer, corev1.ConditionFalse, "NotFound")
+
+			createDv(pvc1Name, testURL(), nil)
+			_ = waitForReadyCondition(ds, corev1.ConditionTrue, "Ready")
+			_ = waitForReadyCondition(dsPointer, corev1.ConditionTrue, "Ready")
+		})
+	})
 })
 
 func deleteDvPvc(f *framework.Framework, pvcName string) {