From 15ef4e2b8af2f5212b6dbbe99b8cd57410efc620 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 2 Apr 2019 19:19:10 -0700 Subject: [PATCH] dockerfile: allow content cache for rw mounts Signed-off-by: Tonis Tiigi --- client/llb/exec.go | 12 ++- .../dockerfile2llb/convert_runmount.go | 2 + frontend/dockerfile/dockerfile_mount_test.go | 81 +++++++++++++++++++ solver/llbsolver/ops/exec.go | 2 +- 4 files changed, 93 insertions(+), 4 deletions(-) diff --git a/client/llb/exec.go b/client/llb/exec.go index 29d71874a..e45a23309 100644 --- a/client/llb/exec.go +++ b/client/llb/exec.go @@ -53,7 +53,7 @@ type mount struct { cacheID string tmpfs bool cacheSharing CacheMountSharingMode - // hasOutput bool + noOutput bool } type ExecOp struct { @@ -80,6 +80,8 @@ func (e *ExecOp) AddMount(target string, source Output, opt ...MountOption) Outp m.output = source } else if m.tmpfs { m.output = &output{vertex: e, err: errors.Errorf("tmpfs mount for %s can't be used as a parent", target)} + } else if m.noOutput { + m.output = &output{vertex: e, err: errors.Errorf("mount marked no-output and %s can't be used as a parent", target)} } else { o := &output{vertex: e, getIndex: e.getMountIndexFn(m)} if p := e.constraints.Platform; p != nil { @@ -248,7 +250,7 @@ func (e *ExecOp) Marshal(c *Constraints) (digest.Digest, []byte, *pb.OpMetadata, } outputIndex := pb.OutputIndex(-1) - if !m.readonly && m.cacheID == "" && !m.tmpfs { + if !m.noOutput && !m.readonly && m.cacheID == "" && !m.tmpfs { outputIndex = pb.OutputIndex(outIndex) outIndex++ } @@ -344,7 +346,7 @@ func (e *ExecOp) getMountIndexFn(m *mount) func() (pb.OutputIndex, error) { i := 0 for _, m2 := range e.mounts { - if m2.readonly || m2.cacheID != "" { + if m2.noOutput || m2.readonly || m2.cacheID != "" { continue } if m == m2 { @@ -385,6 +387,10 @@ func SourcePath(src string) MountOption { } } +func ForceNoOutput(m *mount) { + m.noOutput = true +} + func AsPersistentCacheDir(id string, sharing CacheMountSharingMode) MountOption { return func(m *mount) { m.cacheID = id diff --git a/frontend/dockerfile/dockerfile2llb/convert_runmount.go b/frontend/dockerfile/dockerfile2llb/convert_runmount.go index 8214e188e..0246e992e 100644 --- a/frontend/dockerfile/dockerfile2llb/convert_runmount.go +++ b/frontend/dockerfile/dockerfile2llb/convert_runmount.go @@ -75,6 +75,8 @@ func dispatchRunMounts(d *dispatchState, c *instructions.RunCommand, sources []* } if mount.ReadOnly { mountOpts = append(mountOpts, llb.Readonly) + } else if mount.Type == instructions.MountTypeBind { + mountOpts = append(mountOpts, llb.ForceNoOutput) } if mount.Type == instructions.MountTypeCache { sharing := llb.CacheMountShared diff --git a/frontend/dockerfile/dockerfile_mount_test.go b/frontend/dockerfile/dockerfile_mount_test.go index 2dde2c0b1..dfc10b536 100644 --- a/frontend/dockerfile/dockerfile_mount_test.go +++ b/frontend/dockerfile/dockerfile_mount_test.go @@ -4,7 +4,9 @@ package dockerfile import ( "context" + "io/ioutil" "os" + "path/filepath" "testing" "github.com/containerd/continuity/fs/fstest" @@ -17,6 +19,7 @@ import ( var mountTests = []integration.Test{ testMountContext, testMountTmpfs, + testMountRWCache, } func init() { @@ -78,3 +81,81 @@ RUN [ ! -f /mytmp/foo ] }, nil) require.NoError(t, err) } + +func testMountRWCache(t *testing.T, sb integration.Sandbox) { + f := getFrontend(t, sb) + + dockerfile := []byte(` +from busybox AS build +copy cachebust / +run mkdir out && echo foo > out/foo + +from busybox as second +RUN --mount=from=build,src=out,target=/out,rw cat /dev/urandom | head -c 100 | sha256sum > /unique + +from scratch +COPY --from=second /unique /unique +`) + + dir, err := tmpdir( + fstest.CreateFile("Dockerfile", dockerfile, 0600), + fstest.CreateFile("cachebust", []byte("0"), 0600), + ) + require.NoError(t, err) + defer os.RemoveAll(dir) + + c, err := client.New(context.TODO(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + destDir, err := ioutil.TempDir("", "buildkit") + require.NoError(t, err) + defer os.RemoveAll(destDir) + + _, err = f.Solve(context.TODO(), c, client.SolveOpt{ + Exports: []client.ExportEntry{ + { + Type: client.ExporterLocal, + OutputDir: destDir, + }, + }, + LocalDirs: map[string]string{ + builder.DefaultLocalNameDockerfile: dir, + builder.DefaultLocalNameContext: dir, + }, + }, nil) + require.NoError(t, err) + + dt1, err := ioutil.ReadFile(filepath.Join(destDir, "unique")) + require.NoError(t, err) + + // repeat with changed file that should be still cached by content + dir, err = tmpdir( + fstest.CreateFile("Dockerfile", dockerfile, 0600), + fstest.CreateFile("cachebust", []byte("1"), 0600), + ) + require.NoError(t, err) + defer os.RemoveAll(dir) + + destDir, err = ioutil.TempDir("", "buildkit") + require.NoError(t, err) + defer os.RemoveAll(destDir) + + _, err = f.Solve(context.TODO(), c, client.SolveOpt{ + Exports: []client.ExportEntry{ + { + Type: client.ExporterLocal, + OutputDir: destDir, + }, + }, + LocalDirs: map[string]string{ + builder.DefaultLocalNameDockerfile: dir, + builder.DefaultLocalNameContext: dir, + }, + }, nil) + require.NoError(t, err) + + dt2, err := ioutil.ReadFile(filepath.Join(destDir, "unique")) + require.NoError(t, err) + require.Equal(t, dt1, dt2) +} diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go index 47da19005..065eb7c6a 100644 --- a/solver/llbsolver/ops/exec.go +++ b/solver/llbsolver/ops/exec.go @@ -209,7 +209,7 @@ func (e *execOp) getMountDeps() ([]dep, error) { deps[m.Input].Selectors = append(deps[m.Input].Selectors, sel) } - if !m.Readonly || m.Dest == pb.RootMount { // exclude read-only rootfs + if (!m.Readonly || m.Dest == pb.RootMount) && m.Output != -1 { // exclude read-only rootfs && read-write mounts deps[m.Input].NoContentBasedHash = true } }