restic check and restic repair packs: treat missing packfiles the same as damaged and truncated packfiles (#21845)

Co-authored-by: Michael Eischer <michael.eischer@fau.de>

changelog/unreleased/pull-21845

@@ -0,0 +1,7 @@
+Enhancement: `restic repair packs` can handle missing packfiles
+
+Missing and damaged packfiles previously required separate approaches to fix a repository.
+Now, both cases can be handled by the `repair packs` command, which is now able to handle
+missing packfiles by removing them from the repository index.
+
+https://github.com/restic/restic/pull/21845

cmd/restic/cmd_check.go

@@ -313,7 +313,7 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts global.Options, args
 				orphanedPacks++
 				printer.V("%v\n", err)
 			} else {
-				if packErr.Truncated {
+				if packErr.Truncated || packErr.Missing {
 					salvagePacks.Insert(packErr.ID)
 				}
 				errorsFound = true

cmd/restic/cmd_check_integration_test.go

@@ -11,35 +11,36 @@ import (
 
 func testRunCheck(t testing.TB, gopts global.Options) {
 	t.Helper()
-	output, err := testRunCheckOutput(t, gopts, true)
+	stdout, stderr, err := testRunCheckOutput(t, gopts, true)
 	if err != nil {
-		t.Error(output)
+		t.Error(stdout)
+		t.Error(stderr)
 		t.Fatalf("unexpected error: %+v", err)
 	}
 }
 
 func testRunCheckMustFail(t testing.TB, gopts global.Options) {
 	t.Helper()
-	_, err := testRunCheckOutput(t, gopts, false)
+	_, _, err := testRunCheckOutput(t, gopts, false)
 	rtest.Assert(t, err != nil, "expected non nil error after check of damaged repository")
 }
 
-func testRunCheckOutput(t testing.TB, gopts global.Options, checkUnused bool) (string, error) {
-	buf, err := withCaptureStdout(t, gopts, func(ctx context.Context, gopts global.Options) error {
+func testRunCheckOutput(t testing.TB, gopts global.Options, checkUnused bool) (string, string, error) {
+	stdout, stderr, err := withCaptureStdoutStderr(t, gopts, func(ctx context.Context, gopts global.Options) error {
 		opts := CheckOptions{
 			ReadData:    true,
 			CheckUnused: checkUnused,
 		}
-		_, err := runCheck(context.TODO(), opts, gopts, nil, gopts.Term)
+		_, err := runCheck(ctx, opts, gopts, nil, gopts.Term)
 		return err
 	})
-	return buf.String(), err
+	return stdout.String(), stderr.String(), err
 }
 
 func testRunCheckOutputWithOpts(t testing.TB, gopts global.Options, opts CheckOptions, args []string) (string, error) {
 	buf, err := withCaptureStdout(t, gopts, func(ctx context.Context, gopts global.Options) error {
 		gopts.Verbosity = 2
-		_, err := runCheck(context.TODO(), opts, gopts, args, gopts.Term)
+		_, err := runCheck(ctx, opts, gopts, args, gopts.Term)
 		return err
 	})
 	return buf.String(), err

cmd/restic/cmd_repair_index_integration_test.go

@@ -31,7 +31,7 @@ func testRebuildIndex(t *testing.T, backendTestHook global.BackendWrapper) {
 	datafile := filepath.Join("..", "..", "internal", "checker", "testdata", "duplicate-packs-in-index-test-repo.tar.gz")
 	rtest.SetupTarTestFixture(t, env.base, datafile)
 
-	out, err := testRunCheckOutput(t, env.gopts, false)
+	out, _, err := testRunCheckOutput(t, env.gopts, false)
 	if !strings.Contains(out, "contained in several indexes") {
 		t.Fatalf("did not find checker hint for packs in several indexes")
 	}
@@ -48,7 +48,7 @@ func testRebuildIndex(t *testing.T, backendTestHook global.BackendWrapper) {
 	testRunRebuildIndex(t, env.gopts)
 
 	env.gopts.BackendTestHook = nil
-	out, err = testRunCheckOutput(t, env.gopts, false)
+	out, _, err = testRunCheckOutput(t, env.gopts, false)
 	if len(out) != 0 {
 		t.Fatalf("expected no output from the checker, got: %v", out)
 	}

cmd/restic/cmd_repair_packs.go

@@ -69,9 +69,13 @@ func runRepairPacks(ctx context.Context, gopts global.Options, term ui.Terminal,
 	printer.P("saving backup copies of pack files to current folder")
 	for id := range ids {
 		buf, err := repo.LoadRaw(ctx, restic.PackFile, id)
-		// corrupted data is fine
-		if buf == nil {
-			return err
+		if ctx.Err() != nil {
+			return ctx.Err()
+		}
+		// only skip creating a local copy if no data at all could be loaded
+		if err != nil && buf == nil {
+			printer.E("will remove packfile %v due to failed download: %v", id, err)
+			continue
 		}
 
 		f, err := os.OpenFile("pack-"+id.String(), os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o666)

cmd/restic/cmd_repair_packs_integration_test.go

@@ -0,0 +1,75 @@
+package main
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/restic/restic/internal/global"
+	"github.com/restic/restic/internal/restic"
+	rtest "github.com/restic/restic/internal/test"
+	"github.com/restic/restic/internal/ui/progress"
+)
+
+// testRunRepairPacks runs `restic repair packs` with capturing stdout and stderr
+func testRunRepairPacks(t testing.TB, gopts global.Options, args []string) (string, string, error) {
+	bufStdout, bufStderr, err := withCaptureStdoutStderr(t, gopts, func(ctx context.Context, gopts global.Options) error {
+		return runRepairPacks(ctx, gopts, gopts.Term, args)
+	})
+
+	return bufStdout.String(), bufStderr.String(), err
+}
+
+func TestRunRepairPackfiles(t *testing.T) {
+	env, cleanup := withTestEnvironment(t)
+	defer cleanup()
+
+	testSetupBackupData(t, env)
+	// backup of subtree 0/0/9/42
+	testRunBackup(t, env.testdata, []string{filepath.Join(env.testdata, "0", "0", "9", "42")}, BackupOptions{}, env.gopts)
+	testListSnapshots(t, env.gopts, 1)
+
+	packfileID := restic.ID{}
+	err := withTermStatus(t, env.gopts, func(ctx context.Context, gopts global.Options) error {
+		printer := progress.NewTerminalPrinter(false, gopts.Verbosity, gopts.Term)
+		_, repo, unlock, err := openWithReadLock(ctx, gopts, false, printer)
+		rtest.OK(t, err)
+		defer unlock()
+
+		rtest.OK(t, repo.LoadIndex(ctx, printer))
+		// load packfiles from master index
+		err = repo.ListBlobs(ctx, func(blob restic.PackBlob) {
+			if blob.Handle().Type == restic.DataBlob {
+				packfileID = blob.PackID()
+				return
+			}
+		})
+		rtest.OK(t, err)
+
+		return nil
+	})
+	rtest.OK(t, err)
+
+	rtest.Assert(t, !packfileID.IsNull(), "expected valid packfile ID")
+	packIDString := packfileID.String()
+	filename := filepath.Join(env.gopts.Repo, "data", packIDString[0:2], packIDString)
+	rtest.OK(t, os.Remove(filename))
+
+	_, outError, err := testRunCheckOutput(t, env.gopts, false)
+	rtest.Assert(t, err != nil, "expected check errors, got none")
+	rtest.Assert(t, strings.Contains(string(outError), packIDString), "expected mention of %q", packIDString)
+
+	// change to temporary directory to not pollute the repository with backup files
+	cleanupChdir := rtest.Chdir(t, env.base)
+	defer cleanupChdir()
+	// restic repair packs 'packIDString'
+	_, _, err = testRunRepairPacks(t, env.gopts, []string{packIDString})
+	rtest.OK(t, err)
+
+	// run restic repair snapshots --forget
+	testRunRepairSnapshot(t, env.gopts, true)
+	_, _, err = testRunCheckOutput(t, env.gopts, false)
+	rtest.OK(t, err)
+}

cmd/restic/cmd_repair_snapshots_integration_test.go

@@ -69,7 +69,7 @@ func TestRepairSnapshotsWithLostData(t *testing.T) {
 	// repository must be ok after removing the broken snapshots
 	testRunForget(t, env.gopts, ForgetOptions{}, snapshotIDs[0].String(), snapshotIDs[1].String())
 	testListSnapshots(t, env.gopts, 2)
-	_, err := testRunCheckOutput(t, env.gopts, false)
+	_, _, err := testRunCheckOutput(t, env.gopts, false)
 	rtest.OK(t, err)
 }
 
@@ -98,7 +98,7 @@ func TestRepairSnapshotsWithLostTree(t *testing.T) {
 	testRunRebuildIndex(t, env.gopts)
 	testRunRepairSnapshot(t, env.gopts, true)
 	testListSnapshots(t, env.gopts, 1)
-	_, err := testRunCheckOutput(t, env.gopts, false)
+	_, _, err := testRunCheckOutput(t, env.gopts, false)
 	rtest.OK(t, err)
 }
 
@@ -121,7 +121,7 @@ func TestRepairSnapshotsWithLostRootTree(t *testing.T) {
 	testRunRebuildIndex(t, env.gopts)
 	testRunRepairSnapshot(t, env.gopts, true)
 	testListSnapshots(t, env.gopts, 0)
-	_, err := testRunCheckOutput(t, env.gopts, false)
+	_, _, err := testRunCheckOutput(t, env.gopts, false)
 	rtest.OK(t, err)
 }
 

cmd/restic/integration_helpers_test.go

@@ -423,6 +423,13 @@ func withCaptureStdout(t testing.TB, gopts global.Options, callback func(ctx con
 	return buf, err
 }
 
+func withCaptureStdoutStderr(t testing.TB, gopts global.Options, callback func(ctx context.Context, gopts global.Options) error) (*bytes.Buffer, *bytes.Buffer, error) {
+	bufStdout := bytes.NewBuffer(nil)
+	bufStderr := bytes.NewBuffer(nil)
+	err := withTermStatusRaw(os.Stdin, bufStdout, bufStderr, gopts, callback)
+	return bufStdout, bufStderr, err
+}
+
 func withTermStatus(t testing.TB, gopts global.Options, callback func(ctx context.Context, gopts global.Options) error) error {
 	// stdout and stderr are written to by printer functions etc. That is the written data
 	// usually consists of one or multiple lines and therefore can be handled well

doc/077_troubleshooting.rst

@@ -80,7 +80,7 @@ Similarly, if a repository is repeatedly damaged, please open an `issue on GitHu
 somewhere. Please include the check output and additional information that might
 help locate the problem.
 
-If ``check`` detects damaged pack files, it will show instructions on how to repair
+When ``restic check`` detects damaged or missing packfiles, it will show instructions on how to repair
 them using the ``repair packs`` command. Use that command instead of the "Repairing the
 index" section in this guide.
 
@@ -88,7 +88,7 @@ If ``check`` detects unreadable snapshot files, it will show instructions on how
 them using the ``repair snapshots`` command. Follow those instructions as part of the
 "Removing broken snapshots" section in this guide.
 
-If you are interested to check only specific snapshots, you can now
+If you are interested to check only specific snapshots, you can
 use the standard snapshot filter method specifying ``--host``, ``--path``, ``--tag`` or
 alternatively naming snapshot ID(s) explicitly. The selected subset of packfiles
 will then be checked for consistency and read when either ``--read-data`` or

internal/repository/checker.go

@@ -57,6 +57,7 @@ type ErrPackMetadata struct {
 	ID        restic.ID
 	Orphaned  bool
 	Truncated bool
+	Missing   bool
 	Err       error
 }
 
@@ -218,7 +219,7 @@ func (c *Checker) Packs(ctx context.Context, errChan chan<- error) {
 			select {
 			case <-ctx.Done():
 				return
-			case errChan <- &ErrPackMetadata{ID: id, Err: errors.New("does not exist")}:
+			case errChan <- &ErrPackMetadata{ID: id, Missing: true, Err: errors.New("does not exist")}:
 			}
 			continue
 		}

internal/repository/repair_pack.go

@@ -65,7 +65,13 @@ func RepairPacks(ctx context.Context, repo *Repository, ids restic.IDSet, printe
 	printer.P("removing salvaged pack files")
 	// if we fail to delete the damaged pack files, then prune will remove them later on
 	bar = printer.NewCounter("files deleted")
-	_ = restic.ParallelRemove(ctx, &internalRepository{repo}, ids, restic.PackFile, nil, bar)
+	_ = restic.ParallelRemove(ctx, &internalRepository{repo}, ids, restic.PackFile, func(id restic.ID, err error) error {
+		// only log errors while deleting pack files
+		if err != nil {
+			printer.E("failed to delete pack file %v: %v", id, err)
+		}
+		return nil
+	}, bar)
 	bar.Done()
 
 	return nil