Merge branch 'main' of https://git.threefold.info/herocode/sal

service_manager/README.md

@@ -1,15 +1,21 @@
 # Service Manager
 
 This crate provides a unified interface for managing system services across different platforms.
-It abstracts the underlying service management system (like `launchctl` on macOS or `systemd` on Linux),
+It abstracts the underlying service management system (like `launchctl` on macOS or `zinit` on Linux),
-allowing you to start, stop, and monitor services with a consistent API.
+allowing you to create, start, stop, remove, and monitor services with a consistent API.
+
+The service lifecycle is managed in two distinct steps:
+1.  **`create`**: Creates the service definition on the system (e.g., writes a `.plist` file on macOS).
+2.  **`start`**: Starts a service that has already been created.
+
+This separation ensures that service management is more explicit and robust.
 
 ## Features
 
 - A `ServiceManager` trait defining a common interface for service operations.
 - Platform-specific implementations for:
   - macOS (`launchctl`)
-  - Linux (`systemd`)
+  - Linux (`zinit`) (via the `zinit` feature flag)
 - A factory function `create_service_manager` that returns the appropriate manager for the current platform.
 
 ## Usage
@@ -18,29 +24,34 @@ Add this to your `Cargo.toml`:
 
 ```toml
 [dependencies]
-service_manager = { path = "../service_manager" }
+sal-service-manager = { path = "./", features = ["zinit"] }
 ```
 
 Here is an example of how to use the `ServiceManager`:
 
 ```rust,no_run
-use service_manager::{create_service_manager, ServiceConfig};
+use sal_service_manager::{create_service_manager, ServiceConfig, ServiceManager};
-use std::collections::HashMap;
 
 fn main() -> Result<(), Box<dyn std::error::Error>> {
-    let service_manager = create_service_manager();
+    // On linux, this will default to zinit. On macos, to launchctl.
+    let service_manager = create_service_manager(None)?;
 
     let config = ServiceConfig {
         name: "my-service".to_string(),
         binary_path: "/usr/local/bin/my-service-executable".to_string(),
         args: vec!["--config".to_string(), "/etc/my-service.conf".to_string()],
         working_directory: Some("/var/tmp".to_string()),
-        environment: HashMap::new(),
+        environment: None,
         auto_restart: true,
     };
 
-    // Start a new service
+    // Create the service definition
-    service_manager.start(&config)?;
+    service_manager.create(&config)?;
+    println!("Service 'my-service' created.");
+
+    // Start the service
+    service_manager.start("my-service")?;
+    println!("Service 'my-service' started.");
 
     // Get the status of the service
     let status = service_manager.status("my-service")?;
@@ -48,6 +59,11 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
 
     // Stop the service
     service_manager.stop("my-service")?;
+    println!("Service 'my-service' stopped.");
+
+    // Remove the service
+    service_manager.remove("my-service")?;
+    println!("Service 'my-service' removed.");
 
     Ok(())
 }

service_manager/examples/README.md

@@ -0,0 +1,47 @@
+# Service Manager Examples
+
+This directory contains examples demonstrating the usage of the `sal-service-manager` crate.
+
+## Running Examples
+
+To run any example, use the following command structure from the `service_manager` crate's root directory:
+
+```sh
+cargo run --example <EXAMPLE_NAME>
+```
+
+---
+
+### 1. `simple_service`
+
+This example demonstrates the ideal, clean lifecycle of a service using the separated `create` and `start` steps.
+
+**Behavior:**
+1.  Creates a new service definition.
+2.  Starts the newly created service.
+3.  Checks its status to confirm it's running.
+4.  Stops the service.
+5.  Checks its status again to confirm it's stopped.
+6.  Removes the service definition.
+
+**Run it:**
+```sh
+cargo run --example simple_service
+```
+
+### 2. `service_spaghetti`
+
+This example demonstrates how the service manager handles "messy" or improper sequences of operations, showcasing its error handling and robustness.
+
+**Behavior:**
+1.  Creates a service.
+2.  Starts the service.
+3.  Tries to start the **same service again** (which should fail as it's already running).
+4.  Removes the service **without stopping it first** (the manager should handle this gracefully).
+5.  Tries to stop the **already removed** service (which should fail).
+6.  Tries to remove the service **again** (which should also fail).
+
+**Run it:**
+```sh
+cargo run --example service_spaghetti
+```

service_manager/examples/service_spaghetti.rs

@@ -0,0 +1,95 @@
+//! service_spaghetti - An example of messy service management.
+//!
+//! This example demonstrates how the service manager behaves when commands
+//! are issued in a less-than-ideal order, such as starting a service that's
+//! already running or removing a service that hasn't been stopped.
+
+use sal_service_manager::{create_service_manager, ServiceConfig};
+use std::collections::HashMap;
+use std::thread;
+use std::time::Duration;
+
+fn main() {
+    let manager = create_service_manager(None).expect("Failed to create service manager");
+    let service_name = "com.herocode.examples.spaghetti";
+
+    let service_config = ServiceConfig {
+        name: service_name.to_string(),
+        binary_path: "/bin/sh".to_string(),
+        args: vec![
+            "-c".to_string(),
+            "while true; do echo 'Spaghetti service is running...'; sleep 5; done".to_string(),
+        ],
+        working_directory: None,
+        environment: HashMap::new(),
+        auto_restart: false,
+    };
+
+    println!("--- Service Spaghetti Example ---");
+    println!("This example demonstrates messy, error-prone service management.");
+
+    // Cleanup from previous runs to ensure a clean slate
+    if let Ok(true) = manager.exists(service_name) {
+        println!("\nService '{}' found from a previous run. Cleaning up first.", service_name);
+        let _ = manager.stop(service_name);
+        let _ = manager.remove(service_name);
+        println!("Cleanup complete.");
+    }
+
+    // 1. Create the service
+    println!("\n1. Creating the service...");
+    match manager.create(&service_config) {
+        Ok(()) => println!("   -> Success: Service '{}' created.", service_name),
+        Err(e) => {
+            eprintln!("   -> Error: Failed to create service: {}. Halting example.", e);
+            return;
+        }
+    }
+
+    // 2. Start the service
+    println!("\n2. Starting the service for the first time...");
+    match manager.start(service_name) {
+        Ok(()) => println!("   -> Success: Service '{}' started.", service_name),
+        Err(e) => {
+            eprintln!("   -> Error: Failed to start service: {}. Halting example.", e);
+            return;
+        }
+    }
+
+    thread::sleep(Duration::from_secs(2));
+
+    // 3. Try to start the service again while it's already running
+    println!("\n3. Trying to start the *same service* again...");
+    match manager.start(service_name) {
+        Ok(()) => println!("   -> Unexpected Success: Service started again."),
+        Err(e) => eprintln!("   -> Expected Error: {}. The manager should detect it is already running.", e),
+    }
+
+    // 3. Let it run for a bit
+    println!("\n3. Letting the service run for 5 seconds...");
+    thread::sleep(Duration::from_secs(5));
+
+    // 4. Remove the service without stopping it first
+    // The `remove` function is designed to stop the service if it's running.
+    println!("\n4. Removing the service without explicitly stopping it first...");
+    match manager.remove(service_name) {
+        Ok(()) => println!("   -> Success: Service was stopped and removed."),
+        Err(e) => eprintln!("   -> Error: Failed to remove service: {}", e),
+    }
+
+    // 5. Try to stop the service after it has been removed
+    println!("\n5. Trying to stop the service that was just removed...");
+    match manager.stop(service_name) {
+        Ok(()) => println!("   -> Unexpected Success: Stopped a removed service."),
+        Err(e) => eprintln!("   -> Expected Error: {}. The manager knows the service is gone.", e),
+    }
+
+    // 6. Try to remove the service again
+    println!("\n6. Trying to remove the service again...");
+    match manager.remove(service_name) {
+        Ok(()) => println!("   -> Unexpected Success: Removed a non-existent service."),
+        Err(e) => eprintln!("   -> Expected Error: {}. The manager correctly reports it's not found.", e),
+    }
+
+    println!("\n--- Spaghetti Example Finished ---");
+}

service_manager/examples/simple_service.rs

@@ -0,0 +1,105 @@
+use sal_service_manager::{create_service_manager, ServiceConfig};
+use std::collections::HashMap;
+use std::thread;
+use std::time::Duration;
+
+fn main() {
+    // 1. Create a service manager for the current platform
+    let manager = match create_service_manager(None) {
+        Ok(manager) => manager,
+        Err(e) => {
+            eprintln!("Error: Failed to create service manager: {}", e);
+            return;
+        }
+    };
+
+    // 2. Define the configuration for our new service
+    let service_name = "com.herocode.examples.simpleservice";
+    let service_config = ServiceConfig {
+        name: service_name.to_string(),
+        // A simple command that runs in a loop
+        binary_path: "/bin/sh".to_string(),
+        args: vec![
+            "-c".to_string(),
+            "while true; do echo 'Simple service is running...'; date; sleep 5; done".to_string(),
+        ],
+        working_directory: None,
+        environment: HashMap::new(),
+        auto_restart: false,
+    };
+
+    println!("--- Service Manager Example ---");
+
+    // Cleanup from previous runs, if necessary
+    if let Ok(true) = manager.exists(service_name) {
+        println!("Service '{}' already exists. Cleaning up before starting.", service_name);
+        if let Err(e) = manager.stop(service_name) {
+            println!("Note: could not stop existing service (it might not be running): {}", e);
+        }
+        if let Err(e) = manager.remove(service_name) {
+            eprintln!("Error: failed to remove existing service: {}", e);
+            return;
+        }
+        println!("Cleanup complete.");
+    }
+
+    // 3. Create the service
+    println!("\n1. Creating service: '{}'", service_name);
+    match manager.create(&service_config) {
+        Ok(()) => println!("Service '{}' created successfully.", service_name),
+        Err(e) => {
+            eprintln!("Error: Failed to create service '{}': {}", service_name, e);
+            return;
+        }
+    }
+
+    // 4. Start the service
+    println!("\n2. Starting service: '{}'", service_name);
+    match manager.start(service_name) {
+        Ok(()) => println!("Service '{}' started successfully.", service_name),
+        Err(e) => {
+            eprintln!("Error: Failed to start service '{}': {}", service_name, e);
+            return;
+        }
+    }
+
+    // Give it a moment to run
+    println!("\nWaiting for 2 seconds for the service to initialize...");
+    thread::sleep(Duration::from_secs(2));
+
+    // 4. Check the status of the service
+    println!("\n2. Checking service status...");
+    match manager.status(service_name) {
+        Ok(status) => println!("Service status: {:?}", status),
+        Err(e) => eprintln!("Error: Failed to get status for service '{}': {}", service_name, e),
+    }
+
+    println!("\nLetting the service run for 10 seconds. Check logs if you can.");
+    thread::sleep(Duration::from_secs(10));
+
+    // 5. Stop the service
+    println!("\n3. Stopping service: '{}'", service_name);
+    match manager.stop(service_name) {
+        Ok(()) => println!("Service '{}' stopped successfully.", service_name),
+        Err(e) => eprintln!("Error: Failed to stop service '{}': {}", service_name, e),
+    }
+
+    println!("\nWaiting for 2 seconds for the service to stop...");
+    thread::sleep(Duration::from_secs(2));
+
+    // Check status again
+    println!("\n4. Checking status after stopping...");
+    match manager.status(service_name) {
+        Ok(status) => println!("Service status: {:?}", status),
+        Err(e) => eprintln!("Error: Failed to get status for service '{}': {}", service_name, e),
+    }
+
+    // 6. Remove the service
+    println!("\n5. Removing service: '{}'", service_name);
+    match manager.remove(service_name) {
+        Ok(()) => println!("Service '{}' removed successfully.", service_name),
+        Err(e) => eprintln!("Error: Failed to remove service '{}': {}", service_name, e),
+    }
+
+    println!("\n--- Example Finished ---");
+}

service_manager/src/launchctl.rs

@@ -180,67 +180,42 @@ impl LaunchctlServiceManager {
 
 #[async_trait]
 impl ServiceManager for LaunchctlServiceManager {
+    fn create(&self, config: &ServiceConfig) -> Result<(), ServiceManagerError> {
+        if self.exists(&config.name)? {
+            return Err(ServiceManagerError::ServiceAlreadyExists(config.name.clone()));
+        }
+        let rt = tokio::runtime::Runtime::new().map_err(|e| ServiceManagerError::Other(e.to_string()))?;
+        rt.block_on(async {
+            self.create_plist(config).await
+        })
+    }
+
     fn exists(&self, service_name: &str) -> Result<bool, ServiceManagerError> {
         let plist_path = self.get_plist_path(service_name);
         Ok(plist_path.exists())
     }
 
-    fn start(&self, config: &ServiceConfig) -> Result<(), ServiceManagerError> {
+    fn start(&self, service_name: &str) -> Result<(), ServiceManagerError> {
-        // For synchronous version, we'll use blocking operations
+        if !self.exists(service_name)? {
-        let rt = tokio::runtime::Runtime::new().map_err(|e| ServiceManagerError::Other(e.to_string()))?;
+            return Err(ServiceManagerError::ServiceNotFound(service_name.to_string()));
-        rt.block_on(async {
-            let label = self.get_service_label(&config.name);
-            
-            // Check if service is already loaded
-            let list_output = self.run_launchctl(&["list"]).await?;
-            if list_output.contains(&label) {
-                return Err(ServiceManagerError::ServiceAlreadyExists(config.name.clone()));
         }
 
-            // Create the plist file
+        // Check status before trying to start
-            self.create_plist(config).await?;
+        if let Ok(ServiceStatus::Running) = self.status(service_name) {
-
+            return Err(ServiceManagerError::ServiceAlreadyExists(service_name.to_string()));
-            // Load the service
-            let plist_path = self.get_plist_path(&config.name);
-            self.run_launchctl(&["load", &plist_path.to_string_lossy()])
-                .await
-                .map_err(|e| ServiceManagerError::StartFailed(config.name.clone(), e.to_string()))?;
-
-            Ok(())
-        })
         }
-
-    fn start_existing(&self, service_name: &str) -> Result<(), ServiceManagerError> {
         let rt = tokio::runtime::Runtime::new().map_err(|e| ServiceManagerError::Other(e.to_string()))?;
         rt.block_on(async {
             let label = self.get_service_label(service_name);
             let plist_path = self.get_plist_path(service_name);
 
-            // Check if plist file exists
+            // Load the service. We use -w to make it persistent across reboots.
-            if !plist_path.exists() {
+            self.run_launchctl(&["load", "-w", &plist_path.to_string_lossy()])
-                return Err(ServiceManagerError::ServiceNotFound(service_name.to_string()));
-            }
-            
-            // Check if service is already loaded and running
-            let list_output = self.run_launchctl(&["list"]).await?;
-            if list_output.contains(&label) {
-                // Service is loaded, check if it's running
-                match self.status(service_name)? {
-                    ServiceStatus::Running => {
-                        return Ok(()); // Already running, nothing to do
-                    }
-                    _ => {
-                        // Service is loaded but not running, try to start it
-                        self.run_launchctl(&["start", &label])
                 .await
                 .map_err(|e| ServiceManagerError::StartFailed(service_name.to_string(), e.to_string()))?;
-                        return Ok(());
-                    }
-                }
-            }
             
-            // Service is not loaded, load it
+            // Start the service
-            self.run_launchctl(&["load", &plist_path.to_string_lossy()])
+            self.run_launchctl(&["start", &label])
                 .await
                 .map_err(|e| ServiceManagerError::StartFailed(service_name.to_string(), e.to_string()))?;
 
@@ -248,36 +223,26 @@ impl ServiceManager for LaunchctlServiceManager {
         })
     }
 
-    async fn start_and_confirm(&self, config: &ServiceConfig, timeout_secs: u64) -> Result<(), ServiceManagerError> {
+    async fn start_and_confirm(&self, service_name: &str, timeout_secs: u64) -> Result<(), ServiceManagerError> {
-        // First start the service
+        self.start(service_name)?;
-        self.start(config)?;
-        
-        // Then wait for confirmation
-        self.wait_for_service_status(&config.name, timeout_secs).await
-    }
-
-    async fn run(&self, config: &ServiceConfig, timeout_secs: u64) -> Result<(), ServiceManagerError> {
-        self.start_and_confirm(config, timeout_secs).await
-    }
-
-    async fn start_existing_and_confirm(&self, service_name: &str, timeout_secs: u64) -> Result<(), ServiceManagerError> {
-        // First start the existing service
-        self.start_existing(service_name)?;
-        
-        // Then wait for confirmation
         self.wait_for_service_status(service_name, timeout_secs).await
     }
 
     fn stop(&self, service_name: &str) -> Result<(), ServiceManagerError> {
+        if !self.exists(service_name)? {
+            return Err(ServiceManagerError::ServiceNotFound(service_name.to_string()));
+        }
+
         let rt = tokio::runtime::Runtime::new().map_err(|e| ServiceManagerError::Other(e.to_string()))?;
         rt.block_on(async {
-            let _label = self.get_service_label(service_name);
+            let label = self.get_service_label(service_name);
             let plist_path = self.get_plist_path(service_name);
 
-            // Unload the service
+            // Unload the service. Ignore errors, as it might not be loaded.
-            self.run_launchctl(&["unload", &plist_path.to_string_lossy()])
+            let _ = self.run_launchctl(&["unload", &plist_path.to_string_lossy()]).await;
-                .await
+
-                .map_err(|e| ServiceManagerError::StopFailed(service_name.to_string(), e.to_string()))?;
+            // Also try to stop it directly. Ignore errors.
+            let _ = self.run_launchctl(&["stop", &label]).await;
 
             Ok(())
         })
@@ -383,16 +348,18 @@ impl ServiceManager for LaunchctlServiceManager {
     }
 
     fn remove(&self, service_name: &str) -> Result<(), ServiceManagerError> {
-        // Stop the service first
+        if !self.exists(service_name)? {
-        let _ = self.stop(service_name);
+            return Err(ServiceManagerError::ServiceNotFound(service_name.to_string()));
+        }
+
+        // Stop the service first.
+        self.stop(service_name)?;
 
         // Remove the plist file
         let rt = tokio::runtime::Runtime::new().map_err(|e| ServiceManagerError::Other(e.to_string()))?;
         rt.block_on(async {
             let plist_path = self.get_plist_path(service_name);
-            if plist_path.exists() {
             tokio::fs::remove_file(&plist_path).await?;
-            }
             Ok(())
         })
     }

service_manager/src/lib.rs

@@ -42,23 +42,17 @@ pub enum ServiceStatus {
 
 #[async_trait]
 pub trait ServiceManager: Send + Sync {
+    /// Create a new service definition.
+    fn create(&self, config: &ServiceConfig) -> Result<(), ServiceManagerError>;
+
     /// Check if a service exists
     fn exists(&self, service_name: &str) -> Result<bool, ServiceManagerError>;
 
-    /// Start a service with the given configuration
+    /// Start a previously created service by name.
-    fn start(&self, config: &ServiceConfig) -> Result<(), ServiceManagerError>;
+    fn start(&self, service_name: &str) -> Result<(), ServiceManagerError>;
-    
-    /// Start an existing service by name (load existing plist/config)
-    fn start_existing(&self, service_name: &str) -> Result<(), ServiceManagerError>;
 
     /// Start a service and wait for confirmation that it's running or failed
-    async fn start_and_confirm(&self, config: &ServiceConfig, timeout_secs: u64) -> Result<(), ServiceManagerError>;
+    async fn start_and_confirm(&self, service_name: &str, timeout_secs: u64) -> Result<(), ServiceManagerError>;
-    
-    /// Start a service and wait for confirmation that it's running or failed
-    async fn run(&self, config: &ServiceConfig, timeout_secs: u64) -> Result<(), ServiceManagerError>;
-    
-    /// Start an existing service and wait for confirmation that it's running or failed
-    async fn start_existing_and_confirm(&self, service_name: &str, timeout_secs: u64) -> Result<(), ServiceManagerError>;
     
     /// Stop a service by name
     fn stop(&self, service_name: &str) -> Result<(), ServiceManagerError>;
@@ -95,15 +89,63 @@ mod zinit;
 #[cfg(feature = "zinit")]
 pub use zinit::ZinitServiceManager;
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum ServiceManagerChoice {
+    Launchctl,
+    Systemd,
+    Zinit,
+}
+
 // Factory function to create the appropriate service manager for the platform
-pub fn create_service_manager() -> Box<dyn ServiceManager> {
+pub fn create_service_manager(
+    choice: Option<ServiceManagerChoice>,
+) -> Result<Box<dyn ServiceManager>, ServiceManagerError> {
     #[cfg(target_os = "macos")]
     {
-        Box::new(LaunchctlServiceManager::new())
+        match choice {
+            Some(ServiceManagerChoice::Launchctl) | None => {
+                Ok(Box::new(LaunchctlServiceManager::new()))
+            }
+            Some(other) => Err(ServiceManagerError::Other(format!(
+                "Service manager '{:?}' is not supported on macOS",
+                other
+            ))),
+        }
     }
     #[cfg(target_os = "linux")]
     {
-        Box::new(SystemdServiceManager::new())
+        match choice {
+            // Default to Zinit on Linux
+            None => {
+                #[cfg(feature = "zinit")]
+                {
+                    Ok(Box::new(ZinitServiceManager::new()))
+                }
+                #[cfg(not(feature = "zinit"))]
+                {
+                    Err(ServiceManagerError::Other(
+                        "Default service manager Zinit is not available. Please enable the 'zinit' feature, or explicitly choose Systemd.".to_string()
+                    ))
+                }
+            }
+            Some(ServiceManagerChoice::Zinit) => {
+                #[cfg(feature = "zinit")]
+                {
+                    Ok(Box::new(ZinitServiceManager::new()))
+                }
+                #[cfg(not(feature = "zinit"))]
+                {
+                    Err(ServiceManagerError::Other(
+                        "Zinit service manager is not available. Please enable the 'zinit' feature.".to_string()
+                    ))
+                }
+            }
+            Some(ServiceManagerChoice::Systemd) => Ok(Box::new(SystemdServiceManager::new())),
+            Some(other) => Err(ServiceManagerError::Other(format!(
+                "Service manager '{:?}' is not supported on Linux",
+                other
+            ))),
+        }
     }
     #[cfg(not(any(target_os = "macos", target_os = "linux")))]
     {

service_manager/src/zinit.rs

@@ -31,7 +31,11 @@ impl ServiceManager for ZinitServiceManager {
         }
     }
 
-    fn start(&self, config: &ServiceConfig) -> Result<(), ServiceManagerError> {
+    fn create(&self, config: &ServiceConfig) -> Result<(), ServiceManagerError> {
+        if self.exists(&config.name)? {
+            return Err(ServiceManagerError::ServiceAlreadyExists(config.name.clone()));
+        }
+
         let service_config = json!({
             "exec": config.binary_path,
             "args": config.args,
@@ -43,28 +47,21 @@ impl ServiceManager for ZinitServiceManager {
         tokio::runtime::Runtime::new()
             .unwrap()
             .block_on(self.client.create_service(&config.name, service_config))
-            .map_err(|e| ServiceManagerError::StartFailed(config.name.clone(), e.to_string()))?;
+            .map_err(|e| ServiceManagerError::Other(e.to_string()))
-
-        self.start_existing(&config.name)
     }
 
-    fn start_existing(&self, service_name: &str) -> Result<(), ServiceManagerError> {
+    fn start(&self, service_name: &str) -> Result<(), ServiceManagerError> {
+        if let Ok(ServiceStatus::Running) = self.status(service_name) {
+            return Err(ServiceManagerError::ServiceAlreadyExists(service_name.to_string()));
+        }
         tokio::runtime::Runtime::new()
             .unwrap()
             .block_on(self.client.start(service_name))
             .map_err(|e| ServiceManagerError::StartFailed(service_name.to_string(), e.to_string()))
     }
 
-    async fn start_and_confirm(&self, config: &ServiceConfig, _timeout_secs: u64) -> Result<(), ServiceManagerError> {
+    async fn start_and_confirm(&self, service_name: &str, _timeout_secs: u64) -> Result<(), ServiceManagerError> {
-        self.start(config)
+        self.start(service_name)
-    }
-
-    async fn run(&self, config: &ServiceConfig, _timeout_secs: u64) -> Result<(), ServiceManagerError> {
-        self.start(config)
-    }
-
-    async fn start_existing_and_confirm(&self, service_name: &str, _timeout_secs: u64) -> Result<(), ServiceManagerError> {
-        self.start_existing(service_name)
     }
 
     fn stop(&self, service_name: &str) -> Result<(), ServiceManagerError> {