From 83a9800b20f8fad273f4ca922066e91cdd9b187c Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 7 Oct 2026 07:42:44 +0000 Subject: [PATCH] Read the snapshot name using the stored hostname (closes #230) A snapshot ID is hostname_name_timestamp, and purge took the name to be everything between the first and the last underscore. With a hostname such as my_host the name home came out as host_home, so `snapshot purge --keep-latest --snapshot home` found nothing to delete and `snapshot create --prune` purged nothing without a message. The name is now read by removing the hostname stored with the snapshot, in the short form the ID uses, so both may contain underscores. This was chosen over rejecting underscores in `hostname` when the config loads, which would also stop restores on such a host. The purge consistency test stored a hostname that did not match its snapshot IDs; it now matches, as it always does in production. Model: opus-5-5 --- TODO.md | 11 ++++ internal/snapshot/snapshot.go | 14 +++-- internal/vaultik/helpers.go | 41 +++++++------- internal/vaultik/helpers_test.go | 28 ++++++++-- .../purge_local_remote_consistency_test.go | 2 +- internal/vaultik/purge_per_name_test.go | 54 ++++++++++++++----- internal/vaultik/snapshot.go | 19 ++++--- 7 files changed, 124 insertions(+), 45 deletions(-) diff --git a/TODO.md b/TODO.md index 0964f46..e8dd4f6 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,17 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-07: Made per-name retention work when the hostname contains `_` + ([issue #230](https://git.eeqj.de/sneak/vaultik/issues/230)). A + snapshot ID is `hostname_name_timestamp`, and the name was read as + everything between the first and the last `_`, so with + `hostname: my_host` the name `home` came out as `host_home`. + `snapshot purge --keep-latest --snapshot home` then printed "No + snapshots to delete", and `snapshot create --prune` purged nothing + without a message. The name is now read using the hostname the + `snapshots` table stores with each snapshot, cut at its first `.` as it + is in the ID. + - 2026-10-07: Made `remote info` stop reporting a snapshot's blobs as orphaned when its manifest cannot be read, and stop printing raw names from under `metadata/` diff --git a/internal/snapshot/snapshot.go b/internal/snapshot/snapshot.go index 2848864..4e59092 100644 --- a/internal/snapshot/snapshot.go +++ b/internal/snapshot/snapshot.go @@ -105,17 +105,21 @@ func (sm *SnapshotManager) CreateSnapshot( return sm.CreateSnapshotWithName(ctx, hostname, "", version, gitRevision) } +// ShortHostname returns hostname up to its first dot. A snapshot ID starts +// with this form, while the snapshots table stores the full hostname. +func ShortHostname(hostname string) string { + short, _, _ := strings.Cut(hostname, ".") + + return short +} + // CreateSnapshotWithName creates a new snapshot record with an optional // snapshot name. The snapshot ID format is: hostname_name_timestamp or // hostname_timestamp if name is empty. func (sm *SnapshotManager) CreateSnapshotWithName( ctx context.Context, hostname, name, version, gitRevision string, ) (string, error) { - // Use short hostname (strip domain if present) - shortHostname := hostname - if before, _, ok := strings.Cut(hostname, "."); ok { - shortHostname = before - } + shortHostname := ShortHostname(hostname) // Build snapshot ID with optional name timestamp := time.Now().UTC().Format("2006-01-02T15:04:05Z") diff --git a/internal/vaultik/helpers.go b/internal/vaultik/helpers.go index 9ca5045..ff5fe19 100644 --- a/internal/vaultik/helpers.go +++ b/internal/vaultik/helpers.go @@ -9,6 +9,7 @@ import ( "time" "github.com/dustin/go-humanize" + "sneak.berlin/go/vaultik/internal/snapshot" "sneak.berlin/go/vaultik/internal/types" ) @@ -46,12 +47,9 @@ const ( year = 365 * day ) -// Snapshot IDs split on "_" into hostname, optional name parts, and a -// trailing timestamp. -const ( - minSnapshotIDParts = 2 - minSnapshotIDNameParts = 3 -) +// A snapshot ID split on "_" has at least a hostname and a trailing +// timestamp. +const minSnapshotIDParts = 2 // SnapshotInfo contains information about a snapshot. // @@ -121,20 +119,27 @@ func parseSnapshotTimestamp(snapshotID string) (time.Time, error) { return timestamp.UTC(), nil } -// parseSnapshotName extracts the snapshot name from a snapshot ID. -// Format: hostname_snapshotname_timestamp — the middle part(s) between hostname -// and the RFC3339 timestamp are the snapshot name (may contain underscores). -// Returns the snapshot name, or empty string if the ID is malformed. -func parseSnapshotName(snapshotID string) string { - parts := strings.Split(snapshotID, "_") - if len(parts) < minSnapshotIDNameParts { - // Format: hostname_timestamp — no snapshot name +// parseSnapshotName extracts the snapshot name from a snapshot ID of the +// form hostname_name_timestamp, given the hostname stored with that +// snapshot. The hostname and the name may both contain underscores, so the +// name is what is left after removing the short hostname and its "_" from +// the front and the last "_" and the timestamp from the end. Returns "" for +// an ID with no name (hostname_timestamp), and for an ID that does not start +// with that hostname, which CreateSnapshotWithName never writes. +func parseSnapshotName(snapshotID, hostname string) string { + prefix := snapshot.ShortHostname(hostname) + "_" + + rest, ok := strings.CutPrefix(snapshotID, prefix) + if !ok { return "" } - // Format: hostname_name_timestamp — middle parts are the name. - // The last part is the RFC3339 timestamp, the first part is the hostname, - // everything in between is the snapshot name (which may itself contain underscores). - return strings.Join(parts[1:len(parts)-1], "_") + + end := strings.LastIndex(rest, "_") + if end < 0 { + return "" + } + + return rest[:end] } // parseDuration parses a duration string with support for human-friendly units: diff --git a/internal/vaultik/helpers_test.go b/internal/vaultik/helpers_test.go index b77b51d..e279319 100644 --- a/internal/vaultik/helpers_test.go +++ b/internal/vaultik/helpers_test.go @@ -11,33 +11,55 @@ func TestParseSnapshotName(t *testing.T) { tests := []struct { name string snapshotID string + hostname string want string }{ { name: "standard format with name", snapshotID: "myhost_home_2026-01-12T14:41:15Z", + hostname: "myhost", want: "home", }, { name: "standard format with different name", snapshotID: "server1_system_2026-02-15T09:30:00Z", + hostname: "server1", want: "system", }, { name: "name with underscores", snapshotID: "myhost_my_special_backup_2026-03-01T00:00:00Z", + hostname: "myhost", want: "my_special_backup", }, + { + name: "hostname with underscores", + snapshotID: "my_host_docs_2026-03-01T00:00:00Z", + hostname: "my_host", + want: "docs", + }, + { + name: "stored hostname with domain", + snapshotID: "my_host_mail_2026-03-01T00:00:00Z", + hostname: "my_host.example.com", + want: "mail", + }, + { + name: "no name", + snapshotID: "my_host_2026-03-01T00:00:00Z", + hostname: "my_host", + want: "", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - got := parseSnapshotName(tt.snapshotID) + got := parseSnapshotName(tt.snapshotID, tt.hostname) if got != tt.want { - t.Errorf("parseSnapshotName(%q) = %q, want %q", - tt.snapshotID, got, tt.want) + t.Errorf("parseSnapshotName(%q, %q) = %q, want %q", + tt.snapshotID, tt.hostname, got, tt.want) } }) } diff --git a/internal/vaultik/purge_local_remote_consistency_test.go b/internal/vaultik/purge_local_remote_consistency_test.go index bf52b45..f170d2f 100644 --- a/internal/vaultik/purge_local_remote_consistency_test.go +++ b/internal/vaultik/purge_local_remote_consistency_test.go @@ -42,7 +42,7 @@ func setupConsistencyTest( completedAt := startedAt.Add(5 * time.Minute) snap := &database.Snapshot{ ID: types.SnapshotID(id), - Hostname: testHostname, + Hostname: snapHostname, VaultikVersion: testLabel, StartedAt: startedAt, CompletedAt: &completedAt, diff --git a/internal/vaultik/purge_per_name_test.go b/internal/vaultik/purge_per_name_test.go index cfc343b..800480c 100644 --- a/internal/vaultik/purge_per_name_test.go +++ b/internal/vaultik/purge_per_name_test.go @@ -17,8 +17,10 @@ import ( "sneak.berlin/go/vaultik/internal/vaultik" ) -// Snapshot IDs reused across the purge tests. +// Snapshot IDs reused across the purge tests, and the hostname they were +// taken on. const ( + snapHostname = "testhost" snapSystemT0 = "testhost_system_2026-01-01T00:00:00Z" snapHomeT0 = "testhost_home_2026-01-01T00:00:00Z" snapHomeT1 = "testhost_home_2026-01-01T01:00:00Z" @@ -26,9 +28,12 @@ const ( ) // setupPurgeTest creates a Vaultik instance with an in-memory database and mock -// storage pre-populated with the given snapshot IDs. Each snapshot is marked as -// completed. Remote metadata stubs are created so syncWithRemote keeps them. -func setupPurgeTest(t *testing.T, snapshotIDs []string) *vaultik.Vaultik { +// storage pre-populated with the given snapshot IDs, all taken on hostname. +// Each snapshot is marked as completed. Remote metadata stubs are created so +// syncWithRemote keeps them. +func setupPurgeTest( + t *testing.T, hostname string, snapshotIDs []string, +) *vaultik.Vaultik { t.Helper() ctx := context.Background() @@ -51,7 +56,7 @@ func setupPurgeTest(t *testing.T, snapshotIDs []string) *vaultik.Vaultik { completedAt := startedAt.Add(5 * time.Minute) snap := &database.Snapshot{ ID: types.SnapshotID(id), - Hostname: "testhost", + Hostname: types.Hostname(hostname), VaultikVersion: testLabel, StartedAt: startedAt, CompletedAt: &completedAt, @@ -120,7 +125,7 @@ func TestPurgeKeepLatest_PerName(t *testing.T) { "testhost_system_2026-01-01T04:00:00Z", } - v := setupPurgeTest(t, snapshotIDs) + v := setupPurgeTest(t, snapHostname, snapshotIDs) err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{ KeepLatest: true, @@ -148,7 +153,7 @@ func TestPurgeKeepLatest_SingleName(t *testing.T) { "testhost_home_2026-01-01T02:00:00Z", } - v := setupPurgeTest(t, snapshotIDs) + v := setupPurgeTest(t, snapHostname, snapshotIDs) err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{ KeepLatest: true, @@ -176,7 +181,7 @@ func TestPurgeKeepLatest_WithNameFilter(t *testing.T) { "testhost_home_2026-01-01T04:00:00Z", } - v := setupPurgeTest(t, snapshotIDs) + v := setupPurgeTest(t, snapHostname, snapshotIDs) err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{ KeepLatest: true, @@ -198,7 +203,7 @@ func TestPurgeKeepLatest_NoSnapshots(t *testing.T) { log.Initialize(log.Config{}) t.Parallel() - v := setupPurgeTest(t, nil) + v := setupPurgeTest(t, snapHostname, nil) err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{ KeepLatest: true, @@ -216,7 +221,7 @@ func TestPurgeKeepLatest_NameFilterNoMatch(t *testing.T) { "testhost_system_2026-01-01T01:00:00Z", } - v := setupPurgeTest(t, snapshotIDs) + v := setupPurgeTest(t, snapHostname, snapshotIDs) err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{ KeepLatest: true, @@ -243,7 +248,7 @@ func TestPurgeOlderThan_WithNameFilter(t *testing.T) { snapHomeT0, } - v := setupPurgeTest(t, snapshotIDs) + v := setupPurgeTest(t, snapHostname, snapshotIDs) // Purge only "home" snapshots older than 365 days err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{ @@ -277,7 +282,7 @@ func TestPurgeKeepLatest_ThreeNames(t *testing.T) { "testhost_home_2026-01-01T06:00:00Z", } - v := setupPurgeTest(t, snapshotIDs) + v := setupPurgeTest(t, snapHostname, snapshotIDs) err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{ KeepLatest: true, @@ -291,3 +296,28 @@ func TestPurgeKeepLatest_ThreeNames(t *testing.T) { assert.Contains(t, remaining, "testhost_system_2026-01-01T04:00:00Z") assert.Contains(t, remaining, "testhost_media_2026-01-01T05:00:00Z") } + +// A hostname may contain underscores, so the snapshot name cannot be found +// by splitting the ID at them. A purge by name must still select "docs". +func TestPurgeKeepLatest_HostnameWithUnderscore(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + const ( + system = "my_host_system_2026-01-01T00:00:00Z" + docsT1 = "my_host_docs_2026-01-01T01:00:00Z" + docsT2 = "my_host_docs_2026-01-01T02:00:00Z" + ) + + v := setupPurgeTest(t, "my_host", []string{system, docsT1, docsT2}) + + err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{ + KeepLatest: true, + Force: true, + Names: []string{"docs"}, + }) + require.NoError(t, err) + + assert.ElementsMatch(t, []string{system, docsT2}, + listRemainingSnapshots(t, v)) +} diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index 7071bd4..2707516 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -14,6 +14,7 @@ import ( "sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/snapshot" + "sneak.berlin/go/vaultik/internal/types" ) // Sentinel errors for snapshot management. @@ -495,19 +496,23 @@ func (v *Vaultik) PurgeSnapshotsWithOptions(opts *SnapshotPurgeOptions) error { nameFilter[n] = struct{}{} } - // Collect completed snapshots, applying the name filter. + // Collect completed snapshots and their names, applying the name filter. snapshots := make([]SnapshotInfo, 0, len(dbSnapshots)) + names := make(map[types.SnapshotID]string, len(dbSnapshots)) + for _, s := range dbSnapshots { if s.CompletedAt == nil { continue } + name := parseSnapshotName(s.ID.String(), s.Hostname.String()) if len(nameFilter) > 0 { - if _, ok := nameFilter[parseSnapshotName(s.ID.String())]; !ok { + if _, ok := nameFilter[name]; !ok { continue } } + names[s.ID] = name snapshots = append(snapshots, SnapshotInfo{ ID: s.ID, Timestamp: s.StartedAt, @@ -520,7 +525,7 @@ func (v *Vaultik) PurgeSnapshotsWithOptions(opts *SnapshotPurgeOptions) error { return snapshots[i].Timestamp.After(snapshots[j].Timestamp) }) - toDelete, err := selectSnapshotsToPurge(snapshots, opts) + toDelete, err := selectSnapshotsToPurge(snapshots, names, opts) if err != nil { return err } @@ -538,9 +543,11 @@ func (v *Vaultik) PurgeSnapshotsWithOptions(opts *SnapshotPurgeOptions) error { // selectSnapshotsToPurge applies the purge retention criteria to the // newest-first sorted snapshot list and returns the deletion -// candidates. +// candidates. names maps each snapshot's ID to its snapshot name. func selectSnapshotsToPurge( - snapshots []SnapshotInfo, opts *SnapshotPurgeOptions, + snapshots []SnapshotInfo, + names map[types.SnapshotID]string, + opts *SnapshotPurgeOptions, ) ([]SnapshotInfo, error) { var toDelete []SnapshotInfo @@ -551,7 +558,7 @@ func selectSnapshotsToPurge( seen := make(map[string]bool) for _, snap := range snapshots { - name := parseSnapshotName(snap.ID.String()) + name := names[snap.ID] if seen[name] { toDelete = append(toDelete, snap) -- 2.54.0