Author SHA1 Message Date
clawbot 2c4af6d0df Stop snapshot purge when a snapshot cannot leave the local index (closes #337)
check / check (push) Waiting to run
snapshot purge, and the purge step of snapshot create --prune, logged a
failed local index delete, then deleted that snapshot's metadata from
the destination store anyway and exited 0, leaving a local record whose
metadata was gone. It now stops at that snapshot with an error, before
touching its metadata, so the command exits non-zero; snapshots already
purged stay purged. This matches how the same loop already handles a
failed metadata delete.

Model: opus-5-5
2026-10-13 19:35:57 +00:00
3 changed files with 98 additions and 4 deletions
+3 -1
View File
@@ -339,7 +339,9 @@ local index alone, and still exits zero.
**`snapshot purge`**: Remove old snapshots based on criteria. Retention is
per-snapshot-name (`--keep-latest` keeps the latest of each name, not the
latest globally).
latest globally). If a snapshot cannot be removed from the local index, the
command stops there, leaves that snapshot's metadata on the destination store
and exits non-zero; `snapshot create --prune` fails the same way.
* `--keep-latest`: Keep only the most recent snapshot of each name
* `--older-than <duration>`: Remove snapshots older than duration (e.g. `30d`,
`4w`, `6mo`, `1y`; `m` is minutes, `mo` is months)
+90
View File
@@ -0,0 +1,90 @@
package cli //nolint:testpackage // shares runEntry and the hermetic config helpers
import (
"context"
"database/sql"
"os"
"path/filepath"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/snapshot"
"sneak.berlin/go/vaultik/internal/types"
)
// The `snapshot purge` command word and flags used below.
const (
cmdPurge = "purge"
flagKeepLatest = "--keep-latest"
flagForce = "--force"
)
// TestEntrySnapshotPurgeKeepsMetadataWhenLocalIndexDeleteFails makes
// removing a snapshot from the local index fail and checks that
// `snapshot purge` exits non-zero and leaves that snapshot's metadata on
// the destination store. It used to log the error, delete the metadata
// anyway and exit 0.
//
//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
func TestEntrySnapshotPurgeKeepsMetadataWhenLocalIndexDeleteFails(t *testing.T) {
ctx := context.Background()
configPath := writeHermeticPruneConfig(t, false)
// writeHermeticPruneConfig puts the index and the store directory
// beside the config file.
dir := filepath.Dir(configPath)
// Two snapshots of the same name: --keep-latest purges the older.
olderStart := time.Date(2026, 5, 1, 9, 0, 0, 0, time.UTC)
newerStart := olderStart.Add(time.Hour)
olderID := "test-host_test_" + olderStart.Format(time.RFC3339)
newerID := "test-host_test_" + newerStart.Format(time.RFC3339)
db, err := database.New(ctx, filepath.Join(dir, "index.sqlite"))
require.NoError(t, err)
repos := database.NewRepositories(db)
manifestPaths := make(map[string]string)
for id, startedAt := range map[string]time.Time{
olderID: olderStart,
newerID: newerStart,
} {
completedAt := startedAt.Add(time.Minute)
snap := &database.Snapshot{
ID: types.SnapshotID(id),
Hostname: "test-host",
VaultikVersion: "test",
StartedAt: startedAt,
CompletedAt: &completedAt,
}
err = repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
return repos.Snapshots.Create(ctx, tx, snap)
})
require.NoError(t, err)
manifestPaths[id] = filepath.Join(dir, "store", "metadata",
snapshot.RemoteSnapshotKey(id), "manifest.json.zst")
require.NoError(t, os.MkdirAll(filepath.Dir(manifestPaths[id]), 0o750))
require.NoError(t, os.WriteFile(manifestPaths[id], []byte("stub"), 0o600))
}
_, err = db.Conn().ExecContext(ctx, `
CREATE TRIGGER refuse_snapshot_delete BEFORE DELETE ON snapshots
BEGIN SELECT RAISE(ABORT, 'simulated index error'); END`)
require.NoError(t, err)
require.NoError(t, db.Close())
code, _, stderr := runEntry(t,
flagConfig, configPath, cmdSnapshot, cmdPurge, flagKeepLatest, flagForce)
assert.Equal(t, 1, code)
assert.Contains(t, stderr, "simulated index error")
assert.FileExists(t, manifestPaths[olderID],
"a snapshot still in the local index keeps its metadata")
}
+5 -3
View File
@@ -627,7 +627,8 @@ func selectSnapshotsToPurge(
}
// confirmAndExecutePurge shows deletion candidates, confirms with the
// user, and deletes snapshots.
// user, and deletes snapshots. It stops with an error at the first
// snapshot it cannot delete.
func (v *Vaultik) confirmAndExecutePurge(
toDelete []SnapshotInfo, force, quiet bool,
) error {
@@ -669,10 +670,11 @@ func (v *Vaultik) confirmAndExecutePurge(
snapshotID := snap.ID.String()
log.Info("Deleting snapshot", "id", snapshotID)
// Stop before the remote delete: a snapshot still in the local
// index must keep its metadata on the destination store.
err := v.deleteSnapshotFromLocalDB(snapshotID)
if err != nil {
log.Error("Failed to delete from local database",
"snapshot_id", snapshotID, "error", err)
return fmt.Errorf("deleting snapshot %s from local index: %w", snapshotID, err)
}
_, err = v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID))