Compare commits

..
Author SHA1 Message Date
sneak defda0921b Limit and retry the module download in the Docker build (closes #351)
check / check (push) Waiting to run
Each `go mod download` in the Dockerfile now runs under a five-minute
time limit and is tried up to three times. Without a limit, a stalled
connection to the module proxy held the step, and every build on the
host sharing it, until the kernel dropped the connection. Five minutes
is well above the three minutes a working download can take on a busy
build host. When all three attempts fail, the step prints "module
download failed" and exits 1, so the build stops there instead of
failing as a test or lint failure. A test checks every download step
for the limit, the retries, the message and the final `exit 1`.

Model: opus-5-5
2026-10-13 15:23:40 +00:00
10 changed files with 119 additions and 413 deletions
+4 -4
View File
@@ -279,8 +279,8 @@ Key methods:
- `Scan(ctx, path, snapshotID)` → Main scan operation
Scan phases:
1. **Phase 1**: Walk directory, identify files needing processing
2. **Phase 1b**: Count as deleted the files that the previous snapshot of the same name lists and the walk did not find; a file that is excluded, or inside an excluded directory, is not counted
1. **Phase 0**: Detect deleted files from previous snapshots
2. **Phase 1**: Walk directory, identify files needing processing
3. **Phase 2**: Process files (chunk → pack → upload)
#### SnapshotManager
@@ -320,13 +320,13 @@ CreateSnapshot(opts)
│ │
│ ├─► scanner.Scan(ctx, path, snapshotID)
│ │ │
│ │ ├─► Phase 0: detectDeletedFiles()
│ │ │
│ │ ├─► Phase 1: scanPhase()
│ │ │ Walk directory
│ │ │ Check file metadata changes
│ │ │ Build list of files to process
│ │ │
│ │ ├─► Phase 1b: detectDeletedFiles()
│ │ │
│ │ └─► Phase 2: processPhase()
│ │ For each file:
│ │ chunker.ChunkReaderStreaming()
+24 -3
View File
@@ -5,7 +5,16 @@
FROM golangci/golangci-lint:v2.14.0@sha256:ad862ba6b3798cbe0fd9fd7408d498fd74fbd2623a92406b2fd3898faf0bf98f AS lint
WORKDIR /src
COPY go.mod go.sum ./
RUN go mod download
# Without the time limit a stalled connection to the module proxy holds
# this step, and every build on the host sharing it, until the kernel
# drops the connection, which can take an hour. The limit is well above
# the three minutes a working download can take on a busy build host.
RUN for attempt in 1 2 3; do \
timeout 300 go mod download && exit 0; \
echo "go mod download attempt $attempt failed or ran over 300s" >&2; \
done; \
echo "module download failed: giving up after 3 attempts" >&2; \
exit 1
COPY . .
# `golangci-lint run` silently ignores an unknown top-level key in
# .golangci.yml, such as a misspelt `linters:`; `config verify` fails on it.
@@ -18,7 +27,13 @@ RUN golangci-lint run --config .golangci.yml ./...
FROM golang:1.26.1@sha256:cd78d88e00afadbedd272f977d375a6247455f3a4b1178f8ae8bbcb201743a8a AS test
WORKDIR /src
COPY go.mod go.sum ./
RUN go mod download
# Limited and retried as in the lint phase above.
RUN for attempt in 1 2 3; do \
timeout 300 go mod download && exit 0; \
echo "go mod download attempt $attempt failed or ran over 300s" >&2; \
done; \
echo "module download failed: giving up after 3 attempts" >&2; \
exit 1
COPY . .
RUN go test -timeout 90s -race -cover ./... || \
{ echo "--- Rerunning with -v for details ---"; \
@@ -36,7 +51,13 @@ RUN apk add --no-cache git
RUN git config --system --add safe.directory /src
WORKDIR /src
COPY go.mod go.sum ./
RUN go mod download
# Limited and retried as in the lint phase above.
RUN for attempt in 1 2 3; do \
timeout 300 go mod download && exit 0; \
echo "go mod download attempt $attempt failed or ran over 300s" >&2; \
done; \
echo "module download failed: giving up after 3 attempts" >&2; \
exit 1
COPY . .
# The VERSION build arg when one is given, otherwise
+45
View File
@@ -0,0 +1,45 @@
package main_test
import (
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestDockerfileBoundsEveryModuleDownload fails unless every
// `go mod download` in the Dockerfile runs under a time limit, is tried
// a fixed number of times, and then fails the build with a message that
// names the module download (issue #351). Like the version tests, this
// parses the Dockerfile; that a stalled download really is cut off and
// retried was checked by hand.
func TestDockerfileBoundsEveryModuleDownload(t *testing.T) {
t.Parallel()
var downloads []string
for _, instruction := range instructions(t, productDockerfile) {
if strings.Contains(instruction, "go mod download") {
downloads = append(downloads, instruction)
}
}
require.NotEmpty(t, downloads, "%s must download modules",
productDockerfile)
for _, download := range downloads {
assert.Contains(t, download, "timeout 300 go mod download",
"%s: a module download must have a time limit: %s",
productDockerfile, download)
assert.Contains(t, download, "for attempt in 1 2 3",
"%s: a module download must be retried: %s",
productDockerfile, download)
assert.Contains(t, download, "module download failed",
"%s: a module download that never succeeds must say so: %s",
productDockerfile, download)
assert.True(t, strings.HasSuffix(download, "exit 1"),
"%s: a module download that never succeeds must fail the"+
" build: %s", productDockerfile, download)
}
}
-44
View File
@@ -277,50 +277,6 @@ func (r *FileRepository) ListUnderPath(
return files, rows.Err()
}
// ListUnderPathInSnapshot returns the files that ListUnderPath returns
// for path and that the snapshot lists, in no particular order.
func (r *FileRepository) ListUnderPathInSnapshot(
ctx context.Context, path, snapshotID string,
) ([]*File, error) {
path = strings.TrimRight(path, "/")
dirPrefix := path + "/"
query := `
SELECT f.id, f.path, f.source_path, f.mtime, f.mtime_nsec, f.size,
f.mode, f.uid, f.gid, f.link_target
FROM files f
JOIN snapshot_files sf ON sf.file_id = f.id
WHERE sf.snapshot_id = ?
AND (f.path = ? OR substr(f.path, 1, length(?)) = ?)
`
rows, err := r.db.conn.QueryContext(ctx, query,
snapshotID, path, dirPrefix, dirPrefix)
if err != nil {
return nil, fmt.Errorf("querying files: %w", err)
}
defer func() {
err := rows.Close()
if err != nil {
Fatalf("failed to close rows: %v", err)
}
}()
var files []*File
for rows.Next() {
file, err := r.scanFileRows(rows)
if err != nil {
return nil, fmt.Errorf("scanning file: %w", err)
}
files = append(files, file)
}
return files, rows.Err()
}
// ListIDsWithChunksNotInUploadedBlobs returns the IDs of the files whose
// path starts with prefix and that list at least one chunk held by no
// blob whose upload has completed (uploaded_ts set). A new snapshot
-59
View File
@@ -212,65 +212,6 @@ func TestFileRepositoryListUnderPath(t *testing.T) {
}
}
// ListUnderPathInSnapshot leaves out files outside path and files the
// snapshot does not list, though another snapshot lists them.
func TestFileRepositoryListUnderPathInSnapshot(t *testing.T) {
t.Parallel()
db, cleanup := setupTestDB(t)
defer cleanup()
ctx := context.Background()
files := database.NewFileRepository(db)
snapshots := database.NewSnapshotRepository(db)
listed := map[string][]string{
"one": {"/src/a.txt", "/src2/c.txt"},
"two": {"/src/b.txt"},
}
for snapshotID, paths := range listed {
err := snapshots.Create(ctx, nil, &database.Snapshot{
ID: types.SnapshotID(snapshotID),
Hostname: "host",
StartedAt: time.Now(),
})
if err != nil {
t.Fatalf("failed to create snapshot %s: %v", snapshotID, err)
}
for _, path := range paths {
err := files.Create(ctx, nil, &database.File{
Path: types.FilePath(path),
MTime: time.Now().Truncate(time.Second),
Mode: 0644,
})
if err != nil {
t.Fatalf("failed to create %s: %v", path, err)
}
err = snapshots.AddFile(ctx, nil, snapshotID, path)
if err != nil {
t.Fatalf("failed to add %s to %s: %v", path, snapshotID, err)
}
}
}
found, err := files.ListUnderPathInSnapshot(ctx, "/src", "one")
if err != nil {
t.Fatalf("failed to list files: %v", err)
}
got := make([]string, 0, len(found))
for _, f := range found {
got = append(got, f.Path.String())
}
if !slices.Equal(got, []string{"/src/a.txt"}) {
t.Errorf("got %q, want only /src/a.txt", got)
}
}
func TestFileRepositorySymlink(t *testing.T) {
t.Parallel()
+11 -15
View File
@@ -16,9 +16,6 @@ type ScannerParams struct {
Fs afero.Fs
Exclude []string // Exclude patterns (combined global + snapshot-specific)
SkipErrors bool // Skip file read errors (log loudly but continue)
// PreviousSnapshotID is the most recent completed snapshot of the
// same name, "" for none.
PreviousSnapshotID string
}
// Module exports backup functionality as an fx module.
@@ -47,18 +44,17 @@ func provideScannerFactory(
}
return NewScanner(ScannerConfig{
FS: params.Fs,
ChunkSize: cfg.ChunkSize.Int64(),
Repositories: repos,
Storage: storer,
MaxBlobSize: cfg.BlobSizeLimit.Int64(),
CompressionLevel: cfg.CompressionLevel,
AgeRecipients: cfg.AgeRecipients,
EnableProgress: params.EnableProgress,
UI: params.UI,
Exclude: excludes,
SkipErrors: params.SkipErrors,
PreviousSnapshotID: params.PreviousSnapshotID,
FS: params.Fs,
ChunkSize: cfg.ChunkSize.Int64(),
Repositories: repos,
Storage: storer,
MaxBlobSize: cfg.BlobSizeLimit.Int64(),
CompressionLevel: cfg.CompressionLevel,
AgeRecipients: cfg.AgeRecipients,
EnableProgress: params.EnableProgress,
UI: params.UI,
Exclude: excludes,
SkipErrors: params.SkipErrors,
})
}
}
+19 -51
View File
@@ -58,10 +58,6 @@ type Scanner struct {
compressionLevel int
ageRecipient string
snapshotID string // Current snapshot being processed
// previousSnapshotID is the most recent completed snapshot of the
// same name, "" for none. Only a file it lists can be counted as
// deleted.
previousSnapshotID string
// currentSourcePath is the source directory being scanned, stored with
// each file record.
currentSourcePath string
@@ -133,9 +129,6 @@ type ScannerConfig struct {
// SkipErrors skips files that cannot be opened or read (log loudly but
// continue); packer, database, encryption, and upload errors still abort.
SkipErrors bool
// PreviousSnapshotID is the most recent completed snapshot of the
// same name, "" for none.
PreviousSnapshotID string
}
// ScanResult contains the results of a scan operation. Files and bytes
@@ -214,7 +207,6 @@ func NewScanner(cfg ScannerConfig) *Scanner {
compiledExclude: compiledExclude,
progress: progress,
skipErrors: cfg.SkipErrors,
previousSnapshotID: cfg.PreviousSnapshotID,
ui: uiw,
pendingChunkHashes: make(map[string]struct{}),
}
@@ -266,9 +258,8 @@ func (s *Scanner) Scan(
filesToProcess := scanResult.FilesToProcess
// Phase 1b: Detect deleted files by comparing the previous snapshot
// of this name against scanned files
err = s.detectDeletedFiles(ctx, path, existingFiles, result)
// Phase 1b: Detect deleted files by comparing DB against scanned files
err = s.detectDeletedFilesFromMap(ctx, knownFiles, existingFiles, result)
if err != nil {
return nil, fmt.Errorf("detecting deleted files: %w", err)
}
@@ -448,8 +439,9 @@ func (s *Scanner) summarizeScanPhase(
}
// loadKnownFiles loads the known files at and beneath path from the
// database into a map for fast lookup. This avoids per-file database
// queries during the scan phase.
// database into a map for fast lookup. Every loaded file the scan does
// not find is counted as deleted. This avoids per-file database queries
// during the scan phase.
func (s *Scanner) loadKnownFiles(
ctx context.Context, path string,
) (map[string]*database.File, error) {
@@ -1950,28 +1942,21 @@ func (s *Scanner) queueFileForBatchInsert(
})
}
// detectDeletedFiles counts as deleted each file at or beneath path that
// the previous snapshot of this name lists and the walk did not find.
// A file that is excluded, or inside an excluded directory, is not
// counted: the walk skips it whether or not it is still on disk.
func (s *Scanner) detectDeletedFiles(
// detectDeletedFilesFromMap finds files that existed in previous snapshots
// but no longer exist. Uses pre-loaded maps to avoid any filesystem or
// database access.
func (s *Scanner) detectDeletedFilesFromMap(
ctx context.Context,
path string,
knownFiles map[string]*database.File,
existingFiles map[string]struct{},
result *ScanResult,
) error {
if s.previousSnapshotID == "" {
if len(knownFiles) == 0 {
return nil
}
previousFiles, err := s.repos.Files.ListUnderPathInSnapshot(
ctx, path, s.previousSnapshotID)
if err != nil {
return fmt.Errorf("listing files of snapshot %s: %w",
s.previousSnapshotID, err)
}
for _, file := range previousFiles {
// Check each known file against the enumerated set (no filesystem access needed)
for path, file := range knownFiles {
// Check context cancellation periodically
select {
case <-ctx.Done():
@@ -1979,16 +1964,13 @@ func (s *Scanner) detectDeletedFiles(
default:
}
filePath := file.Path.String()
_, exists := existingFiles[filePath]
if exists || s.excludedOrInExcludedDir(filePath, path) {
continue
// Check if the file exists in our enumerated set
if _, exists := existingFiles[path]; !exists {
// File has been deleted
result.FilesDeleted++
result.BytesDeleted += file.Size
log.Debug("Detected deleted file", "path", path, "size", file.Size)
}
result.FilesDeleted++
result.BytesDeleted += file.Size
log.Debug("Detected deleted file", "path", filePath, "size", file.Size)
}
if result.FilesDeleted > 0 {
@@ -1999,20 +1981,6 @@ func (s *Scanner) detectDeletedFiles(
return nil
}
// excludedOrInExcludedDir reports whether filePath, which is at or
// beneath rootPath, or any directory between rootPath and it, matches an
// exclude pattern. A pattern such as */.local/share/Trash matches the
// directory but not the files inside it, which the walk skips too.
func (s *Scanner) excludedOrInExcludedDir(filePath, rootPath string) bool {
for p := filePath; len(p) > len(rootPath); p = filepath.Dir(p) {
if s.shouldExclude(p, rootPath) {
return true
}
}
return false
}
// wrapPermissionError augments permission errors with platform-specific
// remediation instructions. On macOS, TCC-protected directories (Calendars,
// Reminders, Photos, etc.) return EPERM unless the running application has
+11 -12
View File
@@ -661,18 +661,17 @@ func faultScannerFactory(
) snapshot.ScannerFactory {
return func(params snapshot.ScannerParams) *snapshot.Scanner {
return snapshot.NewScanner(snapshot.ScannerConfig{
FS: params.Fs,
Storage: storer,
ChunkSize: faultChunkSize,
MaxBlobSize: faultMaxBlobSize,
CompressionLevel: cfg.CompressionLevel,
AgeRecipients: cfg.AgeRecipients,
Repositories: repos,
EnableProgress: params.EnableProgress,
UI: params.UI,
Exclude: params.Exclude,
SkipErrors: params.SkipErrors,
PreviousSnapshotID: params.PreviousSnapshotID,
FS: params.Fs,
Storage: storer,
ChunkSize: faultChunkSize,
MaxBlobSize: faultMaxBlobSize,
CompressionLevel: cfg.CompressionLevel,
AgeRecipients: cfg.AgeRecipients,
Repositories: repos,
EnableProgress: params.EnableProgress,
UI: params.UI,
Exclude: params.Exclude,
SkipErrors: params.SkipErrors,
})
}
}
+5 -29
View File
@@ -241,18 +241,12 @@ func (v *Vaultik) createNamedSnapshot(
return err
}
previousSnapshotID, err := v.previousSnapshotID(snapName)
if err != nil {
return err
}
scanner := v.ScannerFactory(snapshot.ScannerParams{
EnableProgress: !opts.Cron,
UI: v.UI,
Fs: v.Fs,
Exclude: v.Config.GetExcludes(snapName),
SkipErrors: opts.SkipErrors,
PreviousSnapshotID: previousSnapshotID,
EnableProgress: !opts.Cron,
UI: v.UI,
Fs: v.Fs,
Exclude: v.Config.GetExcludes(snapName),
SkipErrors: opts.SkipErrors,
})
snapshotID, err := v.SnapshotManager.CreateSnapshotWithName(
@@ -287,24 +281,6 @@ func (v *Vaultik) createNamedSnapshot(
return nil
}
// previousSnapshotID returns the ID of the most recent completed snapshot
// named snapName in the local index, or "" when there is none.
func (v *Vaultik) previousSnapshotID(snapName string) (string, error) {
snapshots, err := v.Repositories.Snapshots.ListRecent(v.ctx, listRecentLimit)
if err != nil {
return "", fmt.Errorf("listing snapshots: %w", err)
}
for _, s := range snapshots {
if s.CompletedAt != nil &&
parseSnapshotName(s.ID.String(), s.Hostname.String()) == snapName {
return s.ID.String(), nil
}
}
return "", nil
}
// resolveSnapshotPaths resolves source directories to absolute paths
// with symlink resolution.
func (v *Vaultik) resolveSnapshotPaths(snapName string) ([]string, error) {
@@ -1,196 +0,0 @@
package vaultik_test
import (
"bytes"
"context"
"fmt"
"os"
"path/filepath"
"testing"
"time"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/config"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/storage"
"sneak.berlin/go/vaultik/internal/ui"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// These tests cover https://git.eeqj.de/sneak/vaultik/issues/304: a
// backup counts a file as deleted only when the previous snapshot of
// the same name had it and it is gone, and never counts an excluded
// file.
// deletedCountEnv is a backup setup over the source directory src whose
// user-facing output is kept in out. Each test configures its snapshots
// with setSnapshot.
type deletedCountEnv struct {
v *vaultik.Vaultik
out *bytes.Buffer
src string
}
// newDeletedCountEnv writes files, keyed by path relative to src.
func newDeletedCountEnv(t *testing.T, files map[string][]byte) *deletedCountEnv {
t.Helper()
ctx := context.Background()
fs := afero.NewOsFs()
tempDir := t.TempDir()
srcDir := filepath.Join(tempDir, "src")
for rel, content := range files {
path := filepath.Join(srcDir, rel)
require.NoError(t, fs.MkdirAll(filepath.Dir(path), 0o755))
require.NoError(t, afero.WriteFile(fs, path, content, 0o644))
}
cfg := faultTestConfig()
cfg.IndexPath = filepath.Join(tempDir, "index.sqlite")
cfg.Snapshots = map[string]config.SnapshotConfig{}
store, err := storage.NewFileStorer(filepath.Join(tempDir, "remote"))
require.NoError(t, err)
db, err := database.New(ctx, cfg.IndexPath)
require.NoError(t, err)
t.Cleanup(func() { _ = db.Close() })
repos := database.NewRepositories(db)
out := &bytes.Buffer{}
v := newBackupVaultik(ctx, cfg, store, repos, db, fs)
v.UI = ui.NewWithColor(out, false)
return &deletedCountEnv{v: v, out: out, src: srcDir}
}
// setSnapshot configures the snapshot name to back up the path rel
// beneath src, with the given excludes.
func (e *deletedCountEnv) setSnapshot(name, rel string, exclude ...string) {
e.v.Config.Snapshots[name] = config.SnapshotConfig{
Paths: []string{filepath.Join(e.src, rel)},
Exclude: exclude,
}
}
// backUp runs a backup of the named snapshot and returns its output.
func (e *deletedCountEnv) backUp(t *testing.T, name string) string {
t.Helper()
e.out.Reset()
require.NoError(t, e.v.CreateSnapshot(&vaultik.SnapshotCreateOptions{
Snapshots: []string{name},
}))
return e.out.String()
}
// A file this snapshot excludes is not counted as deleted: neither one
// that another snapshot backs up, nor one that the previous snapshot of
// this name had before the exclude was added.
func TestBackupDoesNotCountExcludedFileAsDeleted(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newDeletedCountEnv(t, map[string][]byte{
"top.txt": []byte("top"),
"cache/tmp.bin": []byte("cache"),
"photos/a.jpg": []byte("photo a"),
"photos/b.jpg": []byte("photo b"),
"photos/c.jpg": []byte("photo c"),
})
env.setSnapshot("photos", "photos")
env.setSnapshot("home", "", "/photos")
env.backUp(t, "photos")
assert.Contains(t, env.backUp(t, "home"), filesLine(2, 2, 0))
env.setSnapshot("home", "", "/photos", "/cache")
assert.Contains(t, env.backUp(t, "home"), filesLine(1, 0, 1))
}
// A file inside a directory that an exclude added after a backup
// matches is not counted as deleted, though the pattern matches only the
// directory.
func TestBackupDoesNotCountFileInExcludedDirectoryAsDeleted(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newDeletedCountEnv(t, map[string][]byte{
"top.txt": []byte("top"),
"user/.local/share/Trash/old.txt": []byte("old"),
})
env.setSnapshot("home", "")
assert.Contains(t, env.backUp(t, "home"), filesLine(2, 2, 0))
env.setSnapshot("home", "", "*/.local/share/Trash")
assert.Contains(t, env.backUp(t, "home"), filesLine(1, 0, 1))
}
// A backup counts only files that the previous snapshot of its own name
// had, not one that a later snapshot of another name over the same path
// had and that is since removed.
func TestBackupIgnoresSnapshotOfAnotherName(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
env := newDeletedCountEnv(t, map[string][]byte{
"kept.txt": []byte("kept"),
})
env.setSnapshot("home", "")
env.setSnapshot("other", "")
env.backUp(t, "home")
// Snapshot start times are kept in whole seconds: wait so that other's
// snapshot is strictly the most recent.
time.Sleep(time.Second)
extra := filepath.Join(env.src, "extra.txt")
require.NoError(t, os.WriteFile(extra, []byte("extra"), 0o600))
env.backUp(t, "other")
require.NoError(t, os.Remove(extra))
assert.Contains(t, env.backUp(t, "home"), filesLine(1, 0, 1))
}
// A file removed from disk is counted as deleted by the next backup of
// the snapshot that had it, and not by any later backup of that
// snapshot, nor by the first backup of another name over the same path.
func TestBackupCountsDeletedFileOnce(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
aContent := bytes.Repeat([]byte("a"), 1000)
bContent := bytes.Repeat([]byte("b"), 2000)
keptContent := bytes.Repeat([]byte("k"), 500)
env := newDeletedCountEnv(t, map[string][]byte{
"a.txt": aContent,
"b.txt": bContent,
"kept.txt": keptContent,
})
env.setSnapshot("home", "")
env.setSnapshot("other", "")
assert.Contains(t, env.backUp(t, "home"), filesLine(3, 3, 0))
require.NoError(t, os.Remove(filepath.Join(env.src, "a.txt")))
require.NoError(t, os.Remove(filepath.Join(env.src, "b.txt")))
summary := env.backUp(t, "home")
assert.Contains(t, summary,
"Files: 1 examined, 0 backed up, 1 unchanged, 2 deleted.")
assert.Contains(t, summary,
fmt.Sprintf("Data: %s total (%s backed up), %s deleted.",
env.v.UI.Size(int64(len(keptContent))), env.v.UI.Size(0),
env.v.UI.Size(int64(len(aContent)+len(bContent)))))
assert.Contains(t, env.backUp(t, "home"), filesLine(1, 0, 1))
assert.Contains(t, env.backUp(t, "other"), filesLine(1, 0, 1))
}