Skip to content

Commit e5ff2a8

Browse files
committed
fix(prune): group by source label regardless of configured sources
Prune was branching on whether the local config had a `sources:` block: with sources it grouped snapshots by `source_label` and applied retention per-group; without sources it pooled every snapshot into one flat bucket and ignored labels entirely. That broke central-server setups that run prune against a repo populated by independent clients — the server has no `sources:` and previous backups' labels were silently discarded. Always use `apply_policy_by_label`. The label is intrinsic to each snapshot (recorded at backup time), so the local config's source list shouldn't influence the grouping. Closes #138.
1 parent e8291a4 commit e5ff2a8

2 files changed

Lines changed: 77 additions & 25 deletions

File tree

crates/vykar-core/src/commands/prune.rs

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::sync::atomic::AtomicBool;
44
use chrono::Utc;
55

66
use crate::config::{RetentionConfig, SourceEntry, VykarConfig};
7-
use crate::prune::{apply_policy, apply_policy_by_label, PruneDecision};
7+
use crate::prune::{apply_policy_by_label, PruneDecision};
88
use vykar_types::error::{Result, VykarError};
99

1010
use super::list::load_snapshot_meta;
@@ -75,32 +75,24 @@ pub fn run(
7575
.filter_map(|s| s.retention.as_ref().map(|r| (s.label.clone(), r.clone())))
7676
.collect();
7777

78-
let has_sources = !sources.is_empty();
79-
80-
let decisions = if has_sources {
81-
// Label-aware: group by source_label and apply per-source retention
82-
if !config.retention.has_any_rule()
83-
&& source_retentions.values().all(|r| !r.has_any_rule())
84-
{
85-
return Err(VykarError::Config(
78+
// Always group by source_label — labels are intrinsic to each
79+
// snapshot (recorded at backup time), so retention must apply
80+
// per-label regardless of whether the local config has a
81+
// `sources:` block. (Issue #138.)
82+
if !config.retention.has_any_rule()
83+
&& source_retentions.values().all(|r| !r.has_any_rule())
84+
{
85+
return Err(VykarError::Config(
8686
"no retention rules configured — set at least one keep_* option in the retention section or per-source".into(),
8787
));
88-
}
89-
apply_policy_by_label(
90-
&target_snapshots,
91-
&config.retention,
92-
&source_retentions,
93-
now,
94-
)?
95-
} else {
96-
// No sources — fall back to flat policy
97-
if !config.retention.has_any_rule() {
98-
return Err(VykarError::Config(
99-
"no retention rules configured — set at least one keep_* option in the retention section".into(),
100-
));
101-
}
102-
apply_policy(&target_snapshots, &config.retention, now)?
103-
};
88+
}
89+
90+
let decisions = apply_policy_by_label(
91+
&target_snapshots,
92+
&config.retention,
93+
&source_retentions,
94+
now,
95+
)?;
10496

10597
// Build list output
10698
let mut list_entries = Vec::new();

crates/vykar-core/src/tests/prune_command.rs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,66 @@ fn prune_source_filter_only_prunes_matching_label() {
112112
assert!(names.contains(&"snap-b-1"));
113113
}
114114

115+
/// Regression test for issue #138: when the local config has no `sources:`
116+
/// block (e.g. a central server running prune against a repo populated by
117+
/// other clients), prune must still group snapshots by `source_label`. Each
118+
/// label is its own retention bucket regardless of local config.
119+
#[test]
120+
fn prune_groups_by_label_even_without_configured_sources() {
121+
let tmp = tempfile::tempdir().unwrap();
122+
let repo_dir = tmp.path().join("repo");
123+
let source_a = tmp.path().join("source-a");
124+
let source_b = tmp.path().join("source-b");
125+
std::fs::create_dir_all(&repo_dir).unwrap();
126+
std::fs::create_dir_all(&source_a).unwrap();
127+
std::fs::create_dir_all(&source_b).unwrap();
128+
std::fs::write(source_a.join("a.txt"), b"a").unwrap();
129+
std::fs::write(source_b.join("b.txt"), b"b").unwrap();
130+
131+
let mut config = init_repo(&repo_dir);
132+
config.retention = RetentionConfig {
133+
keep_last: Some(1),
134+
..RetentionConfig::default()
135+
};
136+
137+
backup_single_source(&config, &source_a, "a", "snap-a");
138+
std::thread::sleep(Duration::from_millis(2));
139+
backup_single_source(&config, &source_b, "b", "snap-b");
140+
141+
let (stats, _) = commands::prune::run(&config, None, false, false, &[], &[], None)
142+
.expect("prune should succeed without a sources block");
143+
144+
assert_eq!(stats.pruned, 0);
145+
assert_eq!(stats.kept, 2);
146+
147+
let after = open_local_repo(&repo_dir);
148+
let names: Vec<_> = after
149+
.manifest()
150+
.snapshots
151+
.iter()
152+
.map(|e| e.name.as_str())
153+
.collect();
154+
assert!(names.contains(&"snap-a"));
155+
assert!(names.contains(&"snap-b"));
156+
}
157+
158+
/// Without any configured sources, prune still needs a retention rule to do
159+
/// anything — verify the consolidated guard fires.
160+
#[test]
161+
fn prune_errors_without_retention_rules_when_sources_empty() {
162+
let tmp = tempfile::tempdir().unwrap();
163+
let repo_dir = tmp.path().join("repo");
164+
std::fs::create_dir_all(&repo_dir).unwrap();
165+
166+
let config = init_repo(&repo_dir);
167+
let err = commands::prune::run(&config, None, true, false, &[], &[], None)
168+
.err()
169+
.unwrap();
170+
assert!(
171+
matches!(err, VykarError::Config(msg) if msg.contains("no retention rules configured"))
172+
);
173+
}
174+
115175
/// When the snapshot blobs have been deleted from storage (commit point
116176
/// reached) but Phase 3 refcount cleanup fails, `prune::run` must return
117177
/// `Ok((stats, _))` with `stats.warnings` populated rather than propagating

0 commit comments

Comments
 (0)