From a9d6ff63344023e7ecc7d90904f694714514e19e Mon Sep 17 00:00:00 2001
From: Yuhang Wei <weiyuhang3@huawei.com>
Date: Mon, 29 Jan 2024 15:28:29 +0800
Subject: [PATCH 1/4] fix(os-agent):add context when returning error

In some cases, the error log only contains "No such file" information. It is hard to identify the origin of the error. Provide more context when propagating error.

Signed-off-by: Yuhang Wei <weiyuhang3@huawei.com>
---
 KubeOS-Rust/manager/src/sys_mgmt/config.rs     | 18 ++++++++++++------
 .../manager/src/sys_mgmt/containerd_image.rs   |  6 +++---
 .../manager/src/sys_mgmt/docker_image.rs       |  6 +++---
 KubeOS-Rust/manager/src/utils/common.rs        |  8 ++++----
 4 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/KubeOS-Rust/manager/src/sys_mgmt/config.rs b/KubeOS-Rust/manager/src/sys_mgmt/config.rs
index a629756..33efdca 100644
--- a/KubeOS-Rust/manager/src/sys_mgmt/config.rs
+++ b/KubeOS-Rust/manager/src/sys_mgmt/config.rs
@@ -105,9 +105,10 @@ impl Configuration for KernelSysctlPersist {
             config_path = &config.config_path;
         }
         debug!("kernel.sysctl.persist config_path: \"{}\"", config_path);
-        create_config_file(config_path)?;
-        let configs = get_and_set_configs(&mut config.contents, config_path)?;
-        write_configs_to_file(config_path, &configs)?;
+        create_config_file(config_path).with_context(|| format!("Failed to find config path"))?;
+        let configs = get_and_set_configs(&mut config.contents, config_path)
+            .with_context(|| format!("Failed to set persist kernel configs"))?;
+        write_configs_to_file(config_path, &configs).with_context(|| format!("Failed to write configs to file"))?;
         Ok(())
     }
 }
@@ -254,10 +255,15 @@ impl Configuration for GrubCmdline {
         if !is_file_exist(&self.grub_path) {
             bail!("Failed to find grub.cfg file");
         }
-        let config_partition =
-            if cfg!(test) { self.is_cur_partition } else { self.get_config_partition(RealCommandExecutor {})? };
+        let config_partition = if cfg!(test) {
+            self.is_cur_partition
+        } else {
+            self.get_config_partition(RealCommandExecutor {})
+                .with_context(|| format!("Failed to get config partition"))?
+        };
         debug!("Config_partition: {} (false means partition A, true means partition B)", config_partition);
-        let configs = get_and_set_grubcfg(&mut config.contents, &self.grub_path, config_partition)?;
+        let configs = get_and_set_grubcfg(&mut config.contents, &self.grub_path, config_partition)
+            .with_context(|| format!("Failed to set grub configs"))?;
         write_configs_to_file(&self.grub_path, &configs)?;
         Ok(())
     }
diff --git a/KubeOS-Rust/manager/src/sys_mgmt/containerd_image.rs b/KubeOS-Rust/manager/src/sys_mgmt/containerd_image.rs
index 5b0d0b7..727949b 100644
--- a/KubeOS-Rust/manager/src/sys_mgmt/containerd_image.rs
+++ b/KubeOS-Rust/manager/src/sys_mgmt/containerd_image.rs
@@ -12,7 +12,7 @@
 
 use std::{fs, os::unix::fs::PermissionsExt, path::Path};
 
-use anyhow::{anyhow, Result};
+use anyhow::{anyhow, Context, Result};
 use log::{debug, info};
 
 use crate::{
@@ -73,12 +73,12 @@ impl<T: CommandExecutor> CtrImageHandler<T> {
             .to_str()
             .ok_or_else(|| anyhow!("Failed to get mount path: {}", self.paths.mount_path.display()))?;
         info!("Start getting rootfs {}", image_name);
-        self.check_and_unmount(mount_path)?;
+        self.check_and_unmount(mount_path).with_context(|| format!("Failed to clean containerd environment"))?;
         self.executor
             .run_command("ctr", &["-n", DEFAULT_NAMESPACE, "images", "mount", "--rw", image_name, mount_path])?;
         // copy os.tar from mount_path to its partent dir
         self.copy_file(self.paths.mount_path.join(&self.paths.rootfs_file), &self.paths.tar_path, permission)?;
-        self.check_and_unmount(mount_path)?;
+        self.check_and_unmount(mount_path).with_context(|| format!("Failed to clean containerd environment"))?;
         Ok(())
     }
 
diff --git a/KubeOS-Rust/manager/src/sys_mgmt/docker_image.rs b/KubeOS-Rust/manager/src/sys_mgmt/docker_image.rs
index a8bbee2..f697142 100644
--- a/KubeOS-Rust/manager/src/sys_mgmt/docker_image.rs
+++ b/KubeOS-Rust/manager/src/sys_mgmt/docker_image.rs
@@ -1,4 +1,4 @@
-use anyhow::Result;
+use anyhow::{Context, Result};
 use log::{debug, info, trace};
 
 use crate::{
@@ -52,7 +52,7 @@ impl<T: CommandExecutor> DockerImageHandler<T> {
     fn get_rootfs_archive(&self, req: &UpgradeRequest) -> Result<()> {
         let image_name = &req.container_image;
         info!("Start getting rootfs {}", image_name);
-        self.check_and_rm_container()?;
+        self.check_and_rm_container().with_context(|| format!("Failed to remove kubeos-temp container"))?;
         debug!("Create container {}", self.container_name);
         let container_id =
             self.executor.run_command_with_output("docker", &["create", "--name", &self.container_name, image_name])?;
@@ -65,7 +65,7 @@ impl<T: CommandExecutor> DockerImageHandler<T> {
                 self.paths.update_path.to_str().unwrap(),
             ],
         )?;
-        self.check_and_rm_container()?;
+        self.check_and_rm_container().with_context(|| format!("Failed to remove kubeos-temp container"))?;
         Ok(())
     }
 
diff --git a/KubeOS-Rust/manager/src/utils/common.rs b/KubeOS-Rust/manager/src/utils/common.rs
index a6d62a0..9baf99e 100644
--- a/KubeOS-Rust/manager/src/utils/common.rs
+++ b/KubeOS-Rust/manager/src/utils/common.rs
@@ -16,7 +16,7 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use anyhow::{anyhow, bail, Result};
+use anyhow::{anyhow, bail, Context, Result};
 use log::{debug, info, trace};
 use nix::{mount, mount::MntFlags};
 
@@ -85,7 +85,7 @@ pub fn check_disk_size<P: AsRef<Path>>(need_bytes: i64, path: P) -> Result<()> {
 /// clean_env will umount the mount path and delete directory /persist/KubeOS-Update and /persist/update.img
 pub fn clean_env<P>(update_path: P, mount_path: P, image_path: P) -> Result<()>
 where
-    P: AsRef<Path>,
+    P: AsRef<Path> + std::fmt::Debug,
 {
     if is_mounted(&mount_path)? {
         debug!("Umount \"{}\"", mount_path.as_ref().display());
@@ -94,8 +94,8 @@ where
         }
     }
     // losetup -D?
-    delete_file_or_dir(update_path)?;
-    delete_file_or_dir(image_path)?;
+    delete_file_or_dir(&update_path).with_context(|| format!("Failed to delete {:?}", update_path))?;
+    delete_file_or_dir(&image_path).with_context(|| format!("Failed to delete {:?}", image_path))?;
     Ok(())
 }
 
-- 
2.34.1