Merge pull request #5296 from MichaelEischer/reindex-before-recover

recover: reindex before reassembling snapshot

changelog/unreleased/issue-5287

@@ -0,0 +1,8 @@
+Enhancement: `recover` automatically runs `repair index`
+
+When trying to recover data from an interrupted snapshot, it was necessary
+to manually run `restic repair index` before runnning `restic recover`.
+This now happens automatically.
+
+https://github.com/restic/restic/issues/52897
+https://github.com/restic/restic/pull/5296

cmd/restic/cmd_check.go

@@ -262,10 +262,10 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts GlobalOptions, args
 	for _, hint := range hints {
 		switch hint.(type) {
 		case *checker.ErrDuplicatePacks:
-			term.Print(hint.Error())
+			printer.S("%s", hint.Error())
 			summary.HintRepairIndex = true
 		case *checker.ErrMixedPack:
-			term.Print(hint.Error())
+			printer.S("%s", hint.Error())
 			summary.HintPrune = true
 		default:
 			printer.E("error: %v\n", hint)
@@ -274,10 +274,10 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts GlobalOptions, args
 	}
 
 	if summary.HintRepairIndex {
-		term.Print("Duplicate packs are non-critical, you can run `restic repair index' to correct this.\n")
+		printer.S("Duplicate packs are non-critical, you can run `restic repair index' to correct this.\n")
 	}
 	if summary.HintPrune {
-		term.Print("Mixed packs with tree and data blobs are non-critical, you can run `restic prune` to correct this.\n")
+		printer.S("Mixed packs with tree and data blobs are non-critical, you can run `restic prune` to correct this.\n")
 	}
 
 	if len(errs) > 0 {
@@ -534,6 +534,7 @@ func (p *jsonErrorPrinter) E(msg string, args ...interface{}) {
 	}
 	p.term.Error(ui.ToJSONString(status))
 }
+func (*jsonErrorPrinter) S(_ string, _ ...interface{})  {}
 func (*jsonErrorPrinter) P(_ string, _ ...interface{})  {}
 func (*jsonErrorPrinter) V(_ string, _ ...interface{})  {}
 func (*jsonErrorPrinter) VV(_ string, _ ...interface{}) {}

cmd/restic/cmd_recover.go

@@ -6,7 +6,10 @@ import (
 	"time"
 
 	"github.com/restic/restic/internal/errors"
+	"github.com/restic/restic/internal/repository"
 	"github.com/restic/restic/internal/restic"
+	"github.com/restic/restic/internal/ui/progress"
+	"github.com/restic/restic/internal/ui/termstatus"
 	"github.com/spf13/cobra"
 	"golang.org/x/sync/errgroup"
 )
@@ -32,31 +35,41 @@ Exit status is 12 if the password is incorrect.
 		GroupID:           cmdGroupDefault,
 		DisableAutoGenTag: true,
 		RunE: func(cmd *cobra.Command, _ []string) error {
-			return runRecover(cmd.Context(), globalOptions)
+			term, cancel := setupTermstatus()
+			defer cancel()
+			return runRecover(cmd.Context(), globalOptions, term)
 		},
 	}
 	return cmd
 }
 
