refactor: some code refactoring

complying some rust best practices
updated devcontainer to built rpms

Signed-off-by: Sayan Paul <paul.sayan@gmail.com>

.devcontainer/Dockerfile

@@ -4,6 +4,7 @@ RUN bash -c "$(curl -fsSL "https://raw.githubusercontent.com/microsoft/vscode-de
 
 RUN dnf install -y \
     sudo git cargo rust rust-src git-core clippy rustfmt \
+    rust-packaging rpmdevtools rpmlint \
     && dnf clean all
 
 USER vscode

greenboot.spec

@@ -6,7 +6,7 @@
 %global __cargo_is_lib() false
 %global forgeurl https://github.com/fedora-iot/greenboot
 
-Version:            1.1.12
+Version:            1.1.13
 
 %forgemeta
 

src/handler/mod.rs

@@ -1,18 +1,19 @@
-/// This module most of low-level commands
+/// This module contains most of the low-level commands
 /// and grub variable modifications
-use anyhow::{bail, Error, Result};
-use std::fs::OpenOptions;
-use std::io::Write;
+use anyhow::{bail, Result};
 use std::process::Command;
 use std::str;
 
-/// reboots the system if boot_counter is greater 0 or can be forced too
-pub fn handle_reboot(force: bool) -> Result<(), Error> {
+/// reboots the system if boot_counter is greater than 0 or can be forced too
+pub fn handle_reboot(force: bool) -> Result<()> {
     if !force {
         match get_boot_counter() {
-            Some(t) if t <= 0 => bail!("countdown ended, check greenboot-rollback status"),
+            Some(t) => {
+                if t <= 0 {
+                    bail!("countdown ended, check greenboot-rollback status")
+                }
+            }
             None => bail!("boot_counter is not set"),
-            _ => {}
         }
     }
     log::info!("restarting system");
@@ -23,37 +24,32 @@ pub fn handle_reboot(force: bool) -> Result<(), Error> {
     bail!("systemd returned error");
 }
 
-/// rollback to previous ostree deployment if boot counter is les than 0
-pub fn handle_rollback() -> Result<(), Error> {
-    match get_boot_counter() {
-        Some(t) if t <= 0 => {
-            log::info!("Greenboot will now attempt rollback");
+/// rollback to previous ostree deployment if boot counter is less than 0
+pub fn handle_rollback() -> Result<()> {
+    if let Some(t) = get_boot_counter() {
+        if t <= 0 {
+            log::info!("Greenboot will now attempt to rollback");
             let status = Command::new("rpm-ostree").arg("rollback").status()?;
             if status.success() {
                 return Ok(());
             }
             bail!(status.to_string());
         }
-        _ => log::info!("Rollback not initiated as boot_counter is either unset or not equal to 0"),
     }
+    log::info!("Rollback not initiated as boot_counter is either unset or not equal to 0");
     Ok(())
 }
 
 /// sets grub variable boot_counter if not set
 pub fn set_boot_counter(reboot_count: i32) -> Result<()> {
-    match get_boot_counter() {
-        Some(counter) => {
-            log::info!("boot_counter={counter}");
-            Ok(())
-        }
-        None => {
-            if set_grub_var("boot_counter", reboot_count) {
-                log::info!("boot_counter={reboot_count}");
-                return Ok(());
-            }
-            bail!("grub returned error");
-        }
+    if let Some(current_counter) = get_boot_counter() {
+        log::info!("boot_counter={current_counter}");
+        return Ok(());
+    } else if set_grub_var("boot_counter", reboot_count) {
+        log::info!("Set boot_counter={reboot_count}");
+        return Ok(());
     }
+    bail!("Failed to set GRUB variable: boot_counter");
 }
 
 /// resets grub variable boot_counter
@@ -66,38 +62,34 @@ pub fn unset_boot_counter() -> Result<()> {
     if status.success() {
         return Ok(());
     }
-    bail!("grub returned error")
+    bail!("grub returned an error")
 }
 
 /// sets grub variable boot_success
 pub fn handle_boot_success(success: bool) -> Result<()> {
     if success {
         if !set_grub_var("boot_success", 1) {
-            bail!("unable to mark boot as success, grub returned error")
+            bail!("unable to mark boot as success, grub returned an error")
         }
         match unset_boot_counter() {
             Ok(_) => return Ok(()),
             Err(e) => bail!("unable to remove boot_counter, {e}"),
         }
     } else if !set_grub_var("boot_success", 0) {
-        bail!("unable to mark boot as failure, grub returned error")
+        bail!("unable to mark boot as failure, grub returned an error")
     }
     Ok(())
 }
 
 /// writes greenboot status to motd.d/boot-status
-pub fn handle_motd(state: &str) -> Result<(), Error> {
+pub fn handle_motd(state: &str) -> Result<()> {
     let motd = format!("Greenboot {state}.");
 
-    let mut motd_file = OpenOptions::new()
-        .create(true)
-        .write(true)
-        .open("/etc/motd.d/boot-status")?;
-    motd_file.write_all(motd.as_bytes())?;
+    std::fs::write("/etc/motd.d/boot-status", motd.as_bytes())?;
     Ok(())
 }
 
-/// fetches boot_counter value, none is not set  
+/// fetches boot_counter value, none if not set
 pub fn get_boot_counter() -> Option<i32> {
     let grub_vars = Command::new("grub2-editenv").arg("-").arg("list").output();
     if grub_vars.is_err() {

src/main.rs

@@ -9,7 +9,7 @@ use std::path::Path;
 use std::process::Command;
 use std::str;
 
-/// dir that greenboot looks for the health-check and other scripts
+/// dir that greenboot looks for the health check and other scripts
 static GREENBOOT_INSTALL_PATHS: [&str; 2] = ["/usr/lib/greenboot", "/etc/greenboot"];
 
 /// greenboot config path
@@ -37,7 +37,7 @@ impl GreenbootConfig {
     fn set_default() -> Self {
         Self { max_reboot: 3 }
     }
-    /// gets the config form the config file
+    /// gets the config from the config file
     fn get_config() -> Self {
         let mut config = Self::set_default();
         let parsed = Config::builder()
@@ -97,7 +97,7 @@ enum Commands {
     Rollback,
 }
 
-/// this run the diagnostics in require.d and check.d
+/// this runs the scripts in required.d and wanted.d
 fn run_diagnostics() -> Result<(), Error> {
     let mut script_failure: bool = false;
     let mut path_exists: bool = false;
@@ -124,12 +124,11 @@ fn run_diagnostics() -> Result<(), Error> {
     }
 
     for path in GREENBOOT_INSTALL_PATHS {
-        let gereenboot_wanted_path = format!("{path}/check/wanted.d/*.sh");
-        for entry in glob(&gereenboot_wanted_path)?.flatten() {
+        let greenboot_wanted_path = format!("{path}/check/wanted.d/*.sh");
+        for entry in glob(&greenboot_wanted_path)?.flatten() {
             log::info!("running wanted check {}", entry.to_string_lossy());
             let output = Command::new("bash").arg("-C").arg(entry.clone()).output()?;
             if !output.status.success() {
-                // combine and print stderr/stdout
                 log::warn!("wanted script {} failed!", entry.to_string_lossy());
                 log::warn!("reason: {}", String::from_utf8_lossy(&output.stderr));
             }
@@ -150,7 +149,6 @@ fn run_red() -> Result<(), Error> {
             log::info!("running red check {}", entry.to_string_lossy());
             let output = Command::new("bash").arg("-C").arg(entry.clone()).output()?;
             if !output.status.success() {
-                // combine and print stderr/stdout
                 log::warn!("red script: {} failed!", entry.to_string_lossy());
                 log::warn!("reason: {}", String::from_utf8_lossy(&output.stderr));
             }
@@ -167,7 +165,6 @@ fn run_green() -> Result<(), Error> {
             log::info!("running green check {}", entry.to_string_lossy());
             let output = Command::new("bash").arg("-C").arg(entry.clone()).output()?;
             if !output.status.success() {
-                // combine and print stderr/stdout
                 log::warn!("green script {} failed!", entry.to_string_lossy());
                 log::warn!("reason: {}", String::from_utf8_lossy(&output.stderr));
             }
@@ -181,7 +178,7 @@ fn run_green() -> Result<(), Error> {
 fn health_check() -> Result<()> {
     let config = GreenbootConfig::get_config();
     log::info!("{config:?}");
-    handle_motd("healthcheck is in progress").ok();
+    handle_motd("healthcheck is in progress")?;
     let run_status = run_diagnostics();
     match run_status {
         Ok(()) => {
@@ -205,7 +202,7 @@ fn health_check() -> Result<()> {
                 .unwrap_or_else(|e| log::error!("cannot set boot_counter as: {}", e.to_string()));
             handle_reboot(false)
                 .unwrap_or_else(|e| log::error!("cannot reboot as: {}", e.to_string()));
-            bail!(e);
+            Err(e)
         }
     }
 }
@@ -216,8 +213,7 @@ fn trigger_rollback() -> Result<()> {
         Ok(()) => {
             log::info!("Rollback successful");
             unset_boot_counter()?;
-            handle_reboot(true)?;
-            Ok(())
+            handle_reboot(true)
         }
         Err(e) => {
             bail!("Rollback not initiated as {}", e);
@@ -233,7 +229,7 @@ fn main() -> Result<()> {
 
     match cli.command {
         Commands::HealthCheck => health_check(),
-        Commands::Rollback => trigger_rollback(), // will tackle the functionality later
+        Commands::Rollback => trigger_rollback(),
     }
 }
 
@@ -245,7 +241,7 @@ mod tests {
 
     use super::*;
 
-    //validate when required folder is not found
+    ///validate when the required folder is not found
     #[test]
     fn missing_required_folder() {
         assert_eq!(