From 8389128377129b5e3b2f20131d1b9b74dcd752fd Mon Sep 17 00:00:00 2001 From: Daniel Gerlag Date: Thu, 19 Sep 2024 10:08:36 -0700 Subject: [PATCH] resource error messages (#59) --- cli/cmd/list.go | 27 ++++- .../kubernetes_provider/src/actors/mod.rs | 12 +- .../src/actors/querycontainer_actor.rs | 23 +++- .../src/actors/reaction_actor.rs | 22 +++- .../src/actors/source_actor.rs | 24 +++- .../src/controller/reconciler.rs | 110 +++++++++++++++--- .../src/api/v1/mappings/query_container.rs | 1 + .../mgmt_api/src/api/v1/mappings/reaction.rs | 1 + .../mgmt_api/src/api/v1/mappings/source.rs | 9 +- .../src/api/v1/models/query_container.rs | 3 +- .../mgmt_api/src/api/v1/models/reaction.rs | 3 +- .../mgmt_api/src/api/v1/models/source.rs | 2 + .../mgmt_api/src/domain/mappings.rs | 27 +---- control-planes/mgmt_api/src/domain/models.rs | 9 +- .../resource_provider_api/src/models.rs | 5 +- 15 files changed, 200 insertions(+), 78 deletions(-) diff --git a/cli/cmd/list.go b/cli/cmd/list.go index 4c120b07..51a53721 100644 --- a/cli/cmd/list.go +++ b/cli/cmd/list.go @@ -1,15 +1,15 @@ package cmd import ( - "fmt" - "os" - "reflect" - "sort" - "drasi.io/cli/api" "drasi.io/cli/service" + "fmt" "github.com/olekukonko/tablewriter" "github.com/spf13/cobra" + "os" + "reflect" + "sort" + "strings" ) func NewListCommand() *cobra.Command { @@ -74,7 +74,22 @@ Example: for k := 0; k < len(itemStatus); k++ { statusFieldName := itemStatus[k].String() statusFields[statusFieldName] = nil - item[statusFieldName] = fmt.Sprintf("%v", reflect.ValueOf(result[i].Status).MapIndex(itemStatus[k]).Elem()) + val := reflect.ValueOf(result[i].Status).MapIndex(itemStatus[k]) + + switch val.Elem().Kind() { + case reflect.Map: + var builder strings.Builder + iter := val.Elem().MapRange() + for iter.Next() { + builder.WriteString(iter.Key().String()) + builder.WriteString(" - ") + builder.WriteString(iter.Value().Elem().String()) + builder.WriteString("\n") + } + item[statusFieldName] = builder.String() + default: + item[statusFieldName] = fmt.Sprintf(" %v", val.Elem()) + } } } items = append(items, item) diff --git a/control-planes/kubernetes_provider/src/actors/mod.rs b/control-planes/kubernetes_provider/src/actors/mod.rs index a5138f2a..78681e30 100644 --- a/control-planes/kubernetes_provider/src/actors/mod.rs +++ b/control-planes/kubernetes_provider/src/actors/mod.rs @@ -14,7 +14,7 @@ use dapr::server::{ utils::DaprJson, }; use resource_provider_api::models::ResourceRequest; -use std::collections::HashMap; +use std::collections::BTreeMap; use std::marker; use tokio::sync::RwLock; @@ -29,7 +29,7 @@ pub struct ResourceActor { resource_type: String, runtime_config: RuntimeConfig, spec_builder: Box + Send + Sync>, - controllers: RwLock>, + controllers: RwLock>, kube_config: kube::Config, _owns_tstatus: marker::PhantomData, } @@ -108,7 +108,7 @@ impl ResourceActor { let controllers = self.controllers.read().await; - for controller in controllers.iter() { + for (_, controller) in controllers.iter() { controller.deprovision(); } @@ -162,7 +162,7 @@ impl ResourceActor { let mut controllers = self.controllers.write().await; controllers.clear(); for spec in specs { - controllers.push(ResourceController::start(self.kube_config.clone(), spec)); + controllers.insert(spec.service_name.clone(), ResourceController::start(self.kube_config.clone(), spec)); } } @@ -172,7 +172,7 @@ impl ResourceActor { let mut recievers = Vec::new(); let controllers = self.controllers.read().await; - for controller in controllers.iter() { + for (_, controller) in controllers.iter() { recievers.push(controller.reconcile()); } @@ -185,7 +185,7 @@ impl ResourceActor { let mut recievers = Vec::new(); let controllers = self.controllers.read().await; - for controller in controllers.iter() { + for (_, controller) in controllers.iter() { recievers.push(controller.reconcile()); } diff --git a/control-planes/kubernetes_provider/src/actors/querycontainer_actor.rs b/control-planes/kubernetes_provider/src/actors/querycontainer_actor.rs index 5a0c2907..a978b418 100644 --- a/control-planes/kubernetes_provider/src/actors/querycontainer_actor.rs +++ b/control-planes/kubernetes_provider/src/actors/querycontainer_actor.rs @@ -6,7 +6,7 @@ use axum::{response::IntoResponse, Json}; use dapr::{server::actor::context_client::ActorContextClient}; use dapr_macros::actor; use resource_provider_api::models::{QueryContainerSpec, QueryContainerStatus}; -use std::marker; +use std::{collections::BTreeMap, marker}; use tokio::sync::RwLock; use super::ResourceActor; @@ -29,7 +29,7 @@ impl QueryContainerActor { resource_type: "querycontainer".to_string(), runtime_config, spec_builder: Box::new(QueryContainerSpecBuilder {}), - controllers: RwLock::new(Vec::new()), + controllers: RwLock::new(BTreeMap::new()), kube_config, _owns_tstatus: marker::PhantomData, } @@ -39,7 +39,22 @@ impl QueryContainerActor { let controllers = self.controllers.read().await; let available = controllers .iter() - .all(|c| c.status() == ReconcileStatus::Online); - Json(QueryContainerStatus { available }) + .all(|c| c.1.status() == ReconcileStatus::Online); + + let mut messages = BTreeMap::new(); + for (name, controller) in controllers.iter() { + match controller.status() { + ReconcileStatus::Unknown => messages.insert(name.clone(), "Unknown".to_string()), + ReconcileStatus::Offline(msg) => messages.insert(name.clone(), msg), + ReconcileStatus::Online => continue, + }; + } + + Json(QueryContainerStatus { + available, + messages: Some(messages), + + }) } + } diff --git a/control-planes/kubernetes_provider/src/actors/reaction_actor.rs b/control-planes/kubernetes_provider/src/actors/reaction_actor.rs index 93e6a630..f8af397f 100644 --- a/control-planes/kubernetes_provider/src/actors/reaction_actor.rs +++ b/control-planes/kubernetes_provider/src/actors/reaction_actor.rs @@ -6,7 +6,7 @@ use axum::{response::IntoResponse, Json}; use dapr::{server::actor::context_client::ActorContextClient}; use dapr_macros::actor; use resource_provider_api::models::{ReactionSpec, ReactionStatus}; -use std::marker; +use std::{collections::BTreeMap, marker}; use tokio::sync::RwLock; use super::ResourceActor; @@ -29,7 +29,7 @@ impl ReactionActor { resource_type: "reaction".to_string(), runtime_config, spec_builder: Box::new(ReactionSpecBuilder {}), - controllers: RwLock::new(Vec::new()), + controllers: RwLock::new(BTreeMap::new()), kube_config, _owns_tstatus: marker::PhantomData, } @@ -40,7 +40,21 @@ impl ReactionActor { let controllers = self.controllers.read().await; let available = controllers .iter() - .all(|c| c.status() == ReconcileStatus::Online); - Json(ReactionStatus { available }) + .all(|c| c.1.status() == ReconcileStatus::Online); + + let mut messages = BTreeMap::new(); + for (name, controller) in controllers.iter() { + match controller.status() { + ReconcileStatus::Unknown => messages.insert(name.clone(), "Unknown".to_string()), + ReconcileStatus::Offline(msg) => messages.insert(name.clone(), msg), + ReconcileStatus::Online => continue, + }; + } + + Json(ReactionStatus { + available, + messages: Some(messages), + + }) } } diff --git a/control-planes/kubernetes_provider/src/actors/source_actor.rs b/control-planes/kubernetes_provider/src/actors/source_actor.rs index abc72dd8..65559d72 100644 --- a/control-planes/kubernetes_provider/src/actors/source_actor.rs +++ b/control-planes/kubernetes_provider/src/actors/source_actor.rs @@ -5,8 +5,8 @@ use crate::{ use axum::{response::IntoResponse, Json}; use dapr::{server::actor::context_client::ActorContextClient}; use dapr_macros::actor; -use resource_provider_api::models::{SourceProviderStatus, SourceSpec, SourceStatus}; -use std::{collections::HashMap, marker}; +use resource_provider_api::models::{SourceSpec, SourceStatus}; +use std::{collections::BTreeMap, marker}; use tokio::sync::RwLock; use super::ResourceActor; @@ -29,7 +29,7 @@ impl SourceActor { resource_type: "source".to_string(), runtime_config, spec_builder: Box::new(SourceSpecBuilder {}), - controllers: RwLock::new(Vec::new()), + controllers: RwLock::new(BTreeMap::new()), kube_config, _owns_tstatus: marker::PhantomData, } @@ -39,7 +39,21 @@ impl SourceActor { let controllers = self.controllers.read().await; let available = controllers .iter() - .all(|c| c.status() == ReconcileStatus::Online); - Json(SourceStatus { available }) + .all(|c| c.1.status() == ReconcileStatus::Online); + + let mut messages = BTreeMap::new(); + for (name, controller) in controllers.iter() { + match controller.status() { + ReconcileStatus::Unknown => messages.insert(name.clone(), "Unknown".to_string()), + ReconcileStatus::Offline(msg) => messages.insert(name.clone(), msg), + ReconcileStatus::Online => continue, + }; + } + + Json(SourceStatus { + available, + messages: Some(messages), + + }) } } diff --git a/control-planes/kubernetes_provider/src/controller/reconciler.rs b/control-planes/kubernetes_provider/src/controller/reconciler.rs index cf5bac71..dc42bff2 100644 --- a/control-planes/kubernetes_provider/src/controller/reconciler.rs +++ b/control-planes/kubernetes_provider/src/controller/reconciler.rs @@ -4,15 +4,13 @@ use either::Either; use hashers::jenkins::spooky_hash::SpookyHasher; use k8s_openapi::api::{ apps::v1::{ - Deployment, DeploymentSpec, DeploymentStatus, DeploymentStrategy, RollingUpdateDeployment, + Deployment, DeploymentSpec, DeploymentStrategy, }, - core::v1::{ConfigMap, PersistentVolumeClaim, Service}, + core::v1::{ConfigMap, PersistentVolumeClaim, Pod, Service}, }; use kube::{ - api::{DeleteParams, Patch, PatchParams, PostParams}, - config, + api::{DeleteParams, ListParams, Patch, PatchParams, PostParams}, core::ObjectMeta, - runtime::wait::delete, Api, }; use serde::Serialize; @@ -26,6 +24,7 @@ pub struct ResourceReconciler { hash: String, component_api: Api, deployment_api: Api, + pod_api: Api, cm_api: Api, pvc_api: Api, service_api: Api, @@ -36,7 +35,7 @@ pub struct ResourceReconciler { #[derive(Clone, Debug, Serialize, PartialEq)] pub enum ReconcileStatus { Unknown, - Offline, + Offline(String), Online, } @@ -62,6 +61,7 @@ impl ResourceReconciler { spec, component_api: Api::default_namespaced(client.clone()), deployment_api: Api::default_namespaced(client.clone()), + pod_api: Api::default_namespaced(client.clone()), cm_api: Api::default_namespaced(client.clone()), pvc_api: Api::default_namespaced(client.clone()), service_api: Api::default_namespaced(client.clone()), @@ -70,14 +70,90 @@ impl ResourceReconciler { } } - fn update_deployment_status(&mut self, status: Option) { - match status { + async fn update_deployment_status(&mut self, deployment: &Deployment) { + match &deployment.status { Some(status) => { - if status.available_replicas.unwrap_or(0) > 0 { - self.status = ReconcileStatus::Online; - } else { - self.status = ReconcileStatus::Offline; - } + if status.available_replicas.unwrap_or(0) == 0 { + + let spec = match &deployment.spec { + Some(spec) => spec, + None => { + self.status = ReconcileStatus::Offline("-".to_string()); + return; + }, + }; + + let deployment_labels = match &spec.selector.match_labels { + Some(labels) => labels, + None => { + self.status = ReconcileStatus::Offline("-".to_string()); + return; + }, + }; + + let label_selector = deployment_labels + .into_iter() + .map(|(k, v)| format!("{}={}", k, v)) + .collect::>() + .join(","); + + let lp = ListParams::default().labels(&label_selector); + let pod_list = match self.pod_api.list(&lp).await { + Ok(pod_list) => pod_list, + Err(_) => { + self.status = ReconcileStatus::Offline("-".to_string()); + return; + }, + }; + + let mut errors = Vec::new(); + for pod in pod_list.items { + match pod.status { + Some(pod_status) => { + match pod_status.container_statuses { + Some(container_statuses) => { + for container_status in container_statuses { + match container_status.state { + Some(state) => { + match state.terminated { + Some(terminated) => { + errors.push(format!( + "{}: {} {}", + container_status.name, + terminated.reason.unwrap_or_default(), + terminated.message.unwrap_or("Terminated".to_string()) + )); + } + None => {}, + } + match state.waiting { + Some(waiting) => { + errors.push(format!( + "{}: waiting: {} {}", + container_status.name, + waiting.reason.unwrap_or_default(), + waiting.message.unwrap_or_default() + )); + } + None => {}, + } + } + None => continue, + } + } + } + None => continue, + } + } + None => continue, + } + } + + self.status = ReconcileStatus::Offline(errors.join("\n")); + return; + } + + self.status = ReconcileStatus::Online; } None => self.status = ReconcileStatus::Unknown, } @@ -93,7 +169,7 @@ impl ResourceReconciler { let pp = DeleteParams::default(); match self.deployment_api.delete(&name, &pp).await { Ok(delete_result) => match delete_result { - Either::Left(dep) => self.update_deployment_status(dep.status), + Either::Left(dep) => self.update_deployment_status(&dep).await, Either::Right(_) => {} }, Err(err) => return Err(Box::new(err)), @@ -174,14 +250,14 @@ impl ResourceReconciler { match self.deployment_api.get(&name).await { Ok(current) => { - self.update_deployment_status(current.status); + self.update_deployment_status(¤t).await; let current_hash = current.metadata.annotations.unwrap()["drasi/spechash"].clone(); if current_hash != self.hash { log::info!("Updating deployment {}", name); let pp = PatchParams::default(); let pat = Patch::Merge(&dep); let update_result = self.deployment_api.patch(&name, &pp, &pat).await?; - self.update_deployment_status(update_result.status); + self.update_deployment_status(&update_result).await; } } Err(e) => match e { @@ -193,7 +269,7 @@ impl ResourceReconciler { log::info!("Creating deployment {}", name); let pp = PostParams::default(); let create_result = self.deployment_api.create(&pp, &dep).await?; - self.update_deployment_status(create_result.status); + self.update_deployment_status(&create_result).await; } _ => return Err(Box::new(e)), }, diff --git a/control-planes/mgmt_api/src/api/v1/mappings/query_container.rs b/control-planes/mgmt_api/src/api/v1/mappings/query_container.rs index 99ee3b68..8c62fc1c 100644 --- a/control-planes/mgmt_api/src/api/v1/mappings/query_container.rs +++ b/control-planes/mgmt_api/src/api/v1/mappings/query_container.rs @@ -6,6 +6,7 @@ impl From for QueryContainerStatusDto { fn from(status: QueryContainerStatus) -> Self { QueryContainerStatusDto { available: status.available, + messages: status.messages, } } } diff --git a/control-planes/mgmt_api/src/api/v1/mappings/reaction.rs b/control-planes/mgmt_api/src/api/v1/mappings/reaction.rs index 7f7e7402..fd013899 100644 --- a/control-planes/mgmt_api/src/api/v1/mappings/reaction.rs +++ b/control-planes/mgmt_api/src/api/v1/mappings/reaction.rs @@ -6,6 +6,7 @@ impl From for ReactionStatusDto { fn from(status: ReactionStatus) -> Self { ReactionStatusDto { available: status.available, + messages: status.messages, } } } diff --git a/control-planes/mgmt_api/src/api/v1/mappings/source.rs b/control-planes/mgmt_api/src/api/v1/mappings/source.rs index 52b8cd35..1d9a697a 100644 --- a/control-planes/mgmt_api/src/api/v1/mappings/source.rs +++ b/control-planes/mgmt_api/src/api/v1/mappings/source.rs @@ -2,18 +2,11 @@ use crate::domain::models::{SourceSpec, SourceStatus}; use super::{SourceSpecDto, SourceStatusDto}; -impl Into for SourceStatusDto { - fn into(self) -> SourceStatus { - SourceStatus { - available: self.available, - } - } -} - impl From for SourceStatusDto { fn from(status: SourceStatus) -> Self { SourceStatusDto { available: status.available, + messages: status.messages, } } } diff --git a/control-planes/mgmt_api/src/api/v1/models/query_container.rs b/control-planes/mgmt_api/src/api/v1/models/query_container.rs index 8c82033c..3422b7a3 100644 --- a/control-planes/mgmt_api/src/api/v1/models/query_container.rs +++ b/control-planes/mgmt_api/src/api/v1/models/query_container.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use serde::{Deserialize, Serialize}; @@ -38,4 +38,5 @@ pub struct QueryContainerSpecDto { #[derive(Serialize, Deserialize, Debug, Clone)] pub struct QueryContainerStatusDto { pub available: bool, + pub messages: Option>, } diff --git a/control-planes/mgmt_api/src/api/v1/models/reaction.rs b/control-planes/mgmt_api/src/api/v1/models/reaction.rs index 5d967030..3bd94483 100644 --- a/control-planes/mgmt_api/src/api/v1/models/reaction.rs +++ b/control-planes/mgmt_api/src/api/v1/models/reaction.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use serde::{Deserialize, Serialize}; @@ -16,4 +16,5 @@ pub struct ReactionSpecDto { #[derive(Serialize, Deserialize, Debug, Clone)] pub struct ReactionStatusDto { pub available: bool, + pub messages: Option>, } diff --git a/control-planes/mgmt_api/src/api/v1/models/source.rs b/control-planes/mgmt_api/src/api/v1/models/source.rs index b4eba94b..9b4537fe 100644 --- a/control-planes/mgmt_api/src/api/v1/models/source.rs +++ b/control-planes/mgmt_api/src/api/v1/models/source.rs @@ -5,6 +5,7 @@ use super::ConfigValueDto; use super::ServiceDto; +use std::collections::BTreeMap; use std::collections::HashMap; #[derive(Serialize, Deserialize, Debug)] @@ -18,4 +19,5 @@ pub struct SourceSpecDto { #[derive(Serialize, Deserialize, Debug)] pub struct SourceStatusDto { pub available: bool, + pub messages: Option>, } diff --git a/control-planes/mgmt_api/src/domain/mappings.rs b/control-planes/mgmt_api/src/domain/mappings.rs index 95cbdb40..71b58377 100644 --- a/control-planes/mgmt_api/src/domain/mappings.rs +++ b/control-planes/mgmt_api/src/domain/mappings.rs @@ -290,26 +290,11 @@ impl Into for QuerySpec { } } -impl Into for SourceStatus { - fn into(self) -> resource_provider_api::models::SourceStatus { - resource_provider_api::models::SourceStatus { - available: self.available.into(), - } - } -} - impl Into for resource_provider_api::models::SourceStatus { fn into(self) -> SourceStatus { SourceStatus { available: self.available.into(), - } - } -} - -impl Into for QueryContainerStatus { - fn into(self) -> resource_provider_api::models::QueryContainerStatus { - resource_provider_api::models::QueryContainerStatus { - available: self.available.into(), + messages: self.messages, } } } @@ -318,14 +303,7 @@ impl Into for resource_provider_api::models::QueryContaine fn into(self) -> QueryContainerStatus { QueryContainerStatus { available: self.available.into(), - } - } -} - -impl Into for ReactionStatus { - fn into(self) -> resource_provider_api::models::ReactionStatus { - resource_provider_api::models::ReactionStatus { - available: self.available.into(), + messages: self.messages, } } } @@ -334,6 +312,7 @@ impl Into for resource_provider_api::models::ReactionStatus { fn into(self) -> ReactionStatus { ReactionStatus { available: self.available.into(), + messages: self.messages, } } } diff --git a/control-planes/mgmt_api/src/domain/models.rs b/control-planes/mgmt_api/src/domain/models.rs index 82b79050..6cb6c807 100644 --- a/control-planes/mgmt_api/src/domain/models.rs +++ b/control-planes/mgmt_api/src/domain/models.rs @@ -1,6 +1,10 @@ use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; -use std::{collections::HashMap, hash::Hash, pin::Pin}; +use std::{ + collections::{BTreeMap, HashMap}, + hash::Hash, + pin::Pin, +}; use thiserror::Error; #[derive(Clone, Debug, Serialize, Deserialize)] @@ -58,6 +62,7 @@ pub struct SourceSpec { #[derive(Serialize, Deserialize, Debug)] pub struct SourceStatus { pub available: bool, + pub messages: Option>, } #[derive(Serialize, Deserialize, Debug, Clone)] @@ -89,6 +94,7 @@ pub struct QueryContainerSpec { #[derive(Serialize, Deserialize, Debug, Clone)] pub struct QueryContainerStatus { pub available: bool, + pub messages: Option>, } #[derive(Serialize, Deserialize, Debug, Clone)] @@ -103,6 +109,7 @@ pub struct ReactionSpec { #[derive(Serialize, Deserialize, Debug, Clone)] pub struct ReactionStatus { pub available: bool, + pub messages: Option>, } #[derive(Serialize, Deserialize, Debug, Clone)] diff --git a/control-planes/resource_provider_api/src/models.rs b/control-planes/resource_provider_api/src/models.rs index de480908..5943f33e 100644 --- a/control-planes/resource_provider_api/src/models.rs +++ b/control-planes/resource_provider_api/src/models.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::fmt; use std::marker::PhantomData; use std::str::FromStr; @@ -93,6 +93,7 @@ pub struct SourceSpec { #[derive(Serialize, Deserialize, Debug)] pub struct SourceStatus { pub available: bool, + pub messages: Option>, } #[derive(Serialize, Deserialize, Debug, Clone)] @@ -125,6 +126,7 @@ pub struct QueryContainerSpec { #[derive(Serialize, Deserialize, Debug)] pub struct QueryContainerStatus { pub available: bool, + pub messages: Option>, } #[derive(Serialize, Deserialize, Debug)] @@ -139,6 +141,7 @@ pub struct ReactionSpec { #[derive(Serialize, Deserialize, Debug)] pub struct ReactionStatus { pub available: bool, + pub messages: Option>, } #[derive(Serialize, Deserialize, Debug)]