From 3330050aa265f8dc2b7a0643dda62b5e1d2f3baf Mon Sep 17 00:00:00 2001 From: Zach Robinson Date: Fri, 27 Sep 2024 10:34:56 -0500 Subject: [PATCH 1/6] chore: ignore ide paraphernalia --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index ea8c4bf..40d9aca 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ /target +/.idea \ No newline at end of file From 20bef5f5fa8fe83f1541549542beffdc38d4399a Mon Sep 17 00:00:00 2001 From: Zach Robinson Date: Fri, 27 Sep 2024 11:48:39 -0500 Subject: [PATCH 2/6] feat: add conditions to ResourceSync status --- Cargo.lock | 1 + Cargo.toml | 2 +- manifests/crd.yml | 78 ++++++++++++++++++++++++++++++++++++++++++++--- src/resources.rs | 3 +- 4 files changed, 78 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 971a69e..85548a8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -849,6 +849,7 @@ dependencies = [ "base64 0.21.7", "bytes", "chrono", + "schemars", "serde", "serde-value", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index c15b62b..d8d97c7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,7 @@ clap = { version = "4.5", features = ["derive", "help", "env", "std"] } futures = "0.3" kube = { version = "0.87.2", features = ["runtime", "derive", "unstable-runtime"] } kube-derive = "0.87.2" -k8s-openapi = { version = "0.20.0", features = ["v1_26"] } +k8s-openapi = { version = "0.20.0", features = ["v1_26", "schemars"] } kubert = { version = "0.21.2", features = [ "clap", "runtime", diff --git a/manifests/crd.yml b/manifests/crd.yml index 61e62c7..129b74f 100644 --- a/manifests/crd.yml +++ b/manifests/crd.yml @@ -136,10 +136,80 @@ spec: status: nullable: true properties: - demo: - type: string - required: - - demo + conditions: + description: Conditions describes the current observed state of the + CloudDedicatedCluster + items: + description: "Condition contains details for one aspect of the current + state of this API Resource.\n---\nThis struct is intended for + direct use as an array at the field path .status.conditions. For + example,\n\n\n\ttype FooStatus struct{\n\t // Represents the + observations of a foo's current state.\n\t // Known .status.conditions.type + are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // + +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t + \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" + patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t + \ // other fields\n\t}" + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: |- + type of condition in CamelCase or in foo.example.com/CamelCase. + --- + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be + useful (see .node.status.conditions), the ability to deconflict is important. + The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map type: object required: - spec diff --git a/src/resources.rs b/src/resources.rs index ae288a4..6b11be6 100644 --- a/src/resources.rs +++ b/src/resources.rs @@ -1,6 +1,7 @@ use k8s_openapi::apiextensions_apiserver::pkg::apis::apiextensions::v1::{ CustomResourceDefinition, CustomResourceValidation, JSONSchemaProps, }; +use k8s_openapi::apimachinery::pkg::apis::meta::v1::Condition; use kube::{ core::{gvk::ParseGroupVersionError, GroupVersionKind, TypeMeta}, CustomResource, @@ -36,7 +37,7 @@ pub struct Mapping { #[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct ResourceSyncStatus { - pub demo: String, + pub conditions: Option>, } #[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema, Hash, PartialEq, Eq)] From a9bc4ed09ba9376918061a27a56a0fcfbdba256e Mon Sep 17 00:00:00 2001 From: Zach Robinson Date: Fri, 27 Sep 2024 13:37:58 -0500 Subject: [PATCH 3/6] feat: implement status patching --- src/controller.rs | 118 ++++++++++++++++++++++++++++++++++++---------- src/resources.rs | 2 +- 2 files changed, 93 insertions(+), 27 deletions(-) diff --git a/src/controller.rs b/src/controller.rs index 9c8b7b9..a400f71 100644 --- a/src/controller.rs +++ b/src/controller.rs @@ -1,7 +1,6 @@ -use std::{sync::Arc, time::Duration}; - use futures::StreamExt; -use k8s_openapi::apimachinery::pkg::apis::meta::v1::OwnerReference; +use k8s_openapi::apimachinery::pkg::apis::meta::v1::{Condition, OwnerReference, Time}; +use k8s_openapi::chrono::Utc; use kube::api::DeleteParams; use kube::api::Patch::Merge; use kube::runtime::{predicates, reflector, WatchStreamExt}; @@ -14,6 +13,8 @@ use kube::{ Api, Client, Resource, ResourceExt, }; use serde_json::json; +use std::string::ToString; +use std::{sync::Arc, time::Duration}; #[allow(unused_imports)] use tracing::{debug, error, info, warn}; @@ -22,18 +23,27 @@ use util::{WithItemAdded, WithItemRemoved}; use crate::mapping::{apply_mappings, clone_resource}; use crate::remote_watcher_manager::RemoteWatcherManager; use crate::resource_extensions::NamespacedApi; +use crate::resources::ResourceSyncStatus; use crate::{requeue_after, resources::ResourceSync, util, Error, Result, FINALIZER}; +static RESOURCE_SYNC_FAILING_CONDITION: &str = "ResourceSyncFailing"; + pub struct Context { pub client: Client, pub remote_watcher_manager: RemoteWatcherManager, } +macro_rules! apply_patch_params { + () => { + PatchParams::apply(&ResourceSync::group(&())).force() + }; +} + async fn reconcile_deleted_resource( resource_sync: Arc, name: &str, target_api: NamespacedApi, - parent_api: Api, + parent_api: &Api, ctx: Arc, ) -> Result { if !resource_sync.has_target_finalizer() { @@ -88,7 +98,7 @@ async fn reconcile_deleted_resource( async fn add_target_finalizer( resource_sync: Arc, name: &str, - parent_api: Api, + parent_api: &Api, ) -> Result { let patched_finalizers = resource_sync .finalizers_clone_or_empty() @@ -161,7 +171,7 @@ async fn reconcile_normally( debug!(?target, "produced target object"); - let ssapply = PatchParams::apply(&ResourceSync::group(&())).force(); + let ssapply = apply_patch_params!(); target_api .patch(&target_ref.name, &ssapply, &Patch::Apply(&target)) .await?; @@ -185,31 +195,87 @@ async fn reconcile(resource_sync: Arc, ctx: Arc) -> Resul .name .to_owned() .ok_or(Error::NameRequired)?; - info!(?name, "running reconciler"); + let parent_api = resource_sync.api(ctx.client.clone()); - debug!(?resource_sync.spec, "got"); - let local_ns = resource_sync.namespace().ok_or(Error::NamespaceRequired)?; + let result = { + let resource_sync = Arc::clone(&resource_sync); - let target_api = resource_sync - .spec - .target - .api_for(ctx.client.clone(), &local_ns) - .await?; - let source_api = resource_sync - .spec - .source - .api_for(ctx.client.clone(), &local_ns) - .await?; - let parent_api = resource_sync.api(ctx.client.clone()); + info!(?name, "running reconciler"); + + debug!(?resource_sync.spec, "got"); + let local_ns = resource_sync.namespace().ok_or(Error::NamespaceRequired)?; + + let target_api = resource_sync + .spec + .target + .api_for(ctx.client.clone(), &local_ns) + .await?; + let source_api = resource_sync + .spec + .source + .api_for(ctx.client.clone(), &local_ns) + .await?; - match resource_sync { - resource_sync if resource_sync.has_been_deleted() => { - reconcile_deleted_resource(resource_sync, &name, target_api, parent_api, ctx).await + match resource_sync { + resource_sync if resource_sync.has_been_deleted() => { + reconcile_deleted_resource(resource_sync, &name, target_api, &parent_api, ctx).await + } + resource_sync if !resource_sync.has_target_finalizer() => { + add_target_finalizer(resource_sync, &name, &parent_api).await + } + _ => reconcile_normally(resource_sync, &name, source_api, target_api, ctx).await, } - resource_sync if !resource_sync.has_target_finalizer() => { - add_target_finalizer(resource_sync, &name, parent_api).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]), + }) } - _ => reconcile_normally(resource_sync, &name, source_api, target_api, ctx).await, + _ => None, + }; + + if status != resource_sync.status { + let patch_params = apply_patch_params!(); + parent_api + .patch_status( + &name, + &patch_params, + &Patch::Apply(json!({"status": status})), + ) + .await?; + } + + result +} + +// TODO: Unit Test +fn sync_failing_transition_time(status: &Option) -> Time { + let now = Time(Utc::now()); + + 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) + } + }, } } diff --git a/src/resources.rs b/src/resources.rs index 6b11be6..e92ae65 100644 --- a/src/resources.rs +++ b/src/resources.rs @@ -34,7 +34,7 @@ pub struct Mapping { pub to_field_path: Option, } -#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema)] +#[derive(Deserialize, Serialize, Clone, Debug, Default, JsonSchema, PartialEq)] #[serde(rename_all = "camelCase")] pub struct ResourceSyncStatus { pub conditions: Option>, From 9c8cf3662c7c74fedb6570ebd2a7a8f8b1ed0285 Mon Sep 17 00:00:00 2001 From: Zach Robinson Date: Fri, 27 Sep 2024 16:20:17 -0500 Subject: [PATCH 4/6] feat: add a few unit tests --- src/controller.rs | 48 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/src/controller.rs b/src/controller.rs index a400f71..5582c5c 100644 --- a/src/controller.rs +++ b/src/controller.rs @@ -259,7 +259,6 @@ async fn reconcile(resource_sync: Arc, ctx: Arc) -> Resul result } -// TODO: Unit Test fn sync_failing_transition_time(status: &Option) -> Time { let now = Time(Utc::now()); @@ -323,4 +322,49 @@ pub async fn run(client: Client) -> Result<()> { } #[cfg(test)] -mod tests {} +mod tests { + use super::{sync_failing_transition_time, RESOURCE_SYNC_FAILING_CONDITION}; + use crate::resources::ResourceSyncStatus; + use chrono::{TimeDelta, TimeZone}; + use k8s_openapi::apimachinery::pkg::apis::meta::v1::Condition; + use k8s_openapi::apimachinery::pkg::apis::meta::v1::Time; + use once_cell::sync::Lazy; + use rstest::rstest; + + static NOW: Lazy