Skip to content

Commit 5086f39

Browse files
urbanikbclaude
andcommitted
Fix status reconciler requeue logic: RequeueAfter and skip equal-status write
Two issues in the previous requeue logic: 1. The guard `equalStatus && newAvailable` caused the unavailable-but-equal case to fall through to a status write even when nothing had changed, producing a no-op write on every reconcile while the instance remained unavailable. The new `if equalStatus` guard short-circuits both the available and unavailable steady states, avoiding the unnecessary write. 2. Both unavailable return paths used `Requeue: true` (immediate requeue), which causes tight-loop reconciliation against an instance that is still unavailable. Switching to `RequeueAfter: 30s` gives components time to recover between checks and avoids extending backoff counter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 1d57c8d commit 5086f39

2 files changed

Lines changed: 12 additions & 12 deletions

File tree

controllers/apps/apimanager_status_reconciler.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"sort"
77
"strings"
8+
"time"
89

910
appsv1alpha1 "github.com/3scale/3scale-operator/apis/apps/v1alpha1"
1011
subController "github.com/3scale/3scale-operator/controllers/subscription"
@@ -49,16 +50,15 @@ func (s *APIManagerStatusReconciler) Reconcile() (reconcile.Result, error) {
4950
return reconcile.Result{}, fmt.Errorf("failed to calculate status: %w", err)
5051
}
5152

52-
// Read availability from the newly computed status, not the stale pre-reconcile snapshot.
53-
// Using old status caused a True-to-False requeue gap: old=Available=True would suppress
54-
// the requeue even after writing Available=False to the API server (THREESCALE-10754).
5553
newAvailable := newStatus.Conditions.IsTrueFor(appsv1alpha1.APIManagerAvailableConditionType)
5654

5755
equalStatus := s.apimanagerResource.Status.Equals(newStatus, s.logger)
5856
s.logger.V(1).Info("Status", "status is different", !equalStatus)
59-
if equalStatus && newAvailable {
60-
// Steady state
57+
if equalStatus {
6158
s.logger.V(1).Info("Status was not updated")
59+
if !newAvailable {
60+
return reconcile.Result{RequeueAfter: 30 * time.Second}, nil
61+
}
6262
return reconcile.Result{}, nil
6363
}
6464

@@ -75,7 +75,7 @@ func (s *APIManagerStatusReconciler) Reconcile() (reconcile.Result, error) {
7575
}
7676

7777
if !newAvailable {
78-
return reconcile.Result{Requeue: true}, nil
78+
return reconcile.Result{RequeueAfter: 30 * time.Second}, nil
7979
}
8080

8181
return reconcile.Result{}, nil

controllers/apps/apimanager_status_reconciler_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -609,10 +609,10 @@ func TestAPIManagerStatusReconciler_Reconcile_statusConditions(t *testing.T) {
609609
}
610610
}
611611

612-
// TestAPIManagerStatusReconciler_Reconcile_requeueOnTrueToFalseTransition is a regression
613-
// test for the stale-read requeue bug: when Available transitions from True to False the
614-
// reconciler must requeue, not settle silently at Available=False.
615-
func TestAPIManagerStatusReconciler_Reconcile_requeueOnTrueToFalseTransition(t *testing.T) {
612+
// TestAPIManagerStatusReconciler_Reconcile_requeueAfterWhenUnavailable verifies that when
613+
// Available transitions from True to False the reconciler schedules a requeue rather than
614+
// settling silently at Available=False.
615+
func TestAPIManagerStatusReconciler_Reconcile_requeueAfterWhenUnavailable(t *testing.T) {
616616
namespace := "test-namespace"
617617
t.Setenv("PREFLIGHT_CHECKS_BYPASS", "true")
618618

@@ -640,7 +640,7 @@ func TestAPIManagerStatusReconciler_Reconcile_requeueOnTrueToFalseTransition(t *
640640
if err != nil {
641641
t.Fatalf("Reconcile() unexpected error: %v", err)
642642
}
643-
if !result.Requeue {
644-
t.Errorf("Reconcile() Requeue = false, want true on Available True-to-False transition")
643+
if result.RequeueAfter == 0 {
644+
t.Errorf("Reconcile() RequeueAfter = 0, want non-zero on Available True-to-False transition")
645645
}
646646
}

0 commit comments

Comments
 (0)