fix: check consecutive failures before in-progress restore early return

The in-progress restore check at line 898 was running before the
consecutive failures check, setting the phase back to Restoring and
returning early. This prevented the suspension block from correcting
the phase, causing it to stay stuck at Restoring even while suspended.

Author: passcod

src/controllers/replica.rs

@@ -894,6 +894,85 @@ pub async fn reconcile(replica: Arc<PostgresPhysicalReplica>, ctx: Arc<Context>)
 		}
 	}
 
+	// Check consecutive failures before the in-progress restore early return,
+	// so that suspension takes priority and the phase doesn't get stuck at
+	// Restoring when all restores are failing.
+	let consecutive_failures = replica
+		.status
+		.as_ref()
+		.and_then(|s| s.consecutive_restore_failures)
+		.unwrap_or(0);
+
+	const MAX_CONSECUTIVE_FAILURES: u32 = 3;
+	if consecutive_failures >= MAX_CONSECUTIVE_FAILURES {
+		if active_restore.is_some() {
+			replica.update_phase(client, ReplicaPhase::Ready).await?;
+		} else {
+			replica.update_phase(client, ReplicaPhase::Pending).await?;
+		}
+
+		let already_suspended = replica
+			.status
+			.as_ref()
+			.and_then(|s| {
+				s.conditions
+					.iter()
+					.find(|c| c.type_ == "RestoreSchedulingSuspended")
+			})
+			.is_some_and(|c| c.status == "True");
+
+		if !already_suspended {
+			warn!(
+				replica = name,
+				consecutive_failures,
+				"restore scheduling suspended after {MAX_CONSECUTIVE_FAILURES} consecutive failures"
+			);
+			replica
+				.update_condition(
+					client,
+					"RestoreSchedulingSuspended",
+					"True",
+					"ConsecutiveFailures",
+					&format!(
+						"Scheduling suspended after {consecutive_failures} consecutive restore failures. \
+						 Fix the underlying issue and reset with: kubectl patch postgresphysicalreplica {name} \
+						 -n {namespace} --subresource=status --type=merge \
+						 -p '{{\"status\":{{\"consecutiveRestoreFailures\":0}}}}'"
+					),
+				)
+				.await?;
+		}
+		return Ok(Action::requeue(Duration::from_secs(300)));
+	}
+
+	// Clear RestoreSchedulingSuspended if it was previously set but the
+	// counter has since been reset below the threshold (e.g. manual patch).
+	if replica
+		.status
+		.as_ref()
+		.and_then(|s| {
+			s.conditions
+				.iter()
+				.find(|c| c.type_ == "RestoreSchedulingSuspended")
+		})
+		.is_some_and(|c| c.status == "True")
+	{
+		info!(
+			replica = name,
+			consecutive_failures,
+			"clearing RestoreSchedulingSuspended condition (counter below threshold)"
+		);
+		replica
+			.update_condition(
+				client,
+				"RestoreSchedulingSuspended",
+				"False",
+				"CounterReset",
+				"Consecutive failure counter reset below threshold",
+			)
+			.await?;
+	}
+
 	// Decide whether to trigger a new restore
 	if let Some(in_progress) = in_progress_restore {
 		let phase = in_progress.status.as_ref().and_then(|s| s.phase.as_ref());
@@ -972,84 +1051,6 @@ pub async fn reconcile(replica: Arc<PostgresPhysicalReplica>, ctx: Arc<Context>)
 
 	let schedule_decision = replica.check_schedule();
 
-	let consecutive_failures = replica
-		.status
-		.as_ref()
-		.and_then(|s| s.consecutive_restore_failures)
-		.unwrap_or(0);
-
-	const MAX_CONSECUTIVE_FAILURES: u32 = 3;
-	if consecutive_failures >= MAX_CONSECUTIVE_FAILURES {
-		// Update phase even while suspended so it doesn't stay stuck at
-		// "Restoring" after all in-progress restores have failed.
-		if active_restore.is_some() {
-			replica.update_phase(client, ReplicaPhase::Ready).await?;
-		} else {
-			replica.update_phase(client, ReplicaPhase::Pending).await?;
-		}
-
-		let already_suspended = replica
-			.status
-			.as_ref()
-			.and_then(|s| {
-				s.conditions
-					.iter()
-					.find(|c| c.type_ == "RestoreSchedulingSuspended")
-			})
-			.is_some_and(|c| c.status == "True");
-
-		if !already_suspended {
-			warn!(
-				replica = name,
-				consecutive_failures,
-				"restore scheduling suspended after {MAX_CONSECUTIVE_FAILURES} consecutive failures"
-			);
-			replica
-				.update_condition(
-					client,
-					"RestoreSchedulingSuspended",
-					"True",
-					"ConsecutiveFailures",
-					&format!(
-						"Scheduling suspended after {consecutive_failures} consecutive restore failures. \
-						 Fix the underlying issue and reset with: kubectl patch postgresphysicalreplica {name} \
-						 -n {namespace} --subresource=status --type=merge \
-						 -p '{{\"status\":{{\"consecutiveRestoreFailures\":0}}}}'"
-					),
-				)
-				.await?;
-		}
-		return Ok(Action::requeue(Duration::from_secs(300)));
-	}
-
-	// Clear RestoreSchedulingSuspended if it was previously set but the
-	// counter has since been reset below the threshold (e.g. manual patch).
-	if replica
-		.status
-		.as_ref()
-		.and_then(|s| {
-			s.conditions
-				.iter()
-				.find(|c| c.type_ == "RestoreSchedulingSuspended")
-		})
-		.is_some_and(|c| c.status == "True")
-	{
-		info!(
-			replica = name,
-			consecutive_failures,
-			"clearing RestoreSchedulingSuspended condition (counter below threshold)"
-		);
-		replica
-			.update_condition(
-				client,
-				"RestoreSchedulingSuspended",
-				"False",
-				"CounterReset",
-				"Consecutive failure counter reset below threshold",
-			)
-			.await?;
-	}
-
 	let should_restore = never_restored
 		|| active_restore_deleted
 		|| matches!(schedule_decision, ScheduleDecision::Trigger);

tests/persistent_schemas.rs

@@ -1,6 +1,9 @@
 use k8s_openapi::api::core::v1::{LocalObjectReference, Secret};
 use kube::{Api, ResourceExt as _, api::PostParams};
-use postgres_restore_operator::types::{PostgresPhysicalReplica, PostgresPhysicalRestore, PostgresPhysicalRestoreSpec, ReplicaPhase, RestorePhase};
+use postgres_restore_operator::types::{
+	PostgresPhysicalReplica, PostgresPhysicalRestore, PostgresPhysicalRestoreSpec, ReplicaPhase,
+	RestorePhase,
+};
 use tokio::time::{sleep, timeout};
 
 use helpers::*;