From 566e28c1746b3ff729749f20fbb3d4a25ea1ebca Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Mon, 16 Apr 2018 15:23:10 -0700 Subject: [PATCH] snapshot: update mounts to mountable interface Signed-off-by: Tonis Tiigi --- cache/blobs/blobs.go | 14 ++- cache/cacheimport/import.go | 9 +- cache/contenthash/checksum_test.go | 2 +- cache/manager.go | 5 +- cache/manager_test.go | 2 +- cache/refs.go | 23 ++++- executor/containerdexecutor/executor.go | 10 +- executor/oci/spec_unix.go | 24 ++++- executor/runcexecutor/executor.go | 8 +- snapshot/blobmapping/snapshotter.go | 12 +-- snapshot/containerd/snapshotter.go | 2 +- snapshot/localmounter.go | 37 +++++--- snapshot/localmounter_unix.go | 4 + snapshot/localmounter_windows.go | 4 + snapshot/snapshotter.go | 120 +++++++++++++++++++++++- source/containerimage/pull.go | 9 +- source/git/gitsource_test.go | 2 +- source/http/httpsource_test.go | 2 +- 18 files changed, 243 insertions(+), 46 deletions(-) diff --git a/cache/blobs/blobs.go b/cache/blobs/blobs.go index b026cbcb..393be2ed 100644 --- a/cache/blobs/blobs.go +++ b/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()), diff --git a/cache/cacheimport/import.go b/cache/cacheimport/import.go index b61719e1..d7c84f98 100644 --- a/cache/cacheimport/import.go +++ b/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) diff --git a/cache/contenthash/checksum_test.go b/cache/contenthash/checksum_test.go index a8598874..270258f4 100644 --- a/cache/contenthash/checksum_test.go +++ b/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) diff --git a/cache/manager.go b/cache/manager.go index a6e4cc6c..2e2dff57 100644 --- a/cache/manager.go +++ b/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()) } diff --git a/cache/manager_test.go b/cache/manager_test.go index 63d5582f..1b0458c1 100644 --- a/cache/manager_test.go +++ b/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)) diff --git a/cache/refs.go b/cache/refs.go index fbcbf6ca..2cf14c1e 100644 --- a/cache/refs.go +++ b/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 } diff --git a/executor/containerdexecutor/executor.go b/executor/containerdexecutor/executor.go index b43ca15e..ab7849c9 100644 --- a/executor/containerdexecutor/executor.go +++ b/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 diff --git a/executor/oci/spec_unix.go b/executor/oci/spec_unix.go index ba7aa79d..2382efdc 100644 --- a/executor/oci/spec_unix.go +++ b/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 { diff --git a/executor/runcexecutor/executor.go b/executor/runcexecutor/executor.go index 3e5b5421..52140987 100644 --- a/executor/runcexecutor/executor.go +++ b/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) diff --git a/snapshot/blobmapping/snapshotter.go b/snapshot/blobmapping/snapshotter.go index 646a1819..e91241b7 100644 --- a/snapshot/blobmapping/snapshotter.go +++ b/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, - opt: opt, + SnapshotterBase: opt.Snapshotter, + 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 } diff --git a/snapshot/containerd/snapshotter.go b/snapshot/containerd/snapshotter.go index e288ece4..ec8eb23e 100644 --- a/snapshot/containerd/snapshotter.go +++ b/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, }) } diff --git a/snapshot/localmounter.go b/snapshot/localmounter.go index 083139db..73224416 100644 --- a/snapshot/localmounter.go +++ b/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 -// mount binds without privileges -func LocalMounter(m []mount.Mount) Mounter { - return &localMounter{m: m} +// LocalMounter is a helper for mounting mountfactory to temporary path. In +// addition it can mount binds without privileges +func LocalMounter(mountable Mountable) Mounter { + 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 - m []mount.Mount - target string + mu sync.Mutex + mounts []mount.Mount + 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 } diff --git a/snapshot/localmounter_unix.go b/snapshot/localmounter_unix.go index 914cdae4..c44e435e 100644 --- a/snapshot/localmounter_unix.go +++ b/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 } diff --git a/snapshot/localmounter_windows.go b/snapshot/localmounter_windows.go index 47717593..4e1287b0 100644 --- a/snapshot/localmounter_windows.go +++ b/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 } diff --git a/snapshot/snapshotter.go b/snapshot/snapshotter.go index 987f721d..ad7fcaf2 100644 --- a/snapshot/snapshotter.go +++ b/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) +} diff --git a/source/containerimage/pull.go b/source/containerimage/pull.go index 73a87640..e8fc32fa 100644 --- a/source/containerimage/pull.go +++ b/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) diff --git a/source/git/gitsource_test.go b/source/git/gitsource_test.go index 0ee68df5..6f0b9fee 100644 --- a/source/git/gitsource_test.go +++ b/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) diff --git a/source/http/httpsource_test.go b/source/http/httpsource_test.go index 9df80628..eeb3ccb2 100644 --- a/source/http/httpsource_test.go +++ b/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 {