diff --git a/src/controller.rs b/src/controller.rs index bc72457..f83c0a2 100644 --- a/src/controller.rs +++ b/src/controller.rs @@ -27,6 +27,7 @@ use crate::resources::ResourceSyncStatus; use crate::{requeue_after, resources::ResourceSync, util, Error, Result, FINALIZER}; const RESOURCE_SYNC_FAILING_CONDITION: &str = "ResourceSyncFailing"; +const RESOURCE_SYNC_SUCCEEDED_REASON: &str = "ResourceSyncSucceeded"; pub struct Context { pub client: Client, @@ -234,23 +235,7 @@ async fn reconcile(resource_sync: Arc, ctx: Arc) -> Resul ) .await; - let status = match &result { - Err(err) => { - let sync_failing_condition = Condition { - last_transition_time: sync_failing_transition_time(&(resource_sync.status)), - message: err.to_string(), - observed_generation: resource_sync.metadata.generation, - reason: RESOURCE_SYNC_FAILING_CONDITION.to_string(), - status: "True".to_string(), - type_: RESOURCE_SYNC_FAILING_CONDITION.to_string(), - }; - - Some(ResourceSyncStatus { - conditions: Some(vec![sync_failing_condition]), - }) - } - _ => None, - }; + let status = reconcile_status(&resource_sync, &result); if status != resource_sync.status { parent_api @@ -327,25 +312,67 @@ async fn source_and_target_apis( Ok((source_api, target_api)) } -fn sync_failing_transition_time(status: &Option) -> Time { - let now = Time(Utc::now()); +fn reconcile_status( + resource_sync: &ResourceSync, + result: &Result, +) -> Option { + match result { + Err(err) => Some(ResourceSyncStatus { + conditions: Some(vec![sync_failing_condition( + resource_sync, + "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. + Ok(_) if !resource_sync.has_been_deleted() => Some(ResourceSyncStatus { + conditions: Some(vec![sync_failing_condition( + resource_sync, + "False", + RESOURCE_SYNC_SUCCEEDED_REASON, + "Sync succeeded".to_string(), + )]), + }), + Ok(_) => resource_sync.status.clone(), + } +} - match status { - None => now, - Some(status) => match &status.conditions { - None => now, - Some(conditions) => { - let sync_failing_condition = conditions - .iter() - .find(|c| c.type_ == RESOURCE_SYNC_FAILING_CONDITION); - sync_failing_condition - .map(|c| c.last_transition_time.clone()) - .unwrap_or(now) - } - }, +fn sync_failing_condition( + resource_sync: &ResourceSync, + status: &str, + reason: &str, + message: String, +) -> Condition { + Condition { + last_transition_time: sync_failing_transition_time(&resource_sync.status, status), + message, + observed_generation: resource_sync.metadata.generation, + reason: reason.to_string(), + status: status.to_string(), + type_: RESOURCE_SYNC_FAILING_CONDITION.to_string(), } } +// The transition time is only carried over while the condition value is unchanged; a True<->False +// flip records a new transition. +fn sync_failing_transition_time(status: &Option, new_status: &str) -> Time { + let now = Time(Utc::now()); + + status + .as_ref() + .and_then(|status| status.conditions.as_ref()) + .and_then(|conditions| { + conditions + .iter() + .find(|c| c.type_ == RESOURCE_SYNC_FAILING_CONDITION) + }) + .filter(|c| c.status == new_status) + .map(|c| c.last_transition_time.clone()) + .unwrap_or(now) +} + // TODO: Exponential Backoff using DefaultBackoff for watcher fn error_policy(resource_sync: Arc, error: &Error, _ctx: Arc) -> Action { let name = resource_sync.name_any(); @@ -391,48 +418,103 @@ pub async fn run(client: Client) -> Result<()> { #[cfg(test)] mod tests { - use super::{sync_failing_transition_time, RESOURCE_SYNC_FAILING_CONDITION}; - use crate::resources::ResourceSyncStatus; + use super::{ + reconcile_status, sync_failing_transition_time, RESOURCE_SYNC_FAILING_CONDITION, + RESOURCE_SYNC_SUCCEEDED_REASON, + }; + use crate::resources::{ResourceSync, ResourceSyncStatus}; + use crate::{Error, Result}; use chrono::{TimeDelta, TimeZone}; use k8s_openapi::apimachinery::pkg::apis::meta::v1::Condition; + use k8s_openapi::apimachinery::pkg::apis::meta::v1::ObjectMeta; use k8s_openapi::apimachinery::pkg::apis::meta::v1::Time; + use kube::runtime::controller::Action; use once_cell::sync::Lazy; use rstest::rstest; static NOW: Lazy