From fdde46f7ffa9015e1b359bba177588563a5d5a65 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 30 May 2017 14:17:04 -0700 Subject: [PATCH] cachemanager: check pulled snapshot Signed-off-by: Tonis Tiigi --- cache/manager.go | 29 +++++++++++++++++-- cache/refs.go | 29 ++++++++++++++++--- control/control_test.go | 52 +++++++++++++++++++++-------------- snapshot/snapshotter.go | 4 +++ source/containerimage/pull.go | 28 +++++++++++-------- 5 files changed, 104 insertions(+), 38 deletions(-) diff --git a/cache/manager.go b/cache/manager.go index fb2d78f6..55e1b5fd 100644 --- a/cache/manager.go +++ b/cache/manager.go @@ -7,6 +7,7 @@ import ( "sync" "github.com/boltdb/bolt" + cdsnapshot "github.com/containerd/containerd/snapshot" "github.com/pkg/errors" "github.com/tonistiigi/buildkit_poc/snapshot" ) @@ -101,10 +102,34 @@ func (cm *cacheManager) Close() error { func (cm *cacheManager) Get(id string) (ImmutableRef, error) { cm.mu.Lock() defer cm.mu.Unlock() + return cm.get(id) +} +func (cm *cacheManager) get(id string) (ImmutableRef, error) { rec, ok := cm.records[id] if !ok { - // TODO: lazy-load from Snapshotter - return nil, errors.Wrapf(errNotFound, "%s not found", id) + info, err := cm.Snapshotter.Stat(context.TODO(), id) + if err != nil { + return nil, err + } + if info.Kind != cdsnapshot.KindCommitted { + return nil, errors.Wrapf(errInvalid, "can't lazy load active %s", id) + } + + var parent ImmutableRef + if info.Parent != "" { + parent, err = cm.get(info.Parent) + if err != nil { + return nil, err + } + } + + rec = &cacheRecord{ + id: id, + cm: cm, + refs: make(map[*cacheRef]struct{}), + parent: parent, + } + cm.records[id] = rec // TODO: store to db } rec.mu.Lock() diff --git a/cache/refs.go b/cache/refs.go index fe591d29..4209baeb 100644 --- a/cache/refs.go +++ b/cache/refs.go @@ -35,10 +35,12 @@ type cacheRecord struct { mutable bool frozen bool // meta SnapMeta - refs map[*cacheRef]struct{} - id string - cm *cacheManager - parent ImmutableRef + refs map[*cacheRef]struct{} + id string + cm *cacheManager + parent ImmutableRef + view string + viewMount []mount.Mount } // hold manager lock before calling @@ -62,6 +64,17 @@ func (sr *cacheRef) Mount() ([]mount.Mount, error) { return nil, errors.Wrapf(err, "failed to mount %s", sr.id) } return m, nil + } else { + if sr.viewMount == nil { // TODO: handle this better + sr.view = generateID() + m, err := sr.cm.Snapshotter.View(context.TODO(), sr.view, sr.id) + if err != nil { + sr.view = "" + return nil, errors.Wrapf(err, "failed to mount %s", sr.id) + } + sr.viewMount = m + } + return sr.viewMount, nil } return nil, errors.New("snapshot mount not implemented") @@ -83,6 +96,14 @@ func (sr *cacheRef) release() error { return err } } + if sr.viewMount != nil { + if err := sr.cm.Snapshotter.Remove(context.TODO(), sr.view); err != nil { + return err + } + sr.view = "" + sr.viewMount = nil + } + delete(sr.refs, sr) sr.frozen = false diff --git a/control/control_test.go b/control/control_test.go index 5089a04c..523cccef 100644 --- a/control/control_test.go +++ b/control/control_test.go @@ -15,21 +15,23 @@ import ( "github.com/containerd/containerd/content" "github.com/containerd/containerd/mount" "github.com/containerd/containerd/rootfs" - "github.com/containerd/containerd/snapshot" + cdsnapshot "github.com/containerd/containerd/snapshot" "github.com/containerd/containerd/snapshot/naive" digest "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/tonistiigi/buildkit_poc/cache" + "github.com/tonistiigi/buildkit_poc/snapshot" "github.com/tonistiigi/buildkit_poc/source" "github.com/tonistiigi/buildkit_poc/source/containerimage" ) func TestControl(t *testing.T) { + // this should be an example or e2e test tmpdir, err := ioutil.TempDir("", "controltest") assert.NoError(t, err) - // defer os.RemoveAll(tmpdir) + defer os.RemoveAll(tmpdir) cd, err := localContainerd(tmpdir) assert.NoError(t, err) @@ -43,10 +45,10 @@ func TestControl(t *testing.T) { assert.NoError(t, err) is, err := containerimage.NewSource(containerimage.SourceOpt{ - Snapshotter: cd.Snapshotter, - ContentStore: cd.ContentStore, - Applier: cd.Applier, - Accessor: cm, + Snapshotter: cd.Snapshotter, + ContentStore: cd.ContentStore, + Applier: cd.Applier, + CacheAccessor: cm, }) assert.NoError(t, err) @@ -58,11 +60,33 @@ func TestControl(t *testing.T) { snap, err := sm.Pull(context.TODO(), img) assert.NoError(t, err) - _ = snap + mounts, err := snap.Mount() + assert.NoError(t, err) + + lm := snapshot.LocalMounter(mounts) + + target, err := lm.Mount() + assert.NoError(t, err) + + f, err := os.Open(target) + assert.NoError(t, err) + + names, err := f.Readdirnames(-1) + assert.NoError(t, err) + assert.True(t, len(names) > 10) + + err = f.Close() + assert.NoError(t, err) + + lm.Unmount() + assert.NoError(t, err) + + err = snap.Release() + assert.NoError(t, err) } type containerd struct { - Snapshotter snapshot.Snapshotter + Snapshotter cdsnapshot.Snapshotter ContentStore content.Store Applier rootfs.Applier } @@ -149,15 +173,3 @@ func (rc *readCounter) Read(p []byte) (n int, err error) { rc.c += int64(n) return } - -// req := &diffapi.ApplyRequest{ -// Diff: fromDescriptor(diff), -// Mounts: fromMounts(mounts), -// } -// resp, err := r.client.Apply(ctx, req) -// if err != nil { -// return ocispec.Descriptor{}, err -// } -// return toDescriptor(resp.Applied), nil - -// Apply(context.Context, ocispec.Descriptor, []mount.Mount) (ocispec.Descriptor, error) diff --git a/snapshot/snapshotter.go b/snapshot/snapshotter.go index 3cdc7b30..0fbf9de8 100644 --- a/snapshot/snapshotter.go +++ b/snapshot/snapshotter.go @@ -4,11 +4,15 @@ import ( "context" "github.com/containerd/containerd/mount" + "github.com/containerd/containerd/snapshot" ) // Snapshotter defines interface that any snapshot implementation should satisfy type Snapshotter interface { + Stat(ctx context.Context, key string) (snapshot.Info, error) Prepare(ctx context.Context, key, parent string) ([]mount.Mount, error) Mounts(ctx context.Context, key string) ([]mount.Mount, error) Commit(ctx context.Context, name, key string) error + View(ctx context.Context, key, parent string) ([]mount.Mount, error) + Remove(ctx context.Context, key string) error } diff --git a/source/containerimage/pull.go b/source/containerimage/pull.go index d1631c67..8a98a706 100644 --- a/source/containerimage/pull.go +++ b/source/containerimage/pull.go @@ -21,10 +21,10 @@ import ( // code can be used with any implementation type SourceOpt struct { - Snapshotter snapshot.Snapshotter - ContentStore content.Store - Applier rootfs.Applier - Accessor cache.Accessor + Snapshotter snapshot.Snapshotter + ContentStore content.Store + Applier rootfs.Applier + CacheAccessor cache.Accessor } type imageSource struct { @@ -64,6 +64,9 @@ func (is *imageSource) Pull(ctx context.Context, id source.Identifier) (cache.Im return nil, err } + // TODO: need a wrapper snapshot interface that combines content + // and snapshots as 1) buildkit shouldn't have a dependency on contentstore + // or 2) cachemanager should manage the contentstore handlers := []images.Handler{ remotes.FetchHandler(is.ContentStore, fetcher), images.ChildrenHandler(is.ContentStore), @@ -72,24 +75,25 @@ func (is *imageSource) Pull(ctx context.Context, id source.Identifier) (cache.Im return nil, err } - if err := is.unpack(ctx, desc); err != nil { + chainid, err := is.unpack(ctx, desc) + if err != nil { return nil, err } - return nil, nil + + return is.CacheAccessor.Get(chainid) } -func (is *imageSource) unpack(ctx context.Context, desc ocispec.Descriptor) error { +func (is *imageSource) unpack(ctx context.Context, desc ocispec.Descriptor) (string, error) { layers, err := getLayers(ctx, is.ContentStore, desc) if err != nil { - return err + return "", err } - chainid, err := rootfs.ApplyLayers(ctx, layers, is.Snapshotter, is.Applier) + chainID, err := rootfs.ApplyLayers(ctx, layers, is.Snapshotter, is.Applier) if err != nil { - return err + return "", err } - _ = chainid - return nil + return string(chainID), nil } func getLayers(ctx context.Context, provider content.Provider, desc ocispec.Descriptor) ([]rootfs.Layer, error) {