[pbs-devel] [RFC v2 proxmox-backup 15/21] client: expose skip trash flags for cli commands

[Christian Ebner <c.ebner@proxmox.com>]

Allows to explicitly set/clear the `skip-trash` flag when pruning
namespaces, groups or snapshots via the client cli command.

Set defaults for `skip-trash` to false in order to use the trash.

Further, never add the flag to the api call parameters in the client
if not explicitly set, in order to keep backwards compatibility to
older PBS instances.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/datastore.rs         |  6 ++++--
 proxmox-backup-client/src/group.rs     | 14 +++++++++++++-
 proxmox-backup-client/src/namespace.rs | 14 +++++++++++++-
 proxmox-backup-client/src/snapshot.rs  | 16 ++++++++++++----
 src/api2/admin/datastore.rs            | 11 +++++++++--
 src/api2/admin/namespace.rs            | 12 ++++++++++--
 src/api2/backup/environment.rs         |  1 +
 src/server/prune_job.rs                |  4 +++-
 src/server/pull.rs                     | 12 +++++++-----
 9 files changed, 72 insertions(+), 18 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index d88af4c68..0f86754bb 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -808,10 +808,11 @@ impl DataStore {
         self: &Arc<Self>,
         ns: &BackupNamespace,
         backup_group: &pbs_api_types::BackupGroup,
+        skip_trash: bool,
     ) -> Result<BackupGroupDeleteStats, Error> {
         let backup_group = self.backup_group(ns.clone(), backup_group.clone());
 
-        backup_group.destroy(true)
+        backup_group.destroy(skip_trash)
     }
 
     /// Remove a backup directory including all content
@@ -820,10 +821,11 @@ impl DataStore {
         ns: &BackupNamespace,
         backup_dir: &pbs_api_types::BackupDir,
         force: bool,
+        skip_trash: bool,
     ) -> Result<(), Error> {
         let backup_dir = self.backup_dir(ns.clone(), backup_dir.clone())?;
 
-        backup_dir.destroy(force, true)
+        backup_dir.destroy(force, skip_trash)
     }
 
     /// Returns the time of the last successful backup
diff --git a/proxmox-backup-client/src/group.rs b/proxmox-backup-client/src/group.rs
index 67f26e261..42f8e1e61 100644
--- a/proxmox-backup-client/src/group.rs
+++ b/proxmox-backup-client/src/group.rs
@@ -37,11 +37,20 @@ pub fn group_mgmt_cli() -> CliCommandMap {
                 type: BackupNamespace,
                 optional: true,
             },
+            "skip-trash": {
+                type: bool,
+                optional: true,
+                description: "Immediately remove the group, not marking contents as trash.",
+            },
         }
     }
 )]
 /// Forget (remove) backup snapshots.
-async fn forget_group(group: String, mut param: Value) -> Result<(), Error> {
+async fn forget_group(
+    group: String,
+    skip_trash: Option<bool>,
+    mut param: Value,
+) -> Result<(), Error> {
     let backup_group: BackupGroup = group.parse()?;
     let repo = remove_repository_from_value(&mut param)?;
     let client = connect(&repo)?;
@@ -63,6 +72,9 @@ async fn forget_group(group: String, mut param: Value) -> Result<(), Error> {
     )?;
     if confirmation.is_yes() {
         let path = format!("api2/json/admin/datastore/{}/groups", repo.store());
+        if let Some(skip_trash) = skip_trash {
+            api_param["skip-trash"] = Value::from(skip_trash);
+        }
         if let Err(err) = client.delete(&path, Some(api_param)).await {
             // "ENOENT: No such file or directory" is part of the error returned when the group
             // has not been found. The full error contains the full datastore path and we would
diff --git a/proxmox-backup-client/src/namespace.rs b/proxmox-backup-client/src/namespace.rs
index 2929e394b..204a11b1d 100644
--- a/proxmox-backup-client/src/namespace.rs
+++ b/proxmox-backup-client/src/namespace.rs
@@ -136,11 +136,20 @@ async fn create_namespace(param: Value) -> Result<(), Error> {
                 description: "Destroys all groups in the hierarchy.",
                 optional: true,
             },
+            "skip-trash": {
+                type: bool,
+                optional: true,
+                description: "Immediately remove the namespace, not marking contents as trash.",
+            },
         }
     },
 )]
 /// Delete an existing namespace.
-async fn delete_namespace(param: Value, delete_groups: Option<bool>) -> Result<(), Error> {
+async fn delete_namespace(
+    param: Value,
+    delete_groups: Option<bool>,
+    skip_trash: Option<bool>,
+) -> Result<(), Error> {
     let repo = extract_repository_from_value(&param)?;
     let backup_ns = optional_ns_param(&param)?;
 
@@ -150,6 +159,9 @@ async fn delete_namespace(param: Value, delete_groups: Option<bool>) -> Result<(
 
     let path = format!("api2/json/admin/datastore/{}/namespace", repo.store());
     let mut param = json!({ "ns": backup_ns });
+    if let Some(skip_trash) = skip_trash {
+        param["skip-trash"] = Value::from(skip_trash);
+    }
 
     if let Some(value) = delete_groups {
         param["delete-groups"] = serde_json::to_value(value)?;
diff --git a/proxmox-backup-client/src/snapshot.rs b/proxmox-backup-client/src/snapshot.rs
index f195c23b7..a9b46726a 100644
--- a/proxmox-backup-client/src/snapshot.rs
+++ b/proxmox-backup-client/src/snapshot.rs
@@ -173,11 +173,16 @@ async fn list_snapshot_files(param: Value) -> Result<Value, Error> {
                 type: String,
                 description: "Snapshot path.",
             },
+            "skip-trash": {
+                type: bool,
+                optional: true,
+                description: "Immediately remove the snapshot, not marking it as trash.",
+            },
         }
     }
 )]
 /// Forget (remove) backup snapshots.
