wip: rook-ceph install score

Reviewed-on: #119

Cargo.lock

@@ -105,7 +105,7 @@ dependencies = [
  "futures-core",
  "futures-util",
  "mio 1.0.4",
- "socket2",
+ "socket2 0.5.10",
  "tokio",
  "tracing",
 ]
@@ -167,7 +167,7 @@ dependencies = [
  "serde_json",
  "serde_urlencoded",
  "smallvec",
- "socket2",
+ "socket2 0.5.10",
  "time",
  "tracing",
  "url",
@@ -2178,7 +2178,6 @@ dependencies = [
  "serde",
  "serde_json",
  "sysinfo",
- "uuid",
 ]
 
 [[package]]

Dockerfile

@@ -1,4 +1,4 @@
-FROM docker.io/rust:1.87.0 AS build
+FROM docker.io/rust:1.89.0 AS build
 
 WORKDIR /app
 
@@ -6,7 +6,7 @@ COPY . .
 
 RUN cargo build --release --bin harmony_composer
 
-FROM docker.io/rust:1.87.0
+FROM docker.io/rust:1.89.0
 
 WORKDIR /app
 

harmony/src/modules/storage/ceph/mod.rs

@@ -1 +1,4 @@
-pub mod ceph_osd_replacement_score;
+pub mod ceph_remove_osd_score;
+pub mod rook_ceph_helm_chart_score;
+pub mod rook_ceph_cluster_helm_chart_score;
+pub mod rook_ceph_install_score;

harmony/src/modules/storage/ceph/rook_ceph_cluster_helm_chart_score.rs

@@ -0,0 +1,44 @@
+
+use std::str::FromStr;
+
+use non_blank_string_rs::NonBlankString;
+
+use crate::modules::helm::chart::HelmChartScore;
+
+pub fn rook_ceph_cluster_helm_chart(ns: &str) -> HelmChartScore {
+    let values = r#"
+monitoring:
+  enabled: true
+  createPrometheusRules: true
+cephClusterSpec:
+  placement:
+    all:
+      nodeAffinity:
+        requiredDuringSchedulingIgnoredDuringExecution:
+          nodeSelectorTerms:
+            - matchExpressions:
+              - key: storage-node
+                operator: In
+                values:
+                - "true"
+dashboard:
+  ssl: false
+  prometheusEndpoint: http://prometheus-operated:9090
+  prometheusEndpointSSLVerify: false
+toolbox:
+  enabled: true
+
+        "#
+        .to_string();
+    HelmChartScore {
+        namespace: Some(NonBlankString::from_str(ns).unwrap()),
+        release_name: NonBlankString::from_str("rook-ceph").unwrap(),
+        chart_name: NonBlankString::from_str("https://charts.rook.io/release/rook-release/rook-ceph-cluster").unwrap(),
+        chart_version: todo!(),
+        values_overrides: todo!(),
+        values_yaml: Some(values.to_string()),
+        create_namespace: todo!(),
+        install_only: todo!(),
+        repository: todo!(),
+    }
+}

harmony/src/modules/storage/ceph/rook_ceph_helm_chart_score.rs

@@ -0,0 +1,24 @@
+use std::str::FromStr;
+
+use non_blank_string_rs::NonBlankString;
+
+use crate::modules::helm::chart::HelmChartScore;
+
+pub fn rook_ceph_helm_chart(ns: &str) -> HelmChartScore {
+    let values = r#"
+monitoring:
+  enabled: true
+        "#
+    .to_string();
+    HelmChartScore {
+        namespace: Some(NonBlankString::from_str(ns).unwrap()),
+        release_name: NonBlankString::from_str("rook-ceph").unwrap(),
+        chart_name: NonBlankString::from_str("https://charts.rook.io/release/rook-release/rook-ceph").unwrap(),
+        chart_version: todo!(),
+        values_overrides: todo!(),
+        values_yaml: Some(values.to_string()),
+        create_namespace: todo!(),
+        install_only: todo!(),
+        repository: todo!(),
+    }
+}

harmony/src/modules/storage/ceph/rook_ceph_install_score.rs

@@ -0,0 +1,81 @@
+use serde::Serialize;
+
+use crate::{
+    data::{Id, Version},
+    interpret::{Interpret, InterpretError, InterpretName, InterpretStatus, Outcome},
+    inventory::Inventory,
+    score::Score,
+    topology::{HelmCommand, Topology},
+};
+
+#[derive(Debug, Clone, Serialize)]
+pub struct RookCephInstall {
+    namespace: String,
+}
+
+impl<T: Topology + HelmCommand> Score<T> for RookCephInstall {
+    fn name(&self) -> String {
+        "RookCephInstall".to_string()
+    }
+
+    fn create_interpret(&self) -> Box<dyn Interpret<T>> {
+        Box::new(RookCephInstallInterpret {
+            score: self.score.clone(),
+        })
+    }
+}
+
+#[derive(Debug, Clone)]
+pub struct RookCephInstallInterpret {
+    score: RookCephInstall,
+}
+
+#[async_trait]
+impl<T: Topology + HelmCommand> Interpret<T> for RookCephInstallInterpret {
+    async fn execute(
+        &self,
+        inventory: &Inventory,
+        topology: &T,
+    ) -> Result<InterpretError, Outcome> {
+        self.label_nodes();
+        self.install_rook_helm_chart(self.score.namespace);
+        self.install_rook_cluster_helm_chart(self.score.namespace);
+        //TODO I think we will need to add a capability OCClient to interact with the okd
+        //cli tool 
+        self.add_oc_adm_policy(self.score.namespace);
+    }
+
+    fn get_name(&self) -> InterpretName {
+        todo!()
+    }
+
+    fn get_version(&self) -> Version {
+        todo!()
+    }
+
+    fn get_status(&self) -> InterpretStatus {
+        todo!()
+    }
+
+    fn get_children(&self) -> Vec<Id> {
+        todo!()
+    }
+}
+
+impl RookCephInstallInterpret {
+    fn label_nodes(&self) -> _ {
+        todo!()
+    }
+
+    fn install_rook_helm_chart(&self, namespace: String) -> _ {
+        todo!()
+    }
+
+    fn install_rook_cluster_helm_chart(&self, namespace: String) -> _ {
+        todo!()
+    }
+
+    fn add_oc_adm_policy(&self, namespace: String) -> _ {
+        todo!()
+    }
+}

harmony_inventory_agent/Cargo.toml

@@ -10,4 +10,3 @@ serde.workspace = true
 serde_json.workspace = true
 log.workspace = true
 env_logger.workspace = true
-uuid.workspace = true

harmony_inventory_agent/src/hwinfo.rs

@@ -1,3 +1,4 @@
+use log::debug;
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
 use std::fs;
@@ -83,27 +84,108 @@ pub struct ManagementInterface {
 }
 
 impl PhysicalHost {
-    pub fn gather() -> Self {
+    pub fn gather() -> Result<Self, String> {
         let mut sys = System::new_all();
         sys.refresh_all();
 
-        Self {
-            storage_drives: Self::gather_storage_drives(),
-            storage_controller: Self::gather_storage_controller(),
-            memory_modules: Self::gather_memory_modules(),
-            cpus: Self::gather_cpus(&sys),
-            chipset: Self::gather_chipset(),
-            network_interfaces: Self::gather_network_interfaces(),
-            management_interface: Self::gather_management_interface(),
-            host_uuid: Self::get_host_uuid(),
+        Self::all_tools_available()?;
+
+        Ok(Self {
+            storage_drives: Self::gather_storage_drives()?,
+            storage_controller: Self::gather_storage_controller()?,
+            memory_modules: Self::gather_memory_modules()?,
+            cpus: Self::gather_cpus(&sys)?,
+            chipset: Self::gather_chipset()?,
+            network_interfaces: Self::gather_network_interfaces()?,
+            management_interface: Self::gather_management_interface()?,
+            host_uuid: Self::get_host_uuid()?,
+        })
+    }
+
+    fn all_tools_available() -> Result<(), String> {
+        let required_tools = [
+            ("lsblk", "--version"),
+            ("lspci", "--version"),
+            ("lsmod", "--version"),
+            ("dmidecode", "--version"),
+            ("smartctl", "--version"),
+            ("ip", "route"), // No version flag available
+        ];
+
+        let mut missing_tools = Vec::new();
+
+        for (tool, tool_arg) in required_tools.iter() {
+            // First check if tool exists in PATH using which(1)
+            let exists = if let Ok(output) = Command::new("which").arg(tool).output() {
+                output.status.success()
+            } else {
+                // Fallback: manual PATH search if which(1) is unavailable
+                if let Ok(path_var) = std::env::var("PATH") {
+                    path_var.split(':').any(|dir| {
+                        let tool_path = std::path::Path::new(dir).join(tool);
+                        tool_path.exists() && Self::is_executable(&tool_path)
+                    })
+                } else {
+                    false
+                }
+            };
+
+            if !exists {
+                missing_tools.push(*tool);
+                continue;
+            }
+
+            // Verify tool is functional by checking version/help output
+            let mut cmd = Command::new(tool);
+            cmd.arg(tool_arg);
+            cmd.stdout(std::process::Stdio::null());
+            cmd.stderr(std::process::Stdio::null());
+
+            if let Ok(status) = cmd.status() {
+                if !status.success() {
+                    missing_tools.push(*tool);
+                }
+            } else {
+                missing_tools.push(*tool);
             }
         }
 
-    fn gather_storage_drives() -> Vec<StorageDrive> {
+        if !missing_tools.is_empty() {
+            let missing_str = missing_tools
+                .iter()
+                .map(|s| s.to_string())
+                .collect::<Vec<String>>()
+                .join(", ");
+            return Err(format!(
+                "The following required tools are not available: {}. Please install these tools to use PhysicalHost::gather()",
+                missing_str
+            ));
+        }
+
+        Ok(())
+    }
+
+    #[cfg(unix)]
+    fn is_executable(path: &std::path::Path) -> bool {
+        use std::os::unix::fs::PermissionsExt;
+
+        match std::fs::metadata(path) {
+            Ok(meta) => meta.permissions().mode() & 0o111 != 0,
+            Err(_) => false,
+        }
+    }
+
+    #[cfg(not(unix))]
+    fn is_executable(_path: &std::path::Path) -> bool {
+        // On non-Unix systems, we assume existence implies executability
+        true
+    }
+
+    fn gather_storage_drives() -> Result<Vec<StorageDrive>, String> {
         let mut drives = Vec::new();
 
         // Use lsblk with JSON output for robust parsing
-        if let Ok(output) = Command::new("lsblk")
+        let output = Command::new("lsblk")
             .args([
                 "-d",
                 "-o",
@@ -114,16 +196,30 @@ impl PhysicalHost {
                 "--json",
             ])
             .output()
-            && output.status.success()
-            && let Ok(json) = serde_json::from_slice::<Value>(&output.stdout)
-            && let Some(blockdevices) = json.get("blockdevices").and_then(|v| v.as_array())
-        {
+            .map_err(|e| format!("Failed to execute lsblk: {}", e))?;
+
+        if !output.status.success() {
+            return Err(format!(
+                "lsblk command failed: {}",
+                String::from_utf8_lossy(&output.stderr)
+            ));
+        }
+
+        let json: Value = serde_json::from_slice(&output.stdout)
+            .map_err(|e| format!("Failed to parse lsblk JSON output: {}", e))?;
+
+        let blockdevices = json
+            .get("blockdevices")
+            .and_then(|v| v.as_array())
+            .ok_or("Invalid lsblk JSON: missing 'blockdevices' array")?;
+
         for device in blockdevices {
             let name = device
                 .get("name")
                 .and_then(|v| v.as_str())
-                    .unwrap_or("")
+                .ok_or("Missing 'name' in lsblk device")?
                 .to_string();
+
             if name.is_empty() {
                 continue;
             }
@@ -140,13 +236,16 @@ impl PhysicalHost {
                 .map(|s| s.trim().to_string())
                 .unwrap_or_default();
 
-                let size_str = device.get("size").and_then(|v| v.as_str()).unwrap_or("0");
-                let size_bytes = Self::parse_size(size_str).unwrap_or(0);
+            let size_str = device
+                .get("size")
+                .and_then(|v| v.as_str())
+                .ok_or("Missing 'size' in lsblk device")?;
+            let size_bytes = Self::parse_size(size_str)?;
 
             let rotational = device
                 .get("rota")
                 .and_then(|v| v.as_bool())
-                    .unwrap_or(false);
+                .ok_or("Missing 'rota' in lsblk device")?;
 
             let wwn = device
                 .get("wwn")
@@ -156,56 +255,67 @@ impl PhysicalHost {
 
             let device_path = Path::new("/sys/block").join(&name);
 
+            let logical_block_size = Self::read_sysfs_u32(
+                &device_path.join("queue/logical_block_size"),
+            )
+            .map_err(|e| format!("Failed to read logical block size for {}: {}", name, e))?;
+
+            let physical_block_size = Self::read_sysfs_u32(
+                &device_path.join("queue/physical_block_size"),
+            )
+            .map_err(|e| format!("Failed to read physical block size for {}: {}", name, e))?;
+
+            let interface_type = Self::get_interface_type(&name, &device_path)?;
+            let smart_status = Self::get_smart_status(&name)?;
+
             let mut drive = StorageDrive {
                 name: name.clone(),
                 model,
                 serial,
                 size_bytes,
-                    logical_block_size: Self::read_sysfs_u32(
-                        &device_path.join("queue/logical_block_size"),
-                    )
-                    .unwrap_or(512),
-                    physical_block_size: Self::read_sysfs_u32(
-                        &device_path.join("queue/physical_block_size"),
-                    )
-                    .unwrap_or(512),
+                logical_block_size,
+                physical_block_size,
                 rotational,
                 wwn,
-                    interface_type: Self::get_interface_type(&name, &device_path),
-                    smart_status: Self::get_smart_status(&name),
+                interface_type,
+                smart_status,
             };
 
             // Enhance with additional sysfs info if available
             if device_path.exists() {
                 if drive.model.is_empty() {
-                        drive.model = Self::read_sysfs_string(&device_path.join("device/model"));
+                    drive.model = Self::read_sysfs_string(&device_path.join("device/model"))
+                        .map_err(|e| format!("Failed to read model for {}: {}", name, e))?;
                 }
                 if drive.serial.is_empty() {
-                        drive.serial = Self::read_sysfs_string(&device_path.join("device/serial"));
+                    drive.serial = Self::read_sysfs_string(&device_path.join("device/serial"))
+                        .map_err(|e| format!("Failed to read serial for {}: {}", name, e))?;
                 }
             }
 
             drives.push(drive);
         }
+
+        Ok(drives)
     }
 
-        drives
-    }
-
-    fn gather_storage_controller() -> StorageController {
+    fn gather_storage_controller() -> Result<StorageController, String> {
         let mut controller = StorageController {
             name: "Unknown".to_string(),
             driver: "Unknown".to_string(),
         };
 
         // Use lspci with JSON output if available
-        if let Ok(output) = Command::new("lspci")
+        let output = Command::new("lspci")
             .args(["-nn", "-d", "::0100", "-J"]) // Storage controllers class with JSON
             .output()
-            && output.status.success()
-            && let Ok(json) = serde_json::from_slice::<Value>(&output.stdout)
-            && let Some(devices) = json.as_array()
-        {
+            .map_err(|e| format!("Failed to execute lspci: {}", e))?;
+
+        if output.status.success() {
+            let json: Value = serde_json::from_slice(&output.stdout)
+                .map_err(|e| format!("Failed to parse lspci JSON output: {}", e))?;
+
+            if let Some(devices) = json.as_array() {
                 for device in devices {
                     if let Some(device_info) = device.as_object()
                         && let Some(name) = device_info
@@ -219,14 +329,16 @@ impl PhysicalHost {
                     }
                 }
             }
+        }
 
-        // Fallback to text output if JSON fails
-        if controller.name == "Unknown"
-            && let Ok(output) = Command::new("lspci")
+        // Fallback to text output if JSON fails or no device found
+        if controller.name == "Unknown" {
+            let output = Command::new("lspci")
                 .args(["-nn", "-d", "::0100"]) // Storage controllers class
                 .output()
-            && output.status.success()
-        {
+                .map_err(|e| format!("Failed to execute lspci (fallback): {}", e))?;
+
+            if output.status.success() {
                 let output_str = String::from_utf8_lossy(&output.stdout);
                 if let Some(line) = output_str.lines().next() {
                     let parts: Vec<&str> = line.split(':').collect();
@@ -235,11 +347,14 @@ impl PhysicalHost {
                     }
                 }
             }
+        }
 
         // Try to get driver info from lsmod
-        if let Ok(output) = Command::new("lsmod").output()
-            && output.status.success()
-        {
+        let output = Command::new("lsmod")
+            .output()
+            .map_err(|e| format!("Failed to execute lsmod: {}", e))?;
+
+        if output.status.success() {
             let output_str = String::from_utf8_lossy(&output.stdout);
             for line in output_str.lines() {
                 if line.contains("ahci")
@@ -256,16 +371,28 @@ impl PhysicalHost {
             }
         }
 
-        controller
+        Ok(controller)
     }
 
-    fn gather_memory_modules() -> Vec<MemoryModule> {
+    fn gather_memory_modules() -> Result<Vec<MemoryModule>, String> {
         let mut modules = Vec::new();
 
-        if let Ok(output) = Command::new("dmidecode").arg("--type").arg("17").output()
-            && output.status.success()
-        {
-            let output_str = String::from_utf8_lossy(&output.stdout);
+        let output = Command::new("dmidecode")
+            .arg("--type")
+            .arg("17")
+            .output()
+            .map_err(|e| format!("Failed to execute dmidecode: {}", e))?;
+
+        if !output.status.success() {
+            return Err(format!(
+                "dmidecode command failed: {}",
+                String::from_utf8_lossy(&output.stderr)
+            ));
+        }
+
+        let output_str = String::from_utf8(output.stdout)
+            .map_err(|e| format!("Failed to parse dmidecode output: {}", e))?;
+
         let sections: Vec<&str> = output_str.split("Memory Device").collect();
 
         for section in sections.into_iter().skip(1) {
@@ -311,12 +438,11 @@ impl PhysicalHost {
                 modules.push(module);
             }
         }
+
+        Ok(modules)
     }
 
-        modules
-    }
-
-    fn gather_cpus(sys: &System) -> Vec<CPU> {
+    fn gather_cpus(sys: &System) -> Result<Vec<CPU>, String> {
         let mut cpus = Vec::new();
         let global_cpu = sys.global_cpu_info();
 
@@ -328,23 +454,29 @@ impl PhysicalHost {
             frequency_mhz: global_cpu.frequency(),
         });
 
-        cpus
+        Ok(cpus)
     }
 
-    fn gather_chipset() -> Chipset {
-        Chipset {
-            name: Self::read_dmi("board-product-name").unwrap_or_else(|| "Unknown".to_string()),
-            vendor: Self::read_dmi("board-manufacturer").unwrap_or_else(|| "Unknown".to_string()),
-        }
+    fn gather_chipset() -> Result<Chipset, String> {
+        Ok(Chipset {
+            name: Self::read_dmi("baseboard-product-name")?,
+            vendor: Self::read_dmi("baseboard-manufacturer")?,
+        })
     }
 
-    fn gather_network_interfaces() -> Vec<NetworkInterface> {
+    fn gather_network_interfaces() -> Result<Vec<NetworkInterface>, String> {
         let mut interfaces = Vec::new();
         let sys_net_path = Path::new("/sys/class/net");
 
-        if let Ok(entries) = fs::read_dir(sys_net_path) {
-            for entry in entries.flatten() {
-                let iface_name = entry.file_name().into_string().unwrap_or_default();
+        let entries = fs::read_dir(sys_net_path)
+            .map_err(|e| format!("Failed to read /sys/class/net: {}", e))?;
+
+        for entry in entries {
+            let entry = entry.map_err(|e| format!("Failed to read directory entry: {}", e))?;
+            let iface_name = entry
+                .file_name()
+                .into_string()
+                .map_err(|_| "Invalid UTF-8 in interface name")?;
             let iface_path = entry.path();
 
             // Skip virtual interfaces
@@ -364,16 +496,42 @@ impl PhysicalHost {
                 continue;
             }
 
-                let mac_address = Self::read_sysfs_string(&iface_path.join("address"));
-                let speed_mbps = Self::read_sysfs_u32(&iface_path.join("speed"));
-                let operstate = Self::read_sysfs_string(&iface_path.join("operstate"));
-                let mtu = Self::read_sysfs_u32(&iface_path.join("mtu")).unwrap_or(1500);
-                let driver = Self::read_sysfs_string(&iface_path.join("device/driver/module"));
-                let firmware_version =
-                    Self::read_sysfs_opt_string(&iface_path.join("device/firmware_version"));
+            let mac_address = Self::read_sysfs_string(&iface_path.join("address"))
+                .map_err(|e| format!("Failed to read MAC address for {}: {}", iface_name, e))?;
+
+            let speed_mbps = if iface_path.join("speed").exists() {
+                match Self::read_sysfs_u32(&iface_path.join("speed")) {
+                    Ok(speed) => Some(speed),
+                    Err(e) => {
+                        debug!(
+                            "Failed to read speed for {}: {} . This is expected to fail on wifi interfaces.",
+                            iface_name, e
+                        );
+                        None
+                    }
+                }
+            } else {
+                None
+            };
+
+            let operstate = Self::read_sysfs_string(&iface_path.join("operstate"))
+                .map_err(|e| format!("Failed to read operstate for {}: {}", iface_name, e))?;
+
+            let mtu = Self::read_sysfs_u32(&iface_path.join("mtu"))
+                .map_err(|e| format!("Failed to read MTU for {}: {}", iface_name, e))?;
+
+            let driver =
+                Self::read_sysfs_symlink_basename(&iface_path.join("device/driver/module"))
+                    .map_err(|e| format!("Failed to read driver for {}: {}", iface_name, e))?;
+
+            let firmware_version = Self::read_sysfs_opt_string(
+                &iface_path.join("device/firmware_version"),
+            )
+            .map_err(|e| format!("Failed to read firmware version for {}: {}", iface_name, e))?;
 
             // Get IP addresses using ip command with JSON output
-                let (ipv4_addresses, ipv6_addresses) = Self::get_interface_ips_json(&iface_name);
+            let (ipv4_addresses, ipv6_addresses) = Self::get_interface_ips_json(&iface_name)
+                .map_err(|e| format!("Failed to get IP addresses for {}: {}", iface_name, e))?;
 
             interfaces.push(NetworkInterface {
                 name: iface_name,
@@ -387,142 +545,219 @@ impl PhysicalHost {
                 firmware_version,
             });
         }
+
+        Ok(interfaces)
     }
 
-        interfaces
-    }
-
-    fn gather_management_interface() -> Option<ManagementInterface> {
-        // Try to detect common management interfaces
+    fn gather_management_interface() -> Result<Option<ManagementInterface>, String> {
         if Path::new("/dev/ipmi0").exists() {
-            Some(ManagementInterface {
+            Ok(Some(ManagementInterface {
                 kind: "IPMI".to_string(),
                 address: None,
-                firmware: Self::read_dmi("bios-version"),
-            })
+                firmware: Some(Self::read_dmi("bios-version")?),
+            }))
         } else if Path::new("/sys/class/misc/mei").exists() {
-            Some(ManagementInterface {
+            Ok(Some(ManagementInterface {
                 kind: "Intel ME".to_string(),
                 address: None,
                 firmware: None,
-            })
+            }))
         } else {
-            None
+            Ok(None)
         }
     }
 
-    fn get_host_uuid() -> String {
-        Self::read_dmi("system-uuid").unwrap()
+    fn get_host_uuid() -> Result<String, String> {
+        Self::read_dmi("system-uuid")
     }
 
     // Helper methods
-    fn read_sysfs_string(path: &Path) -> String {
+    fn read_sysfs_string(path: &Path) -> Result<String, String> {
         fs::read_to_string(path)
-            .unwrap_or_default()
-            .trim()
-            .to_string()
-    }
-
-    fn read_sysfs_opt_string(path: &Path) -> Option<String> {
-        fs::read_to_string(path)
-            .ok()
             .map(|s| s.trim().to_string())
-            .filter(|s| !s.is_empty())
+            .map_err(|e| format!("Failed to read {}: {}", path.display(), e))
     }
 
-    fn read_sysfs_u32(path: &Path) -> Option<u32> {
+    fn read_sysfs_opt_string(path: &Path) -> Result<Option<String>, String> {
+        match fs::read_to_string(path) {
+            Ok(s) => {
+                let s = s.trim().to_string();
+                Ok(if s.is_empty() { None } else { Some(s) })
+            }
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None),
+            Err(e) => Err(format!("Failed to read {}: {}", path.display(), e)),
+        }
+    }
+
+    fn read_sysfs_u32(path: &Path) -> Result<u32, String> {
         fs::read_to_string(path)
-            .ok()
-            .and_then(|s| s.trim().parse().ok())
+            .map_err(|e| format!("Failed to read {}: {}", path.display(), e))?
+            .trim()
+            .parse()
+            .map_err(|e| format!("Failed to parse {}: {}", path.display(), e))
     }
 
-    fn read_dmi(field: &str) -> Option<String> {
-        Command::new("dmidecode")
+    fn read_sysfs_symlink_basename(path: &Path) -> Result<String, String> {
+        match fs::read_link(path) {
+            Ok(target_path) => match target_path.file_name() {
+                Some(name_osstr) => match name_osstr.to_str() {
+                    Some(name_str) => Ok(name_str.to_string()),
+                    None => Err(format!(
+                        "Symlink target basename is not valid UTF-8: {}",
+                        target_path.display()
+                    )),
+                },
+                None => Err(format!(
+                    "Symlink target has no basename: {} -> {}",
+                    path.display(),
+                    target_path.display()
+                )),
+            },
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => Err(format!(
+                "Could not resolve symlink for path : {}",
+                path.display()
+            )),
+            Err(e) => Err(format!("Failed to read symlink {}: {}", path.display(), e)),
+        }
+    }
+
+    fn read_dmi(field: &str) -> Result<String, String> {
+        let output = Command::new("dmidecode")
             .arg("-s")
             .arg(field)
             .output()
-            .ok()
-            .filter(|output| output.status.success())
-            .and_then(|output| String::from_utf8(output.stdout).ok())
-            .map(|s| s.trim().to_string())
-            .filter(|s| !s.is_empty())
+            .map_err(|e| format!("Failed to execute dmidecode for field {}: {}", field, e))?;
+
+        if !output.status.success() {
+            return Err(format!(
+                "dmidecode command failed for field {}: {}",
+                field,
+                String::from_utf8_lossy(&output.stderr)
+            ));
         }
 
-    fn get_interface_type(device_name: &str, device_path: &Path) -> String {
-        if device_name.starts_with("nvme") {
-            "NVMe".to_string()
-        } else if device_name.starts_with("sd") {
-            "SATA".to_string()
-        } else if device_name.starts_with("hd") {
-            "IDE".to_string()
-        } else if device_name.starts_with("vd") {
-            "VirtIO".to_string()
-        } else {
-            // Try to determine from device path
-            Self::read_sysfs_string(&device_path.join("device/subsystem"))
-                .split('/')
-                .next_back()
-                .unwrap_or("Unknown")
-                .to_string()
-        }
-    }
-
-    fn get_smart_status(device_name: &str) -> Option<String> {
-        Command::new("smartctl")
-            .arg("-H")
-            .arg(format!("/dev/{}", device_name))
-            .output()
-            .ok()
-            .filter(|output| output.status.success())
-            .and_then(|output| String::from_utf8(output.stdout).ok())
-            .and_then(|s| {
-                s.lines()
-                    .find(|line| line.contains("SMART overall-health self-assessment"))
-                    .and_then(|line| line.split(':').nth(1))
+        String::from_utf8(output.stdout)
             .map(|s| s.trim().to_string())
+            .map_err(|e| {
+                format!(
+                    "Failed to parse dmidecode output for field {}: {}",
+                    field, e
+                )
             })
     }
 
-    fn parse_size(size_str: &str) -> Option<u64> {
-        if size_str.ends_with('T') {
-            size_str[..size_str.len() - 1]
-                .parse::<u64>()
-                .ok()
-                .map(|t| t * 1024 * 1024 * 1024 * 1024)
-        } else if size_str.ends_with('G') {
-            size_str[..size_str.len() - 1]
-                .parse::<u64>()
-                .ok()
-                .map(|g| g * 1024 * 1024 * 1024)
-        } else if size_str.ends_with('M') {
-            size_str[..size_str.len() - 1]
-                .parse::<u64>()
-                .ok()
-                .map(|m| m * 1024 * 1024)
-        } else if size_str.ends_with('K') {
-            size_str[..size_str.len() - 1]
-                .parse::<u64>()
-                .ok()
-                .map(|k| k * 1024)
-        } else if size_str.ends_with('B') {
-            size_str[..size_str.len() - 1].parse::<u64>().ok()
+    fn get_interface_type(device_name: &str, device_path: &Path) -> Result<String, String> {
+        if device_name.starts_with("nvme") {
+            Ok("NVMe".to_string())
+        } else if device_name.starts_with("sd") {
+            Ok("SATA".to_string())
+        } else if device_name.starts_with("hd") {
+            Ok("IDE".to_string())
+        } else if device_name.starts_with("vd") {
+            Ok("VirtIO".to_string())
         } else {
-            size_str.parse::<u64>().ok()
+            // Try to determine from device path
+            let subsystem = Self::read_sysfs_string(&device_path.join("device/subsystem"))?;
+            Ok(subsystem
+                .split('/')
+                .next_back()
+                .unwrap_or("Unknown")
+                .to_string())
         }
     }
 
-    fn get_interface_ips_json(iface_name: &str) -> (Vec<String>, Vec<String>) {
+    fn get_smart_status(device_name: &str) -> Result<Option<String>, String> {
+        let output = Command::new("smartctl")
+            .arg("-H")
+            .arg(format!("/dev/{}", device_name))
+            .output()
+            .map_err(|e| format!("Failed to execute smartctl for {}: {}", device_name, e))?;
+
+        if !output.status.success() {
+            return Ok(None);
+        }
+
+        let stdout = String::from_utf8(output.stdout)
+            .map_err(|e| format!("Failed to parse smartctl output for {}: {}", device_name, e))?;
+
+        for line in stdout.lines() {
+            if line.contains("SMART overall-health self-assessment") {
+                if let Some(status) = line.split(':').nth(1) {
+                    return Ok(Some(status.trim().to_string()));
+                }
+            }
+        }
+
+        Ok(None)
+    }
+
+    fn parse_size(size_str: &str) -> Result<u64, String> {
+        debug!("Parsing size_str '{size_str}'");
+        let size;
+        if size_str.ends_with('T') {
+            size = size_str[..size_str.len() - 1]
+                .parse::<f64>()
+                .map(|t| t * 1024.0 * 1024.0 * 1024.0 * 1024.0)
+                .map_err(|e| format!("Failed to parse T size '{}': {}", size_str, e))
+        } else if size_str.ends_with('G') {
+            size = size_str[..size_str.len() - 1]
+                .parse::<f64>()
+                .map(|g| g * 1024.0 * 1024.0 * 1024.0)
+                .map_err(|e| format!("Failed to parse G size '{}': {}", size_str, e))
+        } else if size_str.ends_with('M') {
+            size = size_str[..size_str.len() - 1]
+                .parse::<f64>()
+                .map(|m| m * 1024.0 * 1024.0)
+                .map_err(|e| format!("Failed to parse M size '{}': {}", size_str, e))
+        } else if size_str.ends_with('K') {
+            size = size_str[..size_str.len() - 1]
+                .parse::<f64>()
+                .map(|k| k * 1024.0)
+                .map_err(|e| format!("Failed to parse K size '{}': {}", size_str, e))
+        } else if size_str.ends_with('B') {
+            size = size_str[..size_str.len() - 1]
+                .parse::<f64>()
+                .map_err(|e| format!("Failed to parse B size '{}': {}", size_str, e))
+        } else {
+            size = size_str
+                .parse::<f64>()
+                .map_err(|e| format!("Failed to parse size '{}': {}", size_str, e))
+        }
+
+        size.map(|s| s as u64)
+    }
+
+    fn get_interface_ips_json(iface_name: &str) -> Result<(Vec<String>, Vec<String>), String> {
         let mut ipv4 = Vec::new();
         let mut ipv6 = Vec::new();
 
         // Get IPv4 addresses using JSON output
-        if let Ok(output) = Command::new("ip")
+        let output = Command::new("ip")
             .args(["-j", "-4", "addr", "show", iface_name])
             .output()
-            && output.status.success()
-            && let Ok(json) = serde_json::from_slice::<Value>(&output.stdout)
-            && let Some(addrs) = json.as_array()
-        {
+            .map_err(|e| {
+                format!(
+                    "Failed to execute ip command for IPv4 on {}: {}",
+                    iface_name, e
+                )
+            })?;
+
+        if !output.status.success() {
+            return Err(format!(
+                "ip command for IPv4 on {} failed: {}",
+                iface_name,
+                String::from_utf8_lossy(&output.stderr)
+            ));
+        }
+
+        let json: Value = serde_json::from_slice(&output.stdout).map_err(|e| {
+            format!(
+                "Failed to parse ip JSON output for IPv4 on {}: {}",
+                iface_name, e
+            )
+        })?;
+
+        if let Some(addrs) = json.as_array() {
             for addr_info in addrs {
                 if let Some(addr_info_obj) = addr_info.as_object()
                     && let Some(addr_info) =
@@ -540,13 +775,32 @@ impl PhysicalHost {
         }
 
         // Get IPv6 addresses using JSON output
-        if let Ok(output) = Command::new("ip")
+        let output = Command::new("ip")
             .args(["-j", "-6", "addr", "show", iface_name])
             .output()
-            && output.status.success()
-            && let Ok(json) = serde_json::from_slice::<Value>(&output.stdout)
-            && let Some(addrs) = json.as_array()
-        {
+            .map_err(|e| {
+                format!(
+                    "Failed to execute ip command for IPv6 on {}: {}",
+                    iface_name, e
+                )
+            })?;
+
+        if !output.status.success() {
+            return Err(format!(
+                "ip command for IPv6 on {} failed: {}",
+                iface_name,
+                String::from_utf8_lossy(&output.stderr)
+            ));
+        }
+
+        let json: Value = serde_json::from_slice(&output.stdout).map_err(|e| {
+            format!(
+                "Failed to parse ip JSON output for IPv6 on {}: {}",
+                iface_name, e
+            )
+        })?;
+
+        if let Some(addrs) = json.as_array() {
             for addr_info in addrs {
                 if let Some(addr_info_obj) = addr_info.as_object()
                     && let Some(addr_info) =
@@ -566,6 +820,6 @@ impl PhysicalHost {
             }
         }
 
-        (ipv4, ipv6)
+        Ok((ipv4, ipv6))
     }
 }

harmony_inventory_agent/src/main.rs

@@ -9,9 +9,17 @@ mod hwinfo;
 async fn inventory() -> impl Responder {
     log::info!("Received inventory request");
     let host = PhysicalHost::gather();
+    match host {
+        Ok(host) => {
             log::info!("Inventory data gathered successfully");
             actix_web::HttpResponse::Ok().json(host)
         }
+        Err(error) => {
+            log::error!("Inventory data gathering FAILED");
+            actix_web::HttpResponse::InternalServerError().json(error)
+        }
+    }
+}
 
 #[actix_web::main]
 async fn main() -> std::io::Result<()> {