[pbs-devel] [PATCH proxmox-backup v2 1/3] rest-server: cleanup_old_tasks: improve error handling

[Dominik Csapak <d.csapak@proxmox.com>]

by not bubbling up most errors, and continuing on. this avoids that we
stop cleaning up because e.g. one directory was missing.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* ignore only ENOENT error for task log archive dirs,
  log all other errors
* use task_log instead of log::errors, this way the task will end on a
  'WARNINGS' and the user sees that something was wrong with it
  this also has the advantage that the errors really go into the task
  log instead of the syslog
* improve error messages

 proxmox-rest-server/src/worker_task.rs | 38 ++++++++++++++++++++------
 src/bin/proxmox-backup-proxy.rs        |  2 +-
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/proxmox-rest-server/src/worker_task.rs b/proxmox-rest-server/src/worker_task.rs
index 643e1872..cd1a4457 100644
--- a/proxmox-rest-server/src/worker_task.rs
+++ b/proxmox-rest-server/src/worker_task.rs
@@ -16,6 +16,7 @@ use tokio::sync::oneshot;
 use nix::fcntl::OFlag;
 use once_cell::sync::OnceCell;
 
+use proxmox_sys::task_warn;
 use proxmox_sys::linux::procfs;
 use proxmox_sys::fs::{create_path, replace_file, atomic_open_or_create_file, CreateOptions};
 use proxmox_lang::try_block;
@@ -232,7 +233,7 @@ pub fn rotate_task_log_archive(
 
 /// removes all task logs that are older than the oldest task entry in the
 /// task archive
-pub fn cleanup_old_tasks(compressed: bool) -> Result<(), Error> {
+pub fn cleanup_old_tasks(worker: &dyn WorkerTaskContext, compressed: bool) -> Result<(), Error> {
     let setup = worker_task_setup()?;
 
     let _lock = setup.lock_task_list_files(true)?;
@@ -270,18 +271,37 @@ pub fn cleanup_old_tasks(compressed: bool) -> Result<(), Error> {
         for i in 0..256 {
             let mut path = setup.taskdir.clone();
             path.push(format!("{:02X}", i));
-            for file in std::fs::read_dir(path)? {
-                let file = file?;
+            let files = match std::fs::read_dir(path) {
+                Ok(files) => files,
+                Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue,
+                Err(err) => {
+                    task_warn!(worker, "could not check task logs in '{:02X}': {}", i, err);
+                    continue;
+                }
+            };
+            for file in files {
+                let file = match file {
+                    Ok(file) => file,
+                    Err(err) => {
+                        task_warn!(worker, "could not check some task log in '{:02X}': {}", i, err);
+                        continue;
+                    }
+                };
                 let path = file.path();
 
-                let modified = get_modified(file)
-                    .map_err(|err| format_err!("error getting mtime for {:?}: {}", path, err))?;
+                let modified = match get_modified(file) {
+                    Ok(modified) => modified,
+                    Err(err) => {
+                        task_warn!(worker, "error getting mtime for '{:?}': {}", path, err);
+                        continue;
+                    }
+                };
 
                 if modified < cutoff_time {
-                    match std::fs::remove_file(path) {
-                        Ok(()) => {},
-                        Err(err) if err.kind() == std::io::ErrorKind::NotFound => {},
-                        Err(err) => bail!("could not remove file: {}", err),
+                    match std::fs::remove_file(&path) {
+                        Ok(()) => {}
+                        Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
+                        Err(err) => task_warn!(worker, "could not remove file '{:?}': {}", path, err)
                     }
                 }
             }
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index a4f20af1..0d2271a8 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -900,7 +900,7 @@ async fn schedule_task_log_rotate() {
 
                 if has_rotated {
                     task_log!(worker, "cleaning up old task logs");
-                    if let Err(err) = cleanup_old_tasks(true) {
+                    if let Err(err) = cleanup_old_tasks(&worker, true) {
                         task_warn!(worker, "could not completely cleanup old tasks: {}", err);
                     }
                 }
-- 
2.30.2