Do not prune after a backup that found a volume not mounted (closes #395)

snapshot create --prune purged the older snapshots holding a volume's
files when the volume, mounted below a configured path, was not
mounted: its empty mount point read as deleted files.

The scan now records in the local index each directory at or below a
configured path that is on a different device from its parent
directory. Under --prune, a run that does not find such a directory
recorded by an earlier snapshot of that name skips the prune, keeps
every older snapshot, names the directory and its snapshot, and exits
non-zero, as for an empty configured path. A volume retired on purpose
is cleared by removing the older snapshots with snapshot purge.

Model: opus-5-5

README.md

@@ -342,10 +342,9 @@ value where a list belongs or a word where a number belongs), is refused.
   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.
+  or its configured path was removed. For a volume retired 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 <duration>`: 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

docs/DATAMODEL.md

@@ -161,7 +161,7 @@ The directories at or below a configured path that a snapshot's backup found on
 - `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.
+**Purpose:** Lets `snapshot create --prune` skip the prune when a volume an earlier snapshot of the same name found mounted is not mounted now.
 
 ### 11. `uploads`
 Tracks blob upload metrics.

internal/cli/entry_snapshot_create_prune_test.go

@@ -1,7 +1,6 @@
 package cli //nolint:testpackage // shares runEntry and the hermetic config helpers
 
 import (
-	"context"
 	"os"
 	"path/filepath"
 	"strings"
@@ -9,7 +8,6 @@ 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.
@@ -119,51 +117,6 @@ 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.

internal/cli/snapshot.go

@@ -138,8 +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, found a configured path empty, or did not "+
-			"find a mount point an earlier snapshot found)")
+			"unreadable files, found a configured path empty, or found "+
+			"no volume where an earlier snapshot found one mounted)")
 	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) "+

internal/snapshot/device_number_test.go

@@ -1,40 +0,0 @@
-//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"))
-}

internal/vaultik/snapshot.go

@@ -55,14 +55,12 @@ 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")
+// found as a mount point was not one in this run, as when the volume
+// mounted there is not mounted. 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("volume(s) were not mounted")
 
 // listRecentLimit caps how many snapshot rows are fetched from the
 // local index for listing and purge operations.
@@ -235,13 +233,11 @@ func (v *Vaultik) skipPostBackupPrune(
 	}
 
 	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'.",
+		v.UI.Warningf("Skipped the post-backup prune because no volume "+
+			"is mounted at %s, where an earlier snapshot found one; older "+
+			"snapshots are kept. If a volume was retired on purpose, "+
+			"remove the older snapshots by hand with "+
+			"'vaultik snapshot purge'.",
 			strings.Join(unmountedPaths, ", "))
 	}
 

internal/vaultik/snapshot_prune_test.go

@@ -325,8 +325,7 @@ func TestBackupWithVolumeNotMountedDoesNotPrune(t *testing.T) {
 		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)
+			"no volume is mounted at "+volumeDir+" of snapshot "+snapshotName)
 		assert.Contains(t, env.out.String(), "'vaultik snapshot purge'")
 		assert.NotContains(t, env.out.String(), "Running post-backup prune.")
 	}