From e940b5ac18e8f9e31d7cb4fd3336b85a67557690 Mon Sep 17 00:00:00 2001 From: Chris Ayoub Date: Sun, 9 Aug 2026 20:25:28 +0000 Subject: [PATCH] snapshots/erofs: protect snapshot staging from cleanup A new-* directory can exist before createSnapshot acquires the metadata writer transaction. A concurrent Remove performs a global orphan scan and can delete the staging directory because it is not in the ID map yet. Exclude new-* staging directories from orphan cleanup and use a shared local constant for directory creation and filtering. Signed-off-by: Chris Ayoub --- plugins/snapshots/erofs/erofs.go | 11 +++++++- plugins/snapshots/erofs/erofs_linux_test.go | 28 +++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/plugins/snapshots/erofs/erofs.go b/plugins/snapshots/erofs/erofs.go index 46352cd4c01a4..12d0e44b127a0 100644 --- a/plugins/snapshots/erofs/erofs.go +++ b/plugins/snapshots/erofs/erofs.go @@ -24,6 +24,7 @@ import ( "path/filepath" "runtime" "strconv" + "strings" "github.com/containerd/continuity/fs" "github.com/containerd/errdefs" @@ -39,6 +40,8 @@ import ( "github.com/containerd/containerd/v2/internal/userns" ) +const snapshotTempDirPrefix = "new-" + // SnapshotterConfig is used to configure the erofs snapshotter instance type SnapshotterConfig struct { // ovlOptions are the base options added to the overlayfs mount (defaults to [""]) @@ -293,7 +296,7 @@ func (s *snapshotter) lowerPath(id string) (string, error) { } func (s *snapshotter) prepareDirectory(ctx context.Context, snapshotDir string, kind snapshots.Kind, cacheBlob string) (string, error) { - td, err := os.MkdirTemp(snapshotDir, "new-") + td, err := os.MkdirTemp(snapshotDir, snapshotTempDirPrefix) if err != nil { return "", fmt.Errorf("failed to create temp dir: %w", err) } @@ -891,6 +894,12 @@ func (s *snapshotter) getCleanupDirectories(ctx context.Context) ([]string, erro cleanup := []string{} for _, d := range dirs { + // A new-* directory may belong to a concurrent snapshot creation. It is + // renamed to its metadata ID after the writer transaction is acquired, so + // Remove must not treat it as an orphan in the meantime. + if strings.HasPrefix(d, snapshotTempDirPrefix) { + continue + } if _, ok := ids[d]; ok { continue } diff --git a/plugins/snapshots/erofs/erofs_linux_test.go b/plugins/snapshots/erofs/erofs_linux_test.go index ec9c15c6b752b..b350de2721468 100644 --- a/plugins/snapshots/erofs/erofs_linux_test.go +++ b/plugins/snapshots/erofs/erofs_linux_test.go @@ -113,6 +113,34 @@ func TestErofsWithQuota(t *testing.T) { testsuite.SnapshotterSuite(t, "erofs", newSnapshotter(t, WithDefaultSize(16*1024*1024))) } +func TestGetCleanupDirectoriesSkipsSnapshotTempDirs(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + snapshotDir := filepath.Join(root, "snapshots") + require.NoError(t, os.Mkdir(snapshotDir, 0700)) + + ms, err := storage.NewMetaStore(filepath.Join(root, "metadata.db")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, ms.Close()) }) + s := &snapshotter{root: root, ms: ms} + + _, err = os.MkdirTemp(snapshotDir, snapshotTempDirPrefix) + require.NoError(t, err) + orphanDir := filepath.Join(snapshotDir, "orphan") + require.NoError(t, os.Mkdir(orphanDir, 0700)) + require.NoError(t, ms.WithTransaction(ctx, true, func(ctx context.Context) error { + _, err := storage.CreateSnapshot(ctx, snapshots.KindActive, "existing", "") + return err + })) + + var cleanup []string + require.NoError(t, ms.WithTransaction(ctx, true, func(ctx context.Context) error { + cleanup, err = s.getCleanupDirectories(ctx) + return err + })) + assert.Equal(t, []string{orphanDir}, cleanup) +} + // TestWritableSize exercises the LabelSnapshotMaxSize override that the // block-mode mkfs path passes to X-containerd.mkfs.size. Covers the // happy path (label overrides default), fallback cases (missing, empty,