Don't reconcile datasource on status condition changes

internal/knowledge/datasources/plugins/openstack/controller.go

@@ -17,12 +17,14 @@ import (
 	"github.com/cobaltcore-dev/cortex/pkg/sso"
 	"github.com/sapcc/go-bits/jobloop"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
@@ -257,6 +259,27 @@ func (r *OpenStackDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.
 	return ctrl.Result{RequeueAfter: datasource.Spec.OpenStack.SyncInterval.Duration}, nil
 }
 
+// predicateIgnoreStatusConditions returns a predicate that ignores changes to
+// the status conditions, because these are only updated by the controller
+// itself and should not trigger a new reconciliation.
+func predicateIgnoreStatusConditions() predicate.Predicate {
+	return predicate.Funcs{
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			// Remove the status conditions from the old and new object
+			// before comparing them. This is fine for now, because
+			// the controller itself doesn't need to react to status
+			// condition changes.
+			oldObj := e.ObjectOld.DeepCopyObject().(*v1alpha1.Datasource)
+			newObj := e.ObjectNew.DeepCopyObject().(*v1alpha1.Datasource)
+			oldObj.Status.Conditions = nil
+			newObj.Status.Conditions = nil
+			// If anything else in the object has changed except the status
+			// conditions, then trigger a reconciliation.
+			return !equality.Semantic.DeepEqual(oldObj, newObj)
+		},
+	}
+}
+
 func (r *OpenStackDatasourceReconciler) SetupWithManager(mgr manager.Manager, mcl *multicluster.Client) error {
 	bldr := multicluster.BuildController(mcl, mgr)
 	// Watch datasource changes across all clusters.
@@ -266,12 +289,17 @@ func (r *OpenStackDatasourceReconciler) SetupWithManager(mgr manager.Manager, mc
 		predicate.NewPredicateFuncs(func(obj client.Object) bool {
 			// Only react to datasources matching the operator.
 			ds := obj.(*v1alpha1.Datasource)
+			// Ignore all datasources outside our scheduling domain.
 			if ds.Spec.SchedulingDomain != r.Conf.SchedulingDomain {
 				return false
 			}
-			// Only react to openstack datasources.
-			return ds.Spec.Type == v1alpha1.DatasourceTypeOpenStack
+			// Ignore all datasources that are not of type openstack.
+			if ds.Spec.Type != v1alpha1.DatasourceTypeOpenStack {
+				return false
+			}
+			return true
 		}),
+		predicateIgnoreStatusConditions(),
 	)
 	if err != nil {
 		return err

internal/knowledge/datasources/plugins/openstack/controller_test.go

@@ -14,6 +14,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 )
 
 func TestOpenStackDatasourceReconciler_Creation(t *testing.T) {
@@ -430,3 +431,118 @@ func TestOpenStackDatasourceDefaults(t *testing.T) {
 		t.Errorf("Expected type %s, got %s", v1alpha1.OpenStackDatasourceTypeNova, ds.Type)
 	}
 }
+
+func TestUpdatePredicateIgnoresStatusConditionChanges(t *testing.T) {
+	tests := []struct {
+		name     string
+		oldObj   *v1alpha1.Datasource
+		newObj   *v1alpha1.Datasource
+		expected bool
+	}{
+		{
+			name: "only status conditions change - should not trigger reconcile",
+			oldObj: &v1alpha1.Datasource{
+				ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"},
+				Spec:       v1alpha1.DatasourceSpec{Type: v1alpha1.DatasourceTypeOpenStack},
+				Status: v1alpha1.DatasourceStatus{
+					Conditions: []metav1.Condition{
+						{Type: v1alpha1.DatasourceConditionReady, Status: metav1.ConditionFalse},
+					},
+				},
+			},
+			newObj: &v1alpha1.Datasource{
+				ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"},
+				Spec:       v1alpha1.DatasourceSpec{Type: v1alpha1.DatasourceTypeOpenStack},
+				Status: v1alpha1.DatasourceStatus{
+					Conditions: []metav1.Condition{
+						{Type: v1alpha1.DatasourceConditionReady, Status: metav1.ConditionTrue},
+					},
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "spec stays equal - should not trigger reconcile",
+			oldObj: &v1alpha1.Datasource{
+				ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"},
+				Spec:       v1alpha1.DatasourceSpec{Type: v1alpha1.DatasourceTypeOpenStack},
+			},
+			newObj: &v1alpha1.Datasource{
+				ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"},
+				Spec:       v1alpha1.DatasourceSpec{Type: v1alpha1.DatasourceTypeOpenStack},
+			},
+			expected: false,
+		},
+		{
+			name: "metadata labels change - should trigger reconcile",
+			oldObj: &v1alpha1.Datasource{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test",
+					Namespace: "default",
+					Labels:    map[string]string{"env": "dev"},
+				},
+			},
+			newObj: &v1alpha1.Datasource{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test",
+					Namespace: "default",
+					Labels:    map[string]string{"env": "prod"},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "status NumberOfObjects changes - should trigger reconcile",
+			oldObj: &v1alpha1.Datasource{
+				ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"},
+				Status:     v1alpha1.DatasourceStatus{NumberOfObjects: 10},
+			},
+			newObj: &v1alpha1.Datasource{
+				ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"},
+				Status:     v1alpha1.DatasourceStatus{NumberOfObjects: 20},
+			},
+			expected: true,
+		},
+		{
+			name: "both conditions and other fields change - should trigger reconcile",
+			oldObj: &v1alpha1.Datasource{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test",
+					Namespace: "default",
+					Labels:    map[string]string{"version": "v1"},
+				},
+				Status: v1alpha1.DatasourceStatus{
+					Conditions: []metav1.Condition{
+						{Type: v1alpha1.DatasourceConditionReady, Status: metav1.ConditionFalse},
+					},
+				},
+			},
+			newObj: &v1alpha1.Datasource{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test",
+					Namespace: "default",
+					Labels:    map[string]string{"version": "v2"},
+				},
+				Status: v1alpha1.DatasourceStatus{
+					Conditions: []metav1.Condition{
+						{Type: v1alpha1.DatasourceConditionReady, Status: metav1.ConditionTrue},
+					},
+				},
+			},
+			expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			e := event.UpdateEvent{
+				ObjectOld: tt.oldObj,
+				ObjectNew: tt.newObj,
+			}
+			result := predicateIgnoreStatusConditions().Update(e)
+			if result != tt.expected {
+				t.Errorf("expected %v, got %v", tt.expected, result)
+			}
+		})
+	}
+}