From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 72CCF1FF0AF for ; Fri, 06 Nov 2026 18:24:59 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 3D10921883; Fri, 06 Nov 2026 18:23:48 +0100 (CET) From: Christian Ebner To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup v2 37/43] prune: skip snapshots with active retentions Date: Fri, 6 Nov 2026 18:22:29 +0100 Message-ID: <20261106172235.890542-38-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261106172235.890542-1-c.ebner@proxmox.com> References: <20261106172235.890542-1-c.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793985780654 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.440 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 2K3L5M4GOWL7EX4DGUSGDF3LR4OKM72X X-Message-ID-Hash: 2K3L5M4GOWL7EX4DGUSGDF3LR4OKM72X X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Make sure that snapshots which have an active retention are not tried to be pruned but rather skipped. This is analogous to the checks performed for protected snapshots. Signed-off-by: Christian Ebner --- pbs-datastore/src/prune.rs | 54 +++++++++++++++++++++++++++++++++---- src/api2/admin/datastore.rs | 7 +++-- src/server/prune_job.rs | 5 ++-- tests/prune.rs | 49 +++++++++++++++++++++++++++++++-- www/datastore/Prune.js | 4 +++ 5 files changed, 108 insertions(+), 11 deletions(-) diff --git a/pbs-datastore/src/prune.rs b/pbs-datastore/src/prune.rs index 6d8a215d7..5ece66e43 100644 --- a/pbs-datastore/src/prune.rs +++ b/pbs-datastore/src/prune.rs @@ -2,6 +2,7 @@ use std::collections::{HashMap, HashSet}; use std::path::PathBuf; use anyhow::Error; +use tracing::warn; use pbs_api_types::KeepOptions; @@ -10,6 +11,7 @@ use super::BackupInfo; #[derive(Clone, Copy, PartialEq, Eq)] pub enum PruneMark { Protected, + Retained, Keep, KeepPartial, Remove, @@ -24,12 +26,17 @@ impl PruneMark { pub fn protected(self) -> bool { self == PruneMark::Protected } + + pub fn retained(self) -> bool { + self == PruneMark::Retained + } } impl std::fmt::Display for PruneMark { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str(match self { PruneMark::Protected => "protected", + PruneMark::Retained => "retained", PruneMark::Keep => "keep", PruneMark::KeepPartial => "keep-partial", PruneMark::Remove => "remove", @@ -115,10 +122,39 @@ fn remove_incomplete_and_ignore_future( } } +/// Check if the snapshot is still protected by its retention timestamp. +pub fn has_active_retention(info: &BackupInfo) -> bool { + match info.backup_dir.active_retention_lock() { + Ok(retain_until) => retain_until.is_some(), + Err(err) => { + warn!( + "failed to check retention of {:?} - {err:#}", + info.backup_dir.relative_path(), + ); + true + } + } +} + +/// Keep snapshots marked for removal which are still protected by their retention timestamp. +/// +/// This must run after the keep options were applied by [`compute_prune_info`]. +pub fn mark_active_retained( + prune_info: &mut [(BackupInfo, PruneMark)], + has_active_retention: impl Fn(&BackupInfo) -> bool, +) { + for (info, mark) in prune_info.iter_mut() { + if *mark == PruneMark::Remove && has_active_retention(info) { + *mark = PruneMark::Retained; + } + } +} + /// This filters incomplete and kept backups. pub fn compute_prune_info( mut list: Vec, options: &KeepOptions, + has_active_retention: impl Fn(&BackupInfo) -> bool, ) -> Result, Error> { let mut mark = HashMap::new(); @@ -126,6 +162,18 @@ pub fn compute_prune_info( remove_incomplete_and_ignore_future(&mut mark, &list); + for info in list.iter() { + let backup_id = info.backup_dir.relative_path(); + if mark.contains_key(&backup_id) { + continue; + } + if info.protected { + mark.insert(backup_id, PruneMark::Protected); + } else if has_active_retention(info) { + mark.insert(backup_id, PruneMark::Retained); + } + } + if let Some(keep_last) = options.keep_last { mark_selections(&mut mark, &list, keep_last as usize, |info| { Ok(info.backup_dir.backup_time_string().to_owned()) @@ -170,11 +218,7 @@ pub fn compute_prune_info( .into_iter() .map(|info| { let backup_id = info.backup_dir.relative_path(); - let mark = if info.protected { - PruneMark::Protected - } else { - mark.get(&backup_id).copied().unwrap_or(PruneMark::Remove) - }; + let mark = mark.get(&backup_id).copied().unwrap_or(PruneMark::Remove); (info, mark) }) diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs index 097d74dc1..563590430 100644 --- a/src/api2/admin/datastore.rs +++ b/src/api2/admin/datastore.rs @@ -58,7 +58,7 @@ use pbs_datastore::dynamic_index::{BufferedDynamicReader, DynamicIndexReader, Lo use pbs_datastore::fixed_index::FixedIndexReader; use pbs_datastore::index::IndexFile; use pbs_datastore::manifest::BackupManifest; -use pbs_datastore::prune::compute_prune_info; +use pbs_datastore::prune::{compute_prune_info, has_active_retention}; use pbs_datastore::{ BackupDir, DataStore, LocalChunkReader, StoreProgress, check_backup_owner, ensure_datastore_is_mounted, task_tracking, @@ -1016,6 +1016,7 @@ pub fn prune( backup_time: i64, keep: bool, protected: bool, + retained: bool, #[serde(skip_serializing_if = "Option::is_none")] ns: Option, } @@ -1023,7 +1024,7 @@ pub fn prune( let list = group.list_backups()?; - let mut prune_info = compute_prune_info(list, &keep_options)?; + let mut prune_info = compute_prune_info(list, &keep_options, has_active_retention)?; prune_info.reverse(); // delete older snapshots first @@ -1040,6 +1041,7 @@ pub fn prune( backup_time: backup_dir.backup_time(), keep, protected: mark.protected(), + retained: mark.retained(), ns: None, }; let prune_ns = backup_dir.backup_ns(); @@ -1087,6 +1089,7 @@ pub fn prune( backup_time, keep, protected: mark.protected(), + retained: mark.retained(), ns: None, }); diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs index c94e34f43..79e0df804 100644 --- a/src/server/prune_job.rs +++ b/src/server/prune_job.rs @@ -8,7 +8,7 @@ use pbs_api_types::{ PRIV_DATASTORE_PRUNE, PruneJobOptions, print_store_and_ns, }; use pbs_datastore::DataStore; -use pbs_datastore::prune::{PruneMark, compute_prune_info}; +use pbs_datastore::prune::{PruneMark, compute_prune_info, has_active_retention}; use proxmox_rest_server::WorkerTask; use crate::backup::ListAccessibleBackupGroups; @@ -56,7 +56,7 @@ pub fn prune_datastore( let ns = group.backup_ns(); let list = group.list_backups()?; - let mut prune_info = compute_prune_info(list, &prune_options.keep)?; + let mut prune_info = compute_prune_info(list, &prune_options.keep, has_active_retention)?; prune_info.reverse(); // delete older snapshots first info!( @@ -75,6 +75,7 @@ pub fn prune_datastore( info.backup_dir.backup_time_string(), mark = match mark { PruneMark::Protected => "keep protected", + PruneMark::Retained => "keep retained", PruneMark::Keep => "keep", PruneMark::KeepPartial => "keep partial", PruneMark::Remove => "remove", diff --git a/tests/prune.rs b/tests/prune.rs index 612a915db..bb5184b76 100644 --- a/tests/prune.rs +++ b/tests/prune.rs @@ -3,7 +3,7 @@ use std::path::PathBuf; use anyhow::Error; use pbs_api_types::{MANIFEST_BLOB_NAME, PruneJobOptions}; -use pbs_datastore::prune::compute_prune_info; +use pbs_datastore::prune::{PruneMark, compute_prune_info}; use pbs_datastore::{BackupDir, BackupInfo}; fn get_prune_list( @@ -11,7 +11,7 @@ fn get_prune_list( return_kept: bool, options: &PruneJobOptions, ) -> Vec { - let mut prune_info = compute_prune_info(list, &options.keep).unwrap(); + let mut prune_info = compute_prune_info(list, &options.keep, |_| false).unwrap(); prune_info.reverse(); @@ -74,6 +74,51 @@ fn test_prune_protected() -> Result<(), Error> { Ok(()) } +#[test] +fn test_prune_retained() -> Result<(), Error> { + let orig_list = vec![ + create_info("host/elsa/2019-11-15T09:39:15Z", false), + create_info("host/elsa/2019-11-15T10:39:15Z", false), + create_info("host/elsa/2019-11-15T10:49:15Z", false), + create_info("host/elsa/2019-11-15T10:59:15Z", false), + ]; + let retained = [ + PathBuf::from("host/elsa/2019-11-15T10:39:15Z"), + PathBuf::from("host/elsa/2019-11-15T10:59:15Z"), + ]; + + let mut options = PruneJobOptions::default(); + options.keep.keep_last = Some(1); + let prune_info = compute_prune_info(orig_list, &options.keep, |info| { + retained.contains(&info.backup_dir.relative_path()) + })?; + + let marks: Vec<(PathBuf, PruneMark)> = prune_info + .iter() + .map(|(info, mark)| (info.backup_dir.relative_path(), *mark)) + .collect(); + let expect = vec![ + ( + PathBuf::from("host/elsa/2019-11-15T10:59:15Z"), + PruneMark::Retained, + ), + ( + PathBuf::from("host/elsa/2019-11-15T10:49:15Z"), + PruneMark::Keep, + ), + ( + PathBuf::from("host/elsa/2019-11-15T10:39:15Z"), + PruneMark::Retained, + ), + ( + PathBuf::from("host/elsa/2019-11-15T09:39:15Z"), + PruneMark::Remove, + ), + ]; + assert!(marks == expect); + Ok(()) +} + #[test] fn test_prune_hourly() -> Result<(), Error> { let orig_list = vec![ diff --git a/www/datastore/Prune.js b/www/datastore/Prune.js index 7722db2c7..abb732540 100644 --- a/www/datastore/Prune.js +++ b/www/datastore/Prune.js @@ -110,6 +110,10 @@ Ext.define('PBS.Datastore.PruneInputPanel', { backup.keepReason = 'protected'; continue; } + if (backup.retained) { + backup.keepReason = 'retained'; + continue; + } counter[rule]++; if (rule !== 'keep-all') { backup.keepReason = rule + ': ' + counter[rule]; -- 2.47.3