diff --git a/storage/drivers/overlay/overlay.go b/storage/drivers/overlay/overlay.go index b488256097..4d0ecaa2a4 100644 --- a/storage/drivers/overlay/overlay.go +++ b/storage/drivers/overlay/overlay.go @@ -84,7 +84,13 @@ const ( stagingDir = "staging" tempDirName = "tempdirs" lowerFile = "lower" - maxDepth = 500 + // lowerLayersFile references lower layers directly by layer ID + // instead of going through the l/ symlinks. The code appends + // "/diff" itself when consuming entries. It is preferred over + // lowerFile when present. The old lowerFile is still written + // for backward compatibility with older tools. + lowerLayersFile = "lower-layers" + maxDepth = 500 stagingLockFile = "staging.lock" @@ -1153,6 +1159,35 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnl } } + // Write a lower-layers file referencing layers by ID instead of + // l/ symlink references. The reading side appends "/diff" itself. + parentDir := d.dir(parent) + layerLower := parent + parentLower, err := os.ReadFile(path.Join(parentDir, lowerLayersFile)) + if err == nil { + layerLower += ":" + string(parentLower) + } else if !errors.Is(err, unix.ENOENT) { + return err + } else { + // Parent has no lower-layers file. Convert old-format lower + // entries (l/ symlinks) to layer IDs. + oldLower, err := os.ReadFile(path.Join(parentDir, lowerFile)) + if err == nil { + for _, s := range strings.Split(string(oldLower), ":") { + target, err := os.Readlink(d.dir(s)) + if err != nil { + return fmt.Errorf("reading symlink for lower %q: %w", s, err) + } + layerLower += ":" + filepath.Base(filepath.Dir(target)) + } + } else if !errors.Is(err, unix.ENOENT) { + return err + } + } + if err := os.WriteFile(path.Join(dir, lowerLayersFile), []byte(layerLower), 0o666); err != nil { + return err + } + return nil } @@ -1190,20 +1225,9 @@ func (d *Driver) getLower(parent string) (string, error) { return "", err } - // Read Parent link fileA parentLink, err := os.ReadFile(path.Join(parentDir, "link")) if err != nil { - 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 "", fmt.Errorf("recreating the links: %w", err) - } - parentLink, err = os.ReadFile(path.Join(parentDir, "link")) - if err != nil { - return "", err - } + return "", err } lowers := []string{path.Join(linkDir, string(parentLink))} @@ -1258,27 +1282,25 @@ func (d *Driver) dir2(id string, useImageStore bool) (string, string, bool) { func (d *Driver) getLowerDirs(id string) ([]string, error) { var lowersArray []string - lowers, err := os.ReadFile(path.Join(d.dir(id), lowerFile)) + dir := d.dir(id) + lowers, err := os.ReadFile(path.Join(dir, lowerLayersFile)) + if err != nil { + if !errors.Is(err, unix.ENOENT) { + return nil, err + } + lowers, err = os.ReadFile(path.Join(dir, lowerFile)) + } if err == nil { for s := range strings.SplitSeq(string(lowers), ":") { lower := d.dir(s) lp, err := os.Readlink(lower) - // if the link does not exist, we lost the symlinks during a sudden reboot. - // Let's go ahead and recreate those symlinks. if err != nil { - if os.IsNotExist(err) { - logrus.Warnf("Can't read link %q because it does not exist. A storage corruption might have occurred, attempting to recreate the missing symlinks. It might be best wipe the storage to avoid further errors due to storage corruption.", lower) - if err := d.recreateSymlinks(); err != nil { - return nil, fmt.Errorf("recreating the missing symlinks: %w", err) - } - // let's call Readlink on lower again now that we have recreated the missing symlinks - lp, err = os.Readlink(lower) - if err != nil { - return nil, err - } - } else { - return nil, err + if errors.Is(err, syscall.EINVAL) { + // Not a symlink: layer ID, append /diff. + lowersArray = append(lowersArray, path.Join(lower, "diff")) + continue } + return nil, err } lowersArray = append(lowersArray, path.Clean(d.dir(path.Join("link", lp)))) } @@ -1390,112 +1412,6 @@ func (d *Driver) DeferredRemove(id string) (tempdir.CleanupTempDirFunc, error) { return t.Cleanup, nil } -// recreateSymlinks goes through the driver's home directory and checks if the diff directory -// under each layer has a symlink created for it under the linkDir. If the symlink does not -// exist, it creates them -func (d *Driver) recreateSymlinks() error { - // We have at most 3 corrective actions per layer, so 10 iterations is plenty. - const maxIterations = 10 - - // List all the directories under the home directory - dirs, err := os.ReadDir(d.home) - if err != nil { - return fmt.Errorf("reading driver home directory %q: %w", d.home, err) - } - // This makes the link directory if it doesn't exist - if err := idtools.MkdirAllAndChown(path.Join(d.home, linkDir), 0o755, idtools.IDPair{UID: 0, GID: 0}); err != nil { - return err - } - // Keep looping as long as we take some corrective action in each iteration - var errs error - madeProgress := true - iterations := 0 - 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, stagingDir, tempDirName and anything that is not a directory - if dir.Name() == linkDir || dir.Name() == stagingDir || dir.Name() == tempDirName || !dir.IsDir() { - continue - } - // Read the "link" file under each layer to get the name of the symlink - data, err := os.ReadFile(path.Join(d.dir(dir.Name()), "link")) - if err != nil { - errs = errors.Join(errs, fmt.Errorf("reading name of symlink for %q: %w", dir.Name(), err)) - 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 = fileutils.Lexists(linkPath) - if err != nil && os.IsNotExist(err) { - if err := os.Symlink(path.Join("..", dir.Name(), "diff"), linkPath); err != nil { - errs = errors.Join(errs, err) - continue - } - madeProgress = true - } else if err != nil { - errs = errors.Join(errs, err) - continue - } - } - - // linkDirFullPath is the full path to the linkDir - linkDirFullPath := filepath.Join(d.home, "l") - // Now check if we somehow lost a "link" file, by making sure - // that each symlink we have corresponds to one. - links, err := os.ReadDir(linkDirFullPath) - if err != nil { - errs = errors.Join(errs, err) - 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(linkDirFullPath, link.Name())) - if err != nil { - errs = errors.Join(errs, err) - continue - } - targetComponents := strings.Split(target, string(os.PathSeparator)) - if len(targetComponents) != 3 || targetComponents[0] != ".." || targetComponents[2] != "diff" { - errs = errors.Join(errs, fmt.Errorf("link target of %q looks weird: %q", link, target)) - // force the link to be recreated on the next pass - if err := os.Remove(filepath.Join(linkDirFullPath, link.Name())); err != nil { - if !os.IsNotExist(err) { - errs = errors.Join(errs, fmt.Errorf("removing link %q: %w", link, err)) - } // else don’t report any error, but also don’t set madeProgress. - continue - } - 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 := os.ReadFile(linkFile) - if err != nil || string(data) != link.Name() { - // NOTE: If two or more links point to the same target, we will update linkFile - // with every value of link.Name(), and set madeProgress = true every time. - if err := os.WriteFile(linkFile, []byte(link.Name()), 0o644); err != nil { - errs = errors.Join(errs, fmt.Errorf("correcting link for layer %s: %w", targetID, err)) - continue - } - madeProgress = true - } - } - iterations++ - if iterations >= maxIterations { - errs = errors.Join(errs, fmt.Errorf("reached %d iterations in overlay graph driver’s recreateSymlink, giving up", iterations)) - break - } - } - return errs -} - // Get creates and mounts the required file system for the given id and returns the mount path. func (d *Driver) Get(id string, options graphdriver.MountOpts) (string, error) { return d.get(id, false, options) @@ -1590,9 +1506,15 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO readWrite = false } - lowers, err := os.ReadFile(path.Join(dir, lowerFile)) - if err != nil && !os.IsNotExist(err) { - return "", err + lowers, err := os.ReadFile(path.Join(dir, lowerLayersFile)) + if err != nil { + if !errors.Is(err, unix.ENOENT) { + return "", err + } + lowers, err = os.ReadFile(path.Join(dir, lowerFile)) + if err != nil && !os.IsNotExist(err) { + return "", err + } } splitLowers := strings.Split(string(lowers), ":") if len(splitLowers) > maxDepth { @@ -1704,16 +1626,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO } lower = "" } - // if it is a "not found" error, that means the symlinks were lost in a sudden reboot - // so call the recreateSymlinks function to go through all the layer dirs and recreate - // the symlinks with the name from their respective "link" files - if lower == "" && os.IsNotExist(err) { - logrus.Warnf("Can't stat lower layer %q because it does not exist. Going through storage to recreate the missing symlinks.", newpath) - if err := d.recreateSymlinks(); err != nil { - return "", fmt.Errorf("recreating the missing symlinks: %w", err) - } - lower = newpath - } else if lower == "" { + if lower == "" { return "", fmt.Errorf("can't stat lower layer %q: %w", newpath, err) } } else { @@ -1726,7 +1639,12 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO linkContent, err := os.Readlink(lower) if err != nil { - return "", err + if !errors.Is(err, syscall.EINVAL) { + return "", err + } + // Not a symlink: layer ID from lower-layers, append /diff. + lower = path.Join(lower, "diff") + linkContent = lower } lowerID := filepath.Base(filepath.Dir(linkContent)) composefsMount, err := maybeAddComposefsMount(lowerID, i+1, readWrite) diff --git a/storage/tests/overlay-recreate.bats b/storage/tests/overlay-recreate.bats deleted file mode 100644 index c3ebbb3297..0000000000 --- a/storage/tests/overlay-recreate.bats +++ /dev/null @@ -1,36 +0,0 @@ -#!/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" - run storage --debug=false mount "$upperlayer" - merged_dir="$output" - 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" - # check it works if we delete the merged directory. - rmdir "$merged_dir" - storage mount "$upperlayer" - storage unmount "$upperlayer" - # okay, not bad, kid. -}