Merge pull request #3469 from jarhodes314/refactor/service-managers

refactor: separate service management from system toml configurations

Author: jarhodes314

crates/common/tedge_config/src/lib.rs

@@ -1,15 +1,14 @@
-pub mod mqtt_config;
 mod sudo;
-pub mod system_services;
-pub mod tedge_config_cli;
+pub mod tedge_toml;
 pub use sudo::SudoCommandBuilder;
 pub mod cli;
+mod system_toml;
 
-pub use self::tedge_config_cli::config_setting::*;
-pub use self::tedge_config_cli::error::*;
-pub use self::tedge_config_cli::models::*;
-pub use self::tedge_config_cli::tedge_config::*;
-pub use self::tedge_config_cli::tedge_config_location::*;
+pub use self::system_toml::*;
+pub use self::tedge_toml::error::*;
+pub use self::tedge_toml::models::*;
+pub use self::tedge_toml::tedge_config::*;
+pub use self::tedge_toml::tedge_config_location::*;
 pub use camino::Utf8Path as Path;
 pub use camino::Utf8PathBuf as PathBuf;
 pub use certificate::CertificateError;

crates/common/tedge_config/src/mqtt_config.rs

@@ -1,8 +1,8 @@
 use camino::Utf8Path;
 
+use super::SystemConfig;
+use super::SystemTomlError;
 use crate::cli::LogConfigArgs;
-use crate::system_services::SystemConfig;
-use crate::system_services::SystemServiceError;
 use std::io::IsTerminal;
 use std::str::FromStr;
 
