fix: remove WD watch predicate that wedged new WorkloadDeployments

The #37 predicate (wdReferencedDataChangedPredicate) on the WorkloadDeployment
For() watch dropped metadata-only Update events. A new WD's first reconcile adds
the workload-controller finalizer and returns; that finalizer-add produces an
Update with no generation bump and no ReferencedDataReady change, so the predicate
filtered it and the WD never got a second reconcile — wedging every new WD at the
finalizer stage until a controller restart.

Remove the predicate entirely. The equality.Semantic.DeepEqual guard before
Status().Update (added in the same #37 change) already prevents the self-trigger
loop the predicate was meant to stop, and watching all updates lets new WDs
proceed past the finalizer stage immediately.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit b94a40a)

Author: scotwells

internal/controller/workloaddeployment_controller.go

@@ -18,11 +18,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/cluster"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/finalizer"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	mcbuilder "sigs.k8s.io/multicluster-runtime/pkg/builder"
 	mccontext "sigs.k8s.io/multicluster-runtime/pkg/context"
@@ -238,12 +236,10 @@ func (r *WorkloadDeploymentReconciler) Reconcile(ctx context.Context, req mcreco
 		apimeta.SetStatusCondition(&deployment.Status.Conditions, availCond)
 	}
 
-	// Skip the write when the status is unchanged. Without this guard the
-	// reconciler's own Status().Update would always produce a new resourceVersion,
-	// firing another Update event on the WD and creating an infinite reconcile loop
-	// before the predicate on For() was added. The guard is a belt-and-suspenders
-	// complement to the predicate: the predicate prevents re-queuing on own writes,
-	// and this guard avoids the superfluous API call entirely.
+	// Skip the write when the status is unchanged. The unfiltered For() watch
+	// re-enqueues this deployment on every status write, so an unconditional
+	// Status().Update would re-trigger the reconciler on its own writes; this
+	// guard breaks that loop and avoids the superfluous API call entirely.
 	if !equality.Semantic.DeepEqual(existingStatus, deployment.Status) {
 		if err := cl.GetClient().Status().Update(ctx, &deployment); err != nil {
 			return ctrl.Result{}, fmt.Errorf("failed updating deployment status: %w", err)
@@ -313,68 +309,6 @@ func (r *WorkloadDeploymentReconciler) reconcileInstanceGates(
 	return currentReplicas, updatedReplicas, readyReplicas, quotaBlockedReplicas, referencedDataBlockedReplicas, nil
 }
 
-// wdReferencedDataChangedPredicate returns a predicate for the WorkloadDeployment
-// For() watch that fires on:
-//   - Any Create, Delete, or Generic event (always enqueue).
-//   - An Update event where metadata.generation changed (spec updated), OR where
-//     the ReferencedDataReady condition's Status, Reason, or Message changed.
-//
-// The predicate intentionally does NOT fire when only the Available or
-// ReplicasReady conditions change, because those are written by this reconciler
-// itself. Without this guard the reconciler's own Status().Update would re-enqueue
-// itself on every run, creating a tight reconcile loop. The equality check before
-// Status().Update is a complementary guard, but the predicate is the primary
-// protection: it prevents re-enqueuing entirely so the workqueue stays quiet between
-// meaningful state transitions.
-//
-// Loop prevention: the ReferencedDataController (the only other writer of the
-// ReferencedDataReady condition) is the intended trigger. When it sets
-// ReferencedDataReady=False/SourceNotFound the predicate passes and this
-// reconciler re-runs, sees the resolver verdict in deployment.Status.Conditions, and
-// promotes Available to ReferencedDataNotReady. Subsequent runs by this reconciler
-// (which write Available but not ReferencedDataReady) are filtered out.
-func wdReferencedDataChangedPredicate() predicate.Predicate {
-	return predicate.Funcs{
-		UpdateFunc: func(e event.UpdateEvent) bool {
-			oldWD, ok1 := e.ObjectOld.(*computev1alpha.WorkloadDeployment)
-			newWD, ok2 := e.ObjectNew.(*computev1alpha.WorkloadDeployment)
-			if !ok1 || !ok2 {
-				return true // be conservative when type assertion fails
-			}
-			// Spec change: always reconcile.
-			if oldWD.Generation != newWD.Generation {
-				return true
-			}
-			// ReferencedDataReady condition changed: reconcile so Available is
-			// updated to reflect the resolver's verdict.
-			return wdRefDataCondChanged(
-				apimeta.FindStatusCondition(oldWD.Status.Conditions, computev1alpha.ReferencedDataReady),
-				apimeta.FindStatusCondition(newWD.Status.Conditions, computev1alpha.ReferencedDataReady),
-			)
-		},
-		CreateFunc:  func(_ event.CreateEvent) bool { return true },
-		DeleteFunc:  func(_ event.DeleteEvent) bool { return true },
-		GenericFunc: func(_ event.GenericEvent) bool { return true },
-	}
-}
-
-// wdRefDataCondChanged returns true when the ReferencedDataReady condition's
-// observable fields (Status, Reason, Message) differ between old and new. Presence
-// changes (nil → non-nil or vice versa) are also treated as a change. The
-// LastTransitionTime field is excluded because it changes on every status flip and
-// would defeat the loop-prevention intent of wdReferencedDataChangedPredicate.
-func wdRefDataCondChanged(old, new *metav1.Condition) bool {
-	if (old == nil) != (new == nil) {
-		return true // condition was added or removed
-	}
-	if old == nil {
-		return false // both nil — no change
-	}
-	return old.Status != new.Status ||
-		old.Reason != new.Reason ||
-		old.Message != new.Message
-}
-
 // selectWDBlockingCondition evaluates all blocking causes for a WorkloadDeployment
 // that has no ready replicas and returns the Available condition reflecting the
 // highest-priority blocker. All causes are evaluated before selecting the winner
@@ -772,15 +706,15 @@ func (r *WorkloadDeploymentReconciler) SetupWithManager(mgr mcmanager.Manager, o
 	}
 
 	b := mcbuilder.ControllerManagedBy(mgr).
-		// The predicate gates re-enqueuing on meaningful WD changes: spec updates
-		// (generation bump) or a ReferencedDataReady condition change written by
-		// ReferencedDataController. Without it, each Status().Update by this
-		// reconciler (writing Available/ReplicasReady) would re-enqueue itself,
-		// creating a tight loop and delaying the ReferencedDataReady signal from
-		// the resolver.
+		// Watch all WorkloadDeployment events. The reconciler's own Status().Update
+		// cannot create a self-trigger loop because the equality.Semantic.DeepEqual
+		// guard skips the write when nothing changed, so no self-event is produced.
+		// We deliberately do NOT filter Update events with a predicate: an earlier
+		// predicate that only passed generation/ReferencedDataReady changes dropped
+		// metadata-only updates such as the initial finalizer-add, which wedged new
+		// WorkloadDeployments until a controller restart.
 		For(&computev1alpha.WorkloadDeployment{},
 			mcbuilder.WithEngageWithLocalCluster(false),
-			mcbuilder.WithPredicates(wdReferencedDataChangedPredicate()),
 		).
 		Owns(&computev1alpha.Instance{})