-func runRecover(ctx context.Context, gopts GlobalOptions) error {
+func runRecover(ctx context.Context, gopts GlobalOptions, term *termstatus.Terminal) error {
 	hostname, err := os.Hostname()
 	if err != nil {
 		return err
 	}
 
-	ctx, repo, unlock, err := openWithAppendLock(ctx, gopts, false)
+	ctx, repo, unlock, err := openWithExclusiveLock(ctx, gopts, false)
 	if err != nil {
 		return err
 	}
 	defer unlock()
 
+	printer := newTerminalProgressPrinter(gopts.verbosity, term)
+
 	snapshotLister, err := restic.MemorizeList(ctx, repo, restic.SnapshotFile)
 	if err != nil {
 		return err
 	}
 
-	Verbosef("load index files\n")
-	bar := newIndexProgress(gopts.Quiet, gopts.JSON)
+	printer.P("ensuring index is complete\n")
+	err = repository.RepairIndex(ctx, repo, repository.RepairIndexOptions{}, printer)
+	if err != nil {
+		return err
+	}
+
+	printer.P("load index files\n")
+	bar := newIndexTerminalProgress(gopts.Quiet, gopts.JSON, term)
 	if err = repo.LoadIndex(ctx, bar); err != nil {
 		return err
 	}
@@ -74,15 +87,15 @@ func runRecover(ctx context.Context, gopts GlobalOptions) error {
 		return err
 	}
 
-	Verbosef("load %d trees\n", len(trees))
-	bar = newProgressMax(!gopts.Quiet, uint64(len(trees)), "trees loaded")
+	printer.P("load %d trees\n", len(trees))
+	bar = newTerminalProgressMax(!gopts.Quiet, uint64(len(trees)), "trees loaded", term)
 	for id := range trees {
 		tree, err := restic.LoadTree(ctx, repo, id)
 		if ctx.Err() != nil {
 			return ctx.Err()
 		}
 		if err != nil {
-			Warnf("unable to load tree %v: %v\n", id.Str(), err)
+			printer.E("unable to load tree %v: %v\n", id.Str(), err)
 			continue
 		}
 
@@ -95,7 +108,7 @@ func runRecover(ctx context.Context, gopts GlobalOptions) error {
 	}
 	bar.Done()
 
-	Verbosef("load snapshots\n")
+	printer.P("load snapshots\n")
 	err = restic.ForAllSnapshots(ctx, snapshotLister, repo, nil, func(_ restic.ID, sn *restic.Snapshot, _ error) error {
 		trees[*sn.Tree] = true
 		return nil
@@ -103,19 +116,19 @@ func runRecover(ctx context.Context, gopts GlobalOptions) error {
 	if err != nil {
 		return err
 	}
-	Verbosef("done\n")
+	printer.P("done\n")
 
 	roots := restic.NewIDSet()
 	for id, seen := range trees {
 		if !seen {
-			Verboseff("found root tree %v\n", id.Str())
+			printer.V("found root tree %v\n", id.Str())
 			roots.Insert(id)
 		}
 	}
-	Printf("\nfound %d unreferenced roots\n", len(roots))
+	printer.S("\nfound %d unreferenced roots\n", len(roots))
 
 	if len(roots) == 0 {
-		Verbosef("no snapshot to write.\n")
+		printer.P("no snapshot to write.\n")
 		return nil
 	}
 
@@ -163,11 +176,11 @@ func runRecover(ctx context.Context, gopts GlobalOptions) error {
 		return err
 	}
 
-	return createSnapshot(ctx, "/recover", hostname, []string{"recovered"}, repo, &treeID)
+	return createSnapshot(ctx, printer, "/recover", hostname, []string{"recovered"}, repo, &treeID)
 
 }
 
-func createSnapshot(ctx context.Context, name, hostname string, tags []string, repo restic.SaverUnpacked[restic.WriteableFileType], tree *restic.ID) error {
+func createSnapshot(ctx context.Context, printer progress.Printer, name, hostname string, tags []string, repo restic.SaverUnpacked[restic.WriteableFileType], tree *restic.ID) error {
 	sn, err := restic.NewSnapshot([]string{name}, tags, hostname, time.Now())
 	if err != nil {
 		return errors.Fatalf("unable to save snapshot: %v", err)
@@ -180,6 +193,6 @@ func createSnapshot(ctx context.Context, name, hostname string, tags []string, r
 		return errors.Fatalf("unable to save snapshot: %v", err)
 	}
 
-	Printf("saved new snapshot %v\n", id.Str())
+	printer.S("saved new snapshot %v\n", id.Str())
 	return nil
 }

cmd/restic/cmd_recover_integration_test.go

@@ -0,0 +1,37 @@
+package main
+
+import (
+	"context"
+	"testing"
+
+	rtest "github.com/restic/restic/internal/test"
+	"github.com/restic/restic/internal/ui/termstatus"
+)
+
+func testRunRecover(t testing.TB, gopts GlobalOptions) {
+	rtest.OK(t, withTermStatus(gopts, func(ctx context.Context, term *termstatus.Terminal) error {
+		return runRecover(context.TODO(), gopts, term)
+	}))
+}
+
+func TestRecover(t *testing.T) {
+	env, cleanup := withTestEnvironment(t)
+	// must list index more than once
+	env.gopts.backendTestHook = nil
+	defer cleanup()
+
+	testSetupBackupData(t, env)
+
+	// create backup and forget it afterwards
+	testRunBackup(t, "", []string{env.testdata}, BackupOptions{}, env.gopts)
+	ids := testListSnapshots(t, env.gopts, 1)
+	sn := testLoadSnapshot(t, env.gopts, ids[0])
+	testRunForget(t, env.gopts, ForgetOptions{}, ids[0].String())
+	testListSnapshots(t, env.gopts, 0)
+
+	testRunRecover(t, env.gopts)
+	ids = testListSnapshots(t, env.gopts, 1)
+	testRunCheck(t, env.gopts)
+	// check that the root tree is included in the snapshot
+	rtest.OK(t, runCat(context.TODO(), env.gopts, []string{"tree", ids[0].String() + ":" + sn.Tree.Str()}))
+}

cmd/restic/integration_helpers_test.go

@@ -354,6 +354,15 @@ func lastSnapshot(old, new map[string]struct{}) (map[string]struct{}, string) {
 	return old, ""
 }
 
+func testLoadSnapshot(t testing.TB, gopts GlobalOptions, id restic.ID) *restic.Snapshot {
+	_, repo, unlock, err := openWithReadLock(context.TODO(), gopts, false)
+	defer unlock()
+	rtest.OK(t, err)
+	snapshot, err := restic.LoadSnapshot(context.TODO(), repo, id)
+	rtest.OK(t, err)
+	return snapshot
+}
+
 func appendRandomData(filename string, bytes uint) error {
 	f, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE, 0666)
 	if err != nil {

internal/ui/message.go

@@ -24,6 +24,12 @@ func (m *Message) E(msg string, args ...interface{}) {
 	m.term.Error(fmt.Sprintf(msg, args...))
 }
 
+// S prints a message, this is should only be used for very important messages
+// that are not errors.
+func (m *Message) S(msg string, args ...interface{}) {
+	m.term.Print(fmt.Sprintf(msg, args...))
+}
+
 // P prints a message if verbosity >= 1, this is used for normal messages which
 // are not errors.
 func (m *Message) P(msg string, args ...interface{}) {

internal/ui/progress/printer.go

@@ -8,9 +8,15 @@ import "testing"
 type Printer interface {
 	NewCounter(description string) *Counter
 
+	// E prints to stderr
 	E(msg string, args ...interface{})
+	// S prints to stdout
+	S(msg string, args ...interface{})
+	// P prints to stdout unless quiet was passed
 	P(msg string, args ...interface{})
+	// V prints to stdout if verbose is set once
 	V(msg string, args ...interface{})
+	// VV prints to stdout if verbose is set twice
 	VV(msg string, args ...interface{})
 }
 
@@ -25,6 +31,8 @@ func (*NoopPrinter) NewCounter(_ string) *Counter {
 
 func (*NoopPrinter) E(_ string, _ ...interface{}) {}
 
+func (*NoopPrinter) S(_ string, _ ...interface{}) {}
+
 func (*NoopPrinter) P(_ string, _ ...interface{}) {}
 
 func (*NoopPrinter) V(_ string, _ ...interface{}) {}
@@ -52,6 +60,10 @@ func (p *TestPrinter) E(msg string, args ...interface{}) {
 	p.t.Logf("error: "+msg, args...)
 }
 
+func (p *TestPrinter) S(msg string, args ...interface{}) {
+	p.t.Logf("stdout: "+msg, args...)
+}
+
 func (p *TestPrinter) P(msg string, args ...interface{}) {
 	p.t.Logf("print: "+msg, args...)
 }