diff --git a/README.md b/README.md index c710ecb..15f4ce5 100644 --- a/README.md +++ b/README.md @@ -335,8 +335,17 @@ value where a list belongs or a word where a number belongs), is refused. either: every older snapshot is kept, and the command names the path and exits non-zero, on every run until the path holds something again. Take a path that is empty on purpose out of the config, or remove the older - snapshots by hand with `snapshot purge`. A file removed during the backup - does not stop the prune. + snapshots by hand with `snapshot purge`. When an earlier snapshot of the + same name in the local index found a volume mounted at or below a + configured path (a directory on a different filesystem from its parent) + and this backup finds none mounted there, the prune does not run either: + every older snapshot is kept, and the command names the directory and + exits non-zero, on every run until the volume is mounted again. The same + happens when that directory is no longer backed up, because it is excluded + or its configured path was removed. If the volume was retired or the + directory is no longer backed up on purpose, remove the older snapshots by + hand with `snapshot purge`. A file removed during the backup does not stop + the prune. * `--keep-newer-than `: With `--prune`, keep snapshots newer than this duration instead of only the latest (e.g. `4w`, `30d`, `6mo`, `1y`). Without `--prune`, or with a value that is not a valid duration, the diff --git a/TODO.md b/TODO.md index 6c73004..715d9f8 100644 --- a/TODO.md +++ b/TODO.md @@ -24,6 +24,14 @@ owner's; a `v*` tag runs the release build in # Completed Steps +- 2026-10-15: Stopped `snapshot create --prune` pruning after a backup + in which a volume mounted below a configured path in an earlier + snapshot of the same name was not mounted + ([issue #395](https://git.eeqj.de/sneak/vaultik/issues/395)). Such a + prune could delete every snapshot still holding the volume's files. + Each snapshot now records in the local index the directories it found + on a different filesystem from their parent. + - 2026-10-14: Stopped `snapshot create --prune` pruning after a backup that found nothing in a configured path ([issue #388](https://git.eeqj.de/sneak/vaultik/issues/388)), as in diff --git a/docs/DATAMODEL.md b/docs/DATAMODEL.md index f2462e1..094b908 100644 --- a/docs/DATAMODEL.md +++ b/docs/DATAMODEL.md @@ -153,7 +153,17 @@ Maps snapshots to the blobs they reference. **Purpose:** Tracks blob dependencies for snapshots and enables manifest generation. -### 10. `uploads` +### 10. `snapshot_mount_points` +The directories at or below a configured path that a snapshot's backup found on a different filesystem from their parent directory (a different device number), as a mount point is, the configured path itself included. + +**Columns:** +- `snapshot_id` (TEXT) - Snapshot ID (FK to snapshots.id) +- `path` (TEXT) - Absolute path of the directory +- PRIMARY KEY (`snapshot_id`, `path`) + +**Purpose:** Lets `snapshot create --prune` skip the prune when a backup does not find a mount point that an earlier snapshot of the same name found, as when the volume is not mounted. + +### 11. `uploads` Tracks blob upload metrics. **Columns:** @@ -173,6 +183,7 @@ Tracks blob upload metrics. - `INSERT OR REPLACE INTO files` - Update file metadata - `SELECT * FROM files WHERE path = ?` - Check if file has changed - `INSERT INTO snapshot_files` - Add file to current snapshot + - `INSERT INTO snapshot_mount_points` - Record each directory on a different filesystem from its parent 2. **Chunking** (for changed files) - `INSERT OR IGNORE INTO chunks` - Store new chunks diff --git a/internal/cli/entry_snapshot_create_prune_test.go b/internal/cli/entry_snapshot_create_prune_test.go index c6b9597..6f64997 100644 --- a/internal/cli/entry_snapshot_create_prune_test.go +++ b/internal/cli/entry_snapshot_create_prune_test.go @@ -1,6 +1,7 @@ package cli //nolint:testpackage // shares runEntry and the hermetic config helpers import ( + "context" "os" "path/filepath" "strings" @@ -8,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/database" ) // Flags of `snapshot create` used to build argument vectors below. @@ -117,6 +119,51 @@ func TestEntrySnapshotCreatePruneExitsNonZeroWhenSourceIsEmpty(t *testing.T) { assert.Len(t, snapshots, 2, "the older snapshot is kept") } +// TestEntrySnapshotCreatePruneExitsNonZeroWhenMountPointIsMissing backs +// up, records in the local index that this backup found the source +// directory as a mount point, and runs `snapshot create --prune`, which +// does not find it as one. The command exits non-zero, reports the +// backup as completed, not as a failed snapshot creation, and keeps the +// older snapshot. See https://git.eeqj.de/sneak/vaultik/issues/395. +// +//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals +func TestEntrySnapshotCreatePruneExitsNonZeroWhenMountPointIsMissing( + t *testing.T, +) { + configPath, storeDir := writeBackupConfig(t) + + code, _, _ := runEntry(t, flagConfig, configPath, cmdSnapshot, cmdCreate) + require.Equal(t, 0, code) + + // writeBackupConfig puts the source directory and the local index + // beside the config file. + dir := filepath.Dir(configPath) + ctx := context.Background() + + db, err := database.New(ctx, filepath.Join(dir, "index.sqlite")) + require.NoError(t, err) + + repos := database.NewRepositories(db) + + backedUp, err := repos.Snapshots.GetOnlySnapshot(ctx) + require.NoError(t, err) + require.NoError(t, repos.Snapshots.AddMountPoint(ctx, nil, + backedUp.ID.String(), filepath.Join(dir, "source"))) + require.NoError(t, db.Close()) + + code, _, stderr := runEntry(t, + flagConfig, configPath, cmdSnapshot, cmdCreate, flagPrune) + + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "Backup completed, but 1 mount point(s) "+ + "an earlier snapshot recorded were not found") + assert.NotContains(t, stderr, "Snapshot creation failed") + + snapshots, err := os.ReadDir(filepath.Join(storeDir, "metadata")) + require.NoError(t, err) + assert.Len(t, snapshots, 2, "the older snapshot is kept") +} + // writeBackupConfig builds a hermetic config with one small file in its // source directory and an empty destination store directory. Returns the // config path and the store directory. diff --git a/internal/cli/snapshot.go b/internal/cli/snapshot.go index 3c67594..8996d35 100644 --- a/internal/cli/snapshot.go +++ b/internal/cli/snapshot.go @@ -116,7 +116,8 @@ on Linux), then /etc/vaultik/config.yml.`, } if errors.Is(err, vaultik.ErrFilesUnreadable) || - errors.Is(err, vaultik.ErrSourcePathEmpty) { + errors.Is(err, vaultik.ErrSourcePathEmpty) || + errors.Is(err, vaultik.ErrVolumeNotMounted) { log.Error("Backup completed with an error", "error", err) ReportErrorf("Backup completed, but %v", err) @@ -137,7 +138,8 @@ on Linux), then /etc/vaultik/config.yml.`, "directory under metadata/ with no manifest once all its "+ "objects are older than prune_grace_period and its snapshot is "+ "not in the local index (not after a backup that skipped "+ - "unreadable files or found a configured path empty)") + "unreadable files, found a configured path empty, or did not "+ + "find a mount point an earlier snapshot found)") cmd.Flags().StringVar(&opts.KeepNewerThan, "keep-newer-than", "", "With --prune: keep snapshots newer than this duration "+ "(e.g. 30d, 4w, 6mo, 1y; m is minutes, mo is months) "+ diff --git a/internal/database/schema/001.sql b/internal/database/schema/001.sql index 9645b56..d9de0ce 100644 --- a/internal/database/schema/001.sql +++ b/internal/database/schema/001.sql @@ -123,6 +123,16 @@ CREATE TABLE IF NOT EXISTS snapshot_blobs ( -- Index for efficient blob lookups (used in orphan detection) CREATE INDEX IF NOT EXISTS idx_snapshot_blobs_blob_id ON snapshot_blobs(blob_id); +-- Snapshot mount points table: the directories at or below a configured +-- path that the snapshot's backup found on a different filesystem from +-- their parent directory, as a mount point is +CREATE TABLE IF NOT EXISTS snapshot_mount_points ( + snapshot_id TEXT NOT NULL, + path TEXT NOT NULL, + PRIMARY KEY (snapshot_id, path), + FOREIGN KEY (snapshot_id) REFERENCES snapshots(id) ON DELETE CASCADE +); + -- Uploads table: tracks blob upload metrics CREATE TABLE IF NOT EXISTS uploads ( blob_hash TEXT PRIMARY KEY, diff --git a/internal/database/snapshots.go b/internal/database/snapshots.go index f4d5e77..7ada98c 100644 --- a/internal/database/snapshots.go +++ b/internal/database/snapshots.go @@ -24,7 +24,8 @@ var ( ) // SnapshotRepository provides access to the snapshots table and its -// snapshot_files / snapshot_blobs association tables. +// snapshot_files / snapshot_blobs association tables, and to the mount +// points each snapshot's backup found (snapshot_mount_points). type SnapshotRepository struct { db *DB } @@ -494,33 +495,63 @@ func (r *SnapshotRepository) GetBlobHashes( ctx context.Context, snapshotID string, ) ([]string, error) { query := ` - SELECT sb.blob_hash + SELECT sb.blob_hash FROM snapshot_blobs sb WHERE sb.snapshot_id = ? ORDER BY sb.blob_hash ` - rows, err := r.db.conn.QueryContext(ctx, query, snapshotID) + blobs, err := r.queryStrings(ctx, query, snapshotID) if err != nil { return nil, fmt.Errorf("querying blob hashes: %w", err) } - defer func() { _ = rows.Close() }() + return blobs, nil +} - var blobs []string +// AddMountPoint records dir as a directory that the backup of a snapshot +// found on a different filesystem from its parent directory, as a mount +// point is, using tx when non-nil. +func (r *SnapshotRepository) AddMountPoint( + ctx context.Context, tx *sql.Tx, snapshotID string, dir string, +) error { + query := ` + INSERT OR IGNORE INTO snapshot_mount_points (snapshot_id, path) + VALUES (?, ?) + ` - for rows.Next() { - var blobHash string - - err := rows.Scan(&blobHash) - if err != nil { - return nil, fmt.Errorf("scanning blob hash: %w", err) - } - - blobs = append(blobs, blobHash) + var err error + if tx != nil { + _, err = tx.ExecContext(ctx, query, snapshotID, dir) + } else { + _, err = r.db.ExecWithLog(ctx, query, snapshotID, dir) } - return blobs, rows.Err() + if err != nil { + return fmt.Errorf("adding mount point to snapshot: %w", err) + } + + return nil +} + +// GetMountPoints returns the directories AddMountPoint recorded for a +// snapshot, sorted. +func (r *SnapshotRepository) GetMountPoints( + ctx context.Context, snapshotID string, +) ([]string, error) { + query := ` + SELECT path + FROM snapshot_mount_points + WHERE snapshot_id = ? + ORDER BY path + ` + + dirs, err := r.queryStrings(ctx, query, snapshotID) + if err != nil { + return nil, fmt.Errorf("querying mount points: %w", err) + } + + return dirs, nil } // GetSnapshotTotalCompressedSize returns the total compressed size of all @@ -709,6 +740,34 @@ func (r *SnapshotRepository) DeleteSnapshotUploads( return nil } +// queryStrings runs query, which selects one text column, and returns +// its values in the order of the rows. +func (r *SnapshotRepository) queryStrings( + ctx context.Context, query string, args ...any, +) ([]string, error) { + rows, err := r.db.conn.QueryContext(ctx, query, args...) + if err != nil { + return nil, err + } + + defer func() { _ = rows.Close() }() + + var values []string + + for rows.Next() { + var value string + + err := rows.Scan(&value) + if err != nil { + return nil, err + } + + values = append(values, value) + } + + return values, rows.Err() +} + // extendedCompressionRatio computes the compression ratio for a snapshot // from its stored blob_size and the given uncompressed size. Returns 1.0 // when the uncompressed size is zero. diff --git a/internal/database/snapshots_test.go b/internal/database/snapshots_test.go index 5ddbd14..bc943fc 100644 --- a/internal/database/snapshots_test.go +++ b/internal/database/snapshots_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "math" + "slices" "testing" "time" @@ -422,3 +423,52 @@ func TestSnapshotRepositoryDuplicate(t *testing.T) { t.Error("expected error for duplicate snapshot") } } + +// Deleting a snapshot from the local index deletes the mount points its +// backup recorded, and no other snapshot's. See +// https://git.eeqj.de/sneak/vaultik/issues/395. +func TestSnapshotRepositoryDeleteRemovesMountPoints(t *testing.T) { + t.Parallel() + + db, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + repo := database.NewSnapshotRepository(db) + + for _, id := range []types.SnapshotID{"kept", "deleted"} { + err := repo.Create(ctx, nil, &database.Snapshot{ + ID: id, + Hostname: testHostname, + VaultikVersion: testVersion, + StartedAt: time.Now().Truncate(time.Second), + }) + if err != nil { + t.Fatalf("failed to create snapshot: %v", err) + } + + err = repo.AddMountPoint(ctx, nil, id.String(), "/data/nas") + if err != nil { + t.Fatalf("failed to add mount point: %v", err) + } + } + + err := repo.Delete(ctx, "deleted") + if err != nil { + t.Fatalf("failed to delete snapshot: %v", err) + } + + for id, want := range map[string][]string{ + "kept": {"/data/nas"}, + "deleted": nil, + } { + got, err := repo.GetMountPoints(ctx, id) + if err != nil { + t.Fatalf("failed to read mount points: %v", err) + } + + if !slices.Equal(got, want) { + t.Errorf("mount points of %s: got %v, want %v", id, got, want) + } + } +} diff --git a/internal/snapshot/device_number_test.go b/internal/snapshot/device_number_test.go new file mode 100644 index 0000000..2b3b0e4 --- /dev/null +++ b/internal/snapshot/device_number_test.go @@ -0,0 +1,40 @@ +//nolint:testpackage // exercises the unexported deviceNumberFromStat +package snapshot + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The device number a real backup reads is the same for two entries of +// one directory and differs between two filesystems. Without it no +// mount point is ever found. See +// https://git.eeqj.de/sneak/vaultik/issues/395. +func TestDeviceNumberFromStat(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + file := filepath.Join(dir, "file.txt") + require.NoError(t, os.WriteFile(file, nil, 0o600)) + + deviceOf := func(path string) uint64 { + t.Helper() + + info, err := os.Stat(path) + require.NoError(t, err) + + device, ok := deviceNumberFromStat(info) + require.True(t, ok, "no device number read for %s", path) + + return device + } + + assert.Equal(t, deviceOf(dir), deviceOf(file)) + + // /dev is a different filesystem from the temp dir on Linux, macOS and in Docker. + assert.NotEqual(t, deviceOf(dir), deviceOf("/dev")) +} diff --git a/internal/snapshot/module.go b/internal/snapshot/module.go index e9f6aab..f83ac07 100644 --- a/internal/snapshot/module.go +++ b/internal/snapshot/module.go @@ -1,6 +1,8 @@ package snapshot import ( + "os" + "github.com/spf13/afero" "go.uber.org/fx" "sneak.berlin/go/vaultik/internal/config" @@ -19,6 +21,9 @@ type ScannerParams struct { // PreviousSnapshotID is the most recent completed snapshot of the // same name, "" for none. PreviousSnapshotID string + // DeviceNumber is ScannerConfig.DeviceNumber; nil reads it from the + // file's stat. + DeviceNumber func(info os.FileInfo) (uint64, bool) } // Module exports backup functionality as an fx module. @@ -59,6 +64,7 @@ func provideScannerFactory( Exclude: excludes, SkipErrors: params.SkipErrors, PreviousSnapshotID: params.PreviousSnapshotID, + DeviceNumber: params.DeviceNumber, }) } } diff --git a/internal/snapshot/mount_point_test.go b/internal/snapshot/mount_point_test.go new file mode 100644 index 0000000..8026b20 --- /dev/null +++ b/internal/snapshot/mount_point_test.go @@ -0,0 +1,97 @@ +package snapshot_test + +import ( + "context" + "os" + "path/filepath" + "slices" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/database" + "sneak.berlin/go/vaultik/internal/snapshot" +) + +// A scan records with the snapshot each directory on a different device +// from its parent directory, as a mount point is, the scanned path +// included. A filesystem that gives no device numbers, as the in-memory +// one, records none. See https://git.eeqj.de/sneak/vaultik/issues/395. +func TestScanRecordsMountPoints(t *testing.T) { + t.Parallel() + + const ( + scannedPath = "/data/source" + snapshotID = "mount-points" + ) + + tests := []struct { + name string + // onVolume names the directories on the volume's device; every + // other directory is on another one. nil leaves the device + // numbers to the in-memory filesystem, which has none. + onVolume []string + want []string + }{ + { + name: "volume below the scanned path", + onVolume: []string{"nas"}, + want: []string{scannedPath + "/nas"}, + }, + { + name: "volume at the scanned path", + onVolume: []string{"source", "nas"}, + want: []string{scannedPath}, + }, + {name: "no device numbers", want: nil}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctx := context.Background() + fs := afero.NewMemMapFs() + + for _, path := range []string{ + scannedPath + "/notes.txt", scannedPath + "/nas/a.txt", + } { + require.NoError(t, fs.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, afero.WriteFile(fs, path, []byte(path), 0o644)) + } + + db, err := database.NewTestDB() + require.NoError(t, err) + t.Cleanup(func() { _ = db.Close() }) + + repos := database.NewRepositories(db) + createTestSnapshotRecord(ctx, t, repos, snapshotID) + + cfg := snapshot.ScannerConfig{ + FS: fs, + ChunkSize: int64(1024 * 16), + Repositories: repos, + MaxBlobSize: int64(1024 * 1024), + CompressionLevel: 3, + AgeRecipients: []string{testAgePublicKey}, + } + if tt.onVolume != nil { + cfg.DeviceNumber = func(info os.FileInfo) (uint64, bool) { + if slices.Contains(tt.onVolume, info.Name()) { + return 2, true + } + + return 1, true + } + } + + _, err = snapshot.NewScanner(cfg).Scan(ctx, scannedPath, snapshotID) + require.NoError(t, err) + + got, err := repos.Snapshots.GetMountPoints(ctx, snapshotID) + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/internal/snapshot/scanner.go b/internal/snapshot/scanner.go index 5c2dd77..ccd6bc5 100644 --- a/internal/snapshot/scanner.go +++ b/internal/snapshot/scanner.go @@ -73,6 +73,9 @@ type Scanner struct { skipErrors bool // ui is the user-facing output; never nil (defaults to a discarding writer). ui *ui.Writer + // deviceNumber returns the number of the device that holds a file; + // never nil (defaults to deviceNumberFromStat). + deviceNumber func(info os.FileInfo) (uint64, bool) // In-memory cache of known chunk hashes for fast existence checks knownChunks map[string]struct{} @@ -136,6 +139,10 @@ type ScannerConfig struct { // PreviousSnapshotID is the most recent completed snapshot of the // same name, "" for none. PreviousSnapshotID string + // DeviceNumber returns the number of the device that holds a file, + // and false when there is none to read. nil reads it from the file's + // stat; tests replace it, since the in-memory test filesystem has none. + DeviceNumber func(info os.FileInfo) (uint64, bool) } // ScanResult contains the results of a scan operation. Files and bytes @@ -214,6 +221,11 @@ func NewScanner(cfg ScannerConfig) *Scanner { uiw = ui.NewWithColor(io.Discard, false) } + deviceNumber := cfg.DeviceNumber + if deviceNumber == nil { + deviceNumber = deviceNumberFromStat + } + return &Scanner{ fs: cfg.FS, chunker: chunker.NewChunker(cfg.ChunkSize), @@ -229,6 +241,7 @@ func NewScanner(cfg ScannerConfig) *Scanner { skipErrors: cfg.SkipErrors, previousSnapshotID: cfg.PreviousSnapshotID, ui: uiw, + deviceNumber: deviceNumber, pendingChunkHashes: make(map[string]struct{}), } } @@ -1290,9 +1303,9 @@ func (s *Scanner) buildDirectoryEntry(path string, info os.FileInfo) *database.F } } -// recordDirectories records every directory the walk found. A directory -// has no data to back up, so it is not among the files counted and -// backed up. +// recordDirectories records every directory the walk found, and its +// mount points (see recordMountPoints). A directory has no data to back +// up, so it is not among the files counted and backed up. func (s *Scanner) recordDirectories( ctx context.Context, directories []*FileToProcess, ) error { @@ -1303,7 +1316,97 @@ func (s *Scanner) recordDirectories( } } - return nil + return s.recordMountPoints(ctx, directories) +} + +// recordMountPoints records with the snapshot each directory the walk +// found on a different device from its parent directory, as a mount +// point is, the scanned path included. +func (s *Scanner) recordMountPoints( + ctx context.Context, directories []*FileToProcess, +) error { + mountPoints, err := s.findMountPoints(directories) + if err != nil { + return err + } + + return s.repos.WithTx(ctx, func(txCtx context.Context, tx *sql.Tx) error { + for _, dir := range mountPoints { + log.Info("Found a mount point", "path", dir) + + err := s.repos.Snapshots.AddMountPoint(txCtx, tx, s.snapshotID, dir) + if err != nil { + return fmt.Errorf("recording mount point %s: %w", dir, err) + } + } + + return nil + }) +} + +// findMountPoints returns the directories, among those the walk found, +// that are on a different device from their parent directory. The +// scanned path is compared with the directory that holds it, which the +// walk did not reach. +func (s *Scanner) findMountPoints( + directories []*FileToProcess, +) ([]string, error) { + walked := make(map[string]os.FileInfo, len(directories)) + for _, dir := range directories { + walked[dir.Path] = dir.FileInfo + } + + var mountPoints []string + + for _, dir := range directories { + parentPath := filepath.Dir(dir.Path) + + parent, ok := walked[parentPath] + if !ok { + var err error + + parent, err = s.fs.Stat(parentPath) + if err != nil { + return nil, fmt.Errorf( + "looking up the parent of %s: %w", dir.Path, err) + } + } + + if s.onOtherDevice(dir.FileInfo, parent) { + mountPoints = append(mountPoints, dir.Path) + } + } + + return mountPoints, nil +} + +// onOtherDevice reports whether dir is on a different device from +// parent. A file whose device number cannot be read counts as on the +// same device as its parent. +func (s *Scanner) onOtherDevice(dir, parent os.FileInfo) bool { + dirDevice, ok := s.deviceNumber(dir) + if !ok { + return false + } + + parentDevice, ok := s.deviceNumber(parent) + if !ok { + return false + } + + return dirDevice != parentDevice +} + +// deviceNumberFromStat returns the number of the device that holds the +// file info describes, and false when info carries no stat, as from the +// in-memory test filesystem. +func deviceNumberFromStat(info os.FileInfo) (uint64, bool) { + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return 0, false + } + + return uint64(stat.Dev), true //nolint:unconvert // Dev is an int32 on darwin } // recordNonRegularFile writes a symlink or directory entry to the database diff --git a/internal/snapshot/snapshot.go b/internal/snapshot/snapshot.go index a959930..80b032c 100644 --- a/internal/snapshot/snapshot.go +++ b/internal/snapshot/snapshot.go @@ -518,8 +518,8 @@ type CleanupStats struct { // // The cleanup is performed in a specific order to maintain referential integrity: // 1. Delete other snapshots -// 2. Delete orphaned snapshot associations (snapshot_files, snapshot_blobs) -// for deleted snapshots +// 2. Delete orphaned snapshot associations (snapshot_files, snapshot_blobs, +// snapshot_mount_points) for deleted snapshots // 3. Delete orphaned files (not in the current snapshot) // 4. Delete orphaned chunk-to-file mappings (references to deleted files) // 5. Delete orphaned blobs (not in the current snapshot) @@ -948,8 +948,8 @@ func (sm *SnapshotManager) deleteOtherSnapshots( return nil } -// deleteOrphanedSnapshotAssociations deletes snapshot_files and -// snapshot_blobs for deleted snapshots +// deleteOrphanedSnapshotAssociations deletes snapshot_files, +// snapshot_blobs and snapshot_mount_points for deleted snapshots func (sm *SnapshotManager) deleteOrphanedSnapshotAssociations( ctx context.Context, tx *sql.Tx, currentSnapshotID string, ) error { @@ -983,6 +983,19 @@ func (sm *SnapshotManager) deleteOrphanedSnapshotAssociations( log.Debug("[Temp DB Cleanup] Deleted snapshot_blobs associations", "count", rowsAffected) + // Delete the mount points other snapshots recorded + query := "DELETE FROM snapshot_mount_points WHERE snapshot_id != ?" + database.LogSQL("Execute", query, currentSnapshotID) + + result, err = tx.ExecContext(ctx, query, currentSnapshotID) + if err != nil { + return fmt.Errorf("deleting orphaned snapshot_mount_points: %w", err) + } + + rowsAffected, _ = result.RowsAffected() + log.Debug("[Temp DB Cleanup] Deleted snapshot_mount_points rows", + "count", rowsAffected) + return nil } diff --git a/internal/snapshot/snapshot_test.go b/internal/snapshot/snapshot_test.go index 38e38d8..b371d72 100644 --- a/internal/snapshot/snapshot_test.go +++ b/internal/snapshot/snapshot_test.go @@ -7,12 +7,14 @@ import ( "database/sql" "io" "path/filepath" + "slices" "testing" "github.com/spf13/afero" "sneak.berlin/go/vaultik/internal/config" "sneak.berlin/go/vaultik/internal/database" "sneak.berlin/go/vaultik/internal/log" + "sneak.berlin/go/vaultik/internal/types" ) const ( @@ -387,6 +389,77 @@ func TestCleanSnapshotDBNonExistentSnapshot(t *testing.T) { } } +// The exported metadata database keeps only the mount points that the +// exported snapshot recorded. See +// https://git.eeqj.de/sneak/vaultik/issues/395. +func TestCleanSnapshotDBKeepsOnlyTheSnapshotsMountPoints(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + ctx := context.Background() + dbPath := filepath.Join(t.TempDir(), "export.db") + + db, err := database.New(ctx, dbPath) + if err != nil { + t.Fatalf("failed to create database: %v", err) + } + + repos := database.NewRepositories(db) + + for _, id := range []types.SnapshotID{"exported", "other"} { + err = repos.Snapshots.Create(ctx, nil, &database.Snapshot{ID: id}) + if err != nil { + t.Fatalf("failed to create snapshot: %v", err) + } + + err = repos.Snapshots.AddMountPoint(ctx, nil, id.String(), "/data/"+id.String()) + if err != nil { + t.Fatalf("failed to add mount point: %v", err) + } + } + + err = db.Close() + if err != nil { + t.Fatalf("failed to close database: %v", err) + } + + sm := &SnapshotManager{ + config: &config.Config{ + CompressionLevel: 3, + AgeRecipients: []string{testAgeRecipient}, + }, + fs: afero.NewOsFs(), + } + + _, err = sm.cleanSnapshotDB(ctx, dbPath, "exported") + if err != nil { + t.Fatalf("failed to clean snapshot database: %v", err) + } + + cleaned, err := database.New(ctx, dbPath) + if err != nil { + t.Fatalf("failed to open cleaned database: %v", err) + } + + defer func() { _ = cleaned.Close() }() + + cleanedRepos := database.NewRepositories(cleaned) + + for id, want := range map[string][]string{ + "exported": {"/data/exported"}, + "other": nil, + } { + got, err := cleanedRepos.Snapshots.GetMountPoints(ctx, id) + if err != nil { + t.Fatalf("failed to read mount points: %v", err) + } + + if !slices.Equal(got, want) { + t.Errorf("mount points of %s: got %v, want %v", id, got, want) + } + } +} + // Two creates of one snapshot name back to back start within one second, // the resolution of the timestamp in a snapshot ID. See // https://git.eeqj.de/sneak/vaultik/issues/270. diff --git a/internal/vaultik/fault_injection_test.go b/internal/vaultik/fault_injection_test.go index c3f2f5d..3cccec3 100644 --- a/internal/vaultik/fault_injection_test.go +++ b/internal/vaultik/fault_injection_test.go @@ -679,6 +679,7 @@ func faultScannerFactory( Exclude: params.Exclude, SkipErrors: params.SkipErrors, PreviousSnapshotID: params.PreviousSnapshotID, + DeviceNumber: params.DeviceNumber, }) } } diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index 68a44d5..b519fe1 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "regexp" + "slices" "sort" "strconv" "strings" @@ -52,6 +53,17 @@ var ErrFilesUnreadable = errors.New("file(s) could not be read") // returns ErrFilesUnreadable instead. var ErrSourcePathEmpty = errors.New("source path(s) were empty") +// ErrVolumeNotMounted is returned when --prune skipped the post-backup +// prune because a directory that an earlier snapshot of the same name +// found as a mount point was not found as one in this run: the volume +// mounted there is not mounted, or the directory is no longer backed up. +// The snapshots are complete and kept, so the caller can report it apart +// from a failed backup. A run that also skipped unreadable files or +// found a configured path empty returns ErrFilesUnreadable or +// ErrSourcePathEmpty instead. +var ErrVolumeNotMounted = errors.New( + "mount point(s) an earlier snapshot recorded were not found") + // listRecentLimit caps how many snapshot rows are fetched from the // local index for listing and purge operations. const listRecentLimit = 10000 @@ -142,7 +154,7 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error { return err } - filesUnreadable, emptyPaths, err := v.createNamedSnapshots( + filesUnreadable, emptyPaths, unmountedPaths, err := v.createNamedSnapshots( opts, hostname, snapshotNames) if err != nil { return err @@ -155,8 +167,10 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error { } switch { - case opts.Prune && (filesUnreadable > 0 || len(emptyPaths) > 0): - return v.skipPostBackupPrune(filesUnreadable, emptyPaths) + case opts.Prune && (filesUnreadable > 0 || len(emptyPaths) > 0 || + len(unmountedPaths) > 0): + return v.skipPostBackupPrune( + filesUnreadable, emptyPaths, unmountedPaths) case opts.Prune: err = v.runPostBackupPrune(snapshotNames, opts.KeepNewerThan) if err != nil { @@ -169,20 +183,22 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error { // createNamedSnapshots creates each named snapshot in turn. It returns // the number of entries --skip-errors skipped because it could not read -// or list them, and each configured path in which the scan found -// nothing, named with its snapshot. +// or list them, each configured path in which the scan found nothing, +// and, under --prune, each directory where an earlier snapshot found a +// volume mounted and this run did not, the paths named with their +// snapshot. func (v *Vaultik) createNamedSnapshots( opts *SnapshotCreateOptions, hostname string, snapshotNames []string, -) (int, []string, error) { +) (int, []string, []string, error) { filesUnreadable := 0 - var emptyPaths []string + var emptyPaths, unmountedPaths []string for snapIdx, snapName := range snapshotNames { stats, err := v.createNamedSnapshot( opts, hostname, snapName, snapIdx+1, len(snapshotNames)) if err != nil { - return 0, nil, err + return 0, nil, nil, err } filesUnreadable += stats.totalFilesUnreadable @@ -190,21 +206,27 @@ func (v *Vaultik) createNamedSnapshots( for _, dir := range stats.emptyPaths { emptyPaths = append(emptyPaths, dir+" of snapshot "+snapName) } + + for _, dir := range stats.unmountedPaths { + unmountedPaths = append(unmountedPaths, + dir+" of snapshot "+snapName) + } } - return filesUnreadable, emptyPaths, nil + return filesUnreadable, emptyPaths, unmountedPaths, nil } // skipPostBackupPrune says why the post-backup prune does not run, naming // every cause, and returns the error the run ends with. An older snapshot // may be the only one that still holds a file this run could not read, -// or what an empty path held. The previous snapshot is no guide: after -// one run with an empty path it is empty too. +// or what an empty path or a volume that is not mounted held. The +// previous snapshot is no guide: after one such run it lacks them too. func (v *Vaultik) skipPostBackupPrune( - filesUnreadable int, emptyPaths []string, + filesUnreadable int, emptyPaths, unmountedPaths []string, ) error { log.Warn("Skipping post-backup prune", - "files_unreadable", filesUnreadable, "empty_paths", emptyPaths) + "files_unreadable", filesUnreadable, "empty_paths", emptyPaths, + "unmounted_paths", unmountedPaths) if len(emptyPaths) > 0 { v.UI.Warningf("Skipped the post-backup prune because nothing was "+ @@ -212,6 +234,17 @@ func (v *Vaultik) skipPostBackupPrune( strings.Join(emptyPaths, ", ")) } + if len(unmountedPaths) > 0 { + v.UI.Warningf("Skipped the post-backup prune because this backup "+ + "did not find a mount point at %s, where an earlier snapshot "+ + "found one: either the volume is not mounted or the directory "+ + "is no longer backed up. Older snapshots are kept. Mount the "+ + "volume again, or, if the volume was retired or the directory "+ + "is no longer backed up on purpose, remove the older snapshots "+ + "by hand with 'vaultik snapshot purge'.", + strings.Join(unmountedPaths, ", ")) + } + if filesUnreadable > 0 { v.UI.Warningf("Skipped the post-backup prune because %d file(s) "+ "could not be read; older snapshots are kept.", filesUnreadable) @@ -219,7 +252,11 @@ func (v *Vaultik) skipPostBackupPrune( return fmt.Errorf("%d %w", filesUnreadable, ErrFilesUnreadable) } - return fmt.Errorf("%d %w", len(emptyPaths), ErrSourcePathEmpty) + if len(emptyPaths) > 0 { + return fmt.Errorf("%d %w", len(emptyPaths), ErrSourcePathEmpty) + } + + return fmt.Errorf("%d %w", len(unmountedPaths), ErrVolumeNotMounted) } // finishCreateSnapshot ends a run whose snapshots are all complete and @@ -329,6 +366,10 @@ type snapshotStats struct { // The configured paths in which the scan found nothing. emptyPaths []string + // Under --prune, the directories where an earlier snapshot of the + // same name found a volume mounted and this one did not. + unmountedPaths []string + // The sizes of all blobs the snapshot references, set by // finalizeSnapshotMetadata once snapshot_blobs is populated. blobSize int64 @@ -394,9 +435,57 @@ func (v *Vaultik) createNamedSnapshot( v.printSnapshotSummary(snapshotID, snapshotStartTime, stats) + if opts.Prune { + stats.unmountedPaths, err = v.findUnmountedPaths(snapName, snapshotID) + if err != nil { + return nil, err + } + } + return stats, nil } +// findUnmountedPaths returns each directory that an earlier snapshot +// named snapName in the local index recorded as a mount point and +// snapshot snapshotID did not, as when the volume mounted there is not +// mounted. +func (v *Vaultik) findUnmountedPaths( + snapName, snapshotID string, +) ([]string, error) { + found, err := v.Repositories.Snapshots.GetMountPoints(v.ctx, snapshotID) + if err != nil { + return nil, err + } + + snapshots, err := v.Repositories.Snapshots.ListRecent(v.ctx, listRecentLimit) + if err != nil { + return nil, fmt.Errorf("listing snapshots: %w", err) + } + + var unmounted []string + + for _, s := range snapshots { + if s.ID.String() == snapshotID || + parseSnapshotName(s.ID.String(), s.Hostname.String()) != snapName { + continue + } + + recorded, err := v.Repositories.Snapshots.GetMountPoints( + v.ctx, s.ID.String()) + if err != nil { + return nil, err + } + + for _, dir := range recorded { + if !slices.Contains(found, dir) && !slices.Contains(unmounted, dir) { + unmounted = append(unmounted, dir) + } + } + } + + return unmounted, 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) { diff --git a/internal/vaultik/snapshot_prune_test.go b/internal/vaultik/snapshot_prune_test.go index 035dd5a..bb36031 100644 --- a/internal/vaultik/snapshot_prune_test.go +++ b/internal/vaultik/snapshot_prune_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "sneak.berlin/go/vaultik/internal/config" "sneak.berlin/go/vaultik/internal/log" + "sneak.berlin/go/vaultik/internal/snapshot" "sneak.berlin/go/vaultik/internal/vaultik" ) @@ -259,3 +260,99 @@ func TestBackupWithNoEmptySourcePathPrunes(t *testing.T) { }) } } + +// A backup under --prune in which a volume that an earlier snapshot found +// mounted below the configured path is not mounted does not prune: run +// after run, the snapshot that holds the volume's files is kept with +// everything it stored, and the run fails. Once the volume is mounted +// again, the prune runs. See https://git.eeqj.de/sneak/vaultik/issues/395. +func TestBackupWithVolumeNotMountedDoesNotPrune(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + // volume is the directory, below the configured path, where the + // volume is mounted. + const ( + snapshotName = "data" + volume = "nas" + ) + + ctx := context.Background() + env := newDeletedCountEnv(t, map[string][]byte{ + volume + "/a.txt": []byte("file a"), + "notes.txt": []byte("notes"), + }) + env.v.Config.Snapshots[snapshotName] = config.SnapshotConfig{ + Paths: []string{env.src}, + } + prune := &vaultik.SnapshotCreateOptions{ + Prune: true, Snapshots: []string{snapshotName}, + } + + // While the volume is mounted, its directory is on a device of its + // own. + mounted := true + newScanner := env.v.ScannerFactory + env.v.ScannerFactory = func(params snapshot.ScannerParams) *snapshot.Scanner { + params.DeviceNumber = func(info os.FileInfo) (uint64, bool) { + if mounted && info.Name() == volume { + return 2, true + } + + return 1, true + } + + return newScanner(params) + } + + env.backUp(t, snapshotName) + + firstID := localSnapshotID(ctx, t, env.v.Repositories, snapshotName) + stored, err := env.v.Storage.List(ctx, "") + require.NoError(t, err) + + // A volume that is not mounted leaves its mount point behind as an + // empty directory on its parent's device. + volumeDir := filepath.Join(env.src, volume) + require.NoError(t, os.RemoveAll(volumeDir)) + require.NoError(t, os.Mkdir(volumeDir, 0o750)) + + mounted = false + + for range 2 { + env.out.Reset() + + err := env.v.CreateSnapshot(prune) + require.ErrorIs(t, err, vaultik.ErrVolumeNotMounted) + assert.Contains(t, env.out.String(), + "did not find a mount point at "+volumeDir+" of snapshot "+ + snapshotName) + assert.Contains(t, env.out.String(), "'vaultik snapshot purge'") + assert.NotContains(t, env.out.String(), "Running post-backup prune.") + } + + snapshots, err := env.v.Repositories.Snapshots.ListRecent( + ctx, listRecentTestLimit) + require.NoError(t, err) + require.Len(t, snapshots, 3) + assert.Equal(t, firstID, snapshots[2].ID.String(), + "the snapshot that holds the volume's files is kept") + + for _, key := range stored { + _, err := env.v.Storage.Stat(ctx, key) + require.NoError(t, err, "%s is kept", key) + } + + // The volume is mounted again. + require.NoError(t, os.WriteFile( + filepath.Join(volumeDir, "a.txt"), []byte("file a"), 0o600)) + + mounted = true + + require.NoError(t, env.v.CreateSnapshot(prune)) + + snapshots, err = env.v.Repositories.Snapshots.ListRecent( + ctx, listRecentTestLimit) + require.NoError(t, err) + require.Len(t, snapshots, 1, "the older snapshots are pruned") +}