fix: degrade condition overwritten by the next reconcile (backport #220)

internal/controller/bootcnodepool_controller.go

@@ -311,14 +311,6 @@ func (r *BootcNodePoolReconciler) Reconcile(
 	// emit events only for persisted transitions.
 	statusOrig := pool.Status.DeepCopy()
 
-	// Start with conditions in a healthy state; sync functions only set
-	// degraded conditions when something is wrong.
-	apimeta.SetStatusCondition(&pool.Status.Conditions, metav1.Condition{
-		Type:   bootcv1alpha1.PoolDegraded,
-		Status: metav1.ConditionFalse,
-		Reason: bootcv1alpha1.PoolHealthy,
-	})
-
 	// From this point on, let's not re-Get() the pool again and just reuse
 	// `pool` so that we have a consistent view for this reconciliation run.
 	// Status writes to `pool` are also permitted; we Update() back once at
@@ -333,7 +325,13 @@ func (r *BootcNodePoolReconciler) Reconcile(
 		}
 		return ctrl.Result{}, nil
 	}
-	resolveResult, tagTargetChanged, err := r.resolveTargetDigest(ctx, &pool, secretData)
+	clearPoolDegradedReason(&pool, bootcv1alpha1.PoolSecretError)
+
+	resolveResult, tagTargetChanged, err := r.resolveTargetDigest(
+		ctx,
+		&pool,
+		secretData,
+	)
 	if err != nil {
 		if isInvalidSpecError(err) {
 			return r.setInvalidSpecCondition(ctx, &pool, statusOrig, err)
@@ -356,6 +354,14 @@ func (r *BootcNodePoolReconciler) Reconcile(
 		return complete(resolveResult)
 	}
 
+	// Clear stale rollout-path degraded conditions so the checks below
+	// can re-evaluate from scratch. SecretError and TagResolutionError
+	// are managed by their own checks above and must not be cleared here.
+	clearPoolDegradedReason(&pool, bootcv1alpha1.PoolNodeConflict)
+	clearPoolDegradedReason(&pool, bootcv1alpha1.PoolNodeDegraded)
+	clearPoolDegradedReason(&pool, bootcv1alpha1.PoolRolloutHalted)
+	clearPoolDegradedReason(&pool, bootcv1alpha1.PoolInvalidSpec)
+
 	// Sync pool membership and retrieve BootcNodes we own.
 	ownedBootcNodes, err := r.syncMembership(ctx, &pool)
 	if err != nil {
@@ -470,6 +476,7 @@ func (r *BootcNodePoolReconciler) resolveTargetDigest(
 		// Reset the NextTagResolutionTime in case we pass from a tag referenced image to a digested one.
 		// Otherwise, it simply a nop
 		pool.Status.NextTagResolutionTime = nil
+		clearPoolDegradedReason(pool, bootcv1alpha1.PoolTagResolutionError)
 		return ctrl.Result{}, false, nil
 	}
 
@@ -493,6 +500,7 @@ func (r *BootcNodePoolReconciler) resolveTargetDigest(
 			log.Info("Resolved tag to new digest", "ref", pool.Spec.Image.Ref, "digest", digest)
 		}
 		pool.Status.TargetDigest = digest
+		clearPoolDegradedReason(pool, bootcv1alpha1.PoolTagResolutionError)
 	}
 
 	next := metav1.NewTime(now.Add(r.TagResolutionInterval))
@@ -916,14 +924,14 @@ var poolDegradedPrecedence = map[string]int{
 }
 
 // setPoolDegraded sets the Degraded condition on the pool, but only if the new
-// reason has higher precedence than the current one. This prevents e.g.
-// RolloutHalted from overwriting NodeConflict. Unknown reasons get precedence
-// 0 (map zero-value), so they silently no-op. Keep poolDegradedPrecedence in
-// sync with the API constants.
+// reason has equal or higher precedence than the current one. Equal precedence
+// allows message updates (e.g. a new error string for the same reason) without
+// changing LastTransitionTime (SetStatusCondition preserves it when the status
+// is unchanged). Keep poolDegradedPrecedence in sync with the API constants.
 func setPoolDegraded(pool *bootcv1alpha1.BootcNodePool, reason, message string) {
 	current := apimeta.FindStatusCondition(pool.Status.Conditions, bootcv1alpha1.PoolDegraded)
 	if current != nil && current.Status == metav1.ConditionTrue {
-		if poolDegradedPrecedence[reason] <= poolDegradedPrecedence[current.Reason] {
+		if poolDegradedPrecedence[reason] < poolDegradedPrecedence[current.Reason] {
 			return
 		}
 	}
@@ -934,3 +942,18 @@ func setPoolDegraded(pool *bootcv1alpha1.BootcNodePool, reason, message string)
 		Message: message,
 	})
 }
+
+// clearPoolDegradedReason clears the Degraded condition to Healthy, but only
+// if the current reason matches. This allows each check to clear its own
+// degraded reason on success without affecting conditions set by other checks.
+func clearPoolDegradedReason(pool *bootcv1alpha1.BootcNodePool, reason string) {
+	current := apimeta.FindStatusCondition(pool.Status.Conditions, bootcv1alpha1.PoolDegraded)
+	if current == nil || current.Status != metav1.ConditionTrue || current.Reason != reason {
+		return
+	}
+	apimeta.SetStatusCondition(&pool.Status.Conditions, metav1.Condition{
+		Type:   bootcv1alpha1.PoolDegraded,
+		Status: metav1.ConditionFalse,
+		Reason: bootcv1alpha1.PoolHealthy,
+	})
+}

internal/controller/events_test.go

@@ -14,6 +14,7 @@ import (
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
 	eventsv1 "k8s.io/api/events/v1"
+	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -396,6 +397,52 @@ func TestRecordPoolEvents(t *testing.T) {
 	}
 }
 
+func TestResolveTargetDigestDeferredPreservesDegraded(t *testing.T) {
+	g := NewWithT(t)
+	pool := testutil.NewPool(
+		"bad-tag",
+		testutil.ImageTaggedRef,
+		testutil.WithWorkerSelector(),
+	)
+	// Simulate a prior failed resolution: NextTagResolutionTime is set
+	// in the future but TargetDigest was never populated.
+	future := metav1.NewTime(time.Now().Add(time.Hour))
+	pool.Status.NextTagResolutionTime = &future
+
+	// Set the degraded condition from a previous reconcile directly
+	// on the pool (no blanket reset, so it persists naturally).
+	origMessage := "dial tcp: lookup registry.example.com: no such host"
+	origTransition := metav1.NewTime(time.Now().Add(-10 * time.Minute))
+	apimeta.SetStatusCondition(&pool.Status.Conditions, metav1.Condition{
+		Type:               bootcv1alpha1.PoolDegraded,
+		Status:             metav1.ConditionTrue,
+		Reason:             bootcv1alpha1.PoolTagResolutionError,
+		Message:            origMessage,
+		LastTransitionTime: origTransition,
+	})
+
+	reconciler := &BootcNodePoolReconciler{
+		TagResolver:           staticTagResolver{digest: testDigestA},
+		TagResolutionInterval: time.Hour,
+	}
+
+	result, changed, err := reconciler.resolveTargetDigest(
+		context.Background(),
+		pool,
+		nil,
+	)
+
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(changed).To(BeFalse())
+	g.Expect(result.RequeueAfter).To(BeNumerically(">", 0))
+	degraded := apimeta.FindStatusCondition(pool.Status.Conditions, bootcv1alpha1.PoolDegraded)
+	g.Expect(degraded).NotTo(BeNil())
+	g.Expect(degraded.Status).To(Equal(metav1.ConditionTrue))
+	g.Expect(degraded.Reason).To(Equal(bootcv1alpha1.PoolTagResolutionError))
+	g.Expect(degraded.Message).To(Equal(origMessage))
+	g.Expect(degraded.LastTransitionTime).To(Equal(origTransition))
+}
+
 func TestResolveTargetDigestReportsTagChange(t *testing.T) {
 	g := NewWithT(t)
 	pool := testutil.NewPool(