From 28cf138b88c0a9c2f8d5510124d4e07f6a98ebf9 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Fri, 19 Mar 2021 11:45:27 -0400 Subject: [PATCH] overlay.recreateSymlinks: handle missing "link" files, add a test Extend the overlay driver's recreateSymlinks() function to not give up at the first sign of trouble, but to keep on going until it can no longer make progress, and add an integration test to exercise it. Signed-off-by: Nalin Dahyabhai --- drivers/overlay/overlay.go | 112 ++++++++++++++++++++++++++++++------ tests/overlay-recreate.bats | 33 +++++++++++ 2 files changed, 126 insertions(+), 19 deletions(-) create mode 100644 tests/overlay-recreate.bats diff --git a/drivers/overlay/overlay.go b/drivers/overlay/overlay.go index 254fd5fabf..b4c78fa137 100644 --- a/drivers/overlay/overlay.go +++ b/drivers/overlay/overlay.go @@ -30,6 +30,7 @@ import ( "github.com/containers/storage/pkg/system" "github.com/containers/storage/pkg/unshare" units "github.com/docker/go-units" + "github.com/hashicorp/go-multierror" rsystem "github.com/opencontainers/runc/libcontainer/system" "github.com/opencontainers/selinux/go-selinux/label" "github.com/pkg/errors" @@ -824,7 +825,17 @@ func (d *Driver) getLower(parent string) (string, error) { // Read Parent link fileA parentLink, err := ioutil.ReadFile(path.Join(parentDir, "link")) if err != nil { - return "", err + if !os.IsNotExist(err) { + return "", err + } + logrus.Warnf("Can't read parent link %q because it does not exist. Going through storage to recreate the missing links.", path.Join(parentDir, "link")) + if err := d.recreateSymlinks(); err != nil { + return "", errors.Wrap(err, "error recreating the links") + } + parentLink, err = ioutil.ReadFile(path.Join(parentDir, "link")) + if err != nil { + return "", err + } } lowers := []string{path.Join(linkDir, string(parentLink))} @@ -946,6 +957,7 @@ func (d *Driver) recreateSymlinks() error { if err != nil { return fmt.Errorf("error reading driver home directory %q: %v", d.home, err) } + linksDir := filepath.Join(d.home, "l") // This makes the link directory if it doesn't exist rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps) if err != nil { @@ -954,28 +966,80 @@ func (d *Driver) recreateSymlinks() error { if err := idtools.MkdirAllAs(path.Join(d.home, linkDir), 0700, rootUID, rootGID); err != nil { return err } - for _, dir := range dirs { - // Skip over the linkDir and anything that is not a directory - if dir.Name() == linkDir || !dir.Mode().IsDir() { - continue + // Keep looping as long as we take some corrective action in each iteration + var errs *multierror.Error + madeProgress := true + for madeProgress { + errs = nil + madeProgress = false + // Check that for each layer, there's a link in "l" with the name in + // the layer's "link" file that points to the layer's "diff" directory. + for _, dir := range dirs { + // Skip over the linkDir and anything that is not a directory + if dir.Name() == linkDir || !dir.Mode().IsDir() { + continue + } + // Read the "link" file under each layer to get the name of the symlink + data, err := ioutil.ReadFile(path.Join(d.dir(dir.Name()), "link")) + if err != nil { + errs = multierror.Append(errs, errors.Wrapf(err, "error reading name of symlink for %q", dir)) + continue + } + linkPath := path.Join(d.home, linkDir, strings.Trim(string(data), "\n")) + // Check if the symlink exists, and if it doesn't, create it again with the + // name we got from the "link" file + _, err = os.Lstat(linkPath) + if err != nil && os.IsNotExist(err) { + if err := os.Symlink(path.Join("..", dir.Name(), "diff"), linkPath); err != nil { + errs = multierror.Append(errs, err) + continue + } + madeProgress = true + } else if err != nil { + errs = multierror.Append(errs, errors.Wrapf(err, "error trying to stat %q", linkPath)) + continue + } } - // Read the "link" file under each layer to get the name of the symlink - data, err := ioutil.ReadFile(path.Join(d.dir(dir.Name()), "link")) + // Now check if we somehow lost a "link" file, by making sure + // that each symlink we have corresponds to one. + links, err := ioutil.ReadDir(linksDir) if err != nil { - return fmt.Errorf("error reading name of symlink for %q: %v", dir, err) - } - linkPath := path.Join(d.home, linkDir, strings.Trim(string(data), "\n")) - // Check if the symlink exists, and if it doesn't create it again with the name we - // got from the "link" file - _, err = os.Stat(linkPath) - if err != nil && os.IsNotExist(err) { - if err := os.Symlink(path.Join("..", dir.Name(), "diff"), linkPath); err != nil { - return err + errs = multierror.Append(errs, errors.Wrapf(err, "error reading links directory %q", linksDir)) + continue + } + // Go through all of the symlinks in the "l" directory + for _, link := range links { + // Read the symlink's target, which should be "../$layer/diff" + target, err := os.Readlink(filepath.Join(linksDir, link.Name())) + if err != nil { + errs = multierror.Append(errs, errors.Wrapf(err, "error reading target of link %q", link)) + continue + } + targetComponents := strings.Split(target, string(os.PathSeparator)) + if len(targetComponents) != 3 || targetComponents[0] != ".." || targetComponents[2] != "diff" { + errs = multierror.Append(errs, errors.Errorf("link target of %q looks weird: %q", link, target)) + // force the link to be recreated on the next pass + os.Remove(filepath.Join(linksDir, link.Name())) + madeProgress = true + continue + } + // Reconstruct the name of the target's link file and check that + // it has the basename of our symlink in it. + targetID := targetComponents[1] + linkFile := filepath.Join(d.dir(targetID), "link") + data, err := ioutil.ReadFile(linkFile) + if err != nil || string(data) != link.Name() { + if err := ioutil.WriteFile(linkFile, []byte(link.Name()), 0644); err != nil { + errs = multierror.Append(errs, errors.Wrapf(err, "error correcting link for layer %q", targetID)) + continue + } + madeProgress = true } - } else if err != nil { - return fmt.Errorf("error trying to stat %q: %v", linkPath, err) } } + if errs != nil { + return errs.ErrorOrNil() + } return nil } @@ -1032,7 +1096,17 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO // lists that we're building. "diff" itself is the upper, so it won't be in the lists. link, err := ioutil.ReadFile(path.Join(dir, "link")) if err != nil { - return "", err + if !os.IsNotExist(err) { + return "", err + } + logrus.Warnf("Can't read parent link %q because it does not exist. Going through storage to recreate the missing links.", path.Join(dir, "link")) + if err := d.recreateSymlinks(); err != nil { + return "", errors.Wrap(err, "error recreating the links") + } + link, err = ioutil.ReadFile(path.Join(dir, "link")) + if err != nil { + return "", err + } } diffN := 1 perms := defaultPerms diff --git a/tests/overlay-recreate.bats b/tests/overlay-recreate.bats new file mode 100644 index 0000000000..038a48810a --- /dev/null +++ b/tests/overlay-recreate.bats @@ -0,0 +1,33 @@ +#!/usr/bin/env bats + +load helpers + +@test "overlay-recreate" { + case "$STORAGE_DRIVER" in + overlay) + ;; + *) + skip "not applicable to driver $STORAGE_DRIVER" + ;; + esac + populate + # behold my destructive power! + rm -v ${TESTDIR}/root/overlay/l/* + # we should be able to recover from that. + storage mount "$lowerlayer" + storage unmount "$lowerlayer" + storage mount "$midlayer" + storage unmount "$midlayer" + storage mount "$upperlayer" + storage unmount "$upperlayer" + # okay, but how about this? + rm -v ${TESTDIR}/root/overlay/*/link + # yeah, we can handle that, too. + storage mount "$lowerlayer" + storage unmount "$lowerlayer" + storage mount "$midlayer" + storage unmount "$midlayer" + storage mount "$upperlayer" + storage unmount "$upperlayer" + # okay, not bad, kid. +}