@@ -18,7 +18,7 @@ pub fn log_init(
     sname: &str,
     flags: &LogConfigArgs,
     config_dir: &Utf8Path,
-) -> Result<(), SystemServiceError> {
+) -> Result<(), SystemTomlError> {
     let subscriber = tracing_subscriber::fmt()
         .with_writer(std::io::stderr)
         .with_ansi(std::io::stderr().is_terminal())
@@ -51,11 +51,11 @@ pub fn log_init(
 pub fn get_log_level(
     sname: &str,
     config_dir: &Utf8Path,
-) -> Result<tracing::Level, SystemServiceError> {
+) -> Result<tracing::Level, SystemTomlError> {
     let loglevel = SystemConfig::try_new(config_dir)?.log;
     match loglevel.get(sname) {
         Some(ll) => tracing::Level::from_str(&ll.to_uppercase()).map_err(|_| {
-            SystemServiceError::InvalidLogLevel {
+            SystemTomlError::InvalidLogLevel {
                 name: ll.to_string(),
             }
         }),

crates/common/tedge_config/src/system_services/log_config.rs renamed to crates/common/tedge_config/src/system_toml/log_level.rs

@@ -1,60 +1,51 @@
-use crate::system_services::SystemServiceError;
-use camino::Utf8Path;
-use serde::Deserialize;
 use std::collections::HashMap;
-use std::fs;
 use std::time::Duration;
 
-pub const SERVICE_CONFIG_FILE: &str = "system.toml";
+use serde::Deserialize;
+
+mod log_level;
+mod services;
+
+pub use self::log_level::*;
+pub use self::services::*;
+use camino::Utf8Path;
+use camino::Utf8PathBuf;
+use std::fs;
+
+pub const SYSTEM_CONFIG_FILE: &str = "system.toml";
 const REBOOT_COMMAND: &[&str] = &["init", "6"];
 
+#[derive(thiserror::Error, Debug)]
+pub enum SystemTomlError {
+    #[error("Toml syntax error in the system config file '{path}': {reason}")]
+    InvalidSyntax { path: Utf8PathBuf, reason: String },
+
+    #[error("Invalid log level: {name:?}, supported levels are info, warn, error and debug")]
+    InvalidLogLevel { name: String },
+}
+
 #[derive(Deserialize, Debug, Default, Eq, PartialEq)]
 pub struct SystemConfig {
     #[serde(default)]
-    pub init: InitConfig,
+    pub init: services::InitConfig,
     #[serde(default)]
     pub log: HashMap<String, String>,
     #[serde(default)]
     pub system: SystemSpecificCommands,
 }
 
-#[derive(Deserialize, Debug, Eq, PartialEq)]
-#[serde(from = "InitConfigToml")]
-pub struct InitConfig {
-    pub name: String,
-    pub is_available: Vec<String>,
-    pub restart: Vec<String>,
-    pub stop: Vec<String>,
-    pub start: Vec<String>,
-    pub enable: Vec<String>,
-    pub disable: Vec<String>,
-    pub is_active: Vec<String>,
-}
-
-#[derive(Deserialize, Debug, Eq, PartialEq)]
-#[serde(deny_unknown_fields)]
-struct InitConfigToml {
-    name: String,
-    is_available: Vec<String>,
-    restart: Vec<String>,
-    stop: Vec<String>,
-    start: Option<Vec<String>>,
-    enable: Vec<String>,
-    disable: Vec<String>,
-    is_active: Vec<String>,
-}
+impl SystemConfig {
+    pub fn try_new(config_root: &Utf8Path) -> Result<Self, SystemTomlError> {
+        let config_path = config_root.join(SYSTEM_CONFIG_FILE);
 
-impl From<InitConfigToml> for InitConfig {
-    fn from(value: InitConfigToml) -> Self {
-        Self {
-            name: value.name,
-            is_available: value.is_available,
-            start: value.start.unwrap_or(value.restart.clone()),
-            restart: value.restart,
-            stop: value.stop,
-            enable: value.enable,
-            disable: value.disable,
-            is_active: value.is_active,
+        match fs::read_to_string(config_path.clone()) {
+            Ok(contents) => {
+                toml::from_str(contents.as_str()).map_err(|e| SystemTomlError::InvalidSyntax {
+                    path: config_path,
+                    reason: e.to_string(),
+                })
+            }
+            Err(_) => Ok(Self::default()),
         }
     }
 }
@@ -94,37 +85,6 @@ impl Default for SystemSpecificCommands {
     }
 }
 
-impl Default for InitConfig {
-    fn default() -> Self {
-        Self {
-            name: "systemd".to_string(),
-            is_available: vec!["/bin/systemctl".into(), "--version".into()],
-            restart: vec!["/bin/systemctl".into(), "restart".into(), "{}".into()],
-            stop: vec!["/bin/systemctl".into(), "stop".into(), "{}".into()],
-            start: vec!["/bin/systemctl".into(), "start".into(), "{}".into()],
-            enable: vec!["/bin/systemctl".into(), "enable".into(), "{}".into()],
-            disable: vec!["/bin/systemctl".into(), "disable".into(), "{}".into()],
-            is_active: vec!["/bin/systemctl".into(), "is-active".into(), "{}".into()],
-        }
-    }
-}
-
-impl SystemConfig {
-    pub fn try_new(config_root: &Utf8Path) -> Result<Self, SystemServiceError> {
-        let config_path = config_root.join(SERVICE_CONFIG_FILE);
-
-        match fs::read_to_string(config_path.clone()) {
-            Ok(contents) => toml::from_str(contents.as_str()).map_err(|e| {
-                SystemServiceError::SystemConfigInvalidToml {
-                    path: config_path,
-                    reason: e.to_string(),
-                }
-            }),
-            Err(_) => Ok(Self::default()),
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -235,7 +195,7 @@ mod tests {
     fn create_temp_system_config(content: &str) -> std::io::Result<(TempDir, Utf8PathBuf)> {
         let temp_dir = TempDir::new()?;
         let config_root = Utf8Path::from_path(temp_dir.path()).unwrap().to_owned();
-        let config_file_path = config_root.join(SERVICE_CONFIG_FILE);
+        let config_file_path = config_root.join(SYSTEM_CONFIG_FILE);
         let mut file = std::fs::File::create(config_file_path.as_path())?;
         file.write_all(content.as_bytes())?;
         Ok((temp_dir, config_root))

crates/common/tedge_config/src/system_services/managers/config.rs renamed to crates/common/tedge_config/src/system_toml/mod.rs

@@ -0,0 +1,57 @@
+use serde::Deserialize;
+
+#[derive(Deserialize, Debug, Eq, PartialEq)]
+#[serde(from = "InitConfigToml")]
+pub struct InitConfig {
+    pub name: String,
+    pub is_available: Vec<String>,
+    pub restart: Vec<String>,
+    pub stop: Vec<String>,
+    pub start: Vec<String>,
+    pub enable: Vec<String>,
+    pub disable: Vec<String>,
+    pub is_active: Vec<String>,
+}
+
+#[derive(Deserialize, Debug, Eq, PartialEq)]
+#[serde(deny_unknown_fields)]
+struct InitConfigToml {
+    name: String,
+    is_available: Vec<String>,
+    restart: Vec<String>,
+    stop: Vec<String>,
+    start: Option<Vec<String>>,
+    enable: Vec<String>,
+    disable: Vec<String>,
+    is_active: Vec<String>,
+}
+
+impl From<InitConfigToml> for InitConfig {
+    fn from(value: InitConfigToml) -> Self {
+        Self {
+            name: value.name,
+            is_available: value.is_available,
+            start: value.start.unwrap_or(value.restart.clone()),
+            restart: value.restart,
+            stop: value.stop,
+            enable: value.enable,
+            disable: value.disable,
+            is_active: value.is_active,
+        }
+    }
+}
+
+impl Default for InitConfig {
+    fn default() -> Self {
+        Self {
+            name: "systemd".to_string(),
+            is_available: vec!["/bin/systemctl".into(), "--version".into()],
+            restart: vec!["/bin/systemctl".into(), "restart".into(), "{}".into()],
+            stop: vec!["/bin/systemctl".into(), "stop".into(), "{}".into()],
+            start: vec!["/bin/systemctl".into(), "start".into(), "{}".into()],
+            enable: vec!["/bin/systemctl".into(), "enable".into(), "{}".into()],
+            disable: vec!["/bin/systemctl".into(), "disable".into(), "{}".into()],
+            is_active: vec!["/bin/systemctl".into(), "is-active".into(), "{}".into()],
+        }
+    }
+}

crates/common/tedge_config/src/system_toml/services.rs

@@ -13,7 +13,7 @@ pub enum TEdgeConfigError {
     FromConfigSetting(#[from] crate::ConfigSettingError),
 
     #[error(transparent)]
-    FromInvalidConfigUrl(#[from] crate::tedge_config_cli::models::InvalidConnectUrl),
+    FromInvalidConfigUrl(#[from] crate::tedge_toml::models::InvalidConnectUrl),
 
     #[error("Config file not found: {0}")]
     ConfigFileNotFound(std::path::PathBuf),
@@ -31,12 +31,48 @@ pub enum TEdgeConfigError {
     DirNotFound(#[from] tedge_utils::paths::PathsError),
 
     #[error(transparent)]
-    FromParseHostPortError(#[from] crate::tedge_config_cli::models::host_port::ParseHostPortError),
+    FromParseHostPortError(#[from] crate::tedge_toml::models::host_port::ParseHostPortError),
 
     #[error(transparent)]
     FromAtomFileError(#[from] tedge_utils::fs::AtomFileError),
 }
 
+pub type ConfigSettingResult<T> = Result<T, ConfigSettingError>;
+
+#[derive(thiserror::Error, Debug)]
+/// An error encountered while updating a value in tedge.toml
+pub enum ConfigSettingError {
+    #[error(
+        r#"A value for `{key}` is missing.
+    A value can be set with `tedge config set {key} <value>`"#
+    )]
+    ConfigNotSet { key: &'static str },
+
+    #[error("Readonly setting: {message}")]
+    ReadonlySetting { message: &'static str },
+
+    #[error("Conversion from String failed")]
+    ConversionFromStringFailed,
+
+    #[error("Conversion into String failed")]
+    ConversionIntoStringFailed,
+
+    #[error("Derivation for `{key}` failed: {cause}")]
+    DerivationFailed { key: &'static str, cause: String },
+
+    #[error("Config value {key}, cannot be configured: {message} ")]
+    SettingIsNotConfigurable {
+        key: &'static str,
+        message: &'static str,
+    },
+
+    #[error("An error occurred: {msg}")]
+    Other { msg: &'static str },
+
+    #[error(transparent)]
+    Write(#[from] crate::WriteError),
+}
+
 impl TEdgeConfigError {
     pub fn multiple_errors(mut errors: Vec<Self>) -> Self {
         match errors.len() {

crates/common/tedge_config/src/tedge_config_cli/config_setting.rs

@@ -1,7 +1,5 @@
-pub mod config_setting;
 pub mod error;
-pub mod tedge_config_location;
-
 mod figment;
 pub mod models;
 pub mod tedge_config;
+pub mod tedge_config_location;