From b39d765374146cc2dec48b73f82c4fe320bdb909 Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 28 Jun 2026 06:10:26 +0200 Subject: [PATCH] Make snapshot rm clean up the remote by default snapshot rm now does the full cleanup: removes the local index entry, strips the snapshot's metadata from the destination store, and prunes any blobs that were only referenced by the just-removed manifest. The --remote flag is retired; --local-only opts out for the rare case where the user wants to forget a snapshot locally without touching the remote. If the destination store is unreachable, the local-DB removal still completes and a warning is emitted; the user can rerun 'vaultik prune' to retry the remote half later. RemoveAllSnapshots gets the same treatment: after deleting every snapshot's metadata (local + remote + orphan keys), an automatic blob prune sweep removes the now-unreferenced blob set. --- README.md | 16 ++- internal/cli/snapshot.go | 19 ++- internal/vaultik/prune.go | 4 +- internal/vaultik/remove_snapshot_test.go | 114 ++++++++------- internal/vaultik/snapshot.go | 172 ++++++++++++++++------- 5 files changed, 208 insertions(+), 117 deletions(-) diff --git a/README.md b/README.md index 9bdf67f..76ead69 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,7 @@ vaultik [--config ] snapshot create [snapshot-names...] [--cron] [--prune] vaultik [--config ] snapshot list [--json] vaultik [--config ] snapshot verify [--deep] [--json] vaultik [--config ] snapshot purge [--keep-latest | --older-than ] [--snapshot ...] [--force] -vaultik [--config ] snapshot remove [--dry-run] [--force] [--remote] [--json] +vaultik [--config ] snapshot remove [--dry-run] [--force] [--local-only] [--json] vaultik [--config ] snapshot cleanup vaultik [--config ] snapshot restore [paths...] [--verify] vaultik [--config ] prune [--force] [--json] @@ -198,11 +198,15 @@ latest globally). * `--snapshot `: Restrict to specific snapshot names (repeat for multiple) * `--force`: Skip confirmation prompt -**`snapshot remove`**: Remove a specific snapshot from the local database. -Automatically cleans up local rows (files, chunks, blobs) that the removed -snapshot was the last referrer for — you don't need a separate prune step -after removal. -* `--remote`: Also remove snapshot metadata from remote storage +**`snapshot remove`**: Remove a snapshot. By default this removes the +snapshot from the local index, strips the snapshot's metadata from the +backup destination store, and prunes any blobs that are no longer +referenced by any remaining remote manifest. Local row cleanup (files, +chunks, blobs the snapshot was the last referrer for) runs automatically; +no separate prune step is needed. If the destination store is unreachable, +the local-DB removal still completes and a warning is emitted; rerun +`vaultik prune` once the store is reachable to finish remote cleanup. +* `--local-only`: Skip remote cleanup; only touch the local index * `--all`: Remove all snapshots (requires `--force`) * `--dry-run`: Show what would be deleted without deleting * `--force`: Skip confirmation prompt diff --git a/internal/cli/snapshot.go b/internal/cli/snapshot.go index c6bf575..d83c02f 100644 --- a/internal/cli/snapshot.go +++ b/internal/cli/snapshot.go @@ -324,14 +324,19 @@ func newSnapshotRemoveCommand() *cobra.Command { cmd := &cobra.Command{ Use: "remove [snapshot-id]", Aliases: []string{"rm"}, - Short: "Remove a snapshot from the local database", - Long: `Removes a snapshot from the local database. + Short: "Remove a snapshot from local index and remote storage", + Long: `Removes a snapshot. -By default, only removes from the local database. Use --remote to also remove -the snapshot metadata from remote storage. +By default, this removes the snapshot from the local index database, strips +the snapshot's metadata from the backup destination store, and prunes any +blobs that are no longer referenced by any remaining remote snapshot. -Note: This does NOT remove blobs. Use 'vaultik prune' to remove orphaned blobs -after removing snapshots. +Use --local-only to skip the remote half (e.g. when you want to forget a +snapshot locally without touching the destination store). + +If the remote is unreachable, the local-database removal still completes +and a warning is emitted; rerun 'vaultik prune' once the destination store +is reachable to finish remote cleanup. Use --all --force to remove all snapshots.`, Args: func(cmd *cobra.Command, args []string) error { @@ -408,7 +413,7 @@ Use --all --force to remove all snapshots.`, cmd.Flags().BoolVarP(&opts.Force, "force", "f", false, "Skip confirmation prompt") cmd.Flags().BoolVar(&opts.DryRun, "dry-run", false, "Show what would be removed without removing") cmd.Flags().BoolVar(&opts.JSON, "json", false, "Output result as JSON") - cmd.Flags().BoolVar(&opts.Remote, "remote", false, "Also remove snapshot metadata from remote storage") + cmd.Flags().BoolVar(&opts.LocalOnly, "local-only", false, "Skip remote cleanup; only touch the local index") cmd.Flags().BoolVar(&opts.All, "all", false, "Remove all snapshots (requires --force)") return cmd diff --git a/internal/vaultik/prune.go b/internal/vaultik/prune.go index 90f503e..714fb25 100644 --- a/internal/vaultik/prune.go +++ b/internal/vaultik/prune.go @@ -27,11 +27,11 @@ func (v *Vaultik) NukeRemote(force bool) error { } v.UI.Begin("Removing all snapshot metadata from backup destination store.") - if _, err := v.RemoveAllSnapshots(&RemoveOptions{Force: true, Remote: true}); err != nil { + if _, err := v.RemoveAllSnapshots(&RemoveOptions{Force: true}); err != nil { return fmt.Errorf("removing all snapshots: %w", err) } - v.UI.Begin("Removing all blobs from backup destination store.") + v.UI.Begin("Removing any blobs still present in backup destination store.") if err := v.PruneBlobs(&PruneOptions{Force: true}); err != nil { return fmt.Errorf("pruning blobs: %w", err) } diff --git a/internal/vaultik/remove_snapshot_test.go b/internal/vaultik/remove_snapshot_test.go index 8164c81..7c719a7 100644 --- a/internal/vaultik/remove_snapshot_test.go +++ b/internal/vaultik/remove_snapshot_test.go @@ -188,7 +188,11 @@ func addBlob(t *testing.T, store *testStorer, hash string) { // Unit Tests for RemoveSnapshot // ============================================================================ -func TestRemoveSnapshot_LocalOnly(t *testing.T) { +// TestRemoveSnapshot_LocalOnly_PreservesRemote confirms that +// --local-only opts out of the remote-cleanup half: the snapshot is +// removed from the local index, but the remote metadata and blobs are +// untouched. +func TestRemoveSnapshot_LocalOnly_PreservesRemote(t *testing.T) { log.Initialize(log.Config{}) store := newTestStorer() @@ -199,49 +203,61 @@ func TestRemoveSnapshot_LocalOnly(t *testing.T) { tv := vaultik.NewForTesting(store) + opts := &vaultik.RemoveOptions{Force: true, LocalOnly: true} + result, err := tv.RemoveSnapshot("snapshot-001", opts) + + require.NoError(t, err) + assert.Equal(t, "snapshot-001", result.SnapshotID) + assert.False(t, result.RemoteRemoved) + assert.Equal(t, 0, result.BlobsDeleted) + + assert.True(t, store.hasKey("blobs/aa/aa/"+blobA)) + assert.True(t, store.hasKey(remoteKeyPath("snapshot-001", "manifest.json.zst"))) + + assert.Contains(t, tv.Stdout.String(), "Removed snapshot 'snapshot-001' from local database") +} + +// TestRemoveSnapshot_DefaultFullCleanup is the canonical case: no +// flags. The local-DB entry is removed, the snapshot's metadata is +// removed from the destination store, and any blob that was unique to +// this snapshot (i.e. not referenced by any remaining manifest) is +// pruned from the destination store too. +func TestRemoveSnapshot_DefaultFullCleanup(t *testing.T) { + log.Initialize(log.Config{}) + + store := newTestStorer() + + blobUnique := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + blobShared := "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + + addManifest(t, store, "snapshot-001", []string{blobUnique, blobShared}) + addManifest(t, store, "snapshot-002", []string{blobShared}) + addBlob(t, store, blobUnique) + addBlob(t, store, blobShared) + + tv := vaultik.NewForTesting(store) + opts := &vaultik.RemoveOptions{Force: true} result, err := tv.RemoveSnapshot("snapshot-001", opts) - require.NoError(t, err) - assert.Equal(t, "snapshot-001", result.SnapshotID) - assert.False(t, result.RemoteRemoved) - - // Blobs should NOT be deleted (that's what prune is for) - assert.True(t, store.hasKey("blobs/aa/aa/"+blobA)) - // Remote metadata should NOT be deleted (no --remote flag) - assert.True(t, store.hasKey(remoteKeyPath("snapshot-001", "manifest.json.zst"))) - - // Verify output - assert.Contains(t, tv.Stdout.String(), "Removed snapshot 'snapshot-001' from local database") -} - -func TestRemoveSnapshot_WithRemote(t *testing.T) { - log.Initialize(log.Config{}) - - store := newTestStorer() - - blobA := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" - addManifest(t, store, "snapshot-001", []string{blobA}) - addBlob(t, store, blobA) - - tv := vaultik.NewForTesting(store) - - opts := &vaultik.RemoveOptions{Force: true, Remote: true} - result, err := tv.RemoveSnapshot("snapshot-001", opts) - require.NoError(t, err) assert.Equal(t, "snapshot-001", result.SnapshotID) assert.True(t, result.RemoteRemoved) + assert.Equal(t, 1, result.BlobsDeleted, "exactly the unique blob should be deleted") - // Blobs should NOT be deleted - assert.True(t, store.hasKey("blobs/aa/aa/"+blobA)) - // Remote metadata SHOULD be deleted + // Snapshot-001's metadata gone. assert.False(t, store.hasKey(remoteKeyPath("snapshot-001", "manifest.json.zst"))) + // Snapshot-002 untouched. + assert.True(t, store.hasKey(remoteKeyPath("snapshot-002", "manifest.json.zst"))) + // Unique blob deleted. + assert.False(t, store.hasKey("blobs/aa/aa/"+blobUnique)) + // Shared blob preserved (still referenced by snapshot-002). + assert.True(t, store.hasKey("blobs/bb/bb/"+blobShared)) - // Verify output mentions prune - assert.Contains(t, tv.Stdout.String(), "Removed snapshot 'snapshot-001' from local database") - assert.Contains(t, tv.Stdout.String(), "Removed snapshot metadata from remote storage") - assert.Contains(t, tv.Stdout.String(), "Run 'vaultik prune' to remove orphaned blobs") + out := tv.Stdout.String() + assert.Contains(t, out, "Removed snapshot 'snapshot-001' from local database") + assert.Contains(t, out, "Removed snapshot metadata from remote storage") + assert.Contains(t, out, "Removed 1 unreferenced blob") } func TestRemoveSnapshot_DryRun(t *testing.T) { @@ -257,18 +273,16 @@ func TestRemoveSnapshot_DryRun(t *testing.T) { tv := vaultik.NewForTesting(store) - opts := &vaultik.RemoveOptions{Force: true, DryRun: true, Remote: true} + opts := &vaultik.RemoveOptions{Force: true, DryRun: true} result, err := tv.RemoveSnapshot("snapshot-001", opts) require.NoError(t, err) assert.True(t, result.DryRun) - // Nothing should be deleted assert.Equal(t, initialCount, store.keyCount()) assert.True(t, store.hasKey("blobs/aa/aa/"+blobA)) assert.True(t, store.hasKey(remoteKeyPath("snapshot-001", "manifest.json.zst"))) - // Verify dry run message assert.Contains(t, tv.Stdout.String(), "[Dry run - no changes made]") } @@ -300,22 +314,22 @@ func TestRemoveAllSnapshots_WithForce(t *testing.T) { tv := vaultik.NewForTesting(store) - opts := &vaultik.RemoveOptions{All: true, Force: true, Remote: true} + opts := &vaultik.RemoveOptions{All: true, Force: true} result, err := tv.RemoveAllSnapshots(opts) require.NoError(t, err) assert.Len(t, result.SnapshotsRemoved, 2) assert.True(t, result.RemoteRemoved) + assert.Equal(t, 1, result.BlobsDeleted) - // Blobs should NOT be deleted - assert.True(t, store.hasKey("blobs/aa/aa/"+blobA)) - // Remote metadata SHOULD be deleted + assert.False(t, store.hasKey("blobs/aa/aa/"+blobA)) assert.False(t, store.hasKey(remoteKeyPath("snapshot-001", "manifest.json.zst"))) assert.False(t, store.hasKey(remoteKeyPath("snapshot-002", "manifest.json.zst"))) - // Verify output - assert.Contains(t, tv.Stdout.String(), "Removed 2 snapshot(s)") - assert.Contains(t, tv.Stdout.String(), "Run 'vaultik prune' to remove orphaned blobs") + out := tv.Stdout.String() + assert.Contains(t, out, "Removed 2 snapshot(s)") + assert.Contains(t, out, "Removed snapshot metadata from remote storage") + assert.Contains(t, out, "Removed 1 unreferenced blob") } func TestRemoveAllSnapshots_DryRun(t *testing.T) { @@ -329,20 +343,18 @@ func TestRemoveAllSnapshots_DryRun(t *testing.T) { tv := vaultik.NewForTesting(store) - // --remote is required to enumerate orphan remote keys; without - // it, RemoveAll only acts on local snapshots, and NewForTesting - // has no local DB. - opts := &vaultik.RemoveOptions{All: true, Force: true, DryRun: true, Remote: true} + // Default (no LocalOnly) enumerates the orphan remote keys, which + // matches what NewForTesting has — local DB is empty, so the two + // addManifest calls land as orphan remote keys. + opts := &vaultik.RemoveOptions{All: true, Force: true, DryRun: true} result, err := tv.RemoveAllSnapshots(opts) require.NoError(t, err) assert.True(t, result.DryRun) assert.Len(t, result.SnapshotsRemoved, 2) - // Nothing should be deleted assert.Equal(t, initialCount, store.keyCount()) - // Verify dry run message assert.Contains(t, tv.Stdout.String(), "[Dry run - no changes made]") } diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index 2c6f56a..d678e42 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -973,11 +973,11 @@ func (v *Vaultik) syncWithRemote() error { // RemoveOptions contains options for the snapshot remove command type RemoveOptions struct { - Force bool - DryRun bool - JSON bool - Remote bool // Also remove metadata from remote storage - All bool // Remove all snapshots (requires Force) + Force bool + DryRun bool + JSON bool + LocalOnly bool // Skip remote cleanup; only touch the local index + All bool // Remove all snapshots (requires Force) } // RemoveResult contains the result of a snapshot removal @@ -985,11 +985,17 @@ type RemoveResult struct { SnapshotID string `json:"snapshot_id,omitempty"` SnapshotsRemoved []string `json:"snapshots_removed,omitempty"` RemoteRemoved bool `json:"remote_removed,omitempty"` + BlobsDeleted int `json:"blobs_deleted,omitempty"` + BytesFreed int64 `json:"bytes_freed,omitempty"` DryRun bool `json:"dry_run,omitempty"` } -// RemoveSnapshot removes a snapshot from the local database and optionally from remote storage -// Note: This does NOT remove blobs. Use 'vaultik prune' to remove orphaned blobs. +// RemoveSnapshot removes a snapshot from the local index database and, +// unless LocalOnly is set, also strips the snapshot's metadata from the +// destination store and prunes any blobs that are no longer referenced +// by any remaining remote snapshot. When the remote is unreachable the +// command still completes the local-DB removal and warns; callers can +// retry remote cleanup later with `vaultik prune`. func (v *Vaultik) RemoveSnapshot(snapshotID string, opts *RemoveOptions) (*RemoveResult, error) { result := &RemoveResult{ SnapshotID: snapshotID, @@ -999,8 +1005,8 @@ func (v *Vaultik) RemoveSnapshot(snapshotID string, opts *RemoveOptions) (*Remov result.DryRun = true if !opts.JSON { v.printfStdout("Would remove snapshot: %s\n", snapshotID) - if opts.Remote { - v.printlnStdout("Would also remove from remote storage") + if !opts.LocalOnly { + v.printlnStdout("Would also remove metadata and any unique blobs from remote storage") } v.printlnStdout("[Dry run - no changes made]") } @@ -1010,12 +1016,11 @@ func (v *Vaultik) RemoveSnapshot(snapshotID string, opts *RemoveOptions) (*Remov return result, nil } - // Confirm unless --force is used (skip in JSON mode - require --force) if !opts.Force && !opts.JSON { - if opts.Remote { - v.printfStdout("Remove snapshot '%s' from local database and remote storage? [y/N] ", snapshotID) + if opts.LocalOnly { + v.printfStdout("Remove snapshot '%s' from local database (remote untouched)? [y/N] ", snapshotID) } else { - v.printfStdout("Remove snapshot '%s' from local database? [y/N] ", snapshotID) + v.printfStdout("Remove snapshot '%s' from local database AND remote storage (including any blobs unique to this snapshot)? [y/N] ", snapshotID) } var confirm string if _, err := v.scanStdin(&confirm); err != nil { @@ -1030,45 +1035,90 @@ func (v *Vaultik) RemoveSnapshot(snapshotID string, opts *RemoveOptions) (*Remov log.Info("Removing snapshot from local database", "snapshot_id", snapshotID) - // Remove from local database if err := v.deleteSnapshotFromLocalDB(snapshotID); err != nil { return result, fmt.Errorf("removing from local database: %w", err) } - // If --remote, also remove from remote storage - if opts.Remote { + if !opts.LocalOnly { log.Info("Removing snapshot metadata from remote storage", "snapshot_id", snapshotID) - if err := v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID)); err != nil { - return result, fmt.Errorf("removing from remote storage: %w", err) + remoteKey := snapshot.RemoteSnapshotKey(snapshotID) + if err := v.deleteRemoteSnapshotByKey(remoteKey); err != nil { + // Per design: warn-and-proceed; the local-DB removal has + // already happened, so the user can retry remote cleanup + // with `vaultik prune` later. + log.Warn("Could not remove snapshot metadata from remote storage", "error", err) + if v.UI != nil { + v.UI.Warning("Could not remove snapshot metadata from remote: %v. Run 'vaultik prune' once the remote is reachable to finish cleanup.", err) + } + } else { + result.RemoteRemoved = true + blobsDeleted, bytesFreed, pruneErr := v.pruneUnreferencedBlobsAfterRemoval() + if pruneErr != nil { + log.Warn("Failed to prune unreferenced blobs after snapshot removal", "error", pruneErr) + if v.UI != nil { + v.UI.Warning("Snapshot metadata removed, but blob cleanup failed: %v. Run 'vaultik prune' to retry.", pruneErr) + } + } else { + result.BlobsDeleted = blobsDeleted + result.BytesFreed = bytesFreed + } } - result.RemoteRemoved = true } - // Clean up the local rows that just became orphaned (files, chunks, - // blob_chunks, blobs no longer referenced by any snapshot). This - // used to be a separate `vaultik snapshot prune` step; running it - // inline means `snapshot remove` leaves no ghost rows behind. if v.SnapshotManager != nil { if err := v.SnapshotManager.CleanupOrphanedData(v.ctx); err != nil { log.Warn("Failed to clean up orphaned local data after removal", "error", err) } } - // Output result if opts.JSON { return result, v.outputRemoveJSON(result) } - // Print summary v.printfStdout("Removed snapshot '%s' from local database\n", snapshotID) - if opts.Remote { + if !opts.LocalOnly && result.RemoteRemoved { v.printlnStdout("Removed snapshot metadata from remote storage") - v.printlnStdout("\nNote: Remote blobs were not removed. Run 'vaultik prune' to remove orphaned blobs.") + if result.BlobsDeleted > 0 { + v.printfStdout("Removed %d unreferenced blob(s) (%s freed)\n", + result.BlobsDeleted, humanize.Bytes(uint64(result.BytesFreed))) + } else { + v.printlnStdout("No blobs unique to this snapshot were found.") + } } return result, nil } +// pruneUnreferencedBlobsAfterRemoval deletes blobs no longer referenced +// by any remaining remote manifest. Used by RemoveSnapshot / +// RemoveAllSnapshots after metadata has been stripped from the +// destination store; with the just-removed snapshot's manifest gone, +// any blobs that were only referenced by it become unreferenced and +// are swept here. +func (v *Vaultik) pruneUnreferencedBlobsAfterRemoval() (int, int64, error) { + referenced, err := v.collectReferencedBlobs() + if err != nil { + return 0, 0, fmt.Errorf("collecting referenced blobs: %w", err) + } + + allBlobs, err := v.listAllRemoteBlobs() + if err != nil { + return 0, 0, fmt.Errorf("listing remote blobs: %w", err) + } + + unreferenced, totalSize := v.findUnreferencedBlobs(allBlobs, referenced) + if len(unreferenced) == 0 { + return 0, 0, nil + } + + result := &PruneBlobsResult{BlobsFound: len(unreferenced)} + log.Info("Pruning unreferenced blobs after snapshot removal", + "count", len(unreferenced), + "size", humanize.Bytes(uint64(totalSize))) + v.deleteUnreferencedBlobs(unreferenced, allBlobs, result) + return result.BlobsDeleted, result.BytesFreed, nil +} + // RemoveAllSnapshots removes every snapshot known to the local // database from the local index, and (with --remote) every snapshot // metadata directory in remote storage. Both sides are processed so a @@ -1176,7 +1226,7 @@ func (v *Vaultik) listAllRemoteSnapshotKeys() ([]string, error) { func (v *Vaultik) handleRemoveAllDryRun(localSnaps, orphanRemoteKeys []string, opts *RemoveOptions) (*RemoveResult, error) { result := &RemoveResult{DryRun: true} result.SnapshotsRemoved = append(result.SnapshotsRemoved, localSnaps...) - if opts.Remote { + if !opts.LocalOnly { result.SnapshotsRemoved = append(result.SnapshotsRemoved, orphanRemoteKeys...) } if !opts.JSON { @@ -1184,13 +1234,16 @@ func (v *Vaultik) handleRemoveAllDryRun(localSnaps, orphanRemoteKeys []string, o for _, id := range localSnaps { v.printfStdout(" %s\n", id) } - if opts.Remote && len(orphanRemoteKeys) > 0 { - v.printfStdout("Would also remove %d orphan remote snapshot key(s):\n", len(orphanRemoteKeys)) - for _, key := range orphanRemoteKeys { - v.printfStdout(" %s\n", key) + if !opts.LocalOnly { + if len(orphanRemoteKeys) > 0 { + v.printfStdout("Would also remove %d orphan remote snapshot key(s):\n", len(orphanRemoteKeys)) + for _, key := range orphanRemoteKeys { + v.printfStdout(" %s\n", key) + } + } else { + v.printlnStdout("Would also remove from remote storage") } - } else if opts.Remote { - v.printlnStdout("Would also remove from remote storage") + v.printlnStdout("Would then prune all unreferenced blobs from remote storage") } v.printlnStdout("[Dry run - no changes made]") } @@ -1200,11 +1253,11 @@ func (v *Vaultik) handleRemoveAllDryRun(localSnaps, orphanRemoteKeys []string, o return result, nil } -// executeRemoveAll deletes every local snapshot (and, with --remote, -// every corresponding remote metadata directory plus any orphan remote -// keys that don't match a local snapshot). +// executeRemoveAll deletes every local snapshot and, unless LocalOnly +// is set, every corresponding remote metadata directory plus any +// orphan remote keys, then prunes the resulting set of unreferenced +// blobs from the destination store. func (v *Vaultik) executeRemoveAll(localSnaps, orphanRemoteKeys []string, opts *RemoveOptions) (*RemoveResult, error) { - // --all requires --force if !opts.Force { return nil, fmt.Errorf("--all requires --force") } @@ -1212,6 +1265,7 @@ func (v *Vaultik) executeRemoveAll(localSnaps, orphanRemoteKeys []string, opts * log.Info("Removing all snapshots", "local_count", len(localSnaps), "orphan_remote_count", len(orphanRemoteKeys)) result := &RemoveResult{} + remoteErrors := 0 for _, snapshotID := range localSnaps { log.Info("Removing snapshot", "snapshot_id", snapshotID) @@ -1220,33 +1274,44 @@ func (v *Vaultik) executeRemoveAll(localSnaps, orphanRemoteKeys []string, opts * continue } - if opts.Remote { + if !opts.LocalOnly { if err := v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID)); err != nil { - log.Error("Failed to remove from remote", "snapshot_id", snapshotID, "error", err) - continue + log.Warn("Failed to remove snapshot metadata from remote", "snapshot_id", snapshotID, "error", err) + remoteErrors++ } } result.SnapshotsRemoved = append(result.SnapshotsRemoved, snapshotID) } - if opts.Remote { + if !opts.LocalOnly { for _, key := range orphanRemoteKeys { log.Info("Removing orphan remote snapshot", "remote_key", key) if err := v.deleteRemoteSnapshotByKey(key); err != nil { - log.Error("Failed to remove orphan from remote", "remote_key", key, "error", err) + log.Warn("Failed to remove orphan from remote", "remote_key", key, "error", err) + remoteErrors++ continue } result.SnapshotsRemoved = append(result.SnapshotsRemoved, key) } + + if remoteErrors == 0 { + result.RemoteRemoved = true + blobsDeleted, bytesFreed, pruneErr := v.pruneUnreferencedBlobsAfterRemoval() + if pruneErr != nil { + log.Warn("Failed to prune unreferenced blobs after bulk removal", "error", pruneErr) + if v.UI != nil { + v.UI.Warning("Bulk metadata removal succeeded, but blob cleanup failed: %v. Run 'vaultik prune' to retry.", pruneErr) + } + } else { + result.BlobsDeleted = blobsDeleted + result.BytesFreed = bytesFreed + } + } else if v.UI != nil { + v.UI.Warning("Some remote metadata deletions failed; skipping automatic blob prune. Run 'vaultik prune' once the remote is healthy.") + } } - if opts.Remote { - result.RemoteRemoved = true - } - - // Clean up everything that just became orphaned locally so the - // index database doesn't carry 39k ghost rows after a wipe. if v.SnapshotManager != nil { if err := v.SnapshotManager.CleanupOrphanedData(v.ctx); err != nil { log.Warn("Failed to clean up orphaned local data after bulk removal", "error", err) @@ -1258,9 +1323,14 @@ func (v *Vaultik) executeRemoveAll(localSnaps, orphanRemoteKeys []string, opts * } v.printfStdout("Removed %d snapshot(s)\n", len(result.SnapshotsRemoved)) - if opts.Remote { + if !opts.LocalOnly && result.RemoteRemoved { v.printlnStdout("Removed snapshot metadata from remote storage") - v.printlnStdout("\nNote: Remote blobs were not removed. Run 'vaultik prune' to remove orphaned blobs.") + if result.BlobsDeleted > 0 { + v.printfStdout("Removed %d unreferenced blob(s) (%s freed)\n", + result.BlobsDeleted, humanize.Bytes(uint64(result.BytesFreed))) + } else { + v.printlnStdout("No unreferenced blobs were found.") + } } return result, nil