From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id C944A6A0DB for ; Tue, 11 Aug 2020 11:57:31 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 15E071BD1E for ; Tue, 11 Aug 2020 11:57:31 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 38EEE1BC7B for ; Tue, 11 Aug 2020 11:57:26 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 06F3A44597 for ; Tue, 11 Aug 2020 11:57:26 +0200 (CEST) From: Dominik Csapak To: pbs-devel@lists.proxmox.com Date: Tue, 11 Aug 2020 11:57:16 +0200 Message-Id: <20200811095724.26896-2-d.csapak@proxmox.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200811095724.26896-1-d.csapak@proxmox.com> References: <20200811095724.26896-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.045 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods NO_DNS_FOR_FROM 0.379 Envelope sender has no MX or A DNS records RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [tasks.rs, sync.rs, mod.rs] Subject: [pbs-devel] [PATCH proxmox-backup v2 1/9] server: change status of a task from a string to an enum X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Aug 2020 09:57:31 -0000 representing a state via an enum makes more sense in this case we also implement FromStr and Display to make it easy to convet from/to a string Signed-off-by: Dominik Csapak --- new in v2 src/api2/admin/sync.rs | 2 +- src/api2/node/tasks.rs | 8 ++-- src/api2/types/mod.rs | 2 +- src/server/worker_task.rs | 88 ++++++++++++++++++++++++++++++--------- 4 files changed, 74 insertions(+), 26 deletions(-) diff --git a/src/api2/admin/sync.rs b/src/api2/admin/sync.rs index f06625f6..79af03bc 100644 --- a/src/api2/admin/sync.rs +++ b/src/api2/admin/sync.rs @@ -56,7 +56,7 @@ pub fn list_sync_jobs( if let Some(task) = last_tasks.get(&job.id) { job.last_run_upid = Some(task.upid_str.clone()); if let Some((endtime, status)) = &task.state { - job.last_run_state = Some(String::from(status)); + job.last_run_state = Some(status.to_string()); job.last_run_endtime = Some(*endtime); last = *endtime; } diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs index 6917869e..0bb043d9 100644 --- a/src/api2/node/tasks.rs +++ b/src/api2/node/tasks.rs @@ -10,7 +10,7 @@ use proxmox::{identity, list_subdirs_api_method, sortable}; use crate::tools; use crate::api2::types::*; -use crate::server::{self, UPID}; +use crate::server::{self, UPID, TaskState}; use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY}; use crate::config::cached_user_info::CachedUserInfo; @@ -105,9 +105,9 @@ async fn get_task_status( if crate::server::worker_is_active(&upid).await? { result["status"] = Value::from("running"); } else { - let exitstatus = crate::server::upid_read_status(&upid).unwrap_or(String::from("unknown")); + let exitstatus = crate::server::upid_read_status(&upid).unwrap_or(TaskState::Unknown); result["status"] = Value::from("stopped"); - result["exitstatus"] = Value::from(exitstatus); + result["exitstatus"] = Value::from(exitstatus.to_string()); }; Ok(result) @@ -352,7 +352,7 @@ pub fn list_tasks( if let Some(ref state) = info.state { if running { continue; } - if errors && state.1 == "OK" { + if errors && state.1 == crate::server::TaskState::OK { continue; } } diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs index fb386d46..a619810d 100644 --- a/src/api2/types/mod.rs +++ b/src/api2/types/mod.rs @@ -595,7 +595,7 @@ impl From for TaskListItem { fn from(info: crate::server::TaskListInfo) -> Self { let (endtime, status) = info .state - .map_or_else(|| (None, None), |(a,b)| (Some(a), Some(b))); + .map_or_else(|| (None, None), |(a,b)| (Some(a), Some(b.to_string()))); TaskListItem { upid: info.upid_str, diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs index 852f3ecc..876666d6 100644 --- a/src/server/worker_task.rs +++ b/src/server/worker_task.rs @@ -11,6 +11,7 @@ use futures::*; use lazy_static::lazy_static; use nix::unistd::Pid; use serde_json::{json, Value}; +use serde::{Serialize, Deserialize}; use tokio::sync::oneshot; use proxmox::sys::linux::procfs; @@ -190,8 +191,8 @@ pub fn create_task_log_dirs() -> Result<(), Error> { } /// Read exits status from task log file -pub fn upid_read_status(upid: &UPID) -> Result { - let mut status = String::from("unknown"); +pub fn upid_read_status(upid: &UPID) -> Result { + let mut status = TaskState::Unknown; let path = upid.log_path(); @@ -212,12 +213,8 @@ pub fn upid_read_status(upid: &UPID) -> Result { match iter.next() { None => continue, Some(rest) => { - if rest == "OK" { - status = String::from(rest); - } else if rest.starts_with("WARNINGS: ") { - status = String::from(rest); - } else if rest.starts_with("ERROR: ") { - status = String::from(&rest[7..]); + if let Ok(state) = rest.parse() { + status = state; } } } @@ -226,6 +223,59 @@ pub fn upid_read_status(upid: &UPID) -> Result { Ok(status) } +/// Task State +#[derive(Debug, PartialEq, Serialize, Deserialize)] +pub enum TaskState { + /// The Task ended with an undefined state + Unknown, + /// The Task ended and there were no errors or warnings + OK, + /// The Task had 'count' amount of warnings and no errors + Warning { count: u64 }, + /// The Task ended with the error described in 'message' + Error { message: String }, +} + +impl TaskState { + fn result_text(&self) -> String { + match self { + TaskState::Error { message } => format!("TASK ERROR: {}", message), + other => format!("TASK {}", other), + } + } +} + +impl std::fmt::Display for TaskState { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + TaskState::Unknown => write!(f, "unknown"), + TaskState::OK => write!(f, "OK"), + TaskState::Warning { count } => write!(f, "WARNINGS: {}", count), + TaskState::Error { message } => write!(f, "{}", message), + } + } +} + +impl std::str::FromStr for TaskState { + type Err = Error; + + fn from_str(s: &str) -> Result { + if s == "unknown" { + Ok(TaskState::Unknown) + } else if s == "OK" { + Ok(TaskState::OK) + } else if s.starts_with("WARNINGS: ") { + let count: u64 = s[10..].parse()?; + Ok(TaskState::Warning{ count }) + } else if s.len() > 0 { + let message = if s.starts_with("ERROR: ") { &s[7..] } else { s }.to_string(); + Ok(TaskState::Error{ message }) + } else { + bail!("unable to parse Task Status '{}'", s); + } + } +} + /// Task details including parsed UPID /// /// If there is no `state`, the task is still running. @@ -236,9 +286,7 @@ pub struct TaskListInfo { /// UPID string representation pub upid_str: String, /// Task `(endtime, status)` if already finished - /// - /// The `status` is either `unknown`, `OK`, `WARN`, or `ERROR: ...` - pub state: Option<(i64, String)>, // endtime, status + pub state: Option<(i64, TaskState)>, // endtime, status } // atomically read/update the task list, update status of finished tasks @@ -278,14 +326,14 @@ fn update_active_workers(new_upid: Option<&UPID>) -> Result, E None => { println!("Detected stopped UPID {}", upid_str); let status = upid_read_status(&upid) - .unwrap_or_else(|_| String::from("unknown")); + .unwrap_or_else(|_| TaskState::Unknown); finish_list.push(TaskListInfo { upid, upid_str, state: Some((Local::now().timestamp(), status)) }); }, Some((endtime, status)) => { finish_list.push(TaskListInfo { - upid, upid_str, state: Some((endtime, status)) + upid, upid_str, state: Some((endtime, status.parse()?)) }) } } @@ -503,23 +551,23 @@ impl WorkerTask { Ok(upid_str) } - /// get the Text of the result - pub fn get_log_text(&self, result: &Result<(), Error>) -> String { - + /// create state from self and a result + pub fn create_state(&self, result: &Result<(), Error>) -> TaskState { let warn_count = self.data.lock().unwrap().warn_count; if let Err(err) = result { - format!("ERROR: {}", err) + TaskState::Error { message: err.to_string() } } else if warn_count > 0 { - format!("WARNINGS: {}", warn_count) + TaskState::Warning { count: warn_count } } else { - "OK".to_string() + TaskState::OK } } /// Log task result, remove task from running list pub fn log_result(&self, result: &Result<(), Error>) { - self.log(format!("TASK {}", self.get_log_text(result))); + let state = self.create_state(result); + self.log(state.result_text()); WORKER_TASK_LIST.lock().unwrap().remove(&self.upid.task_id); let _ = update_active_workers(None); -- 2.20.1