Compare commits

..
Author SHA1 Message Date
sneak c0c79cd0a2 Make prune and remote nuke fail when a delete fails (closes #290)
check / check (push) Waiting to run
prune, and the prune step of snapshot create --prune, counted a blob
it could not delete and still exited 0. It now prints its summary or
--json document and then returns an error, which replaces the
"Failed to delete N blob(s)" line.

remote nuke only warned when a snapshot's metadata could not be
deleted, advised vaultik prune, which never removes metadata, and then
reported the store empty. It now stops with an error, does not report
the store empty, and says to run vaultik remote nuke --force again. It
checks that the local index belongs to the configured destination
before deleting anything, and that error carries no such advice. A
successful nuke no longer advises prune.

Model: opus-5-5
2026-10-13 05:44:12 +00:00
10 changed files with 282 additions and 147 deletions
+6 -3
View File
@@ -384,10 +384,12 @@ manifest on the destination store to compute the still-referenced blob
set and delete any blob not in that set. Step (3) reads all remote set and delete any blob not in that set. Step (3) reads all remote
manifests — network cost scales with the number of snapshots. `snapshot manifests — network cost scales with the number of snapshots. `snapshot
create --prune` runs the same cleanup automatically; this is the create --prune` runs the same cleanup automatically; this is the
manual entry point for the same work. manual entry point for the same work. If a blob cannot be deleted, the
command (and `snapshot create --prune`) exits non-zero.
* `--force`: Skip confirmation prompt * `--force`: Skip confirmation prompt
* `--json`: Output stats as JSON. Also skips the confirmation prompt, as * `--json`: Output stats as JSON. Also skips the confirmation prompt, as
`--force` does. `--force` does. A blob that cannot be deleted is counted in
`blobs_failed`, and the document is printed before the non-zero exit.
**`info`**: Display system configuration, storage settings, encryption **`info`**: Display system configuration, storage settings, encryption
recipients, and local database statistics. recipients, and local database statistics.
@@ -407,7 +409,8 @@ snapshot's blob count and blob size unknown, `null` in `--json`.
**`remote nuke`**: Delete every snapshot's metadata and every blob from the **`remote nuke`**: Delete every snapshot's metadata and every blob from the
backup destination store, leaving the bucket prefix empty. Destructive and backup destination store, leaving the bucket prefix empty. Destructive and
irreversible. This is the single supported way to wipe the entire irreversible. This is the single supported way to wipe the entire
destination store. destination store. If anything cannot be deleted, the command exits
non-zero; running it again deletes what is left.
* `--force`: Required to confirm destruction. * `--force`: Required to confirm destruction.
**`database delete`**: Delete the local SQLite state database file **`database delete`**: Delete the local SQLite state database file
+10 -8
View File
@@ -22,14 +22,16 @@ the tag exists and is exercised; what is left is merging `next` to
# Completed Steps # Completed Steps
- 2026-10-13: Made an S3 listing that the server cuts short an error - 2026-10-13: Made `prune` and `remote nuke` exit non-zero when a delete
([issue #297](https://git.eeqj.de/sneak/vaultik/issues/297)). A page fails ([issue #290](https://git.eeqj.de/sneak/vaultik/issues/290)).
marked truncated with no continuation token, as a server answering V2 `prune`, and the prune step of `snapshot create --prune`, counted a
listings V1-style sends, ended the listing without an error, so blob it could not delete and still exited 0; it now prints its summary
`prune` saw only the snapshots on the first page and could delete or `--json` document and then fails. `remote nuke` warned about a
blobs the rest still use. Such a page is now a listing error, and snapshot's metadata it could not delete, said to run `vaultik prune`,
`prune`, `snapshot list` and `remote nuke` handle it as a destination which never removes metadata, and then reported the store empty. It
that cannot be listed. now fails without that report and says to run `vaultik remote nuke
--force` again, and a successful nuke no longer advises `prune` for
blobs it deletes itself.
- 2026-10-13: Made `snapshot remove` fail for a snapshot that does not - 2026-10-13: Made `snapshot remove` fail for a snapshot that does not
exist ([issue #293](https://git.eeqj.de/sneak/vaultik/issues/293)). exist ([issue #293](https://git.eeqj.de/sneak/vaultik/issues/293)).
+2 -29
View File
@@ -18,13 +18,6 @@ import (
"github.com/aws/smithy-go/logging" "github.com/aws/smithy-go/logging"
) )
// ErrNoContinuationToken is returned by ListObjects and ListObjectsStream
// when the server marks a page of a listing truncated but sends no
// continuation token to fetch the rest with, as a server answering V2
// listings V1-style does.
var ErrNoContinuationToken = errors.New(
"server marked the listing truncated but sent no continuation token")
// Client wraps the AWS S3 client for vaultik operations. // Client wraps the AWS S3 client for vaultik operations.
// It provides a simplified interface for S3 operations with automatic // It provides a simplified interface for S3 operations with automatic
// prefix handling and connection management. All operations are performed // prefix handling and connection management. All operations are performed
@@ -221,7 +214,7 @@ func (c *Client) ListObjects(ctx context.Context, prefix string) ([]string, erro
}) })
for paginator.HasMorePages() { for paginator.HasMorePages() {
page, err := nextListPage(ctx, paginator) page, err := paginator.NextPage(ctx)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -242,26 +235,6 @@ func (c *Client) ListObjects(ctx context.Context, prefix string) ([]string, erro
return keys, nil return keys, nil
} }
// nextListPage fetches the next page of a listing. The SDK paginator takes
// a page marked truncated without a continuation token for the last page
// and stops without an error, so such a page is ErrNoContinuationToken
// here instead.
func nextListPage(
ctx context.Context, paginator *s3.ListObjectsV2Paginator,
) (*s3.ListObjectsV2Output, error) {
page, err := paginator.NextPage(ctx)
if err != nil {
return nil, err
}
truncated := aws.ToBool(page.IsTruncated)
if truncated && aws.ToString(page.NextContinuationToken) == "" {
return nil, ErrNoContinuationToken
}
return page, nil
}
// HeadObject checks if an object exists in S3. // HeadObject checks if an object exists in S3.
// Returns true if the object exists, false otherwise. // Returns true if the object exists, false otherwise.
// The key is automatically prefixed with the configured prefix. // The key is automatically prefixed with the configured prefix.
@@ -327,7 +300,7 @@ func (c *Client) ListObjectsStream(
}) })
for paginator.HasMorePages() { for paginator.HasMorePages() {
page, err := nextListPage(ctx, paginator) page, err := paginator.NextPage(ctx)
if err != nil { if err != nil {
ch <- ObjectInfo{Err: err} ch <- ObjectInfo{Err: err}
+23 -9
View File
@@ -1,15 +1,16 @@
// Package faultstore provides a storage.Storer wrapper that injects // Package faultstore provides a storage.Storer wrapper that injects
// faults on demand, so tests can reproduce the failure modes a real // faults on demand, so tests can reproduce the failure modes a real
// backend exhibits: an upload that fails partway, a backend that reports // backend exhibits: an upload that fails partway, a backend that reports
// success while storing nothing, and reads that return corrupt or // success while storing nothing, reads that return corrupt or truncated
// truncated bytes. It is the seam called for by the fault-injection // bytes, and a delete the backend refuses. It is the seam called for by
// tests (sneak/vaultik issue 72) and is meant to be reused by future // the fault-injection tests (sneak/vaultik issue 72) and is meant to be
// tests rather than re-implemented per case. // reused by future tests rather than re-implemented per case.
// //
// The wrapper delegates every method to the inner Storer. Two hooks // The wrapper delegates every method to the inner Storer. Three hooks
// change that: OnPut decides the fate of each write, and OnGet decides // change that: OnPut decides the fate of each write, OnGet decides how
// how each read's bytes are returned. Both are keyed by the object key, // each read's bytes are returned, and OnDelete decides whether each
// so a test can fault only blobs, only metadata, or a single object. // delete fails. All are keyed by the object key, so a test can fault
// only blobs, only metadata, or a single object.
package faultstore package faultstore
import ( import (
@@ -25,6 +26,10 @@ import (
// ErrInjectedUpload is returned by a Put the OnPut hook chose to fail. // ErrInjectedUpload is returned by a Put the OnPut hook chose to fail.
var ErrInjectedUpload = errors.New("faultstore: injected upload failure") var ErrInjectedUpload = errors.New("faultstore: injected upload failure")
// ErrInjectedDelete is returned by a Delete the OnDelete hook chose to
// fail.
var ErrInjectedDelete = errors.New("faultstore: injected delete failure")
// PutAction is the disposition OnPut assigns to a write. // PutAction is the disposition OnPut assigns to a write.
type PutAction int type PutAction int
@@ -66,6 +71,11 @@ type Storer struct {
// OnGet, when set, is consulted for every Get with the object key // OnGet, when set, is consulted for every Get with the object key
// and damages the returned bytes accordingly. // and damages the returned bytes accordingly.
OnGet func(key string) GetFault OnGet func(key string) GetFault
// OnDelete, when set, is consulted before every Delete with the
// object key. Returning true fails the delete and leaves the object
// in place.
OnDelete func(key string) bool
} }
// New wraps inner. inner must be non-nil. // New wraps inner. inner must be non-nil.
@@ -154,8 +164,12 @@ func (f *Storer) Stat(ctx context.Context, key string) (*storage.ObjectInfo, err
return f.inner.Stat(ctx, key) return f.inner.Stat(ctx, key)
} }
// Delete delegates unchanged. // Delete removes the object unless OnDelete fails the delete.
func (f *Storer) Delete(ctx context.Context, key string) error { func (f *Storer) Delete(ctx context.Context, key string) error {
if f.OnDelete != nil && f.OnDelete(key) {
return fmt.Errorf("%w for %q", ErrInjectedDelete, key)
}
return f.inner.Delete(ctx, key) return f.inner.Delete(ctx, key)
} }
+3 -69
View File
@@ -4,7 +4,6 @@ import (
"bytes" "bytes"
"context" "context"
"errors" "errors"
"io"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"slices" "slices"
@@ -23,16 +22,13 @@ import (
// s3TestBucket is the bucket created for each in-process S3 server. // s3TestBucket is the bucket created for each in-process S3 server.
const s3TestBucket = "test-bucket" const s3TestBucket = "test-bucket"
// Credentials for the tests' S3 clients. The test servers accept any. // Credentials for the tests that build a storer from a config.Config. The
// in-process S3 server accepts any.
const ( const (
s3TestAccessKeyID = "key" s3TestAccessKeyID = "key"
s3TestSecretAccessKey = "secret" s3TestSecretAccessKey = "secret"
) )
// s3TestRegion is the region of the S3 clients built here with
// s3.NewClient. The test servers ignore it.
const s3TestRegion = "us-east-1"
// newS3Storer builds an s3:// backend backed by a fresh in-process // newS3Storer builds an s3:// backend backed by a fresh in-process
// S3 server. It reuses the same in-memory S3 harness (gofakes3 + s3mem // S3 server. It reuses the same in-memory S3 harness (gofakes3 + s3mem
// over httptest) that internal/s3 and the not-found regression test use, // over httptest) that internal/s3 and the not-found regression test use,
@@ -59,7 +55,7 @@ func newS3Storer(t *testing.T) storage.Storer {
Bucket: s3TestBucket, Bucket: s3TestBucket,
AccessKeyID: "test", AccessKeyID: "test",
SecretAccessKey: "test", SecretAccessKey: "test",
Region: s3TestRegion, Region: "us-east-1",
}) })
if err != nil { if err != nil {
t.Fatalf("new client: %v", err) t.Fatalf("new client: %v", err)
@@ -97,68 +93,6 @@ func TestS3StorerMissingKeyMapsToErrNotFound(t *testing.T) {
} }
} }
// TestS3ListingCutShortIsAnError checks that List and ListStream fail on a
// listing page marked truncated that carries no continuation token, as a
// server answering V2 listings V1-style sends it. Taken as the last page,
// it would hide every later key, and prune would treat the blobs of the
// snapshots listed there as unreferenced.
func TestS3ListingCutShortIsAnError(t *testing.T) {
t.Parallel()
const (
prefix = "metadata/"
truncatedPage = `<?xml version="1.0" encoding="UTF-8"?>
<ListBucketResult xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
<Name>test-bucket</Name>
<Prefix>metadata/</Prefix>
<KeyCount>1</KeyCount>
<MaxKeys>1</MaxKeys>
<IsTruncated>true</IsTruncated>
<Contents>
<Key>metadata/snap/manifest.json.zst</Key>
<Size>8</Size>
</Contents>
</ListBucketResult>`
)
srv := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
_, _ = io.WriteString(w, truncatedPage)
}))
t.Cleanup(srv.Close)
client, err := s3.NewClient(context.Background(), s3.Config{
Endpoint: srv.URL,
Bucket: s3TestBucket,
AccessKeyID: s3TestAccessKeyID,
SecretAccessKey: s3TestSecretAccessKey,
Region: s3TestRegion,
})
if err != nil {
t.Fatalf("new client: %v", err)
}
storer := storage.NewS3Storer(client)
ctx := context.Background()
keys, err := storer.List(ctx, prefix)
if !errors.Is(err, s3.ErrNoContinuationToken) {
t.Errorf("List = %q, %v; want ErrNoContinuationToken", keys, err)
}
var streamErr error
for obj := range storer.ListStream(ctx, prefix) {
if obj.Err != nil {
streamErr = obj.Err
}
}
if !errors.Is(streamErr, s3.ErrNoContinuationToken) {
t.Errorf("ListStream error = %v, want ErrNoContinuationToken", streamErr)
}
}
// TestS3URLPrefixKeyLayout pins the bucket keys an s3:// URL reads and // TestS3URLPrefixKeyLayout pins the bucket keys an s3:// URL reads and
// writes: the README's remote storage layout, with the prefix joined to // writes: the README's remote storage layout, with the prefix joined to
// each key by one "/". s3://b/p and s3://b/p/ must be the same // each key by one "/". s3://b/p and s3://b/p/ must be the same
+81 -1
View File
@@ -5,12 +5,14 @@ import (
"io/fs" "io/fs"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/log" "sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/snapshot" "sneak.berlin/go/vaultik/internal/snapshot"
"sneak.berlin/go/vaultik/internal/storage/faultstore"
) )
// TestNukeRemoteLeavesNoFiles checks that remote nuke leaves no file // TestNukeRemoteLeavesNoFiles checks that remote nuke leaves no file
@@ -23,7 +25,7 @@ func TestNukeRemoteLeavesNoFiles(t *testing.T) {
ctx := context.Background() ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "store") storeDir := filepath.Join(t.TempDir(), "store")
v, repos, _ := backUpToFileDestination(ctx, t, storeDir) v, repos, out := backUpToFileDestination(ctx, t, storeDir)
snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit) snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
require.NoError(t, err) require.NoError(t, err)
@@ -61,4 +63,82 @@ func TestNukeRemoteLeavesNoFiles(t *testing.T) {
}) })
require.NoError(t, err) require.NoError(t, err)
assert.Empty(t, files) assert.Empty(t, files)
// Nuke deletes the blobs itself, so advice to run prune is wrong.
assert.Contains(t, out.String(), "Backup destination store is now empty.")
assert.NotContains(t, out.String(), "vaultik prune")
}
// TestNukeRemoteFailsWhenADeleteFails checks that remote nuke returns an
// error when a snapshot's metadata or a blob cannot be deleted, does not
// report the store empty, and advises running nuke again: prune deletes
// no metadata.
func TestNukeRemoteFailsWhenADeleteFails(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
for _, refusedPrefix := range []string{"metadata/", "blobs/"} {
t.Run(refusedPrefix, func(t *testing.T) {
t.Parallel()
ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "store")
v, _, out := backUpToFileDestination(ctx, t, storeDir)
store := faultstore.New(v.Storage)
store.OnDelete = func(key string) bool {
return strings.HasPrefix(key, refusedPrefix)
}
v.Storage = store
require.Error(t, v.NukeRemote(true))
assert.NotContains(t, out.String(), "now empty")
assert.Contains(t, out.String(), "vaultik remote nuke --force")
assert.NotContains(t, out.String(), "vaultik prune")
})
}
}
// TestNukeRemoteRefusesAnotherDestinationWithoutRetryAdvice checks that
// remote nuke, when the local index belongs to a different destination
// than the configured one, fails without advising to run nuke again:
// that run would fail the same way.
func TestNukeRemoteRefusesAnotherDestinationWithoutRetryAdvice(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "store")
v, _, out := backUpToFileDestination(ctx, t, storeDir)
v.Config.StorageURL = "file://" + filepath.Join(t.TempDir(), "elsewhere")
err := v.NukeRemote(true)
require.ErrorContains(t, err, "bound to a different backup destination")
assert.NotContains(t, out.String(), "vaultik remote nuke --force")
}
// TestNukeRemoteFailsWhenALocalSnapshotCannotBeRemoved checks that a
// snapshot remote nuke cannot remove from the local index fails the
// run: nuke then leaves that snapshot's metadata on the destination
// store.
func TestNukeRemoteFailsWhenALocalSnapshotCannotBeRemoved(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "store")
v, _, out := backUpToFileDestination(ctx, t, storeDir)
_, err := v.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.Error(t, v.NukeRemote(true))
assert.NotContains(t, out.String(), "now empty")
assert.Contains(t, out.String(), "vaultik remote nuke --force")
} }
+37 -5
View File
@@ -19,6 +19,14 @@ type PruneOptions struct {
var errNukeRequiresForce = errors.New( var errNukeRequiresForce = errors.New(
"nuke requires --force (this deletes ALL remote snapshots and blobs)") "nuke requires --force (this deletes ALL remote snapshots and blobs)")
// errBlobDeleteFailed reports unreferenced blobs prune could not delete.
var errBlobDeleteFailed = errors.New("blob(s) could not be deleted")
// nukeCommandHint is the command suggested when a remote nuke could not
// delete everything: running it again deletes what is left, which
// `vaultik prune` does not do for snapshot metadata.
const nukeCommandHint = "vaultik remote nuke --force"
// metadataDirName is the top-level remote directory holding snapshot // metadataDirName is the top-level remote directory holding snapshot
// metadata. // metadata.
const metadataDirName = "metadata" const metadataDirName = "metadata"
@@ -35,6 +43,29 @@ func (v *Vaultik) NukeRemote(force bool) error {
return errNukeRequiresForce return errNukeRequiresForce
} }
// Checked before anything is deleted: running nuke again would fail
// the same way, so the advice below must not follow this error.
err := v.EnsureStorageBinding()
if err != nil {
return err
}
err = v.emptyDestinationStore()
if err != nil {
v.UI.Warningf("Run '%s' again to delete what is left on the "+
"backup destination store.", nukeCommandHint)
return err
}
v.UI.Completef("Backup destination store is now empty.")
return nil
}
// emptyDestinationStore does the deleting for NukeRemote and stops at
// the first step that fails.
func (v *Vaultik) emptyDestinationStore() error {
v.UI.Beginf("Removing all snapshot metadata from backup destination store.") v.UI.Beginf("Removing all snapshot metadata from backup destination store.")
_, err := v.RemoveAllSnapshots(&RemoveOptions{Force: true}) _, err := v.RemoveAllSnapshots(&RemoveOptions{Force: true})
@@ -58,8 +89,6 @@ func (v *Vaultik) NukeRemote(force bool) error {
} }
} }
v.UI.Completef("Backup destination store is now empty.")
return nil return nil
} }
@@ -162,14 +191,17 @@ func (v *Vaultik) PruneBlobs(opts *PruneOptions) error {
v.deleteUnreferencedBlobs(unreferencedBlobs, allBlobs, result) v.deleteUnreferencedBlobs(unreferencedBlobs, allBlobs, result)
if opts.JSON { if opts.JSON {
return v.outputPruneBlobsJSON(result) err = v.outputPruneBlobsJSON(result)
if err != nil {
return err
} }
} else {
v.stdoutf("\nDeleted %d blob(s) totaling %s\n", v.stdoutf("\nDeleted %d blob(s) totaling %s\n",
result.BlobsDeleted, ubytes(result.BytesFreed)) result.BlobsDeleted, ubytes(result.BytesFreed))
}
if result.BlobsFailed > 0 { if result.BlobsFailed > 0 {
v.stdoutf("Failed to delete %d blob(s)\n", result.BlobsFailed) return fmt.Errorf("%d %w", result.BlobsFailed, errBlobDeleteFailed)
} }
return nil return nil
@@ -0,0 +1,98 @@
package vaultik_test
import (
"context"
"encoding/json"
"os"
"path/filepath"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/storage/faultstore"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// refuseBlobDeletes is an OnDelete hook that fails every blob delete.
func refuseBlobDeletes(key string) bool {
return strings.HasPrefix(key, "blobs/")
}
// TestPruneBlobs_FailedDeleteReturnsError checks that prune returns an
// error, so the command exits non-zero, when an unreferenced blob cannot
// be deleted.
func TestPruneBlobs_FailedDeleteReturnsError(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newListEnv(t)
// The store holds no manifest, so nothing references this blob.
addBlob(t, env.store.testStorer, testBlobHashA)
store := faultstore.New(env.store)
store.OnDelete = refuseBlobDeletes
env.v.Storage = store
err := env.v.PruneBlobs(&vaultik.PruneOptions{Force: true})
require.ErrorContains(t, err, "1 blob(s) could not be deleted")
}
// TestPruneBlobs_JSONFailedDeleteReturnsError checks that prune --json
// prints its document and then returns an error when an unreferenced
// blob cannot be deleted.
func TestPruneBlobs_JSONFailedDeleteReturnsError(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newListEnv(t)
addBlob(t, env.store.testStorer, testBlobHashA)
store := faultstore.New(env.store)
store.OnDelete = refuseBlobDeletes
env.v.Storage = store
err := env.v.PruneBlobs(&vaultik.PruneOptions{JSON: true})
require.Error(t, err, "prune must fail when a blob cannot be deleted")
var result vaultik.PruneBlobsResult
require.NoError(t, json.Unmarshal(env.stdout.Bytes(), &result),
"stdout must hold the JSON document, got:\n%s", env.stdout.String())
assert.Equal(t, 1, result.BlobsFailed)
assert.Equal(t, 0, result.BlobsDeleted)
}
// TestCreateSnapshotPrune_FailedBlobDeleteReturnsError checks that the
// prune step of snapshot create --prune fails the run when an
// unreferenced blob cannot be deleted.
func TestCreateSnapshotPrune_FailedBlobDeleteReturnsError(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "store")
v, _, _ := backUpToFileDestination(ctx, t, storeDir)
// No snapshot references this blob, so the prune step deletes it.
orphan := filepath.Join(storeDir, "blobs", testBlobHashA[:2],
testBlobHashA[2:4], testBlobHashA)
require.NoError(t, os.MkdirAll(filepath.Dir(orphan), 0o750))
require.NoError(t, os.WriteFile(orphan, []byte("orphan"), 0o600))
store := faultstore.New(v.Storage)
store.OnDelete = refuseBlobDeletes
v.Storage = store
err := v.CreateSnapshot(&vaultik.SnapshotCreateOptions{
Cron: true,
Prune: true,
})
require.ErrorContains(t, err, "1 blob(s) could not be deleted")
assert.FileExists(t, orphan)
}
+2 -1
View File
@@ -476,7 +476,8 @@ func TestRemoveAllSnapshots_WithForce(t *testing.T) {
out := tv.Stdout.String() out := tv.Stdout.String()
assert.Contains(t, out, "Removed 2 snapshot(s)") assert.Contains(t, out, "Removed 2 snapshot(s)")
assert.Contains(t, out, "Removed snapshot metadata from remote storage") assert.Contains(t, out, "Removed snapshot metadata from remote storage")
assert.Contains(t, out, "vaultik prune") // Remote nuke, the only caller, deletes the blobs next itself.
assert.NotContains(t, out, "vaultik prune")
} }
func TestRemoveAllSnapshots_DryRun(t *testing.T) { func TestRemoveAllSnapshots_DryRun(t *testing.T) {
+18 -20
View File
@@ -24,6 +24,7 @@ var (
errSnapshotVerifyFailed = errors.New("verification failed") errSnapshotVerifyFailed = errors.New("verification failed")
errRemoveAllNeedsForce = errors.New("--all requires --force") errRemoveAllNeedsForce = errors.New("--all requires --force")
errSnapshotNotFound = errors.New("snapshot not found") errSnapshotNotFound = errors.New("snapshot not found")
errSnapshotRemoveFailed = errors.New("snapshot(s) could not be removed")
errInvalidTableName = errors.New("invalid table name") errInvalidTableName = errors.New("invalid table name")
errNoAgeRecipients = errors.New( errNoAgeRecipients = errors.New(
"creating a snapshot needs at least one public key in " + "creating a snapshot needs at least one public key in " +
@@ -1513,8 +1514,8 @@ func (v *Vaultik) handleRemoveAllDryRun(
// executeRemoveAll deletes every local snapshot and, unless LocalOnly // executeRemoveAll deletes every local snapshot and, unless LocalOnly
// is set, every corresponding remote metadata directory plus any // is set, every corresponding remote metadata directory plus any
// orphan remote keys. Blobs are not touched; the printed prune-command // orphan remote keys. Blobs are not touched. It returns an error when
// hint is the next step. // any snapshot could not be removed.
func (v *Vaultik) executeRemoveAll( func (v *Vaultik) executeRemoveAll(
localSnaps, orphanRemoteKeys []string, opts *RemoveOptions, localSnaps, orphanRemoteKeys []string, opts *RemoveOptions,
) (*RemoveResult, error) { ) (*RemoveResult, error) {
@@ -1528,18 +1529,11 @@ func (v *Vaultik) executeRemoveAll(
result := &RemoveResult{} result := &RemoveResult{}
remoteErrors := v.removeAllLocalSnapshots(localSnaps, opts, result) failures := v.removeAllLocalSnapshots(localSnaps, opts, result)
if !opts.LocalOnly { if !opts.LocalOnly {
remoteErrors += v.removeAllOrphanRemotes(orphanRemoteKeys, result) failures += v.removeAllOrphanRemotes(orphanRemoteKeys, result)
result.RemoteRemoved = failures == 0
if remoteErrors == 0 {
result.RemoteRemoved = true
} else if v.UI != nil {
v.UI.Warningf("Some remote metadata deletions failed. Run '%s' "+
"once the remote is healthy to clean up unreferenced blobs.",
pruneCommandHint)
}
} }
if v.SnapshotManager != nil { if v.SnapshotManager != nil {
@@ -1549,6 +1543,10 @@ func (v *Vaultik) executeRemoveAll(
} }
} }
if failures > 0 {
return result, fmt.Errorf("%d %w", failures, errSnapshotRemoveFailed)
}
if opts.JSON { if opts.JSON {
return result, v.outputRemoveJSON(result) return result, v.outputRemoveJSON(result)
} }
@@ -1557,9 +1555,6 @@ func (v *Vaultik) executeRemoveAll(
if !opts.LocalOnly && result.RemoteRemoved { if !opts.LocalOnly && result.RemoteRemoved {
v.printlnStdout("Removed snapshot metadata from remote storage") v.printlnStdout("Removed snapshot metadata from remote storage")
v.stdoutf("\nNote: Removed snapshots' blobs remain on the remote. "+
"Run '%s' to delete any blobs no longer referenced by any "+
"remaining remote snapshot.\n", pruneCommandHint)
} }
return result, nil return result, nil
@@ -1602,12 +1597,13 @@ func (v *Vaultik) deleteIncompleteSnapshot(snapshotIDStr string, id any) bool {
} }
// removeAllLocalSnapshots removes each known local snapshot (and, unless // removeAllLocalSnapshots removes each known local snapshot (and, unless
// --local-only, its remote metadata), returning the number of remote // --local-only, its remote metadata), returning the number of snapshots
// deletion failures. // it could not remove. A snapshot left in the local index keeps its
// remote metadata too, since it is not an orphan remote key.
func (v *Vaultik) removeAllLocalSnapshots( func (v *Vaultik) removeAllLocalSnapshots(
localSnaps []string, opts *RemoveOptions, result *RemoveResult, localSnaps []string, opts *RemoveOptions, result *RemoveResult,
) int { ) int {
remoteErrors := 0 failures := 0
for _, snapshotID := range localSnaps { for _, snapshotID := range localSnaps {
log.Info("Removing snapshot", "snapshot_id", snapshotID) log.Info("Removing snapshot", "snapshot_id", snapshotID)
@@ -1617,6 +1613,8 @@ func (v *Vaultik) removeAllLocalSnapshots(
log.Error("Failed to remove from local database", log.Error("Failed to remove from local database",
"snapshot_id", snapshotID, "error", err) "snapshot_id", snapshotID, "error", err)
failures++
continue continue
} }
@@ -1627,14 +1625,14 @@ func (v *Vaultik) removeAllLocalSnapshots(
log.Warn("Failed to remove snapshot metadata from remote", log.Warn("Failed to remove snapshot metadata from remote",
"snapshot_id", snapshotID, "error", err) "snapshot_id", snapshotID, "error", err)
remoteErrors++ failures++
} }
} }
result.SnapshotsRemoved = append(result.SnapshotsRemoved, snapshotID) result.SnapshotsRemoved = append(result.SnapshotsRemoved, snapshotID)
} }
return remoteErrors return failures
} }
// removeAllOrphanRemotes deletes remote snapshot metadata that has no // removeAllOrphanRemotes deletes remote snapshot metadata that has no