[pbs-devel] [PATCH proxmox-backup v7 20/38] datastore: prune groups/snapshots from s3 object store backend
Christian Ebner
c.ebner at proxmox.com
Thu Jul 10 19:07:10 CEST 2025
When pruning a backup group or a backup snapshot for a datastore with
S3 object store backend, remove the associated objects by removing
them based on the prefix.
In order to exclude protected contents, add a filtering based on the
presence of the protected marker.
Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
---
changes since version 6:
- no changes
pbs-datastore/src/backup_info.rs | 49 ++++++++++++++++++++++++++++++--
pbs-datastore/src/datastore.rs | 43 ++++++++++++++++++++++++----
src/api2/admin/datastore.rs | 24 ++++++++++------
3 files changed, 100 insertions(+), 16 deletions(-)
diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index 112d82658..2e811f32a 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -9,6 +9,7 @@ use std::time::Duration;
use anyhow::{bail, format_err, Context, Error};
use const_format::concatcp;
+use proxmox_s3_client::S3PathPrefix;
use proxmox_sys::fs::{lock_dir_noblock, lock_dir_noblock_shared, replace_file, CreateOptions};
use proxmox_systemd::escape_unit;
@@ -18,7 +19,9 @@ use pbs_api_types::{
};
use pbs_config::{open_backup_lockfile, BackupLockGuard};
+use crate::datastore::GROUP_NOTES_FILE_NAME;
use crate::manifest::{BackupManifest, MANIFEST_LOCK_NAME};
+use crate::s3::S3_CONTENT_PREFIX;
use crate::{DataBlob, DataStore, DatastoreBackend};
pub const DATASTORE_LOCKS_DIR: &str = "/run/proxmox-backup/locks";
@@ -218,7 +221,7 @@ impl BackupGroup {
///
/// Returns `BackupGroupDeleteStats`, containing the number of deleted snapshots
/// and number of protected snaphsots, which therefore were not removed.
- pub fn destroy(&self) -> Result<BackupGroupDeleteStats, Error> {
+ pub fn destroy(&self, backend: &DatastoreBackend) -> Result<BackupGroupDeleteStats, Error> {
let _guard = self
.lock()
.with_context(|| format!("while destroying group '{self:?}'"))?;
@@ -232,10 +235,30 @@ impl BackupGroup {
delete_stats.increment_protected_snapshots();
continue;
}
- snap.destroy(false)?;
+ // also for S3 cleanup local only, the actual S3 objects will be removed below,
+ // reducing the number of required API calls.
+ snap.destroy(false, &DatastoreBackend::Filesystem)?;
delete_stats.increment_removed_snapshots();
}
+ if let DatastoreBackend::S3(s3_client) = backend {
+ let path = self.relative_group_path();
+ let group_prefix = path
+ .to_str()
+ .ok_or_else(|| format_err!("invalid group path prefix"))?;
+ let prefix = format!("{S3_CONTENT_PREFIX}/{group_prefix}");
+ let delete_objects_error = proxmox_async::runtime::block_on(
+ s3_client.delete_objects_by_prefix_with_suffix_filter(
+ &S3PathPrefix::Some(prefix),
+ PROTECTED_MARKER_FILENAME,
+ &["owner", GROUP_NOTES_FILE_NAME],
+ ),
+ )?;
+ if delete_objects_error {
+ bail!("deleting objects failed");
+ }
+ }
+
// Note: make sure the old locking mechanism isn't used as `remove_dir_all` is not safe in
// that case
if delete_stats.all_removed() && !*OLD_LOCKING {
@@ -588,7 +611,7 @@ impl BackupDir {
/// Destroy the whole snapshot, bails if it's protected
///
/// Setting `force` to true skips locking and thus ignores if the backup is currently in use.
- pub fn destroy(&self, force: bool) -> Result<(), Error> {
+ pub fn destroy(&self, force: bool, backend: &DatastoreBackend) -> Result<(), Error> {
let (_guard, _manifest_guard);
if !force {
_guard = self
@@ -601,6 +624,20 @@ impl BackupDir {
bail!("cannot remove protected snapshot"); // use special error type?
}
+ if let DatastoreBackend::S3(s3_client) = backend {
+ let path = self.relative_path();
+ let snapshot_prefix = path
+ .to_str()
+ .ok_or_else(|| format_err!("invalid snapshot path"))?;
+ let prefix = format!("{S3_CONTENT_PREFIX}/{snapshot_prefix}");
+ let delete_objects_error = proxmox_async::runtime::block_on(
+ s3_client.delete_objects_by_prefix(&S3PathPrefix::Some(prefix)),
+ )?;
+ if delete_objects_error {
+ bail!("deleting objects failed");
+ }
+ }
+
let full_path = self.full_path();
log::info!("removing backup snapshot {:?}", full_path);
std::fs::remove_dir_all(&full_path).map_err(|err| {
@@ -630,6 +667,12 @@ impl BackupDir {
// do to rectify the situation.
if guard.is_ok() && group.list_backups()?.is_empty() && !*OLD_LOCKING {
group.remove_group_dir()?;
+ if let DatastoreBackend::S3(s3_client) = backend {
+ let object_key =
+ super::s3::object_key_from_path(&group.relative_group_path(), "owner")
+ .context("invalid owner file object key")?;
+ proxmox_async::runtime::block_on(s3_client.delete_object(object_key))?;
+ }
} else if let Err(err) = guard {
log::debug!("{err:#}");
}
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 099c65ce2..265624229 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -12,7 +12,9 @@ use pbs_tools::lru_cache::LruCache;
use tracing::{info, warn};
use proxmox_human_byte::HumanByte;
-use proxmox_s3_client::{S3Client, S3ClientConfig, S3ClientOptions, S3ClientSecretsConfig};
+use proxmox_s3_client::{
+ S3Client, S3ClientConfig, S3ClientOptions, S3ClientSecretsConfig, S3PathPrefix,
+};
use proxmox_schema::ApiType;
use proxmox_sys::error::SysError;
@@ -37,13 +39,14 @@ use crate::dynamic_index::{DynamicIndexReader, DynamicIndexWriter};
use crate::fixed_index::{FixedIndexReader, FixedIndexWriter};
use crate::hierarchy::{ListGroups, ListGroupsType, ListNamespaces, ListNamespacesRecursive};
use crate::index::IndexFile;
+use crate::s3::S3_CONTENT_PREFIX;
use crate::task_tracking::{self, update_active_operations};
use crate::DataBlob;
static DATASTORE_MAP: LazyLock<Mutex<HashMap<String, Arc<DataStoreImpl>>>> =
LazyLock::new(|| Mutex::new(HashMap::new()));
-const GROUP_NOTES_FILE_NAME: &str = "notes";
+pub const GROUP_NOTES_FILE_NAME: &str = "notes";
const NAMESPACE_MARKER_FILENAME: &str = ".namespace";
/// checks if auth_id is owner, or, if owner is a token, if
@@ -654,7 +657,9 @@ impl DataStore {
let mut stats = BackupGroupDeleteStats::default();
for group in self.iter_backup_groups(ns.to_owned())? {
- let delete_stats = group?.destroy()?;
+ let group = group?;
+ let backend = self.backend()?;
+ let delete_stats = group.destroy(&backend)?;
stats.add(&delete_stats);
removed_all_groups = removed_all_groups && delete_stats.all_removed();
}
@@ -688,6 +693,8 @@ impl DataStore {
let store = self.name();
let mut removed_all_requested = true;
let mut stats = BackupGroupDeleteStats::default();
+ let backend = self.backend()?;
+
if delete_groups {
log::info!("removing whole namespace recursively below {store}:/{ns}",);
for ns in self.recursive_iter_backup_ns(ns.to_owned())? {
@@ -695,6 +702,24 @@ impl DataStore {
stats.add(&delete_stats);
removed_all_requested = removed_all_requested && removed_ns_groups;
}
+
+ if let DatastoreBackend::S3(s3_client) = &backend {
+ let ns_dir = ns.path();
+ let ns_prefix = ns_dir
+ .to_str()
+ .ok_or_else(|| format_err!("invalid namespace path prefix"))?;
+ let prefix = format!("{S3_CONTENT_PREFIX}/{ns_prefix}");
+ let delete_objects_error = proxmox_async::runtime::block_on(
+ s3_client.delete_objects_by_prefix_with_suffix_filter(
+ &S3PathPrefix::Some(prefix),
+ PROTECTED_MARKER_FILENAME,
+ &["owner", GROUP_NOTES_FILE_NAME],
+ ),
+ )?;
+ if delete_objects_error {
+ bail!("deleting objects failed");
+ }
+ }
} else {
log::info!("pruning empty namespace recursively below {store}:/{ns}");
}
@@ -730,6 +755,14 @@ impl DataStore {
log::warn!("failed to remove namespace {ns} - {err}")
}
}
+ if let DatastoreBackend::S3(s3_client) = &backend {
+ // Only remove the namespace marker, if it was empty,
+ // than this is the same as the namespace being removed.
+ let object_key =
+ crate::s3::object_key_from_path(&ns.path(), NAMESPACE_MARKER_FILENAME)
+ .context("invalid namespace marker object key")?;
+ proxmox_async::runtime::block_on(s3_client.delete_object(object_key))?;
+ }
}
}
@@ -747,7 +780,7 @@ impl DataStore {
) -> Result<BackupGroupDeleteStats, Error> {
let backup_group = self.backup_group(ns.clone(), backup_group.clone());
- backup_group.destroy()
+ backup_group.destroy(&self.backend()?)
}
/// Remove a backup directory including all content
@@ -759,7 +792,7 @@ impl DataStore {
) -> Result<(), Error> {
let backup_dir = self.backup_dir(ns.clone(), backup_dir.clone())?;
- backup_dir.destroy(force)
+ backup_dir.destroy(force, &self.backend()?)
}
/// Returns the time of the last successful backup
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 35fcb2ac5..80740e3fb 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -422,7 +422,7 @@ pub async fn delete_snapshot(
let snapshot = datastore.backup_dir(ns, backup_dir)?;
- snapshot.destroy(false)?;
+ snapshot.destroy(false, &datastore.backend()?)?;
Ok(Value::Null)
})
@@ -1088,13 +1088,21 @@ pub fn prune(
});
if !keep {
- if let Err(err) = backup_dir.destroy(false) {
- warn!(
- "failed to remove dir {:?}: {}",
- backup_dir.relative_path(),
- err,
- );
- }
+ match datastore.backend() {
+ Ok(backend) => {
+ if let Err(err) = backup_dir.destroy(false, &backend) {
+ warn!(
+ "failed to remove dir {:?}: {}",
+ backup_dir.relative_path(),
+ err,
+ );
+ }
+ }
+ Err(err) => warn!(
+ "failed to remove dir {:?}: {err}",
+ backup_dir.relative_path()
+ ),
+ };
}
}
prune_result
--
2.47.2
More information about the pbs-devel
mailing list