engineering: simplify and improve resliency of available rollback detection

crates/trident/src/datastore.rs

@@ -1,6 +1,6 @@
 use std::{fs, path::Path};
 
-use log::debug;
+use log::{debug, warn};
 use sqlite::State;
 
 use trident_api::{
@@ -78,41 +78,33 @@ impl DataStore {
         })
     }
 
-    /// Retrieve all HostStatus entries from the datastore, sorted from oldest to newest.
-    pub(crate) fn get_host_statuses(&self) -> Result<Vec<HostStatus>, TridentError> {
-        let mut all_rows_data: Vec<HostStatus> = Vec::new();
+    /// Retrieve all HostStatus entries from the datastore, sorted from newest to oldest.
+    pub(crate) fn get_host_statuses(&self) -> Result<Vec<Option<HostStatus>>, TridentError> {
+        let mut all_rows_data: Vec<Option<HostStatus>> = Vec::new();
 
         // Read all HostStatus entries from the datastore, parse them into
         // HostStatus structs, and return a slice of them.
         let mut query_statement = self
             .db
             .as_ref()
             .structured(ServicingError::from(DatastoreError::OpenDatastore))?
-            .prepare("SELECT contents FROM hoststatus ORDER BY id ASC")
+            .prepare("SELECT contents FROM hoststatus ORDER BY id DESC")
             .structured(ServicingError::Datastore {
-                inner: DatastoreError::InitializeDatastore,
+                inner: DatastoreError::ReadDatastore,
             })
             .message("Failed to read all database host statuses")?;
 
-        while let State::Row = query_statement
-            .next()
-            .structured(ServicingError::Datastore {
-                inner: DatastoreError::ReadDatastore,
-            })
-            .message("Failed to get next datastore row")?
-        {
-            let host_status_yaml = query_statement
-                .read::<String, _>(0)
-                .structured(ServicingError::Datastore {
-                    inner: DatastoreError::ReadDatastore,
-                })
-                .message("Failed to read datastore row")?;
-            let host_status = serde_yaml::from_str(&host_status_yaml)
-                .structured(ServicingError::Datastore {
-                    inner: DatastoreError::ReadDatastore,
-                })
-                .message("Failed to parse Host Status as YAML")?;
-            all_rows_data.push(host_status);
+        loop {
+            match query_statement.next() {
+                Ok(State::Done) => break,
+                Err(e) => {
+                    warn!("Failed to get next datastore row: {:?}", e);
+                    all_rows_data.push(None);
+                    break;
+                }
+                Ok(State::Row) => {} // continue below
+            }
+            all_rows_data.push(self.parse_host_status(query_statement.read::<String, _>(0)));
         }
         Ok(all_rows_data)
     }
@@ -226,6 +218,39 @@ impl DataStore {
     pub(crate) fn close(&mut self) {
         self.db = None;
     }
+
+    /// Parse a single HostStatus entry from a datastore query result.
+    /// 1. Read each row as a string containing YAML-encoded Host Status.
+    /// 2. Decode the YAML string into a serde_yaml Value.
+    /// 3. Use decode_host_status to convert the serde_yaml Value into a HostStatus struct.
+    ///
+    /// If any step fails, log the error and push None for that row.
+    /// If all steps succeed, push Some(HostStatus) for that row.
+    fn parse_host_status(&self, query_result: Result<String, sqlite::Error>) -> Option<HostStatus> {
+        let host_status_yaml = match query_result {
+            Ok(yaml) => yaml,
+            Err(e) => {
+                warn!("Failed to read datastore row: {:?}", e);
+                return None;
+            }
+        };
+
+        let host_status_value: serde_yaml::Value = match serde_yaml::from_str(&host_status_yaml) {
+            Ok(host_status_value) => host_status_value,
+            Err(e) => {
+                warn!("Failed to parse Host Status as serde value: {:?}", e);
+                return None;
+            }
+        };
+
+        match decode_host_status(host_status_value) {
+            Ok(host_status) => Some(host_status),
+            Err(e) => {
+                warn!("Failed to parse Host Status: {:?}", e);
+                None
+            }
+        }
+    }
 }
 
 #[cfg(test)]
@@ -250,6 +275,37 @@ mod tests {
 
         temp_dir.close().unwrap();
     }
+
+    #[test]
+    fn test_parse_host_status() {
+        let ds = super::DataStore {
+            db: None,
+            host_status: Default::default(),
+            temporary: true,
+        };
+        // Validate when db row is an error
+        assert!(ds
+            .parse_host_status(Err(sqlite::Error {
+                code: None,
+                message: None,
+            }))
+            .is_none());
+        // Validate when db row cannot be parsed into serde_yaml::Value
+        assert!(ds
+            .parse_host_status(Ok("[@ notserdevalue".to_string()))
+            .is_none());
+        // Validate when db row can be parsed into serde_yaml::Value but not HostStatus
+        assert!(ds
+            .parse_host_status(Ok("serdeyaml: but-not-host-status".to_string()))
+            .is_none());
+        // Validate when db row can be parsed into serde_yaml::Value and HostStatus
+        let valid_host_status = super::HostStatus {
+            ..Default::default()
+        };
+        assert!(ds
+            .parse_host_status(Ok(serde_yaml::to_string(&valid_host_status).unwrap()))
+            .is_some());
+    }
 }
 
 #[cfg(feature = "functional-test")]

crates/trident/src/engine/manual_rollback/mod.rs

@@ -41,12 +41,7 @@ pub fn get_rollback_info(datastore: &DataStore, kind: GetKind) -> Result<String,
     // Create ManualRollback context from HostStatus entries.
     let context = ManualRollbackContext::new(&host_statuses)
         .message("Failed to create manual rollback context")?;
-    let rollback_chain =
-        context
-            .get_rollback_chain()
-            .structured(ServicingError::ManualRollback {
-                message: "Failed to get available rollbacks",
-            })?;
+    let rollback_chain = context.get_rollback_chain();
 
     match kind {
         GetKind::RollbackTarget => {