Skip to content

Commit a52287f

Browse files
committed
webhosting-operator: fix edge cases with missing status updates
1 parent c53a1f4 commit a52287f

File tree

1 file changed

+32
-3
lines changed

1 file changed

+32
-3
lines changed

webhosting-operator/pkg/controllers/webhosting/website_controller.go

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -480,9 +480,13 @@ func (r *WebsiteReconciler) SetupWithManager(mgr manager.Manager, enableSharding
480480
}
481481

482482
var (
483-
// trigger on spec change and annotation changes (manual trigger for testing purposes)
484-
websitePredicate = predicate.Or(predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{})
485-
reconciler = SilenceConflicts(reconcile.AsReconciler[*webhostingv1alpha1.Website](r.Client, r))
483+
// trigger on spec, status, and annotation changes (manual trigger for testing purposes)
484+
websitePredicate = predicate.Or(
485+
predicate.GenerationChangedPredicate{},
486+
predicate.AnnotationChangedPredicate{},
487+
WebsiteStatusChanged,
488+
)
489+
reconciler = SilenceConflicts(reconcile.AsReconciler[*webhostingv1alpha1.Website](r.Client, r))
486490
)
487491

488492
if enableSharding {
@@ -551,6 +555,31 @@ func (r *WebsiteReconciler) MapThemeToWebsites(ctx context.Context, theme client
551555
return requests
552556
}
553557

558+
// WebsiteStatusChanged is a predicate that triggers when the Website status changes.
559+
// The controller skips updating the status if it didn't change the cached object. In fast consecutive retries, this
560+
// can lead to a Website in Error state, where the controller doesn't observe its own status update, and skips the
561+
// transition to Ready afterward because the cached object was still in Ready state.
562+
// To fix this, we trigger the controller one more time when observing its own status updates to ensure a correct
563+
// status. This shouldn't hurt the standard case, because the controller doesn't cause any API calls if not needed.
564+
var WebsiteStatusChanged = predicate.Funcs{
565+
UpdateFunc: func(e event.UpdateEvent) bool {
566+
if e.ObjectOld == nil || e.ObjectNew == nil {
567+
return false
568+
}
569+
570+
oldWebsite, ok := e.ObjectOld.(*webhostingv1alpha1.Website)
571+
if !ok {
572+
return false
573+
}
574+
newWebsite, ok := e.ObjectNew.(*webhostingv1alpha1.Website)
575+
if !ok {
576+
return false
577+
}
578+
579+
return !apiequality.Semantic.DeepEqual(oldWebsite.Status, newWebsite.Status)
580+
},
581+
}
582+
554583
// DeploymentAvailabilityChanged is a predicate for filtering relevant Deployment events.
555584
var DeploymentAvailabilityChanged = predicate.Funcs{
556585
UpdateFunc: func(e event.UpdateEvent) bool {

0 commit comments

Comments
 (0)