diff --git a/storage/additional_layer_test.go b/storage/additional_layer_test.go new file mode 100644 index 0000000000..933caabf46 --- /dev/null +++ b/storage/additional_layer_test.go @@ -0,0 +1,83 @@ +//go:build linux + +package storage + +import ( + "encoding/base64" + "os" + "path/filepath" + "testing" + + digest "github.com/opencontainers/go-digest" + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// setupAdditionalLayerStore creates a temporary additional layer store directory +// with the expected structure for the given tocDigest and image reference. +// The info file contains the provided content. +func setupAdditionalLayerStore(t *testing.T, tocDigest digest.Digest, imageRef string, infoContent string) string { + t.Helper() + alsRoot := t.TempDir() + + refDir := base64.StdEncoding.EncodeToString([]byte(imageRef)) + layerDir := filepath.Join(alsRoot, refDir, tocDigest.String()) + require.NoError(t, os.MkdirAll(layerDir, 0o755)) + + require.NoError(t, os.MkdirAll(filepath.Join(layerDir, "diff"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(layerDir, "info"), []byte(infoContent), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(layerDir, "blob"), []byte{}, 0o644)) + + return alsRoot +} + +func TestLookupAdditionalLayerSuccess(t *testing.T) { + prevLevel := logrus.GetLevel() + logrus.SetLevel(logrus.ErrorLevel) + t.Cleanup(func() { logrus.SetLevel(prevLevel) }) + + tocDigest := digest.FromString("test-layer") + imageRef := "fedora" + + info := Layer{ + ID: "test-id", + CompressedSize: 42, + TOCDigest: tocDigest, + } + infoJSON, err := json.Marshal(info) + require.NoError(t, err) + + alsPath := setupAdditionalLayerStore(t, tocDigest, imageRef, string(infoJSON)) + store := newTestStore(t, StoreOptions{ + GraphDriverName: "overlay", + GraphDriverOptions: []string{"additionallayerstore=" + alsPath + ":ref"}, + }) + t.Cleanup(func() { _, _ = store.Shutdown(true) }) + + al, err := store.LookupAdditionalLayer(tocDigest, imageRef) + require.NoError(t, err) + defer al.Release() + + assert.Equal(t, tocDigest, al.TOCDigest()) + assert.Equal(t, int64(42), al.CompressedSize()) +} + +func TestLookupAdditionalLayerDecodeError(t *testing.T) { + prevLevel := logrus.GetLevel() + logrus.SetLevel(logrus.ErrorLevel) + t.Cleanup(func() { logrus.SetLevel(prevLevel) }) + + tocDigest := digest.FromString("test-layer") + imageRef := "fedora" + + alsPath := setupAdditionalLayerStore(t, tocDigest, imageRef, "not valid json") + store := newTestStore(t, StoreOptions{ + GraphDriverName: "overlay", + GraphDriverOptions: []string{"additionallayerstore=" + alsPath + ":ref"}, + }) + t.Cleanup(func() { _, _ = store.Shutdown(true) }) + + _, err := store.LookupAdditionalLayer(tocDigest, imageRef) + assert.Error(t, err, "should fail on invalid JSON in info file") +} diff --git a/storage/drivers/overlay/overlay.go b/storage/drivers/overlay/overlay.go index 00974c890c..10cf7dfa06 100644 --- a/storage/drivers/overlay/overlay.go +++ b/storage/drivers/overlay/overlay.go @@ -648,10 +648,10 @@ func SupportsNativeOverlay(home, runhome string) (bool, error) { // Do nothing. default: needsMountProgram, err := scanForMountProgramIndicators(home) - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, fs.ErrNotExist) { return false, err } - if err := os.WriteFile(getMountProgramFlagFile(home), []byte(fmt.Sprintf("%t", needsMountProgram)), 0o600); err != nil && !os.IsNotExist(err) { + if err := os.WriteFile(getMountProgramFlagFile(home), []byte(fmt.Sprintf("%t", needsMountProgram)), 0o600); err != nil && !errors.Is(err, fs.ErrNotExist) { return false, err } if needsMountProgram { @@ -1193,7 +1193,7 @@ func (d *Driver) getLower(parent string) (string, error) { // Read Parent link fileA parentLink, err := os.ReadFile(path.Join(parentDir, "link")) if err != nil { - if !os.IsNotExist(err) { + if !errors.Is(err, fs.ErrNotExist) { 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")) @@ -1266,7 +1266,7 @@ func (d *Driver) getLowerDirs(id string) ([]string, error) { // 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) { + if errors.Is(err, fs.ErrNotExist) { 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) @@ -1282,7 +1282,7 @@ func (d *Driver) getLowerDirs(id string) ([]string, error) { } lowersArray = append(lowersArray, path.Clean(d.dir(path.Join("link", lp)))) } - } else if !os.IsNotExist(err) { + } else if !errors.Is(err, fs.ErrNotExist) { return nil, err } return lowersArray, nil @@ -1335,7 +1335,7 @@ func (d *Driver) removeCommon(id string, cleanup func(string) error) error { d.releaseAdditionalLayerByID(id) - if err := cleanup(dir); err != nil && !os.IsNotExist(err) { + if err := cleanup(dir); err != nil && !errors.Is(err, fs.ErrNotExist) { return err } if d.quotaCtl != nil { @@ -1430,7 +1430,7 @@ func (d *Driver) recreateSymlinks() error { // 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 != nil && errors.Is(err, fs.ErrNotExist) { if err := os.Symlink(path.Join("..", dir.Name(), "diff"), linkPath); err != nil { errs = errors.Join(errs, err) continue @@ -1464,7 +1464,7 @@ func (d *Driver) recreateSymlinks() error { 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) { + if !errors.Is(err, fs.ErrNotExist) { 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 @@ -1590,7 +1590,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO } lowers, err := os.ReadFile(path.Join(dir, lowerFile)) - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, fs.ErrNotExist) { return "", err } splitLowers := strings.Split(string(lowers), ":") @@ -1645,7 +1645,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO maybeAddComposefsMount := func(lowerID string, i int, readWrite bool) (string, error) { composefsBlob := d.getComposefsData(lowerID) if err := fileutils.Exists(composefsBlob); err != nil { - if os.IsNotExist(err) { + if errors.Is(err, fs.ErrNotExist) { return "", nil } return "", err @@ -1706,7 +1706,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO // 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) { + if lower == "" && errors.Is(err, fs.ErrNotExist) { 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) @@ -1983,7 +1983,7 @@ func (d *Driver) Put(id string) error { if count := d.ctr.Decrement(mountpoint); count > 0 { return nil } - if err := fileutils.Exists(path.Join(dir, lowerFile)); err != nil && !os.IsNotExist(err) { + if err := fileutils.Exists(path.Join(dir, lowerFile)); err != nil && !errors.Is(err, fs.ErrNotExist) { return err } @@ -2028,7 +2028,7 @@ func (d *Driver) Put(id string) error { } if !unmounted { - if err := unix.Unmount(mountpoint, unix.MNT_DETACH); err != nil && !os.IsNotExist(err) { + if err := unix.Unmount(mountpoint, unix.MNT_DETACH); err != nil && !errors.Is(err, fs.ErrNotExist) { logrus.Debugf("Failed to unmount %s overlay: %s - %v", id, mountpoint, err) if !errors.Is(err, unix.EINVAL) { return fmt.Errorf("unmounting %q: %w", mountpoint, err) @@ -2205,7 +2205,7 @@ func (d *Driver) DiffGetter(id string) (_ graphdriver.FileGetCloser, Err error) if Err != nil { for _, f := range composefsMounts { f.Close() - if err := unix.Rmdir(f.Name()); err != nil && !os.IsNotExist(err) { + if err := unix.Rmdir(f.Name()); err != nil && !errors.Is(err, fs.ErrNotExist) { logrus.Warnf("Failed to remove %s: %v", f.Name(), err) } } @@ -2378,7 +2378,7 @@ func (d *Driver) ApplyDiffFromStagingDirectory(id, parent string, diffOutput *gr return err } } - if err := os.RemoveAll(diffPath); err != nil && !os.IsNotExist(err) { + if err := os.RemoveAll(diffPath); err != nil && !errors.Is(err, fs.ErrNotExist) { return err } @@ -2750,7 +2750,7 @@ func (d *Driver) getAdditionalLayerPath(tocDigest digest.Digest, ref string) (st func (d *Driver) releaseAdditionalLayerByID(id string) { if al, err := d.getAdditionalLayerPathByID(id); err == nil { notifyReleaseAdditionalLayer(al) - } else if !os.IsNotExist(err) { + } else if !errors.Is(err, fs.ErrNotExist) { logrus.Warnf("Unexpected error on reading Additional Layer Store pointer %v", err) } } @@ -2827,7 +2827,7 @@ func notifyUseAdditionalLayer(al string) { } useFile := path.Join(al, "use") f, err := os.Create(useFile) - if os.IsNotExist(err) { + if errors.Is(err, fs.ErrNotExist) { return } else if err == nil { f.Close() @@ -2849,7 +2849,7 @@ func notifyReleaseAdditionalLayer(al string) { } // tell the additional layer store that we don't use this layer anymore. err := unix.Rmdir(al) - if os.IsNotExist(err) { + if errors.Is(err, fs.ErrNotExist) { return } logrus.Warnf("Unexpected error by Additional Layer Store %v during release; GC doesn't seem to be supported", err) diff --git a/storage/store.go b/storage/store.go index 3d8ea50759..8b2a699805 100644 --- a/storage/store.go +++ b/storage/store.go @@ -3499,6 +3499,12 @@ func (s *store) LookupAdditionalLayer(tocDigest digest.Digest, imageref string) } return nil, err } + succeeded := false + defer func() { + if !succeeded { + al.Release() + } + }() info, err := al.Info() if err != nil { return nil, err @@ -3508,6 +3514,7 @@ func (s *store) LookupAdditionalLayer(tocDigest digest.Digest, imageref string) if err := json.NewDecoder(info).Decode(&layer); err != nil { return nil, err } + succeeded = true return &additionalLayer{&layer, al, s}, nil }