Skip to content

Commit ffed182

Browse files
scotwellsclaude
andcommitted
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)
1 parent d7ec5a9 commit ffed182

2 files changed

Lines changed: 7 additions & 356 deletions

File tree

internal/controller/workloaddeployment_controller.go

Lines changed: 7 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,9 @@ import (
1818
"sigs.k8s.io/controller-runtime/pkg/client"
1919
"sigs.k8s.io/controller-runtime/pkg/cluster"
2020
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
21-
"sigs.k8s.io/controller-runtime/pkg/event"
2221
"sigs.k8s.io/controller-runtime/pkg/finalizer"
2322
"sigs.k8s.io/controller-runtime/pkg/handler"
2423
"sigs.k8s.io/controller-runtime/pkg/log"
25-
"sigs.k8s.io/controller-runtime/pkg/predicate"
2624
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2725
mcbuilder "sigs.k8s.io/multicluster-runtime/pkg/builder"
2826
mccontext "sigs.k8s.io/multicluster-runtime/pkg/context"
@@ -310,68 +308,6 @@ func (r *WorkloadDeploymentReconciler) reconcileInstanceGates(
310308
return currentReplicas, updatedReplicas, readyReplicas, quotaBlockedReplicas, referencedDataBlockedReplicas, nil
311309
}
312310

313-
// wdReferencedDataChangedPredicate returns a predicate for the WorkloadDeployment
314-
// For() watch that fires on:
315-
// - Any Create, Delete, or Generic event (always enqueue).
316-
// - An Update event where metadata.generation changed (spec updated), OR where
317-
// the ReferencedDataReady condition's Status, Reason, or Message changed.
318-
//
319-
// The predicate intentionally does NOT fire when only the Available or
320-
// ReplicasReady conditions change, because those are written by this reconciler
321-
// itself. Without this guard the reconciler's own Status().Update would re-enqueue
322-
// itself on every run, creating a tight reconcile loop. The equality check before
323-
// Status().Update is a complementary guard, but the predicate is the primary
324-
// protection: it prevents re-enqueuing entirely so the workqueue stays quiet between
325-
// meaningful state transitions.
326-
//
327-
// Loop prevention: the ReferencedDataController (the only other writer of the
328-
// ReferencedDataReady condition) is the intended trigger. When it sets
329-
// ReferencedDataReady=False/SourceNotFound the predicate passes and this
330-
// reconciler re-runs, sees the resolver verdict in deployment.Status.Conditions, and
331-
// promotes Available to ReferencedDataNotReady. Subsequent runs by this reconciler
332-
// (which write Available but not ReferencedDataReady) are filtered out.
333-
func wdReferencedDataChangedPredicate() predicate.Predicate {
334-
return predicate.Funcs{
335-
UpdateFunc: func(e event.UpdateEvent) bool {
336-
oldWD, ok1 := e.ObjectOld.(*computev1alpha.WorkloadDeployment)
337-
newWD, ok2 := e.ObjectNew.(*computev1alpha.WorkloadDeployment)
338-
if !ok1 || !ok2 {
339-
return true // be conservative when type assertion fails
340-
}
341-
// Spec change: always reconcile.
342-
if oldWD.Generation != newWD.Generation {
343-
return true
344-
}
345-
// ReferencedDataReady condition changed: reconcile so Available is
346-
// updated to reflect the resolver's verdict.
347-
return wdRefDataCondChanged(
348-
apimeta.FindStatusCondition(oldWD.Status.Conditions, computev1alpha.ReferencedDataReady),
349-
apimeta.FindStatusCondition(newWD.Status.Conditions, computev1alpha.ReferencedDataReady),
350-
)
351-
},
352-
CreateFunc: func(_ event.CreateEvent) bool { return true },
353-
DeleteFunc: func(_ event.DeleteEvent) bool { return true },
354-
GenericFunc: func(_ event.GenericEvent) bool { return true },
355-
}
356-
}
357-
358-
// wdRefDataCondChanged returns true when the ReferencedDataReady condition's
359-
// observable fields (Status, Reason, Message) differ between old and new. Presence
360-
// changes (nil → non-nil or vice versa) are also treated as a change. The
361-
// LastTransitionTime field is excluded because it changes on every status flip and
362-
// would defeat the loop-prevention intent of wdReferencedDataChangedPredicate.
363-
func wdRefDataCondChanged(old, new *metav1.Condition) bool {
364-
if (old == nil) != (new == nil) {
365-
return true // condition was added or removed
366-
}
367-
if old == nil {
368-
return false // both nil — no change
369-
}
370-
return old.Status != new.Status ||
371-
old.Reason != new.Reason ||
372-
old.Message != new.Message
373-
}
374-
375311
// selectWDBlockingCondition evaluates all blocking causes for a WorkloadDeployment
376312
// that has no ready replicas and returns the Available condition reflecting the
377313
// highest-priority blocker. All causes are evaluated before selecting the winner
@@ -769,15 +705,15 @@ func (r *WorkloadDeploymentReconciler) SetupWithManager(mgr mcmanager.Manager, o
769705
}
770706

771707
b := mcbuilder.ControllerManagedBy(mgr).
772-
// The predicate gates re-enqueuing on meaningful WD changes: spec updates
773-
// (generation bump) or a ReferencedDataReady condition change written by
774-
// ReferencedDataController. Without it, each Status().Update by this
775-
// reconciler (writing Available/ReplicasReady) would re-enqueue itself,
776-
// creating a tight loop and delaying the ReferencedDataReady signal from
777-
// the resolver.
708+
// Watch all WorkloadDeployment events. The reconciler's own Status().Update
709+
// cannot create a self-trigger loop because the equality.Semantic.DeepEqual
710+
// guard skips the write when nothing changed, so no self-event is produced.
711+
// We deliberately do NOT filter Update events with a predicate: an earlier
712+
// predicate that only passed generation/ReferencedDataReady changes dropped
713+
// metadata-only updates such as the initial finalizer-add, which wedged new
714+
// WorkloadDeployments until a controller restart.
778715
For(&computev1alpha.WorkloadDeployment{},
779716
mcbuilder.WithEngageWithLocalCluster(false),
780-
mcbuilder.WithPredicates(wdReferencedDataChangedPredicate()),
781717
).
782718
Owns(&computev1alpha.Instance{})
783719

0 commit comments

Comments
 (0)