diff --git a/plugins/snapshots/erofs/erofs.go b/plugins/snapshots/erofs/erofs.go index 47402f73ba..586d16583c 100644 --- a/plugins/snapshots/erofs/erofs.go +++ b/plugins/snapshots/erofs/erofs.go @@ -20,11 +20,8 @@ import ( "context" "fmt" "os" - "os/exec" "path/filepath" "runtime" - "strings" - "time" "github.com/containerd/continuity/fs" "github.com/containerd/errdefs" @@ -48,9 +45,7 @@ type SnapshotterConfig struct { setImmutable bool // defaultSize creates a default size writable layer for active snapshots defaultSize int64 - // fsMergeThreshold (>0) enables fsmerge when the number of image layers exceeds this value - fsMergeThreshold uint - remapIDs bool + remapIDs bool // dmverityMode controls dm-verity behavior: "auto" (use if .dmverity exists), "on" (require .dmverity), "off" (disable) dmverityMode string } @@ -93,13 +88,6 @@ func WithDefaultSize(size int64) Opt { } } -// WithFsMergeThreshold (>0) enables fsmerge when the number of image layers exceeds this value -func WithFsMergeThreshold(v uint) Opt { - return func(config *SnapshotterConfig) { - config.fsMergeThreshold = v - } -} - // WithRemapIDs enables kernel ID-mapped mounts for user namespace support func WithRemapIDs() Opt { return func(config *SnapshotterConfig) { @@ -114,16 +102,15 @@ type MetaStore interface { } type snapshotter struct { - root string - ms *storage.MetaStore - ovlOptions []string - enableFsverity bool - setImmutable bool - defaultWritable int64 - blockMode bool - fsMergeThreshold uint - remapIDs bool - dmverityMode string + root string + ms *storage.MetaStore + ovlOptions []string + enableFsverity bool + setImmutable bool + defaultWritable int64 + blockMode bool + remapIDs bool + dmverityMode string } // NewSnapshotter returns a Snapshotter which uses EROFS+OverlayFS. The layers @@ -191,16 +178,15 @@ func NewSnapshotter(root string, opts ...Opt) (snapshots.Snapshotter, error) { } return &snapshotter{ - root: root, - ms: ms, - ovlOptions: config.ovlOptions, - enableFsverity: config.enableFsverity, - setImmutable: config.setImmutable, - defaultWritable: config.defaultSize, - blockMode: config.defaultSize > 0, - fsMergeThreshold: config.fsMergeThreshold, - remapIDs: config.remapIDs, - dmverityMode: config.dmverityMode, + root: root, + ms: ms, + ovlOptions: config.ovlOptions, + enableFsverity: config.enableFsverity, + setImmutable: config.setImmutable, + defaultWritable: config.defaultSize, + blockMode: config.defaultSize > 0, + remapIDs: config.remapIDs, + dmverityMode: config.dmverityMode, }, nil } @@ -448,12 +434,10 @@ func (s *snapshotter) mounts(snap storage.Snapshot, info snapshots.Info) ([]moun for i := range snap.ParentIDs { // If a merged fsmeta is valid for this layer, skip the remaining bottom layers. // Why? Because bottom layers have been flattened with the thin fsmeta. - if s.fsMergeThreshold > 0 { - if m, ok := s.mountFsMeta(snap, i); ok { - mounts = append(mounts, m) - first = len(mounts) - 1 - break - } + if m, ok := s.mountFsMeta(snap, i); ok { + mounts = append(mounts, m) + first = len(mounts) - 1 + break } layerBlob, err := s.lowerPath(snap.ParentIDs[i]) @@ -601,11 +585,6 @@ func (s *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k return nil, err } - // Generate fsmeta outside of the transaction since it's unnecessary. - // Also ignore all errors since it's a nice-to-have stuff. - if !strings.Contains(key, snapshots.UnpackKeyPrefix) { - s.generateFsMeta(ctx, snap.ParentIDs) - } return s.mounts(snap, info) } @@ -654,47 +633,6 @@ func (s *snapshotter) commitBlock(ctx context.Context, layerBlob string, id stri return nil } -// generate a metadata-only EROFS fsmeta.erofs if all EROFS layer blobs are valid -func (s *snapshotter) generateFsMeta(ctx context.Context, snapIDs []string) { - var blobs []string - - if s.fsMergeThreshold == 0 || uint(len(snapIDs)) <= s.fsMergeThreshold { - return - } - - t1 := time.Now() - mergedMeta := s.fsMetaPath(snapIDs[0]) - // If the empty placeholder cannot be created (mainly due to os.IsExist), just return - if _, err := os.OpenFile(mergedMeta, os.O_CREATE|os.O_EXCL, 0644); err != nil { - return - } - - for i := len(snapIDs) - 1; i >= 0; i-- { - blob := s.layerBlobPath(snapIDs[i]) - if _, err := os.Stat(blob); err != nil { - return - } - blobs = append(blobs, blob) - } - tmpMergedMeta := mergedMeta + ".tmp" - args := append([]string{"--aufs", "--ovlfs-strip=1", "--quiet", tmpMergedMeta}, blobs...) - log.G(ctx).Infof("merging layers with mkfs.erofs %v", args) - cmd := exec.CommandContext(ctx, "mkfs.erofs", args...) - out, err := cmd.CombinedOutput() - if err != nil { - log.G(ctx).Warnf("failed to generate merged fsmeta for %v: %q: %v", snapIDs[0], string(out), err) - return - } - // Atomically replace the fsmeta with the generated file - if err = os.Rename(tmpMergedMeta, mergedMeta); err != nil { - log.G(ctx).Errorf("failed to rename fsmeta: %v", err) - return - } - log.G(ctx).WithFields(log.Fields{ - "d": time.Since(t1), - }).Infof("merged fsmeta for %v generated", snapIDs[0]) -} - func (s *snapshotter) Commit(ctx context.Context, name, key string, opts ...snapshots.Opt) error { var layerBlob string var id string diff --git a/plugins/snapshots/erofs/erofs_linux_test.go b/plugins/snapshots/erofs/erofs_linux_test.go index 3a13533bb9..07e1600890 100644 --- a/plugins/snapshots/erofs/erofs_linux_test.go +++ b/plugins/snapshots/erofs/erofs_linux_test.go @@ -703,3 +703,74 @@ func TestApplyDmverityPolicy(t *testing.T) { assert.Equal(t, "X-containerd.dmverity="+expectedPath, opt) }) } + +func TestMountFsMeta(t *testing.T) { + root := t.TempDir() + s := &snapshotter{root: root} + + parents := []string{"p0", "p1", "p2"} + for _, id := range parents { + require.NoError(t, os.MkdirAll(filepath.Join(root, "snapshots", id), 0755)) + } + + writeMeta := func(t *testing.T, id string, contents []byte) { + t.Helper() + require.NoError(t, os.WriteFile(s.fsMetaPath(id), contents, 0644)) + } + removeMeta := func(t *testing.T, id string) { + t.Helper() + err := os.Remove(s.fsMetaPath(id)) + if err != nil && !os.IsNotExist(err) { + t.Fatal(err) + } + } + + snap := storage.Snapshot{ParentIDs: parents} + + t.Run("missing fsmeta returns false", func(t *testing.T) { + for _, id := range parents { + removeMeta(t, id) + } + _, ok := s.mountFsMeta(snap, 0) + assert.False(t, ok) + }) + + t.Run("empty fsmeta returns false", func(t *testing.T) { + writeMeta(t, "p0", nil) + t.Cleanup(func() { removeMeta(t, "p0") }) + + _, ok := s.mountFsMeta(snap, 0) + assert.False(t, ok) + }) + + t.Run("non-empty fsmeta on top parent returns mount with all device options", func(t *testing.T) { + writeMeta(t, "p0", []byte("merged")) + t.Cleanup(func() { removeMeta(t, "p0") }) + + m, ok := s.mountFsMeta(snap, 0) + require.True(t, ok) + assert.Equal(t, "erofs", m.Type) + assert.Equal(t, s.fsMetaPath("p0"), m.Source) + // Devices appended in reverse parent order from len-1 down to id. + assert.Equal(t, []string{ + "ro", "loop", + "device=" + s.layerBlobPath("p2"), + "device=" + s.layerBlobPath("p1"), + "device=" + s.layerBlobPath("p0"), + }, m.Options) + }) + + t.Run("non-empty fsmeta on intermediate parent only references parents at or below id", func(t *testing.T) { + writeMeta(t, "p1", []byte("merged")) + t.Cleanup(func() { removeMeta(t, "p1") }) + + m, ok := s.mountFsMeta(snap, 1) + require.True(t, ok) + assert.Equal(t, s.fsMetaPath("p1"), m.Source) + assert.Equal(t, []string{ + "ro", "loop", + "device=" + s.layerBlobPath("p2"), + "device=" + s.layerBlobPath("p1"), + }, m.Options) + }) +} diff --git a/plugins/snapshots/erofs/plugin/plugin.go b/plugins/snapshots/erofs/plugin/plugin.go index ff2a1b4b00..834767f714 100644 --- a/plugins/snapshots/erofs/plugin/plugin.go +++ b/plugins/snapshots/erofs/plugin/plugin.go @@ -52,9 +52,6 @@ type Config struct { // DefaultSize is the default size of a writable layer in string DefaultSize string `toml:"default_size"` - // MaxUnmergedLayers (>0) enables fsmerge when the number of image layers exceeds this value. - MaxUnmergedLayers uint `toml:"max_unmerged_layers"` - // DmverityMode controls dm-verity behavior: "auto" (use if available), "on" (require), "off" (disable) // Linux only DmverityMode string `toml:"dmverity_mode"` @@ -99,10 +96,6 @@ func init() { opts = append(opts, erofs.WithDefaultSize(size)) } - if config.MaxUnmergedLayers > 0 { - opts = append(opts, erofs.WithFsMergeThreshold(config.MaxUnmergedLayers)) - } - if config.DmverityMode != "" { opts = append(opts, erofs.WithDmverityMode(config.DmverityMode)) }