OADP-7943, OADP-7829: Fix DPA annotation changes not triggering reconciliation

controllers/dpa_controller.go

@@ -32,13 +32,16 @@ import (
 	"github.com/go-logr/logr"
 	oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1"
 	oadpClient "github.com/openshift/oadp-operator/pkg/client"
+	"github.com/openshift/oadp-operator/pkg/common"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/event"
+	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
@@ -138,20 +141,45 @@ func (r *DPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
 // SetupWithManager sets up the controller with the Manager.
 func (r *DPAReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
-		For(&oadpv1alpha1.DataProtectionApplication{}).
+		For(&oadpv1alpha1.DataProtectionApplication{}, builder.WithPredicates(veleroPredicate(r.Scheme))).
 		Owns(&appsv1.Deployment{}).
 		Owns(&velerov1.BackupStorageLocation{}).
 		Owns(&velerov1.VolumeSnapshotLocation{}).
 		Owns(&appsv1.DaemonSet{}).
 		Owns(&security.SecurityContextConstraints{}).
 		Owns(&corev1.Service{}).
 		Owns(&routev1.Route{}).
+		// Owns watches ConfigMaps created by the operator (with ownerReference to DPA).
 		Owns(&corev1.ConfigMap{}).
 		Watches(&corev1.Secret{}, &labelHandler{}).
-		WithEventFilter(veleroPredicate(r.Scheme)).
+		// Watches for user-created ConfigMaps referenced by DPA annotations
+		// (not owned by DPA, so Owns above does not cover them).
+		Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(r.mapUnsupportedArgsConfigMap)).
 		Complete(r)
 }
 
+// mapUnsupportedArgsConfigMap maps ConfigMap events to DPA reconcile requests
+// when the ConfigMap is referenced by a DPA's unsupported server args annotation.
+func (r *DPAReconciler) mapUnsupportedArgsConfigMap(ctx context.Context, obj client.Object) []reconcile.Request {
+	var dpas oadpv1alpha1.DataProtectionApplicationList
+	if err := r.List(ctx, &dpas, client.InNamespace(obj.GetNamespace())); err != nil {
+		return nil
+	}
+	var requests []reconcile.Request
+	for i := range dpas.Items {
+		if dpas.Items[i].Annotations[common.UnsupportedVeleroServerArgsAnnotation] == obj.GetName() ||
+			dpas.Items[i].Annotations[common.UnsupportedNodeAgentServerArgsAnnotation] == obj.GetName() {
+			requests = append(requests, reconcile.Request{
+				NamespacedName: types.NamespacedName{
+					Name:      dpas.Items[i].Name,
+					Namespace: dpas.Items[i].Namespace,
+				},
+			})
+		}
+	}
+	return requests
+}
+
 type labelHandler struct {
 }
 

controllers/predicate.go

@@ -2,6 +2,7 @@ package controllers
 
 import (
 	oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1"
+	"github.com/openshift/oadp-operator/pkg/common"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/event"
@@ -12,10 +13,15 @@ func veleroPredicate(scheme *runtime.Scheme) predicate.Predicate {
 	return predicate.Funcs{
 		// Update returns true if the Update event should be processed
 		UpdateFunc: func(e event.UpdateEvent) bool {
-			if e.ObjectOld.GetGeneration() == e.ObjectNew.GetGeneration() {
-				return false
+			if e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration() {
+				return isObjectOurs(scheme, e.ObjectOld)
 			}
-			return isObjectOurs(scheme, e.ObjectOld)
+			// Generation unchanged — still reconcile if a functionally
+			// consumed annotation was added, removed, or modified.
+			if hasRelevantAnnotationChange(e.ObjectOld, e.ObjectNew) {
+				return isObjectOurs(scheme, e.ObjectOld)
+			}
+			return false
 		},
 		// Create returns true if the Create event should be processed
 		CreateFunc: func(e event.CreateEvent) bool {
@@ -28,6 +34,23 @@ func veleroPredicate(scheme *runtime.Scheme) predicate.Predicate {
 	}
 }
 
+// hasRelevantAnnotationChange returns true if any functionally consumed
+// DPA annotation has changed between the old and new object.
+func hasRelevantAnnotationChange(oldObj, newObj client.Object) bool {
+	relevantAnnotations := []string{
+		common.UnsupportedVeleroServerArgsAnnotation,
+		common.UnsupportedNodeAgentServerArgsAnnotation,
+	}
+	oldAnnotations := oldObj.GetAnnotations()
+	newAnnotations := newObj.GetAnnotations()
+	for _, key := range relevantAnnotations {
+		if oldAnnotations[key] != newAnnotations[key] {
+			return true
+		}
+	}
+	return false
+}
+
 // isObjectOurs returns true if the object is ours.
 // it first checks if the object has our group, version, and kind
 // else it will check for non empty OadpOperatorlabel labels

controllers/predicate_test.go

@@ -0,0 +1,217 @@
+package controllers
+
+import (
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/event"
+
+	oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1"
+	"github.com/openshift/oadp-operator/pkg/common"
+)
+
+func TestHasRelevantAnnotationChange(t *testing.T) {
+	tests := []struct {
+		name           string
+		oldAnnotations map[string]string
+		newAnnotations map[string]string
+		want           bool
+	}{
+		{
+			name:           "no annotations on either object",
+			oldAnnotations: nil,
+			newAnnotations: nil,
+			want:           false,
+		},
+		{
+			name:           "velero server args annotation added",
+			oldAnnotations: nil,
+			newAnnotations: map[string]string{
+				common.UnsupportedVeleroServerArgsAnnotation: "my-configmap",
+			},
+			want: true,
+		},
+		{
+			name: "velero server args annotation removed",
+			oldAnnotations: map[string]string{
+				common.UnsupportedVeleroServerArgsAnnotation: "my-configmap",
+			},
+			newAnnotations: nil,
+			want:           true,
+		},
+		{
+			name: "velero server args annotation value changed",
+			oldAnnotations: map[string]string{
+				common.UnsupportedVeleroServerArgsAnnotation: "old-configmap",
+			},
+			newAnnotations: map[string]string{
+				common.UnsupportedVeleroServerArgsAnnotation: "new-configmap",
+			},
+			want: true,
+		},
+		{
+			name:           "node agent server args annotation added",
+			oldAnnotations: nil,
+			newAnnotations: map[string]string{
+				common.UnsupportedNodeAgentServerArgsAnnotation: "my-configmap",
+			},
+			want: true,
+		},
+		{
+			name: "node agent server args annotation removed",
+			oldAnnotations: map[string]string{
+				common.UnsupportedNodeAgentServerArgsAnnotation: "my-configmap",
+			},
+			newAnnotations: nil,
+			want:           true,
+		},
+		{
+			name: "irrelevant annotation changed",
+			oldAnnotations: map[string]string{
+				"some-other-annotation": "old-value",
+			},
+			newAnnotations: map[string]string{
+				"some-other-annotation": "new-value",
+			},
+			want: false,
+		},
+		{
+			name: "relevant annotation unchanged, irrelevant changed",
+			oldAnnotations: map[string]string{
+				common.UnsupportedVeleroServerArgsAnnotation: "same-configmap",
+				"some-other-annotation":                      "old-value",
+			},
+			newAnnotations: map[string]string{
+				common.UnsupportedVeleroServerArgsAnnotation: "same-configmap",
+				"some-other-annotation":                      "new-value",
+			},
+			want: false,
+		},
+		{
+			name: "both relevant annotations changed",
+			oldAnnotations: map[string]string{
+				common.UnsupportedVeleroServerArgsAnnotation:    "old-velero-cm",
+				common.UnsupportedNodeAgentServerArgsAnnotation: "old-nodeagent-cm",
+			},
+			newAnnotations: map[string]string{
+				common.UnsupportedVeleroServerArgsAnnotation:    "new-velero-cm",
+				common.UnsupportedNodeAgentServerArgsAnnotation: "new-nodeagent-cm",
+			},
+			want: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			oldObj := &oadpv1alpha1.DataProtectionApplication{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: tt.oldAnnotations,
+				},
+			}
+			newObj := &oadpv1alpha1.DataProtectionApplication{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: tt.newAnnotations,
+				},
+			}
+			got := hasRelevantAnnotationChange(oldObj, newObj)
+			if got != tt.want {
+				t.Errorf("hasRelevantAnnotationChange() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestVeleroPredicateUpdateFunc_AnnotationChange(t *testing.T) {
+	scheme := runtime.NewScheme()
+	_ = oadpv1alpha1.AddToScheme(scheme)
+
+	pred := veleroPredicate(scheme)
+
+	tests := []struct {
+		name string
+		old  *oadpv1alpha1.DataProtectionApplication
+		new  *oadpv1alpha1.DataProtectionApplication
+		want bool
+	}{
+		{
+			name: "generation changed - should reconcile",
+			old: &oadpv1alpha1.DataProtectionApplication{
+				ObjectMeta: metav1.ObjectMeta{Generation: 1},
+			},
+			new: &oadpv1alpha1.DataProtectionApplication{
+				ObjectMeta: metav1.ObjectMeta{Generation: 2},
+			},
+			want: true,
+		},
+		{
+			name: "generation same, no annotation change - should NOT reconcile",
+			old: &oadpv1alpha1.DataProtectionApplication{
+				ObjectMeta: metav1.ObjectMeta{Generation: 1},
+			},
+			new: &oadpv1alpha1.DataProtectionApplication{
+				ObjectMeta: metav1.ObjectMeta{Generation: 1},
+			},
+			want: false,
+		},
+		{
+			name: "generation same, velero annotation added - should reconcile",
+			old: &oadpv1alpha1.DataProtectionApplication{
+				ObjectMeta: metav1.ObjectMeta{Generation: 1},
+			},
+			new: &oadpv1alpha1.DataProtectionApplication{
+				ObjectMeta: metav1.ObjectMeta{
+					Generation: 1,
+					Annotations: map[string]string{
+						common.UnsupportedVeleroServerArgsAnnotation: "my-cm",
+					},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "generation same, node agent annotation removed - should reconcile",
+			old: &oadpv1alpha1.DataProtectionApplication{
+				ObjectMeta: metav1.ObjectMeta{
+					Generation: 1,
+					Annotations: map[string]string{
+						common.UnsupportedNodeAgentServerArgsAnnotation: "my-cm",
+					},
+				},
+			},
+			new: &oadpv1alpha1.DataProtectionApplication{
+				ObjectMeta: metav1.ObjectMeta{Generation: 1},
+			},
+			want: true,
+		},
+		{
+			name: "generation same, irrelevant annotation changed - should NOT reconcile",
+			old: &oadpv1alpha1.DataProtectionApplication{
+				ObjectMeta: metav1.ObjectMeta{
+					Generation:  1,
+					Annotations: map[string]string{"kubectl.kubernetes.io/last-applied": "old"},
+				},
+			},
+			new: &oadpv1alpha1.DataProtectionApplication{
+				ObjectMeta: metav1.ObjectMeta{
+					Generation:  1,
+					Annotations: map[string]string{"kubectl.kubernetes.io/last-applied": "new"},
+				},
+			},
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			e := event.UpdateEvent{
+				ObjectOld: tt.old,
+				ObjectNew: tt.new,
+			}
+			got := pred.Update(e)
+			if got != tt.want {
+				t.Errorf("veleroPredicate.Update() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}