snapshot: clean up snapshot interface

Signed-off-by: Tonis Tiigi <[email protected]>

cache/blobs/blobs.go

@@ -1,7 +1,6 @@
 package blobs
 
 import (
-	gocontext "context"
 	"time"
 
 	"github.com/containerd/containerd/content"
@@ -26,19 +25,10 @@ type DiffPair struct {
 	Blobsum digest.Digest
 }
 
-type blobmapper interface {
-	GetBlob(ctx gocontext.Context, key string) (digest.Digest, digest.Digest, error)
-	SetBlob(ctx gocontext.Context, key string, diffID, blob digest.Digest) error
-}
-
 func GetDiffPairs(ctx context.Context, contentStore content.Store, snapshotter snapshot.Snapshotter, differ diff.Differ, ref cache.ImmutableRef) ([]DiffPair, error) {
 	if ref == nil {
 		return nil, nil
 	}
-	blobmap, ok := snapshotter.(blobmapper)
-	if !ok {
-		return nil, errors.Errorf("image exporter requires snapshotter with blobs mapping support")
-	}
 
 	eg, ctx := errgroup.WithContext(ctx)
 	var diffPairs []DiffPair
@@ -57,7 +47,7 @@ func GetDiffPairs(ctx context.Context, contentStore content.Store, snapshotter s
 	}
 	eg.Go(func() error {
 		dp, err := g.Do(ctx, ref.ID(), func(ctx context.Context) (interface{}, error) {
-			diffID, blob, err := blobmap.GetBlob(ctx, ref.ID())
+			diffID, blob, err := snapshotter.GetBlob(ctx, ref.ID())
 			if err != nil {
 				return nil, err
 			}
@@ -100,7 +90,7 @@ func GetDiffPairs(ctx context.Context, contentStore content.Store, snapshotter s
 			if err != nil {
 				return nil, err
 			}
-			if err := blobmap.SetBlob(ctx, ref.ID(), diffIDDigest, descr.Digest); err != nil {
+			if err := snapshotter.SetBlob(ctx, ref.ID(), diffIDDigest, descr.Digest); err != nil {
 				return nil, err
 			}
 			return DiffPair{DiffID: diffIDDigest, Blobsum: descr.Digest}, nil

cache/cacheimport/export.go

@@ -2,7 +2,6 @@ package cacheimport
 
 import (
 	"bytes"
-	gocontext "context"
 	"encoding/json"
 	"time"
 
@@ -25,11 +24,6 @@ import (
 
 const mediaTypeConfig = "application/vnd.buildkit.cacheconfig.v0"
 
-type blobmapper interface {
-	GetBlob(ctx gocontext.Context, key string) (digest.Digest, digest.Digest, error)
-	SetBlob(ctx gocontext.Context, key string, diffID, blob digest.Digest) error
-}
-
 type CacheRecord struct {
 	CacheKey   digest.Digest
 	Reference  cache.ImmutableRef

cache/cacheimport/import.go

@@ -269,7 +269,6 @@ func (ii *importInfo) unpack(ctx context.Context, dpairs []blobs.DiffPair) (stri
 	var chain []digest.Digest
 	for _, layer := range layers {
 		labels := map[string]string{
-			"containerd.io/gc.root":      time.Now().UTC().Format(time.RFC3339Nano),
 			"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 {
@@ -291,7 +290,7 @@ func (ii *importInfo) fillBlobMapping(ctx context.Context, layers []rootfs.Layer
 	for _, l := range layers {
 		chain = append(chain, l.Diff.Digest)
 		chainID := identity.ChainID(chain)
-		if err := ii.opt.Snapshotter.(blobmapper).SetBlob(ctx, string(chainID), l.Diff.Digest, l.Blob.Digest); err != nil {
+		if err := ii.opt.Snapshotter.SetBlob(ctx, string(chainID), l.Diff.Digest, l.Blob.Digest); err != nil {
 			return err
 		}
 	}

cache/contenthash/checksum_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/containerd/containerd/snapshots"
 	"github.com/containerd/containerd/snapshots/naive"
 	"github.com/moby/buildkit/cache"
 	"github.com/moby/buildkit/cache/metadata"
@@ -383,7 +384,7 @@ func createRef(t *testing.T, cm cache.Manager, files []string) cache.ImmutableRe
 	return ref
 }
 
-func setupCacheManager(t *testing.T, tmpdir string, snapshotter snapshot.Snapshotter) cache.Manager {
+func setupCacheManager(t *testing.T, tmpdir string, snapshotter snapshots.Snapshotter) cache.Manager {
 	md, err := metadata.NewStore(filepath.Join(tmpdir, "metadata.db"))
 	require.NoError(t, err)
 

cache/manager.go

@@ -6,11 +6,10 @@ import (
 	"sync"
 	"time"
 
-	cdsnapshot "github.com/containerd/containerd/snapshots"
+	"github.com/containerd/containerd/snapshots"
 	"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"
@@ -23,7 +22,7 @@ var (
 )
 
 type ManagerOpt struct {
-	Snapshotter   snapshot.Snapshotter
+	Snapshotter   snapshots.Snapshotter
 	GCPolicy      GCPolicy
 	MetadataStore *metadata.Store
 }
@@ -175,7 +174,7 @@ func (cm *cacheManager) getRecord(ctx context.Context, id string, opts ...RefOpt
 
 	rec := &cacheRecord{
 		mu:      &sync.Mutex{},
-		mutable: info.Kind != cdsnapshot.KindCommitted,
+		mutable: info.Kind != snapshots.KindCommitted,
 		cm:      cm,
 		refs:    make(map[Mountable]struct{}),
 		parent:  parent,
@@ -218,10 +217,7 @@ func (cm *cacheManager) New(ctx context.Context, s ImmutableRef, opts ...RefOpti
 		parentID = parent.ID()
 	}
 
-	labels := map[string]string{
-		"containerd.io/gc.root": time.Now().UTC().Format(time.RFC3339Nano),
-	}
-	if _, err := cm.Snapshotter.Prepare(ctx, id, parentID, cdsnapshot.WithLabels(labels)); err != nil {
+	if _, err := cm.Snapshotter.Prepare(ctx, id, parentID); err != nil {
 		if parent != nil {
 			parent.Release(context.TODO())
 		}
@@ -294,12 +290,17 @@ func (cm *cacheManager) Prune(ctx context.Context, ch chan client.UsageInfo) err
 
 	for _, cr := range cm.records {
 		cr.mu.Lock()
+
 		// ignore duplicates that share data
 		if cr.equalImmutable != nil && len(cr.equalImmutable.refs) > 0 || cr.equalMutable != nil && len(cr.refs) == 0 {
 			cr.mu.Unlock()
 			continue
 		}
 
+		if cr.isDead() {
+			continue
+		}
+
 		if len(cr.refs) == 0 {
 			cr.dead = true
 			toDelete = append(toDelete, cr)

cache/manager_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"github.com/containerd/containerd/namespaces"
+	"github.com/containerd/containerd/snapshots"
 	"github.com/containerd/containerd/snapshots/naive"
 	"github.com/moby/buildkit/cache/metadata"
 	"github.com/moby/buildkit/client"
@@ -368,7 +369,7 @@ func TestLazyCommit(t *testing.T) {
 	require.Equal(t, errNotFound, errors.Cause(err))
 }
 
-func getCacheManager(t *testing.T, tmpdir string, snapshotter snapshot.Snapshotter) Manager {
+func getCacheManager(t *testing.T, tmpdir string, snapshotter snapshots.Snapshotter) Manager {
 	md, err := metadata.NewStore(filepath.Join(tmpdir, "metadata.db"))
 	require.NoError(t, err)
 

cache/refs.go

@@ -2,10 +2,8 @@ package cache
 
 import (
 	"sync"
-	"time"
 
 	"github.com/containerd/containerd/mount"
-	cdsnapshot "github.com/containerd/containerd/snapshots"
 	"github.com/moby/buildkit/cache/metadata"
 	"github.com/moby/buildkit/identity"
 	"github.com/moby/buildkit/util/flightcontrol"
@@ -150,10 +148,7 @@ func (cr *cacheRecord) Mount(ctx context.Context, readonly bool) ([]mount.Mount,
 	}
 	if cr.viewMount == nil { // TODO: handle this better
 		cr.view = identity.NewID()
-		labels := map[string]string{
-			"containerd.io/gc.root": time.Now().UTC().Format(time.RFC3339Nano),
-		}
-		m, err := cr.cm.Snapshotter.View(ctx, cr.view, cr.ID(), cdsnapshot.WithLabels(labels))
+		m, err := cr.cm.Snapshotter.View(ctx, cr.view, cr.ID())
 		if err != nil {
 			cr.view = ""
 			return nil, errors.Wrapf(err, "failed to mount %s", cr.ID())
@@ -243,10 +238,7 @@ func (cr *cacheRecord) finalize(ctx context.Context) error {
 	if mutable == nil {
 		return nil
 	}
-	labels := map[string]string{
-		"containerd.io/gc.root": time.Now().UTC().Format(time.RFC3339Nano),
-	}
-	err := cr.cm.Snapshotter.Commit(ctx, cr.ID(), mutable.ID(), cdsnapshot.WithLabels(labels))
+	err := cr.cm.Snapshotter.Commit(ctx, cr.ID(), mutable.ID())
 	if err != nil {
 		return errors.Wrapf(err, "failed to commit %s", mutable.ID())
 	}

snapshot/blobmapping/snapshotter.go

@@ -7,6 +7,7 @@ import (
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/snapshots"
 	"github.com/moby/buildkit/cache/metadata"
+	"github.com/moby/buildkit/snapshot"
 	digest "github.com/opencontainers/go-digest"
 	"github.com/sirupsen/logrus"
 )
@@ -36,13 +37,13 @@ type Snapshotter struct {
 	opt Opt
 }
 
-func NewSnapshotter(opt Opt) (*Snapshotter, error) {
+func NewSnapshotter(opt Opt) snapshot.Snapshotter {
 	s := &Snapshotter{
 		Snapshotter: opt.Snapshotter,
 		opt:         opt,
 	}
 
-	return s, nil
+	return s
 }
 
 // Remove also removes a reference to a blob. If it is a last reference then it deletes it the blob as well
@@ -64,7 +65,7 @@ func (s *Snapshotter) Remove(ctx context.Context, key string) error {
 
 	if len(blobs) == 1 && blobs[0].ID() == key { // last snapshot
 		if err := s.opt.Content.Delete(ctx, blob); err != nil {
-			logrus.Errorf("failed to delete blob %v", blob)
+			logrus.Errorf("failed to delete blob %v: %+v", blob, err)
 		}
 	}
 	return nil

snapshot/nogc/content.go → snapshot/containerd/content.go

@@ -1,4 +1,4 @@
-package nogc
+package containerd
 
 import (
 	"context"

snapshot/nogc/snapshotter.go → snapshot/containerd/snapshotter.go

@@ -1,15 +1,24 @@
-package nogc
+package containerd
 
 import (
 	"context"
+	"time"
 
+	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/namespaces"
 	ctdsnapshot "github.com/containerd/containerd/snapshots"
+	"github.com/moby/buildkit/cache/metadata"
+	"github.com/moby/buildkit/snapshot"
+	"github.com/moby/buildkit/snapshot/blobmapping"
 )
 
-func NewSnapshotter(snapshotter ctdsnapshot.Snapshotter, ns string, gc func(context.Context) error) ctdsnapshot.Snapshotter {
-	return &nsSnapshotter{ns, snapshotter, gc}
+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},
+		MetadataStore: mdstore,
+	})
 }
 
 type nsSnapshotter struct {
@@ -38,15 +47,15 @@ func (s *nsSnapshotter) Mounts(ctx context.Context, key string) ([]mount.Mount,
 }
 func (s *nsSnapshotter) Prepare(ctx context.Context, key, parent string, opts ...ctdsnapshot.Opt) ([]mount.Mount, error) {
 	ctx = namespaces.WithNamespace(ctx, s.ns)
-	return s.Snapshotter.Prepare(ctx, key, parent, opts...)
+	return s.Snapshotter.Prepare(ctx, key, parent, addRootLabel(opts...))
 }
 func (s *nsSnapshotter) View(ctx context.Context, key, parent string, opts ...ctdsnapshot.Opt) ([]mount.Mount, error) {
 	ctx = namespaces.WithNamespace(ctx, s.ns)
-	return s.Snapshotter.View(ctx, key, parent, opts...)
+	return s.Snapshotter.View(ctx, key, parent, addRootLabel(opts...))
 }
 func (s *nsSnapshotter) Commit(ctx context.Context, name, key string, opts ...ctdsnapshot.Opt) error {
 	ctx = namespaces.WithNamespace(ctx, s.ns)
-	return s.Snapshotter.Commit(ctx, name, key, opts...)
+	return s.Snapshotter.Commit(ctx, name, key, addRootLabel(opts...))
 }
 func (s *nsSnapshotter) Remove(ctx context.Context, key string) error {
 	ctx = namespaces.WithNamespace(ctx, s.ns)
@@ -64,3 +73,18 @@ func (s *nsSnapshotter) Walk(ctx context.Context, fn func(context.Context, ctdsn
 	ctx = namespaces.WithNamespace(ctx, s.ns)
 	return s.Snapshotter.Walk(ctx, fn)
 }
+
+func addRootLabel(opts ...ctdsnapshot.Opt) ctdsnapshot.Opt {
+	return func(info *ctdsnapshot.Info) error {
+		for _, opt := range opts {
+			if err := opt(info); err != nil {
+				return err
+			}
+		}
+		if info.Labels == nil {
+			info.Labels = map[string]string{}
+		}
+		info.Labels["containerd.io/gc.root"] = time.Now().UTC().Format(time.RFC3339Nano)
+		return nil
+	}
+}

snapshot/snapshotter.go

@@ -1,10 +1,19 @@
 package snapshot
 
 import (
+	"context"
+
 	"github.com/containerd/containerd/snapshots"
+	digest "github.com/opencontainers/go-digest"
 )
 
 // Snapshotter defines interface that any snapshot implementation should satisfy
 type Snapshotter interface {
 	snapshots.Snapshotter
+	Blobmapper
+}
+
+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
 }

source/containerimage/pull.go

@@ -20,6 +20,7 @@ import (
 	"github.com/moby/buildkit/cache"
 	"github.com/moby/buildkit/session"
 	"github.com/moby/buildkit/session/auth"
+	"github.com/moby/buildkit/snapshot"
 	"github.com/moby/buildkit/source"
 	"github.com/moby/buildkit/util/flightcontrol"
 	"github.com/moby/buildkit/util/imageutil"
@@ -36,17 +37,12 @@ import (
 
 type SourceOpt struct {
 	SessionManager *session.Manager
-	Snapshotter    snapshots.Snapshotter
+	Snapshotter    snapshot.Snapshotter
 	ContentStore   content.Store
 	Applier        diff.Differ
 	CacheAccessor  cache.Accessor
 }
 
-type blobmapper interface {
-	GetBlob(ctx gocontext.Context, key string) (digest.Digest, digest.Digest, error)
-	SetBlob(ctx gocontext.Context, key string, diffID, blob digest.Digest) error
-}
-
 type resolveRecord struct {
 	desc ocispec.Descriptor
 	ts   time.Time
@@ -62,10 +58,6 @@ func NewSource(opt SourceOpt) (source.Source, error) {
 		SourceOpt: opt,
 	}
 
-	if _, ok := opt.Snapshotter.(blobmapper); !ok {
-		return nil, errors.Errorf("imagesource requires snapshotter with blobs mapping support")
-	}
-
 	return is, nil
 }
 
@@ -280,7 +272,7 @@ func (is *imageSource) fillBlobMapping(ctx context.Context, layers []rootfs.Laye
 	for _, l := range layers {
 		chain = append(chain, l.Diff.Digest)
 		chainID := identity.ChainID(chain)
-		if err := is.SourceOpt.Snapshotter.(blobmapper).SetBlob(ctx, string(chainID), l.Diff.Digest, l.Blob.Digest); err != nil {
+		if err := is.SourceOpt.Snapshotter.SetBlob(ctx, string(chainID), l.Diff.Digest, l.Blob.Digest); err != nil {
 			return err
 		}
 	}