snapshot: update mounts to mountable interface

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

cache/blobs/blobs.go

@@ -59,15 +59,25 @@ func GetDiffPairs(ctx context.Context, contentStore content.Store, snapshotter s
 			var lower []mount.Mount
 			if parent != nil {
 				defer parent.Release(context.TODO())
-				lower, err = parent.Mount(ctx, true)
+				m, err := parent.Mount(ctx, true)
 				if err != nil {
 					return nil, err
 				}
+				lower, err = m.Mount()
+				if err != nil {
+					return nil, err
+				}
+				defer m.Release()
 			}
-			upper, err := ref.Mount(ctx, true)
+			m, err := ref.Mount(ctx, true)
 			if err != nil {
 				return nil, err
 			}
+			upper, err := m.Mount()
+			if err != nil {
+				return nil, err
+			}
+			defer m.Release()
 			descr, err := differ.Compare(ctx, lower, upper,
 				diff.WithMediaType(ocispec.MediaTypeImageLayerGzip),
 				diff.WithReference(ref.ID()),

cache/cacheimport/import.go

@@ -252,7 +252,10 @@ func (ii *importInfo) fetch(ctx context.Context, chain []blobs.DiffPair) (cache.
 		return nil, err
 	}
 
-	chainid, err := ii.unpack(ctx, chain)
+	cs, release := snapshot.NewContainerdSnapshotter(ii.opt.Snapshotter)
+	defer release()
+
+	chainid, err := ii.unpack(ctx, chain, cs)
 	if err != nil {
 		return nil, err
 	}
@@ -260,7 +263,7 @@ func (ii *importInfo) fetch(ctx context.Context, chain []blobs.DiffPair) (cache.
 	return ii.opt.CacheAccessor.Get(ctx, chainid, cache.WithDescription("imported cache")) // TODO: more descriptive name
 }
 
-func (ii *importInfo) unpack(ctx context.Context, dpairs []blobs.DiffPair) (string, error) {
+func (ii *importInfo) unpack(ctx context.Context, dpairs []blobs.DiffPair, s cdsnapshot.Snapshotter) (string, error) {
 	layers, err := ii.getLayers(ctx, dpairs)
 	if err != nil {
 		return "", err
@@ -271,7 +274,7 @@ func (ii *importInfo) unpack(ctx context.Context, dpairs []blobs.DiffPair) (stri
 		labels := map[string]string{
 			"containerd.io/uncompressed": layer.Diff.Digest.String(),
 		}
-		if _, err := rootfs.ApplyLayer(ctx, layer, chain, ii.opt.Snapshotter, ii.opt.Applier, cdsnapshot.WithLabels(labels)); err != nil {
+		if _, err := rootfs.ApplyLayer(ctx, layer, chain, s, ii.opt.Applier, cdsnapshot.WithLabels(labels)); err != nil {
 			return "", err
 		}
 		chain = append(chain, layer.Diff.Digest)

cache/contenthash/checksum_test.go

@@ -442,7 +442,7 @@ func setupCacheManager(t *testing.T, tmpdir string, snapshotter snapshots.Snapsh
 	require.NoError(t, err)
 
 	cm, err := cache.NewManager(cache.ManagerOpt{
-		Snapshotter:   snapshotter,
+		Snapshotter:   snapshot.FromContainerdSnapshotter(snapshotter),
 		MetadataStore: md,
 	})
 	require.NoError(t, err)

cache/manager.go

@@ -10,6 +10,7 @@ import (
 	"github.com/moby/buildkit/cache/metadata"
 	"github.com/moby/buildkit/client"
 	"github.com/moby/buildkit/identity"
+	"github.com/moby/buildkit/snapshot"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sync/errgroup"
@@ -22,7 +23,7 @@ var (
 )
 
 type ManagerOpt struct {
-	Snapshotter   snapshots.Snapshotter
+	Snapshotter   snapshot.SnapshotterBase
 	GCPolicy      GCPolicy
 	MetadataStore *metadata.Store
 }
@@ -219,7 +220,7 @@ func (cm *cacheManager) New(ctx context.Context, s ImmutableRef, opts ...RefOpti
 		parentID = parent.ID()
 	}
 
-	if _, err := cm.Snapshotter.Prepare(ctx, id, parentID); err != nil {
+	if err := cm.Snapshotter.Prepare(ctx, id, parentID); err != nil {
 		if parent != nil {
 			parent.Release(context.TODO())
 		}

cache/manager_test.go

@@ -374,7 +374,7 @@ func getCacheManager(t *testing.T, tmpdir string, snapshotter snapshots.Snapshot
 	require.NoError(t, err)
 
 	cm, err := NewManager(ManagerOpt{
-		Snapshotter:   snapshotter,
+		Snapshotter:   snapshot.FromContainerdSnapshotter(snapshotter),
 		MetadataStore: md,
 	})
 	require.NoError(t, err, fmt.Sprintf("error: %+v", err))

cache/refs.go

@@ -7,6 +7,7 @@ import (
 	"github.com/containerd/containerd/mount"
 	"github.com/moby/buildkit/cache/metadata"
 	"github.com/moby/buildkit/identity"
+	"github.com/moby/buildkit/snapshot"
 	"github.com/moby/buildkit/util/flightcontrol"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -33,7 +34,7 @@ type MutableRef interface {
 }
 
 type Mountable interface {
-	Mount(ctx context.Context, readonly bool) ([]mount.Mount, error)
+	Mount(ctx context.Context, readonly bool) (snapshot.Mountable, error)
 }
 
 type cacheRecord struct {
@@ -49,7 +50,7 @@ type cacheRecord struct {
 	dead bool
 
 	view      string
-	viewMount []mount.Mount
+	viewMount snapshot.Mountable
 
 	sizeG flightcontrol.Group
 
@@ -120,7 +121,7 @@ func (cr *cacheRecord) Parent() ImmutableRef {
 	return p.ref()
 }
 
-func (cr *cacheRecord) Mount(ctx context.Context, readonly bool) ([]mount.Mount, error) {
+func (cr *cacheRecord) Mount(ctx context.Context, readonly bool) (snapshot.Mountable, error) {
 	cr.mu.Lock()
 	defer cr.mu.Unlock()
 
@@ -341,7 +342,19 @@ func (sr *mutableRef) release(ctx context.Context) error {
 	return nil
 }
 
-func setReadonly(mounts []mount.Mount) []mount.Mount {
+func setReadonly(mounts snapshot.Mountable) snapshot.Mountable {
+	return &readOnlyMounter{mounts}
+}
+
+type readOnlyMounter struct {
+	snapshot.Mountable
+}
+
+func (m *readOnlyMounter) Mount() ([]mount.Mount, error) {
+	mounts, err := m.Mountable.Mount()
+	if err != nil {
+		return nil, err
+	}
 	for i, m := range mounts {
 		opts := make([]string, 0, len(m.Options))
 		for _, opt := range m.Options {
@@ -352,5 +365,5 @@ func setReadonly(mounts []mount.Mount) []mount.Mount {
 		opts = append(opts, "ro")
 		mounts[i].Options = opts
 	}
-	return mounts
+	return mounts, nil
 }

executor/containerdexecutor/executor.go

@@ -42,14 +42,20 @@ func (w containerdExecutor) Exec(ctx context.Context, meta executor.Meta, root c
 		return err
 	}
 
-	rootMounts, err := root.Mount(ctx, false)
+	mountable, err := root.Mount(ctx, false)
 	if err != nil {
 		return err
 	}
 
+	rootMounts, err := mountable.Mount()
+	if err != nil {
+		return err
+	}
+	defer mountable.Release()
+
 	uid, gid, err := oci.ParseUser(meta.User)
 	if err != nil {
-		lm := snapshot.LocalMounter(rootMounts)
+		lm := snapshot.LocalMounterWithMounts(rootMounts)
 		rootfsPath, err := lm.Mount()
 		if err != nil {
 			return err

executor/oci/spec_unix.go

@@ -49,19 +49,33 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou
 
 	sm := &submounts{}
 
+	var releasers []func() error
+	releaseAll := func() {
+		sm.cleanup()
+		for _, f := range releasers {
+			f()
+		}
+	}
+
 	for _, m := range mounts {
 		if m.Src == nil {
 			return nil, nil, errors.Errorf("mount %s has no source", m.Dest)
 		}
-		mounts, err := m.Src.Mount(ctx, m.Readonly)
+		mountable, err := m.Src.Mount(ctx, m.Readonly)
 		if err != nil {
-			sm.cleanup()
+			releaseAll()
 			return nil, nil, errors.Wrapf(err, "failed to mount %s", m.Dest)
 		}
+		mounts, err := mountable.Mount()
+		if err != nil {
+			releaseAll()
+			return nil, nil, errors.WithStack(err)
+		}
+		releasers = append(releasers, mountable.Release)
 		for _, mount := range mounts {
 			mount, err = sm.subMount(mount, m.Selector)
 			if err != nil {
-				sm.cleanup()
+				releaseAll()
 				return nil, nil, err
 			}
 			s.Mounts = append(s.Mounts, specs.Mount{
@@ -73,7 +87,7 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou
 		}
 	}
 
-	return s, sm.cleanup, nil
+	return s, releaseAll, nil
 }
 
 func withROBind(src, dest string) func(_ context.Context, _ oci.Client, _ *containers.Container, s *specs.Spec) error {
@@ -112,7 +126,7 @@ func (s *submounts) subMount(m mount.Mount, subPath string) (mount.Mount, error)
 		return sub(mr.mount, subPath), nil
 	}
 
-	lm := snapshot.LocalMounter([]mount.Mount{m})
+	lm := snapshot.LocalMounterWithMounts([]mount.Mount{m})
 
 	mp, err := lm.Mount()
 	if err != nil {

executor/runcexecutor/executor.go

@@ -92,11 +92,17 @@ func (w *runcExecutor) Exec(ctx context.Context, meta executor.Meta, root cache.
 		return err
 	}
 
-	rootMount, err := root.Mount(ctx, false)
+	mountable, err := root.Mount(ctx, false)
 	if err != nil {
 		return err
 	}
 
+	rootMount, err := mountable.Mount()
+	if err != nil {
+		return err
+	}
+	defer mountable.Release()
+
 	id := identity.NewID()
 	bundle := filepath.Join(w.root, id)
 

snapshot/blobmapping/snapshotter.go

@@ -16,7 +16,7 @@ const blobKey = "blobmapping.blob"
 
 type Opt struct {
 	Content       content.Store
-	Snapshotter   snapshots.Snapshotter
+	Snapshotter   snapshot.SnapshotterBase
 	MetadataStore *metadata.Store
 }
 
@@ -33,14 +33,14 @@ type DiffPair struct {
 // this snapshotter keeps an internal mapping between a snapshot and a blob
 
 type Snapshotter struct {
-	snapshots.Snapshotter
+	snapshot.SnapshotterBase
 	opt Opt
 }
 
 func NewSnapshotter(opt Opt) snapshot.Snapshotter {
 	s := &Snapshotter{
-		Snapshotter: opt.Snapshotter,
+		SnapshotterBase: opt.Snapshotter,
-		opt:         opt,
+		opt:             opt,
 	}
 
 	return s
@@ -59,7 +59,7 @@ func (s *Snapshotter) Remove(ctx context.Context, key string) error {
 		return err
 	}
 
-	if err := s.Snapshotter.Remove(ctx, key); err != nil {
+	if err := s.SnapshotterBase.Remove(ctx, key); err != nil {
 		return err
 	}
 
@@ -72,7 +72,7 @@ func (s *Snapshotter) Remove(ctx context.Context, key string) error {
 }
 
 func (s *Snapshotter) Usage(ctx context.Context, key string) (snapshots.Usage, error) {
-	u, err := s.Snapshotter.Usage(ctx, key)
+	u, err := s.SnapshotterBase.Usage(ctx, key)
 	if err != nil {
 		return snapshots.Usage{}, err
 	}

snapshot/containerd/snapshotter.go

@@ -16,7 +16,7 @@ import (
 func NewSnapshotter(snapshotter ctdsnapshot.Snapshotter, store content.Store, mdstore *metadata.Store, ns string, gc func(context.Context) error) snapshot.Snapshotter {
 	return blobmapping.NewSnapshotter(blobmapping.Opt{
 		Content:       store,
-		Snapshotter:   &nsSnapshotter{ns, snapshotter, gc},
+		Snapshotter:   snapshot.FromContainerdSnapshotter(&nsSnapshotter{ns, snapshotter, gc}),
 		MetadataStore: mdstore,
 	})
 }

snapshot/localmounter.go

@@ -14,32 +14,47 @@ type Mounter interface {
 	Unmount() error
 }
 
-// LocalMounter is a helper for mounting to temporary path. In addition it can
+// LocalMounter is a helper for mounting mountfactory to temporary path. In
-// mount binds without privileges
+// addition it can mount binds without privileges
-func LocalMounter(m []mount.Mount) Mounter {
+func LocalMounter(mountable Mountable) Mounter {
-	return &localMounter{m: m}
+	return &localMounter{mountable: mountable}
+}
+
+// LocalMounterWithMounts is a helper for mounting to temporary path. In
+// addition it can mount binds without privileges
+func LocalMounterWithMounts(mounts []mount.Mount) Mounter {
+	return &localMounter{mounts: mounts}
 }
 
 type localMounter struct {
-	mu     sync.Mutex
+	mu        sync.Mutex
-	m      []mount.Mount
+	mounts    []mount.Mount
-	target string
+	mountable Mountable
+	target    string
 }
 
 func (lm *localMounter) Mount() (string, error) {
 	lm.mu.Lock()
 	defer lm.mu.Unlock()
 
-	if len(lm.m) == 1 && lm.m[0].Type == "bind" {
+	if lm.mounts == nil {
+		mounts, err := lm.mountable.Mount()
+		if err != nil {
+			return "", err
+		}
+		lm.mounts = mounts
+	}
+
+	if len(lm.mounts) == 1 && (lm.mounts[0].Type == "bind" || lm.mounts[0].Type == "rbind") {
 		ro := false
-		for _, opt := range lm.m[0].Options {
+		for _, opt := range lm.mounts[0].Options {
 			if opt == "ro" {
 				ro = true
 				break
 			}
 		}
 		if !ro {
-			return lm.m[0].Source, nil
+			return lm.mounts[0].Source, nil
 		}
 	}
 
@@ -48,7 +63,7 @@ func (lm *localMounter) Mount() (string, error) {
 		return "", errors.Wrap(err, "failed to create temp dir")
 	}
 
-	if err := mount.All(lm.m, dir); err != nil {
+	if err := mount.All(lm.mounts, dir); err != nil {
 		os.RemoveAll(dir)
 		return "", err
 	}

snapshot/localmounter_unix.go

@@ -21,5 +21,9 @@ func (lm *localMounter) Unmount() error {
 		lm.target = ""
 	}
 
+	if lm.mountable != nil {
+		return lm.mountable.Release()
+	}
+
 	return nil
 }

snapshot/localmounter_windows.go

@@ -18,5 +18,9 @@ func (lm *localMounter) Unmount() error {
 		lm.target = ""
 	}
 
+	if lm.mountable != nil {
+		return lm.mountable.Release()
+	}
+
 	return nil
 }

snapshot/snapshotter.go

@@ -2,18 +2,136 @@ package snapshot
 
 import (
 	"context"
+	"sync"
 
+	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/snapshots"
 	digest "github.com/opencontainers/go-digest"
 )
 
+type Mountable interface {
+	// ID() string
+	Mount() ([]mount.Mount, error)
+	Release() error
+}
+
+type SnapshotterBase interface {
+	Mounts(ctx context.Context, key string) (Mountable, error)
+	Prepare(ctx context.Context, key, parent string, opts ...snapshots.Opt) error
+	View(ctx context.Context, key, parent string, opts ...snapshots.Opt) (Mountable, error)
+
+	Stat(ctx context.Context, key string) (snapshots.Info, error)
+	Update(ctx context.Context, info snapshots.Info, fieldpaths ...string) (snapshots.Info, error)
+	Usage(ctx context.Context, key string) (snapshots.Usage, error)
+	Commit(ctx context.Context, name, key string, opts ...snapshots.Opt) error
+	Remove(ctx context.Context, key string) error
+	Walk(ctx context.Context, fn func(context.Context, snapshots.Info) error) error
+	Close() error
+}
+
 // Snapshotter defines interface that any snapshot implementation should satisfy
 type Snapshotter interface {
-	snapshots.Snapshotter
 	Blobmapper
+	SnapshotterBase
 }
 
 type Blobmapper interface {
 	GetBlob(ctx context.Context, key string) (digest.Digest, digest.Digest, error)
 	SetBlob(ctx context.Context, key string, diffID, blob digest.Digest) error
 }
+
+func FromContainerdSnapshotter(s snapshots.Snapshotter) SnapshotterBase {
+	return &fromContainerd{Snapshotter: s}
+}
+
+type fromContainerd struct {
+	snapshots.Snapshotter
+}
+
+func (s *fromContainerd) Mounts(ctx context.Context, key string) (Mountable, error) {
+	mounts, err := s.Snapshotter.Mounts(ctx, key)
+	if err != nil {
+		return nil, err
+	}
+	return &staticMountable{mounts}, nil
+}
+func (s *fromContainerd) Prepare(ctx context.Context, key, parent string, opts ...snapshots.Opt) error {
+	_, err := s.Snapshotter.Prepare(ctx, key, parent, opts...)
+	return err
+}
+func (s *fromContainerd) View(ctx context.Context, key, parent string, opts ...snapshots.Opt) (Mountable, error) {
+	mounts, err := s.Snapshotter.View(ctx, key, parent, opts...)
+	if err != nil {
+		return nil, err
+	}
+	return &staticMountable{mounts}, nil
+}
+
+type staticMountable struct {
+	mounts []mount.Mount
+}
+
+func (m *staticMountable) Mount() ([]mount.Mount, error) {
+	return m.mounts, nil
+}
+
+func (cm *staticMountable) Release() error {
+	return nil
+}
+
+// NewContainerdSnapshotter converts snapshotter to containerd snapshotter
+func NewContainerdSnapshotter(s Snapshotter) (snapshots.Snapshotter, func() error) {
+	cs := &containerdSnapshotter{Snapshotter: s}
+	return cs, cs.release
+}
+
+type containerdSnapshotter struct {
+	mu        sync.Mutex
+	releasers []func() error
+	Snapshotter
+}
+
+func (cs *containerdSnapshotter) release() error {
+	cs.mu.Lock()
+	defer cs.mu.Unlock()
+	var err error
+	for _, f := range cs.releasers {
+		if err1 := f(); err != nil && err == nil {
+			err = err1
+		}
+	}
+	return err
+}
+
+func (cs *containerdSnapshotter) returnMounts(mf Mountable) ([]mount.Mount, error) {
+	mounts, err := mf.Mount()
+	if err != nil {
+		return nil, err
+	}
+	cs.mu.Lock()
+	cs.releasers = append(cs.releasers, mf.Release)
+	cs.mu.Unlock()
+	return mounts, nil
+}
+
+func (cs *containerdSnapshotter) Mounts(ctx context.Context, key string) ([]mount.Mount, error) {
+	mf, err := cs.Snapshotter.Mounts(ctx, key)
+	if err != nil {
+		return nil, err
+	}
+	return cs.returnMounts(mf)
+}
+
+func (cs *containerdSnapshotter) Prepare(ctx context.Context, key, parent string, opts ...snapshots.Opt) ([]mount.Mount, error) {
+	if err := cs.Snapshotter.Prepare(ctx, key, parent, opts...); err != nil {
+		return nil, err
+	}
+	return cs.Mounts(ctx, key)
+}
+func (cs *containerdSnapshotter) View(ctx context.Context, key, parent string, opts ...snapshots.Opt) ([]mount.Mount, error) {
+	mf, err := cs.Snapshotter.View(ctx, key, parent, opts...)
+	if err != nil {
+		return nil, err
+	}
+	return cs.returnMounts(mf)
+}

source/containerimage/pull.go

@@ -333,8 +333,11 @@ func (p *puller) Snapshot(ctx context.Context) (cache.ImmutableRef, error) {
 		}
 	}
 
+	csh, release := snapshot.NewContainerdSnapshotter(p.is.Snapshotter)
+	defer release()
+
 	unpackProgressDone := oneOffProgress(ctx, "unpacking "+p.src.Reference.String())
-	chainid, err := p.is.unpack(ctx, p.desc)
+	chainid, err := p.is.unpack(ctx, p.desc, csh)
 	if err != nil {
 		return nil, unpackProgressDone(err)
 	}
@@ -343,7 +346,7 @@ func (p *puller) Snapshot(ctx context.Context) (cache.ImmutableRef, error) {
 	return p.is.CacheAccessor.Get(ctx, chainid, cache.WithDescription(fmt.Sprintf("pulled from %s", p.ref)))
 }
 
-func (is *imageSource) unpack(ctx context.Context, desc ocispec.Descriptor) (string, error) {
+func (is *imageSource) unpack(ctx context.Context, desc ocispec.Descriptor, s snapshots.Snapshotter) (string, error) {
 	layers, err := getLayers(ctx, is.ContentStore, desc)
 	if err != nil {
 		return "", err
@@ -355,7 +358,7 @@ func (is *imageSource) unpack(ctx context.Context, desc ocispec.Descriptor) (str
 			"containerd.io/gc.root":      time.Now().UTC().Format(time.RFC3339Nano),
 			"containerd.io/uncompressed": layer.Diff.Digest.String(),
 		}
-		if _, err := rootfs.ApplyLayer(ctx, layer, chain, is.Snapshotter, is.Applier, snapshots.WithLabels(labels)); err != nil {
+		if _, err := rootfs.ApplyLayer(ctx, layer, chain, s, is.Applier, snapshots.WithLabels(labels)); err != nil {
 			return "", err
 		}
 		chain = append(chain, layer.Diff.Digest)

source/git/gitsource_test.go

@@ -295,7 +295,7 @@ func setupGitSource(t *testing.T, tmpdir string) source.Source {
 	assert.NoError(t, err)
 
 	cm, err := cache.NewManager(cache.ManagerOpt{
-		Snapshotter:   snapshotter,
+		Snapshotter:   snapshot.FromContainerdSnapshotter(snapshotter),
 		MetadataStore: md,
 	})
 	assert.NoError(t, err)

source/http/httpsource_test.go

@@ -315,7 +315,7 @@ func newHTTPSource(tmpdir string) (source.Source, error) {
 	}
 
 	cm, err := cache.NewManager(cache.ManagerOpt{
-		Snapshotter:   snapshotter,
+		Snapshotter:   snapshot.FromContainerdSnapshotter(snapshotter),
 		MetadataStore: md,
 	})
 	if err != nil {