-async fn forget_snapshots(param: Value) -> Result<(), Error> {
+async fn forget_snapshots(skip_trash: Option<bool>, param: Value) -> Result<(), Error> {
     let repo = extract_repository_from_value(&param)?;
 
     let backup_ns = optional_ns_param(&param)?;
@@ -188,9 +193,12 @@ async fn forget_snapshots(param: Value) -> Result<(), Error> {
 
     let path = format!("api2/json/admin/datastore/{}/snapshots", repo.store());
 
-    client
-        .delete(&path, Some(snapshot_args(&backup_ns, &snapshot)?))
-        .await?;
+    let mut args = snapshot_args(&backup_ns, &snapshot)?;
+    if let Some(skip_trash) = skip_trash {
+        args["skip-trash"] = Value::from(skip_trash);
+    }
+
+    client.delete(&path, Some(args)).await?;
 
     record_repository(&repo);
 
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 133a6d658..84c0bf5b4 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -277,7 +277,13 @@ pub fn list_groups(
                 optional: true,
                 default: true,
                 description: "Return error when group cannot be deleted because of protected snapshots",
-            }
+            },
+            "skip-trash": {
+                type: bool,
+                optional: true,
+                default: false,
+                description: "Immediately remove the snapshot, not marking it as trash.",
+            },
         },
     },
     returns: {
@@ -295,6 +301,7 @@ pub async fn delete_group(
     ns: Option<BackupNamespace>,
     error_on_protected: bool,
     group: pbs_api_types::BackupGroup,
+    skip_trash: bool,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<BackupGroupDeleteStats, Error> {
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -312,7 +319,7 @@ pub async fn delete_group(
             &group,
         )?;
 
-        let delete_stats = datastore.remove_backup_group(&ns, &group)?;
+        let delete_stats = datastore.remove_backup_group(&ns, &group, skip_trash)?;
 
         let error_msg = if datastore.old_locking() {
             "could not remove empty groups directories due to old locking mechanism.\n\
diff --git a/src/api2/admin/namespace.rs b/src/api2/admin/namespace.rs
index a12d97883..6f70497a6 100644
--- a/src/api2/admin/namespace.rs
+++ b/src/api2/admin/namespace.rs
@@ -144,7 +144,13 @@ pub fn list_namespaces(
                 optional: true,
                 default: true,
                 description: "Return error when namespace cannot be deleted because of protected snapshots",
-            }
+            },
+            "skip-trash": {
+                type: bool,
+                optional: true,
+                default: true,
+                description: "Remove and namespace immediately, skip moving to trash",
+            },
         },
     },
     access: {
@@ -157,6 +163,7 @@ pub fn delete_namespace(
     ns: BackupNamespace,
     delete_groups: bool,
     error_on_protected: bool,
+    skip_trash: bool,
     _info: &ApiMethod,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<BackupGroupDeleteStats, Error> {
@@ -166,7 +173,8 @@ pub fn delete_namespace(
 
     let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;
 
-    let (removed_all, stats) = datastore.remove_namespace_recursive(&ns, delete_groups, true)?;
+    let (removed_all, stats) =
+        datastore.remove_namespace_recursive(&ns, delete_groups, skip_trash)?;
     if !removed_all {
         let err_msg = if delete_groups {
             if datastore.old_locking() {
diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
index 3d541b461..b5619eb8c 100644
--- a/src/api2/backup/environment.rs
+++ b/src/api2/backup/environment.rs
@@ -730,6 +730,7 @@ impl BackupEnvironment {
             self.backup_dir.backup_ns(),
             self.backup_dir.as_ref(),
             true,
+            true,
         )?;
 
         Ok(())
diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
index 596afe086..2794dc3ab 100644
--- a/src/server/prune_job.rs
+++ b/src/server/prune_job.rs
@@ -76,7 +76,9 @@ pub fn prune_datastore(
                 info.backup_dir.backup_time_string()
             );
             if !keep && !dry_run {
-                if let Err(err) = datastore.remove_backup_dir(ns, info.backup_dir.as_ref(), false) {
+                if let Err(err) =
+                    datastore.remove_backup_dir(ns, info.backup_dir.as_ref(), false, true)
+                {
                     let path = info.backup_dir.relative_path();
                     warn!("failed to remove dir {path:?}: {err}");
                 }
diff --git a/src/server/pull.rs b/src/server/pull.rs
index a60ccbf10..0d5845e85 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -504,6 +504,7 @@ async fn pull_snapshot_from<'a>(
                     snapshot.backup_ns(),
                     snapshot.as_ref(),
                     true,
+                    true,
                 ) {
                     info!("cleanup error - {cleanup_err}");
                 }
@@ -678,7 +679,7 @@ async fn pull_group(
             params
                 .target
                 .store
-                .remove_backup_dir(&target_ns, snapshot.as_ref(), false)?;
+                .remove_backup_dir(&target_ns, snapshot.as_ref(), false, true)?;
             sync_stats.add(SyncStats::from(RemovedVanishedStats {
                 snapshots: 1,
                 groups: 0,
@@ -997,10 +998,11 @@ pub(crate) async fn pull_ns(
                     continue;
                 }
                 info!("delete vanished group '{local_group}'");
-                let delete_stats_result = params
-                    .target
-                    .store
-                    .remove_backup_group(&target_ns, local_group);
+                let delete_stats_result =
+                    params
+                        .target
+                        .store
+                        .remove_backup_group(&target_ns, local_group, false);
 
                 match delete_stats_result {
                     Ok(stats) => {
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel