Merge pull request #331 from tiborvass/19.03-buildkit-fixes

[19.03 backport] Cherry-picking build fixes

builder/builder-next/adapters/snapshot/snapshot.go

@@ -255,24 +255,23 @@ func (s *snapshotter) Mounts(ctx context.Context, key string) (snapshot.Mountabl
 		var rwlayer layer.RWLayer
 		return &mountable{
 			idmap: s.opt.IdentityMapping,
-			acquire: func() ([]mount.Mount, error) {
+			acquire: func() ([]mount.Mount, func() error, error) {
 				rwlayer, err = s.opt.LayerStore.CreateRWLayer(id, l.ChainID(), nil)
 				if err != nil {
-					return nil, err
+					return nil, nil, err
 				}
 				rootfs, err := rwlayer.Mount("")
 				if err != nil {
-					return nil, err
+					return nil, nil, err
 				}
 				return []mount.Mount{{
-					Source:  rootfs.Path(),
-					Type:    "bind",
-					Options: []string{"rbind"},
-				}}, nil
-			},
-			release: func() error {
-				_, err := s.opt.LayerStore.ReleaseRWLayer(rwlayer)
-				return err
+						Source:  rootfs.Path(),
+						Type:    "bind",
+						Options: []string{"rbind"},
+					}}, func() error {
+						_, err := s.opt.LayerStore.ReleaseRWLayer(rwlayer)
+						return err
+					}, nil
 			},
 		}, nil
 	}
@@ -281,19 +280,18 @@ func (s *snapshotter) Mounts(ctx context.Context, key string) (snapshot.Mountabl
 
 	return &mountable{
 		idmap: s.opt.IdentityMapping,
-		acquire: func() ([]mount.Mount, error) {
+		acquire: func() ([]mount.Mount, func() error, error) {
 			rootfs, err := s.opt.GraphDriver.Get(id, "")
 			if err != nil {
-				return nil, err
+				return nil, nil, err
 			}
 			return []mount.Mount{{
-				Source:  rootfs.Path(),
-				Type:    "bind",
-				Options: []string{"rbind"},
-			}}, nil
-		},
-		release: func() error {
-			return s.opt.GraphDriver.Put(id)
+					Source:  rootfs.Path(),
+					Type:    "bind",
+					Options: []string{"rbind"},
+				}}, func() error {
+					return s.opt.GraphDriver.Put(id)
+				}, nil
 		},
 	}, nil
 }
@@ -440,32 +438,33 @@ func (s *snapshotter) Close() error {
 type mountable struct {
 	mu       sync.Mutex
 	mounts   []mount.Mount
-	acquire  func() ([]mount.Mount, error)
+	acquire  func() ([]mount.Mount, func() error, error)
 	release  func() error
 	refCount int
 	idmap    *idtools.IdentityMapping
 }
 
-func (m *mountable) Mount() ([]mount.Mount, error) {
+func (m *mountable) Mount() ([]mount.Mount, func() error, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
 	if m.mounts != nil {
 		m.refCount++
-		return m.mounts, nil
+		return m.mounts, m.releaseMount, nil
 	}
 
-	mounts, err := m.acquire()
+	mounts, release, err := m.acquire()
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	m.mounts = mounts
+	m.release = release
 	m.refCount = 1
 
-	return m.mounts, nil
+	return m.mounts, m.releaseMount, nil
 }
 
-func (m *mountable) Release() error {
+func (m *mountable) releaseMount() error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 

builder/builder-next/exporter/export.go

@@ -117,12 +117,12 @@ func (e *imageExporterInstance) Export(ctx context.Context, inp exporter.Source)
 		layersDone := oneOffProgress(ctx, "exporting layers")
 
 		if err := ref.Finalize(ctx, true); err != nil {
-			return nil, err
+			return nil, layersDone(err)
 		}
 
 		diffIDs, err := e.opt.Differ.EnsureLayer(ctx, ref.ID())
 		if err != nil {
-			return nil, err
+			return nil, layersDone(err)
 		}
 
 		diffs = make([]digest.Digest, len(diffIDs))

vendor.conf

@@ -27,7 +27,7 @@ github.com/imdario/mergo                            7c29201646fa3de8506f70121347
 golang.org/x/sync                                   e225da77a7e68af35c70ccbf71af2b83e6acac3c
 
 # buildkit
-github.com/moby/buildkit                            be0d75f074e7a4b0f5b5877c719213a3f5057e60 # v0.6.1
+github.com/moby/buildkit                            588c73e1e4f0f3d7d3738abaaa7cf8026064b33e
 github.com/tonistiigi/fsutil                        3bbb99cdbd76619ab717299830c60f6f2a533a6b
 github.com/grpc-ecosystem/grpc-opentracing          8e809c8a86450a29b90dcc9efbf062d0fe6d9746
 github.com/opentracing/opentracing-go               1361b9cd60be79c4c3a7fa9841b3c132e40066a7

vendor/github.com/moby/buildkit/cache/contenthash/checksum.go

@@ -792,7 +792,7 @@ func getFollowLinksWalk(root *iradix.Node, k []byte, follow bool, linksWalked *i
 		return k, v.(*CacheRecord), nil
 	}
 	if !follow || len(k) == 0 {
-		return nil, nil, nil
+		return k, nil, nil
 	}
 
 	dir, file := splitKey(k)
@@ -817,14 +817,13 @@ func getFollowLinksWalk(root *iradix.Node, k []byte, follow bool, linksWalked *i
 			}
 			return getFollowLinksWalk(root, append(convertPathToKey([]byte(link)), file...), follow, linksWalked)
 		}
-
-		k = append(k, file...)
-		v, ok = root.Get(k)
-		if ok {
-			return k, v.(*CacheRecord), nil
-		}
 	}
-	return nil, nil, nil
+	k = append(k, file...)
+	v, ok = root.Get(k)
+	if ok {
+		return k, v.(*CacheRecord), nil
+	}
+	return k, nil, nil
 }
 
 func prepareDigest(fp, p string, fi os.FileInfo) (digest.Digest, error) {

vendor/github.com/moby/buildkit/cache/refs.go

@@ -424,10 +424,10 @@ type readOnlyMounter struct {
 	snapshot.Mountable
 }
 
-func (m *readOnlyMounter) Mount() ([]mount.Mount, error) {
-	mounts, err := m.Mountable.Mount()
+func (m *readOnlyMounter) Mount() ([]mount.Mount, func() error, error) {
+	mounts, release, err := m.Mountable.Mount()
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	for i, m := range mounts {
 		if m.Type == "overlay" {
@@ -443,7 +443,7 @@ func (m *readOnlyMounter) Mount() ([]mount.Mount, error) {
 		opts = append(opts, "ro")
 		mounts[i].Options = opts
 	}
-	return mounts, nil
+	return mounts, release, nil
 }
 
 func readonlyOverlay(opt []string) []string {

vendor/github.com/moby/buildkit/executor/oci/spec_unix.go

@@ -136,12 +136,12 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou
 			releaseAll()
 			return nil, nil, errors.Wrapf(err, "failed to mount %s", m.Dest)
 		}
-		mounts, err := mountable.Mount()
+		mounts, release, err := mountable.Mount()
 		if err != nil {
 			releaseAll()
 			return nil, nil, errors.WithStack(err)
 		}
-		releasers = append(releasers, mountable.Release)
+		releasers = append(releasers, release)
 		for _, mount := range mounts {
 			mount, err = sm.subMount(mount, m.Selector)
 			if err != nil {

vendor/github.com/moby/buildkit/executor/runcexecutor/executor.go

@@ -4,11 +4,9 @@ import (
 	"context"
 	"encoding/json"
 	"io"
-	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
-	"strconv"
 	"strings"
 	"syscall"
 	"time"
@@ -25,7 +23,6 @@ import (
 	"github.com/moby/buildkit/solver/pb"
 	"github.com/moby/buildkit/util/network"
 	rootlessspecconv "github.com/moby/buildkit/util/rootless/specconv"
-	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -42,8 +39,9 @@ type Opt struct {
 	ProcessMode     oci.ProcessMode
 	IdentityMapping *idtools.IdentityMapping
 	// runc run --no-pivot (unrecommended)
-	NoPivot bool
-	DNS     *oci.DNSConfig
+	NoPivot     bool
+	DNS         *oci.DNSConfig
+	OOMScoreAdj *int
 }
 
 var defaultCommandCandidates = []string{"buildkit-runc", "runc"}
@@ -59,6 +57,7 @@ type runcExecutor struct {
 	idmap            *idtools.IdentityMapping
 	noPivot          bool
 	dns              *oci.DNSConfig
+	oomScoreAdj      *int
 }
 
 func New(opt Opt, networkProviders map[pb.NetMode]network.Provider) (executor.Executor, error) {
@@ -118,6 +117,7 @@ func New(opt Opt, networkProviders map[pb.NetMode]network.Provider) (executor.Ex
 		idmap:            opt.IdentityMapping,
 		noPivot:          opt.NoPivot,
 		dns:              opt.DNS,
+		oomScoreAdj:      opt.OOMScoreAdj,
 	}
 	return w, nil
 }
@@ -155,11 +155,13 @@ func (w *runcExecutor) Exec(ctx context.Context, meta executor.Meta, root cache.
 		return err
 	}
 
-	rootMount, err := mountable.Mount()
+	rootMount, release, err := mountable.Mount()
 	if err != nil {
 		return err
 	}
-	defer mountable.Release()
+	if release != nil {
+		defer release()
+	}
 
 	id := identity.NewID()
 	bundle := filepath.Join(w.root, id)
@@ -242,9 +244,7 @@ func (w *runcExecutor) Exec(ctx context.Context, meta executor.Meta, root cache.
 		}
 	}
 
-	if err := setOOMScoreAdj(spec); err != nil {
-		return err
-	}
+	spec.Process.OOMScoreAdj = w.oomScoreAdj
 	if w.rootless {
 		if err := rootlessspecconv.ToRootless(spec); err != nil {
 			return err
@@ -336,19 +336,3 @@ func (s *forwardIO) Stdout() io.ReadCloser {
 func (s *forwardIO) Stderr() io.ReadCloser {
 	return nil
 }
-
-// setOOMScoreAdj comes from https://github.com/genuinetools/img/blob/2fabe60b7dc4623aa392b515e013bbc69ad510ab/executor/runc/executor.go#L182-L192
-func setOOMScoreAdj(spec *specs.Spec) error {
-	// Set the oom_score_adj of our children containers to that of the current process.
-	b, err := ioutil.ReadFile("/proc/self/oom_score_adj")
-	if err != nil {
-		return errors.Wrap(err, "failed to read /proc/self/oom_score_adj")
-	}
-	s := strings.TrimSpace(string(b))
-	oom, err := strconv.Atoi(s)
-	if err != nil {
-		return errors.Wrapf(err, "failed to parse %s as int", s)
-	}
-	spec.Process.OOMScoreAdj = &oom
-	return nil
-}

vendor/github.com/moby/buildkit/snapshot/localmounter.go

@@ -31,6 +31,7 @@ type localMounter struct {
 	mounts    []mount.Mount
 	mountable Mountable
 	target    string
+	release   func() error
 }
 
 func (lm *localMounter) Mount() (string, error) {
@@ -38,11 +39,12 @@ func (lm *localMounter) Mount() (string, error) {
 	defer lm.mu.Unlock()
 
 	if lm.mounts == nil {
-		mounts, err := lm.mountable.Mount()
+		mounts, release, err := lm.mountable.Mount()
 		if err != nil {
 			return "", err
 		}
 		lm.mounts = mounts
+		lm.release = release
 	}
 
 	if len(lm.mounts) == 1 && (lm.mounts[0].Type == "bind" || lm.mounts[0].Type == "rbind") {

vendor/github.com/moby/buildkit/snapshot/localmounter_unix.go

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

vendor/github.com/moby/buildkit/snapshot/localmounter_windows.go

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

vendor/github.com/moby/buildkit/snapshot/snapshotter.go

@@ -2,7 +2,9 @@ package snapshot
 
 import (
 	"context"
+	"os"
 	"sync"
+	"sync/atomic"
 
 	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/snapshots"
@@ -12,8 +14,7 @@ import (
 
 type Mountable interface {
 	// ID() string
-	Mount() ([]mount.Mount, error)
-	Release() error
+	Mount() ([]mount.Mount, func() error, error)
 	IdentityMapping() *idtools.IdentityMapping
 }
 
@@ -63,7 +64,7 @@ func (s *fromContainerd) Mounts(ctx context.Context, key string) (Mountable, err
 	if err != nil {
 		return nil, err
 	}
-	return &staticMountable{mounts, s.idmap}, nil
+	return &staticMountable{mounts: mounts, idmap: s.idmap, id: key}, nil
 }
 func (s *fromContainerd) Prepare(ctx context.Context, key, parent string, opts ...snapshots.Opt) error {
 	_, err := s.Snapshotter.Prepare(ctx, key, parent, opts...)
@@ -74,23 +75,29 @@ func (s *fromContainerd) View(ctx context.Context, key, parent string, opts ...s
 	if err != nil {
 		return nil, err
 	}
-	return &staticMountable{mounts, s.idmap}, nil
+	return &staticMountable{mounts: mounts, idmap: s.idmap, id: key}, nil
 }
 func (s *fromContainerd) IdentityMapping() *idtools.IdentityMapping {
 	return s.idmap
 }
 
 type staticMountable struct {
+	count  int32
+	id     string
 	mounts []mount.Mount
 	idmap  *idtools.IdentityMapping
 }
 
-func (m *staticMountable) Mount() ([]mount.Mount, error) {
-	return m.mounts, nil
-}
-
-func (cm *staticMountable) Release() error {
-	return nil
+func (cm *staticMountable) Mount() ([]mount.Mount, func() error, error) {
+	atomic.AddInt32(&cm.count, 1)
+	return cm.mounts, func() error {
+		if atomic.AddInt32(&cm.count, -1) < 0 {
+			if v := os.Getenv("BUILDKIT_DEBUG_PANIC_ON_ERROR"); v == "1" {
+				panic("release of released mount " + cm.id)
+			}
+		}
+		return nil
+	}, nil
 }
 
 func (cm *staticMountable) IdentityMapping() *idtools.IdentityMapping {
@@ -122,12 +129,12 @@ func (cs *containerdSnapshotter) release() error {
 }
 
 func (cs *containerdSnapshotter) returnMounts(mf Mountable) ([]mount.Mount, error) {
-	mounts, err := mf.Mount()
+	mounts, release, err := mf.Mount()
 	if err != nil {
 		return nil, err
 	}
 	cs.mu.Lock()
-	cs.releasers = append(cs.releasers, mf.Release)
+	cs.releasers = append(cs.releasers, release)
 	cs.mu.Unlock()
 	return mounts, nil
 }

vendor/github.com/moby/buildkit/solver/llbsolver/file/refmanager.go

@@ -47,12 +47,12 @@ func (rm *RefManager) Commit(ctx context.Context, mount fileoptypes.Mount) (file
 	if !ok {
 		return nil, errors.Errorf("invalid mount type %T", mount)
 	}
-	if err := m.m.Release(); err != nil {
-		return nil, err
-	}
 	if m.mr == nil {
 		return nil, errors.Errorf("invalid mount without active ref for commit")
 	}
+	defer func() {
+		m.mr = nil
+	}()
 	return m.mr.Commit(ctx)
 }
 
@@ -62,7 +62,6 @@ type Mount struct {
 }
 
 func (m *Mount) Release(ctx context.Context) error {
-	m.m.Release()
 	if m.mr != nil {
 		return m.mr.Release(ctx)
 	}

vendor/github.com/moby/buildkit/solver/llbsolver/ops/exec.go

@@ -349,12 +349,11 @@ func (sm *sshMount) Mount(ctx context.Context, readonly bool) (snapshot.Mountabl
 }
 
 type sshMountInstance struct {
-	sm      *sshMount
-	cleanup func() error
-	idmap   *idtools.IdentityMapping
+	sm    *sshMount
+	idmap *idtools.IdentityMapping
 }
 
-func (sm *sshMountInstance) Mount() ([]mount.Mount, error) {
+func (sm *sshMountInstance) Mount() ([]mount.Mount, func() error, error) {
 	ctx, cancel := context.WithCancel(context.TODO())
 
 	uid := int(sm.sm.mount.SSHOpt.Uid)
@@ -366,7 +365,7 @@ func (sm *sshMountInstance) Mount() ([]mount.Mount, error) {
 			GID: gid,
 		})
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		uid = identity.UID
 		gid = identity.GID
@@ -380,9 +379,9 @@ func (sm *sshMountInstance) Mount() ([]mount.Mount, error) {
 	})
 	if err != nil {
 		cancel()
-		return nil, err
+		return nil, nil, err
 	}
-	sm.cleanup = func() error {
+	release := func() error {
 		var err error
 		if cleanup != nil {
 			err = cleanup()
@@ -395,16 +394,7 @@ func (sm *sshMountInstance) Mount() ([]mount.Mount, error) {
 		Type:    "bind",
 		Source:  sock,
 		Options: []string{"rbind"},
-	}}, nil
-}
-
-func (sm *sshMountInstance) Release() error {
-	if sm.cleanup != nil {
-		if err := sm.cleanup(); err != nil {
-			return err
-		}
-	}
-	return nil
+	}}, release, nil
 }
 
 func (sm *sshMountInstance) IdentityMapping() *idtools.IdentityMapping {
@@ -462,14 +452,18 @@ type secretMountInstance struct {
 	idmap *idtools.IdentityMapping
 }
 
-func (sm *secretMountInstance) Mount() ([]mount.Mount, error) {
+func (sm *secretMountInstance) Mount() ([]mount.Mount, func() error, error) {
 	dir, err := ioutil.TempDir("", "buildkit-secrets")
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to create temp dir")
+		return nil, nil, errors.Wrap(err, "failed to create temp dir")
+	}
+	cleanupDir := func() error {
+		return os.RemoveAll(dir)
 	}
 
 	if err := os.Chmod(dir, 0711); err != nil {
-		return nil, err
+		cleanupDir()
+		return nil, nil, err
 	}
 
 	tmpMount := mount.Mount{
@@ -483,15 +477,23 @@ func (sm *secretMountInstance) Mount() ([]mount.Mount, error) {
 	}
 
 	if err := mount.All([]mount.Mount{tmpMount}, dir); err != nil {
-		return nil, errors.Wrap(err, "unable to setup secret mount")
+		cleanupDir()
+		return nil, nil, errors.Wrap(err, "unable to setup secret mount")
 	}
 	sm.root = dir
 
+	cleanup := func() error {
+		if err := mount.Unmount(dir, 0); err != nil {
+			return err
+		}
+		return cleanupDir()
+	}
+
 	randID := identity.NewID()
 	fp := filepath.Join(dir, randID)
 	if err := ioutil.WriteFile(fp, sm.sm.data, 0600); err != nil {
-		sm.Release()
-		return nil, err
+		cleanup()
+		return nil, nil, err
 	}
 
 	uid := int(sm.sm.mount.SecretOpt.Uid)
@@ -503,35 +505,28 @@ func (sm *secretMountInstance) Mount() ([]mount.Mount, error) {
 			GID: gid,
 		})
 		if err != nil {
-			return nil, err
+			cleanup()
+			return nil, nil, err
 		}
 		uid = identity.UID
 		gid = identity.GID
 	}
 
 	if err := os.Chown(fp, uid, gid); err != nil {
-		return nil, err
+		cleanup()
+		return nil, nil, err
 	}
 
 	if err := os.Chmod(fp, os.FileMode(sm.sm.mount.SecretOpt.Mode&0777)); err != nil {
-		return nil, err
+		cleanup()
+		return nil, nil, err
 	}
 
 	return []mount.Mount{{
 		Type:    "bind",
 		Source:  fp,
 		Options: []string{"ro", "rbind"},
-	}}, nil
-}
-
-func (sm *secretMountInstance) Release() error {
-	if sm.root != "" {
-		if err := mount.Unmount(sm.root, 0); err != nil {
-			return err
-		}
-		return os.RemoveAll(sm.root)
-	}
-	return nil
+	}}, cleanup, nil
 }
 
 func (sm *secretMountInstance) IdentityMapping() *idtools.IdentityMapping {
@@ -767,7 +762,7 @@ type tmpfsMount struct {
 	idmap    *idtools.IdentityMapping
 }
 
-func (m *tmpfsMount) Mount() ([]mount.Mount, error) {
+func (m *tmpfsMount) Mount() ([]mount.Mount, func() error, error) {
 	opt := []string{"nosuid"}
 	if m.readonly {
 		opt = append(opt, "ro")
@@ -776,10 +771,7 @@ func (m *tmpfsMount) Mount() ([]mount.Mount, error) {
 		Type:    "tmpfs",
 		Source:  "tmpfs",
 		Options: opt,
-	}}, nil
-}
-func (m *tmpfsMount) Release() error {
-	return nil
+	}}, func() error { return nil }, nil
 }
 
 func (m *tmpfsMount) IdentityMapping() *idtools.IdentityMapping {