diff --git a/changelog/unreleased/pull-2978 b/changelog/unreleased/pull-2978 new file mode 100644 index 000000000..78b0a5e10 --- /dev/null +++ b/changelog/unreleased/pull-2978 @@ -0,0 +1,8 @@ +Enhancement: Warn if parent snapshot cannot be loaded during backup + +During a backup restic uses the parent snapshot to check whether a file was +changed and has to be backed up again. For this check the backup has to read +the directories contained in the old snapshot. If a tree blob cannot be +loaded, restic now warns about this problem with the backup repository. + +https://github.com/restic/restic/pull/2978 diff --git a/cmd/restic/integration_test.go b/cmd/restic/integration_test.go index 71e03ae5f..9c01939ec 100644 --- a/cmd/restic/integration_test.go +++ b/cmd/restic/integration_test.go @@ -368,7 +368,7 @@ func TestBackupNonExistingFile(t *testing.T) { testRunBackup(t, "", dirs, opts, env.gopts) } -func removeDataPacksExcept(gopts GlobalOptions, t *testing.T, keep restic.IDSet) { +func removePacksExcept(gopts GlobalOptions, t *testing.T, keep restic.IDSet, removeTreePacks bool) { r, err := OpenRepository(gopts) rtest.OK(t, err) @@ -383,7 +383,7 @@ func removeDataPacksExcept(gopts GlobalOptions, t *testing.T, keep restic.IDSet) // remove all packs containing data blobs rtest.OK(t, r.List(gopts.ctx, restic.PackFile, func(id restic.ID, size int64) error { - if treePacks.Has(id) || keep.Has(id) { + if treePacks.Has(id) != removeTreePacks || keep.Has(id) { return nil } return r.Backend().Remove(gopts.ctx, restic.Handle{Type: restic.PackFile, Name: id.String()}) @@ -406,7 +406,7 @@ func TestBackupSelfHealing(t *testing.T) { testRunCheck(t, env.gopts) // remove all data packs - removeDataPacksExcept(env.gopts, t, restic.NewIDSet()) + removePacksExcept(env.gopts, t, restic.NewIDSet(), false) testRunRebuildIndex(t, env.gopts) // now the repo is also missing the data blob in the index; check should report this @@ -420,6 +420,56 @@ func TestBackupSelfHealing(t *testing.T) { testRunCheck(t, env.gopts) } +func TestBackupTreeLoadError(t *testing.T) { + env, cleanup := withTestEnvironment(t) + defer cleanup() + + testRunInit(t, env.gopts) + p := filepath.Join(env.testdata, "test/test") + rtest.OK(t, os.MkdirAll(filepath.Dir(p), 0755)) + rtest.OK(t, appendRandomData(p, 5)) + + opts := BackupOptions{} + // Backup a subdirectory first, such that we can remove the tree pack for the subdirectory + testRunBackup(t, env.testdata, []string{"test"}, opts, env.gopts) + + r, err := OpenRepository(env.gopts) + rtest.OK(t, err) + rtest.OK(t, r.LoadIndex(env.gopts.ctx)) + // collect tree packs of subdirectory + subTreePacks := restic.NewIDSet() + for _, idx := range r.Index().(*repository.MasterIndex).All() { + for _, id := range idx.TreePacks() { + subTreePacks.Insert(id) + } + } + + testRunBackup(t, filepath.Dir(env.testdata), []string{filepath.Base(env.testdata)}, opts, env.gopts) + testRunCheck(t, env.gopts) + + // delete the subdirectory pack first + for id := range subTreePacks { + rtest.OK(t, r.Backend().Remove(env.gopts.ctx, restic.Handle{Type: restic.PackFile, Name: id.String()})) + } + testRunRebuildIndex(t, env.gopts) + // now the repo is missing the tree blob in the index; check should report this + rtest.Assert(t, runCheck(CheckOptions{}, env.gopts, nil) != nil, "check should have reported an error") + // second backup should report an error but "heal" this situation + err = testRunBackupAssumeFailure(t, filepath.Dir(env.testdata), []string{filepath.Base(env.testdata)}, opts, env.gopts) + rtest.Assert(t, err != nil, "backup should have reported an error for the subdirectory") + testRunCheck(t, env.gopts) + + // remove all tree packs + removePacksExcept(env.gopts, t, restic.NewIDSet(), true) + testRunRebuildIndex(t, env.gopts) + // now the repo is also missing the data blob in the index; check should report this + rtest.Assert(t, runCheck(CheckOptions{}, env.gopts, nil) != nil, "check should have reported an error") + // second backup should report an error but "heal" this situation + err = testRunBackupAssumeFailure(t, filepath.Dir(env.testdata), []string{filepath.Base(env.testdata)}, opts, env.gopts) + rtest.Assert(t, err != nil, "backup should have reported an error") + testRunCheck(t, env.gopts) +} + func includes(haystack []string, needle string) bool { for _, s := range haystack { if s == needle { @@ -1396,7 +1446,7 @@ func TestPruneWithDamagedRepository(t *testing.T) { testRunBackup(t, "", []string{filepath.Join(env.testdata, "0", "0", "9", "3")}, opts, env.gopts) snapshotIDs := testRunList(t, "snapshots", env.gopts) - removeDataPacksExcept(env.gopts, t, oldPacks) + removePacksExcept(env.gopts, t, oldPacks, false) rtest.Assert(t, len(snapshotIDs) == 1, "expected one snapshot, got %v", snapshotIDs) diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index d6dd6660a..42fc03310 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -191,19 +191,29 @@ func (arch *Archiver) nodeFromFileInfo(filename string, fi os.FileInfo) (*restic } // loadSubtree tries to load the subtree referenced by node. In case of an error, nil is returned. -func (arch *Archiver) loadSubtree(ctx context.Context, node *restic.Node) *restic.Tree { +// If there is no node to load, then nil is returned without an error. +func (arch *Archiver) loadSubtree(ctx context.Context, node *restic.Node) (*restic.Tree, error) { if node == nil || node.Type != "dir" || node.Subtree == nil { - return nil + return nil, nil } tree, err := arch.Repo.LoadTree(ctx, *node.Subtree) if err != nil { debug.Log("unable to load tree %v: %v", node.Subtree.Str(), err) - // TODO: handle error - return nil + // a tree in the repository is not readable -> warn the user + return nil, arch.wrapLoadTreeError(*node.Subtree, err) } - return tree + return tree, nil +} + +func (arch *Archiver) wrapLoadTreeError(id restic.ID, err error) error { + if arch.Repo.Index().Has(id, restic.TreeBlob) { + err = errors.Errorf("tree %v could not be loaded; the repository could be damaged: %v", id, err) + } else { + err = errors.Errorf("tree %v is not known; the repository could be damaged, run `rebuild-index` to try to repair it", id) + } + return err } // SaveDir stores a directory in the repo and returns the node. snPath is the @@ -434,7 +444,10 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous snItem := snPath + "/" start := time.Now() - oldSubtree := arch.loadSubtree(ctx, previous) + oldSubtree, err := arch.loadSubtree(ctx, previous) + if err != nil { + arch.error(abstarget, fi, err) + } fn.isTree = true fn.tree, err = arch.SaveDir(ctx, snPath, fi, target, oldSubtree, @@ -572,7 +585,10 @@ func (arch *Archiver) SaveTree(ctx context.Context, snPath string, atree *Tree, start := time.Now() oldNode := previous.Find(name) - oldSubtree := arch.loadSubtree(ctx, oldNode) + oldSubtree, err := arch.loadSubtree(ctx, oldNode) + if err != nil { + arch.error(join(snPath, name), nil, err) + } // not a leaf node, archive subtree subtree, err := arch.SaveTree(ctx, join(snPath, name), &subatree, oldSubtree) @@ -730,6 +746,7 @@ func (arch *Archiver) loadParentTree(ctx context.Context, snapshotID restic.ID) tree, err := arch.Repo.LoadTree(ctx, *sn.Tree) if err != nil { debug.Log("unable to load tree %v: %v", *sn.Tree, err) + arch.error("/", nil, arch.wrapLoadTreeError(*sn.Tree, err)) return nil } return tree