fix: Always write sync status instead of comparing against stale cache

src/controller.rs

@@ -235,9 +235,17 @@ async fn reconcile(resource_sync: Arc<ResourceSync>, ctx: Arc<Context>) -> Resul
     )
     .await;
 
-    let status = reconcile_status(&resource_sync, &result);
+    // Always write the status, and compute it from the live object rather than the reflector
+    // cache. The cache can lag our own previous patch: deciding off it can skip the write and
+    // leave the condition latched, and carrying its stale condition over corrupts
+    // lastTransitionTime. Skip the get when we won't write; the object may already be gone.
+    let live_status = if result.is_err() || !resource_sync.has_been_deleted() {
+        parent_api.get_status(&name).await?.status
+    } else {
+        None
+    };
 
-    if status != resource_sync.status {
+    if let Some(status) = reconcile_status(&resource_sync, &live_status, &result) {
         parent_api
             .patch_status(
                 &name,
@@ -314,39 +322,45 @@ async fn source_and_target_apis(
 
 fn reconcile_status(
     resource_sync: &ResourceSync,
+    live_status: &Option<ResourceSyncStatus>,
     result: &Result<Action>,
 ) -> Option<ResourceSyncStatus> {
     match result {
         Err(err) => Some(ResourceSyncStatus {
             conditions: Some(vec![sync_failing_condition(
                 resource_sync,
+                live_status,
                 "True",
                 RESOURCE_SYNC_FAILING_CONDITION,
                 err.to_string(),
             )]),
         }),
         // A successful reconcile must reset the condition to False rather than leave the last
-        // failure latched. Skip this for deleted resources; their finalizer may already be gone.
+        // failure latched.
         Ok(_) if !resource_sync.has_been_deleted() => Some(ResourceSyncStatus {
             conditions: Some(vec![sync_failing_condition(
                 resource_sync,
+                live_status,
                 "False",
                 RESOURCE_SYNC_SUCCEEDED_REASON,
                 "Sync succeeded".to_string(),
             )]),
         }),
-        Ok(_) => resource_sync.status.clone(),
+        // None means don't write: a deleted resource's finalizer may already be gone, so a status
+        // patch could 404.
+        Ok(_) => None,
     }
 }
 
 fn sync_failing_condition(
     resource_sync: &ResourceSync,
+    live_status: &Option<ResourceSyncStatus>,
     status: &str,
     reason: &str,
     message: String,
 ) -> Condition {
     Condition {
-        last_transition_time: sync_failing_transition_time(&resource_sync.status, status),
+        last_transition_time: sync_failing_transition_time(live_status, status),
         message,
         observed_generation: resource_sync.metadata.generation,
         reason: reason.to_string(),
@@ -490,7 +504,7 @@ mod tests {
         let rs = resource_sync(false, None);
         let result: Result<Action> = Err(Error::NameRequired);
 
-        let condition = single_condition(reconcile_status(&rs, &result));
+        let condition = single_condition(reconcile_status(&rs, &rs.status, &result));
 
         assert_eq!(condition.type_, RESOURCE_SYNC_FAILING_CONDITION);
         assert_eq!(condition.status, "True");
@@ -503,18 +517,32 @@ mod tests {
         let rs = resource_sync(false, status_with_condition("True"));
         let result: Result<Action> = Ok(Action::await_change());
 
-        let condition = single_condition(reconcile_status(&rs, &result));
+        let condition = single_condition(reconcile_status(&rs, &rs.status, &result));
 
         assert_eq!(condition.type_, RESOURCE_SYNC_FAILING_CONDITION);
         assert_eq!(condition.status, "False");
         assert_eq!(condition.reason, RESOURCE_SYNC_SUCCEEDED_REASON);
     }
 
     #[test]
-    fn test_reconcile_status_ok_deleted_keeps_existing_status() {
+    fn test_reconcile_status_uses_live_status_for_transition_time() {
+        // The cache still says True but the live object has already transitioned to False; the
+        // carried-over time must come from the live condition.
+        let rs = resource_sync(false, status_with_condition("True"));
+        let live_status = status_with_condition("False");
+        let result: Result<Action> = Ok(Action::await_change());
+
+        let condition = single_condition(reconcile_status(&rs, &live_status, &result));
+
+        assert_eq!(condition.status, "False");
+        assert_eq!(condition.last_transition_time, *EPOCH);
+    }
+
+    #[test]
+    fn test_reconcile_status_ok_deleted_skips_status_write() {
         let rs = resource_sync(true, status_with_condition("True"));
         let result: Result<Action> = Ok(Action::await_change());
 
-        assert_eq!(reconcile_status(&rs, &result), rs.status);
+        assert_eq!(reconcile_status(&rs, &rs.status, &result), None);
     }
 }