Author SHA1 Message Date
sneak 28564cc568 Stop a cancelled backup at once under --skip-errors (closes #286)
check / check (push) Canceled after 0s
After Ctrl-C or SIGTERM, phase 2 of a --skip-errors backup treated the
cancellation error from each remaining file like an unreadable file: it
opened the file, printed an error line, counted it as failed and moved
on to the next. The processing loop now checks for cancellation before
each file, and an error from a file once the run is cancelled stops the
run instead of being skipped. The loop check is what stops a run
cancelled while an empty file is open, since an empty file has no
chunks and nothing reads the context while it is backed up.

The --skip-errors test helper now takes the scan's context.

Model: opus-5-5
2026-10-08 11:28:56 +00:00
18 changed files with 15 additions and 435 deletions
-26
View File
@@ -22,25 +22,6 @@ the tag exists and is exercised; what is left is merging `next` to
# Completed Steps
- 2026-10-13: Made an S3 listing that the server cuts short an error
([issue #297](https://git.eeqj.de/sneak/vaultik/issues/297)). A page
marked truncated with no continuation token, as a server answering V2
listings V1-style sends, ended the listing without an error, so
`prune` saw only the snapshots on the first page and could delete
blobs the rest still use. Such a page is now a listing error, and
`prune`, `snapshot list` and `remote nuke` handle it as a destination
that cannot be listed.
- 2026-10-12: Rejected a retention duration longer than about 292 years
([issue #289](https://git.eeqj.de/sneak/vaultik/issues/289)). The
number given to `--older-than` or `--keep-newer-than` was multiplied
by the length of its unit with no limit, so from `293y`, `15251w`,
`3559mo` or `106752d`, or a sum of parts such as `200y200y`, the
result wrapped to a negative duration. The cutoff then lay in the
future, every snapshot of the name was deleted, the one just made
included, and the command exited 0. Such a duration is now rejected
as invalid and nothing is deleted.
- 2026-10-08: Made a backup cancelled under `--skip-errors` stop at once
([issue #286](https://git.eeqj.de/sneak/vaultik/issues/286)). After
Ctrl-C or SIGTERM, phase 2 treated the cancellation error from each
@@ -49,13 +30,6 @@ the tag exists and is exercised; what is left is merging `next` to
The run now stops at the first file after the cancellation, and no
file is counted as failed because of it.
- 2026-10-08: Made `remote nuke` delete the `.partial` files that
uploads cut off part-way leave on the destination store
([issue #281](https://git.eeqj.de/sneak/vaultik/issues/281)). The
file and rclone listings skip such a file, so the command left it in
place and still reported the store empty. It now removes them under
`metadata/` and `blobs/` as its last step.
- 2026-10-08: Made a local index error while recording a directory or
symlink stop a backup under `--skip-errors`
([issue #284](https://git.eeqj.de/sneak/vaultik/issues/284)). Phase 2
+2 -29
View File
@@ -18,13 +18,6 @@ import (
"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.
// It provides a simplified interface for S3 operations with automatic
// 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() {
page, err := nextListPage(ctx, paginator)
page, err := paginator.NextPage(ctx)
if err != nil {
return nil, err
}
@@ -242,26 +235,6 @@ func (c *Client) ListObjects(ctx context.Context, prefix string) ([]string, erro
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.
// Returns true if the object exists, false otherwise.
// The key is automatically prefixed with the configured prefix.
@@ -327,7 +300,7 @@ func (c *Client) ListObjectsStream(
})
for paginator.HasMorePages() {
page, err := nextListPage(ctx, paginator)
page, err := paginator.NextPage(ctx)
if err != nil {
ch <- ObjectInfo{Err: err}
@@ -171,11 +171,6 @@ func (f *Storer) ListStream(
return f.inner.ListStream(ctx, prefix)
}
// DeletePartialUploads delegates unchanged.
func (f *Storer) DeletePartialUploads(ctx context.Context, prefix string) error {
return f.inner.DeletePartialUploads(ctx, prefix)
}
// Info delegates unchanged.
func (f *Storer) Info() storage.Info {
return f.inner.Info()
-36
View File
@@ -242,42 +242,6 @@ func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan Objec
return ch
}
// DeletePartialUploads removes every file under prefix whose name ends in
// tempSuffix. A missing prefix has none to remove.
func (f *FileStorer) DeletePartialUploads(ctx context.Context, prefix string) error {
basePath := f.fullPath(prefix)
exists, err := afero.Exists(f.fs, basePath)
if err != nil {
return fmt.Errorf("checking path: %w", err)
}
if !exists {
return nil
}
err = afero.Walk(f.fs, basePath, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if ctx.Err() != nil {
return ctx.Err()
}
if info.IsDir() || !strings.HasSuffix(info.Name(), tempSuffix) {
return nil
}
return f.fs.Remove(path)
})
if err != nil {
return fmt.Errorf("walking directory: %w", err)
}
return nil
}
// Info returns human-readable storage location information.
func (f *FileStorer) Info() Info {
return Info{
-42
View File
@@ -120,45 +120,3 @@ func TestFileStorer_ListSkipsPartialFiles(t *testing.T) {
t.Fatalf("ListStream should return only the real key, got %v", streamed)
}
}
// TestFileStorer_DeletePartialUploads checks that a leftover temp file is
// removed and the object at the real key is kept.
func TestFileStorer_DeletePartialUploads(t *testing.T) {
t.Parallel()
base := t.TempDir()
f, err := storage.NewFileStorer(base)
if err != nil {
t.Fatalf("NewFileStorer: %v", err)
}
ctx := context.Background()
err = f.Put(ctx, testBlobKey, strings.NewReader("blob-bytes"))
if err != nil {
t.Fatalf("Put: %v", err)
}
leftover := filepath.Join(base, testBlobKey+"-123456.partial")
err = os.WriteFile(leftover, []byte("half"), 0o600)
if err != nil {
t.Fatalf("writing leftover temp file: %v", err)
}
err = f.DeletePartialUploads(ctx, "blobs/")
if err != nil {
t.Fatalf("DeletePartialUploads: %v", err)
}
_, err = os.Stat(leftover)
if !os.IsNotExist(err) {
t.Errorf("leftover temp file was not removed: %v", err)
}
_, err = f.Stat(ctx, testBlobKey)
if err != nil {
t.Errorf("Stat of the real key: %v", err)
}
}
-25
View File
@@ -213,31 +213,6 @@ func (r *RcloneStorer) ListStream(
return ch
}
// DeletePartialUploads removes every object under prefix whose name ends
// in tempSuffix.
func (r *RcloneStorer) DeletePartialUploads(ctx context.Context, prefix string) error {
var partial []fs.Object
err := operations.ListFn(ctx, r.fsys, func(obj fs.Object) {
key := obj.Remote()
if strings.HasPrefix(key, prefix) && strings.HasSuffix(key, tempSuffix) {
partial = append(partial, obj)
}
})
if err != nil {
return fmt.Errorf("listing objects: %w", err)
}
for _, obj := range partial {
err = obj.Remove(ctx)
if err != nil {
return fmt.Errorf("removing object: %w", err)
}
}
return nil
}
// Info returns human-readable storage location information.
func (r *RcloneStorer) Info() Info {
location := r.remote
-41
View File
@@ -198,47 +198,6 @@ func TestRcloneStorerListSkipsPartialFiles(t *testing.T) {
}
}
// TestRcloneStorerDeletePartialUploads checks that a temporary file left
// by a killed upload is removed and the object at the real key is kept.
//
//nolint:paralleltest // NewRcloneStorer installs the process-global rclone config
func TestRcloneStorerDeletePartialUploads(t *testing.T) {
dir := t.TempDir()
ctx := context.Background()
s, err := storage.NewRcloneStorer(ctx, ":local", dir)
if err != nil {
t.Fatalf("NewRcloneStorer: %v", err)
}
err = s.Put(ctx, testBlobKey, strings.NewReader("blob-bytes"))
if err != nil {
t.Fatalf("Put: %v", err)
}
leftover := filepath.Join(dir, testBlobKey+"-123456.partial")
err = os.WriteFile(leftover, []byte("half"), 0o600)
if err != nil {
t.Fatalf("writing leftover temp file: %v", err)
}
err = s.DeletePartialUploads(ctx, "blobs/")
if err != nil {
t.Fatalf("DeletePartialUploads: %v", err)
}
_, err = os.Stat(leftover)
if !os.IsNotExist(err) {
t.Errorf("leftover temp file was not removed: %v", err)
}
_, err = s.Stat(ctx, testBlobKey)
if err != nil {
t.Errorf("Stat of the real key: %v", err)
}
}
// newRcloneStorerOnWrappedLocal registers name as rclone's local backend
// wrapped by wrap, and builds an rclone backend on it rooted at a fresh
// temp directory. wrap changes the features the local backend reports, so
-6
View File
@@ -99,12 +99,6 @@ func (s *S3Storer) ListStream(ctx context.Context, prefix string) <-chan ObjectI
return ch
}
// DeletePartialUploads has nothing to remove: S3 shows an object only once
// its upload has completed, so an upload cut off part-way leaves none.
func (s *S3Storer) DeletePartialUploads(_ context.Context, _ string) error {
return nil
}
// Info returns human-readable storage location information.
func (s *S3Storer) Info() Info {
return Info{
+3 -69
View File
@@ -4,7 +4,6 @@ import (
"bytes"
"context"
"errors"
"io"
"net/http"
"net/http/httptest"
"slices"
@@ -23,16 +22,13 @@ import (
// s3TestBucket is the bucket created for each in-process S3 server.
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 (
s3TestAccessKeyID = "key"
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
// S3 server. It reuses the same in-memory S3 harness (gofakes3 + s3mem
// 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,
AccessKeyID: "test",
SecretAccessKey: "test",
Region: s3TestRegion,
Region: "us-east-1",
})
if err != nil {
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
// 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
-6
View File
@@ -71,12 +71,6 @@ type Storer interface {
// If an error occurs during listing, the final item will have Err set.
ListStream(ctx context.Context, prefix string) <-chan ObjectInfo
// DeletePartialUploads removes every object under prefix that an
// upload cut off part-way left under a temporary name ending in
// `.partial`. The file and rclone backends' List and ListStream skip
// such an object.
DeletePartialUploads(ctx context.Context, prefix string) error
// Info returns human-readable storage location information.
Info() Info
}
@@ -257,10 +257,6 @@ func (s *stubLister) List(_ context.Context, _ string) ([]string, error) {
return nil, errStubUnused
}
func (s *stubLister) DeletePartialUploads(_ context.Context, _ string) error {
return errStubUnused
}
func (s *stubLister) Info() storage.Info {
return storage.Info{}
}
+8 -24
View File
@@ -3,7 +3,6 @@ package vaultik
import (
"errors"
"fmt"
"math"
"regexp"
"strconv"
"strings"
@@ -48,10 +47,6 @@ const (
year = 365 * day
)
// maxDuration is the largest value a time.Duration can hold, about 292
// years.
const maxDuration = time.Duration(math.MaxInt64)
// A snapshot ID split on "_" has at least a hostname and a trailing
// timestamp.
const minSnapshotIDParts = 2
@@ -150,10 +145,10 @@ func parseSnapshotName(snapshotID, hostname string) string {
// parseDuration parses a duration string with support for human-friendly units:
// d/day/days, w/week/weeks, mo/month/months, y/year/years, plus standard Go
// duration units. Following Go, m is minutes and mo is months. A bare number,
// an unknown unit, a negative value, and a total longer than maxDuration are
// all rejected. Outside the Go units the input must be whole numbers each
// followed directly by a unit (2w3d), so 1.0y and 30 days are errors;
// decimals work only in Go units (1.5h).
// an unknown unit, and a negative value are all rejected. Outside the Go
// units the input must be whole numbers each followed directly by a unit
// (2w3d), so 1.0y and 30 days are errors; decimals work only in Go units
// (1.5h).
func parseDuration(s string) (time.Duration, error) {
if strings.HasPrefix(strings.TrimSpace(s), "-") {
return 0, errNegativeDuration
@@ -186,30 +181,19 @@ func parseDuration(s string) (time.Duration, error) {
return 0, fmt.Errorf("invalid number %q: %w", match[1], err)
}
var unitLength time.Duration
unit := strings.ToLower(match[2])
switch unit {
case "d", "day", "days":
unitLength = day
total += time.Duration(n) * day
case "w", "week", "weeks":
unitLength = week
total += time.Duration(n) * week
case "mo", "month", "months":
unitLength = month
total += time.Duration(n) * month
case "y", "year", "years":
unitLength = year
total += time.Duration(n) * year
default:
return 0, fmt.Errorf("%w %q", errUnknownTimeUnit, unit)
}
// Past maxDuration the sum wraps to a negative value, which would
// put the purge cutoff in the future.
if time.Duration(n) > (maxDuration-total)/unitLength {
return 0, fmt.Errorf("%w: %q is longer than about 292 years",
errInvalidDuration, s)
}
total += time.Duration(n) * unitLength
}
return total, nil
-11
View File
@@ -111,17 +111,6 @@ func TestParseDuration(t *testing.T) {
{"30 days", 0, true},
{"30 days ago", 0, true},
{"x7d", 0, true},
// The largest time.Duration is about 292 years. A larger value
// must be rejected, not wrapped to a negative one, which would
// put the cutoff in the future and select every snapshot.
{"292y", 292 * 365 * 24 * time.Hour, false},
{"293y", 0, true},
{"3559mo", 0, true},
{"15251w", 0, true},
{"106752d", 0, true},
{"2562048h", 0, true},
{"200y200y", 0, true},
{"292y6mo", 0, true},
}
for _, tt := range tests {
-6
View File
@@ -155,12 +155,6 @@ func (m *MockStorer) ListStream(
return ch
}
// DeletePartialUploads has nothing to remove: Put stores each object
// under its key at once.
func (m *MockStorer) DeletePartialUploads(_ context.Context, _ string) error {
return nil
}
func (m *MockStorer) Info() storage.Info {
return storage.Info{
Type: "mock",
-64
View File
@@ -1,64 +0,0 @@
package vaultik_test
import (
"context"
"io/fs"
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/snapshot"
)
// TestNukeRemoteLeavesNoFiles checks that remote nuke leaves no file
// under a file:// destination, including the `.partial` files uploads
// killed part-way leave next to a snapshot's metadata and next to where a
// blob would have been.
func TestNukeRemoteLeavesNoFiles(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "store")
v, repos, _ := backUpToFileDestination(ctx, t, storeDir)
snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
require.NoError(t, err)
require.Len(t, snapshots, 1)
snapshotKey := snapshot.RemoteSnapshotKey(snapshots[0].ID.String())
hash := testBlobHashA
leftovers := []string{
filepath.Join(storeDir, "metadata", snapshotKey,
"db.zst.age-123456.partial"),
filepath.Join(storeDir, "blobs", hash[:2], hash[2:4],
hash+"-123456.partial"),
}
for _, leftover := range leftovers {
require.NoError(t, os.MkdirAll(filepath.Dir(leftover), 0o750))
require.NoError(t, os.WriteFile(leftover, []byte("half an upload"), 0o600))
}
require.NoError(t, v.NukeRemote(true))
var files []string
err = filepath.WalkDir(storeDir,
func(path string, entry fs.DirEntry, err error) error {
if err != nil {
return err
}
if !entry.IsDir() {
files = append(files, path)
}
return nil
})
require.NoError(t, err)
assert.Empty(t, files)
}
+2 -12
View File
@@ -24,9 +24,8 @@ var errNukeRequiresForce = errors.New(
const metadataDirName = "metadata"
// NukeRemote deletes every snapshot's metadata and every blob from remote
// storage, along with any object an upload cut off part-way left under a
// temporary `.partial` name. After this returns successfully the bucket
// prefix is empty and the next backup starts from scratch.
// storage. After this returns successfully the bucket prefix is empty and
// the next backup starts from scratch.
//
// Refuses to run unless force is true. The caller is responsible for
// confirming with the user.
@@ -49,15 +48,6 @@ func (v *Vaultik) NukeRemote(force bool) error {
return fmt.Errorf("pruning blobs: %w", err)
}
// The file and rclone listings skip `.partial` objects, so the two
// steps above never delete them.
for _, prefix := range []string{"metadata/", "blobs/"} {
err = v.Storage.DeletePartialUploads(v.ctx, prefix)
if err != nil {
return fmt.Errorf("deleting partial uploads: %w", err)
}
}
v.UI.Completef("Backup destination store is now empty.")
return nil
-23
View File
@@ -267,29 +267,6 @@ func TestPurgeOlderThan_WithNameFilter(t *testing.T) {
assert.Contains(t, remaining, snapHomeT0)
}
func TestPurgeOlderThan_TooLargeDeletesNothing(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
snapshotIDs := []string{snapHomeT0, snapHomeT1}
v := setupPurgeTest(t, snapHostname, snapshotIDs)
err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
OlderThan: "1000y",
Force: true,
})
require.Error(t, err)
err = v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
OlderThan: "292y",
Force: true,
})
require.NoError(t, err)
assert.ElementsMatch(t, snapshotIDs, listRemainingSnapshots(t, v))
}
func TestPurgeKeepLatest_ThreeNames(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
-6
View File
@@ -125,12 +125,6 @@ func (s *testStorer) ListStream(
return ch
}
// DeletePartialUploads has nothing to remove: Put stores each object
// under its key at once.
func (s *testStorer) DeletePartialUploads(_ context.Context, _ string) error {
return nil
}
func (s *testStorer) Info() storage.Info {
return storage.Info{
Type: testLabel,