From 91a2db416854f218fdfbe1582063f2a7a2636eb1 Mon Sep 17 00:00:00 2001 From: Sayan Paul Date: Thu, 14 Sep 2023 11:13:07 +0000 Subject: [PATCH] refactor: some code refactoring complying some rust best practices updated devcontainer to built rpms Signed-off-by: Sayan Paul --- .devcontainer/Dockerfile | 1 + greenboot.spec | 2 +- src/handler/mod.rs | 64 ++++++++++++++++++---------------------- src/main.rs | 24 +++++++-------- 4 files changed, 40 insertions(+), 51 deletions(-) diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index bf4302f..d0d79a1 100644 --- a/.devcontainer/Dockerfile +++ b/.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 diff --git a/greenboot.spec b/greenboot.spec index ba87d83..234d6a9 100644 --- a/greenboot.spec +++ b/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 diff --git a/src/handler/mod.rs b/src/handler/mod.rs index 08fdea2..c6e5412 100644 --- a/src/handler/mod.rs +++ b/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 { let grub_vars = Command::new("grub2-editenv").arg("-").arg("list").output(); if grub_vars.is_err() { diff --git a/src/main.rs b/src/main.rs index 19d948a..979634d 100644 --- a/src/main.rs +++ b/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!(