diff --git a/internal/knowledge/datasources/plugins/openstack/controller.go b/internal/knowledge/datasources/plugins/openstack/controller.go index ecbbef5f2..f94cebd06 100644 --- a/internal/knowledge/datasources/plugins/openstack/controller.go +++ b/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 diff --git a/internal/knowledge/datasources/plugins/openstack/controller_test.go b/internal/knowledge/datasources/plugins/openstack/controller_test.go index b682da677..c75778cbe 100644 --- a/internal/knowledge/datasources/plugins/openstack/controller_test.go +++ b/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) + } + }) + } +}