diff --git a/changelog/unreleased/pull-21845 b/changelog/unreleased/pull-21845 new file mode 100644 index 000000000..896c6c0a8 --- /dev/null +++ b/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 diff --git a/cmd/restic/cmd_check.go b/cmd/restic/cmd_check.go index 403d94411..b0238c0a1 100644 --- a/cmd/restic/cmd_check.go +++ b/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 diff --git a/cmd/restic/cmd_check_integration_test.go b/cmd/restic/cmd_check_integration_test.go index 0a5bc3521..9f13e46e5 100644 --- a/cmd/restic/cmd_check_integration_test.go +++ b/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 diff --git a/cmd/restic/cmd_repair_index_integration_test.go b/cmd/restic/cmd_repair_index_integration_test.go index 52dc47315..02eab8877 100644 --- a/cmd/restic/cmd_repair_index_integration_test.go +++ b/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) } diff --git a/cmd/restic/cmd_repair_packs.go b/cmd/restic/cmd_repair_packs.go index f32e57706..6b35cb5ee 100644 --- a/cmd/restic/cmd_repair_packs.go +++ b/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) diff --git a/cmd/restic/cmd_repair_packs_integration_test.go b/cmd/restic/cmd_repair_packs_integration_test.go new file mode 100644 index 000000000..05732ae00 --- /dev/null +++ b/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) +} diff --git a/cmd/restic/cmd_repair_snapshots_integration_test.go b/cmd/restic/cmd_repair_snapshots_integration_test.go index e839551ca..c75af6e18 100644 --- a/cmd/restic/cmd_repair_snapshots_integration_test.go +++ b/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) } diff --git a/cmd/restic/integration_helpers_test.go b/cmd/restic/integration_helpers_test.go index c9b287415..b20f63208 100644 --- a/cmd/restic/integration_helpers_test.go +++ b/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 diff --git a/doc/077_troubleshooting.rst b/doc/077_troubleshooting.rst index e1e04169c..ba461a311 100644 --- a/doc/077_troubleshooting.rst +++ b/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 diff --git a/internal/repository/checker.go b/internal/repository/checker.go index 5e85cd89b..bb7e6ff96 100644 --- a/internal/repository/checker.go +++ b/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 } diff --git a/internal/repository/repair_pack.go b/internal/repository/repair_pack.go index b958914f4..6b2c69d66 100644 --- a/internal/repository/repair_pack.go +++ b/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