Merge pull request #2422 from crazy-max/fix-tmpfs-size

use bytes as given size for tmpfs mount
moby · Oct 19, 2021 · 8778943 · 8778943
2 parents 41c0a18 + 94378a8
commit 8778943
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 8 deletions.
diff --git a/client/client_test.go b/client/client_test.go
@@ -537,7 +537,7 @@ func testShmSize(t *testing.T, sb integration.Sandbox) {
 	defer c.Close()
 
 	st := llb.Image("busybox:latest").Run(
-		llb.AddMount("/dev/shm", llb.Scratch(), llb.Tmpfs(llb.TmpfsSize(128*1024))),
+		llb.AddMount("/dev/shm", llb.Scratch(), llb.Tmpfs(llb.TmpfsSize(128*1024*1024))),
 		llb.Shlex(`sh -c 'mount | grep /dev/shm > /out/out'`),
 	)
 

diff --git a/client/llb/exec.go b/client/llb/exec.go
@@ -499,9 +499,9 @@ func (fn tmpfsOptionFunc) SetTmpfsOption(ti *TmpfsInfo) {
 	fn(ti)
 }
 
-func TmpfsSize(kb int64) TmpfsOption {
+func TmpfsSize(b int64) TmpfsOption {
 	return tmpfsOptionFunc(func(ti *TmpfsInfo) {
-		ti.Size = kb
+		ti.Size = b
 	})
 }
 

diff --git a/frontend/dockerfile/dockerfile_mount_test.go b/frontend/dockerfile/dockerfile_mount_test.go
@@ -25,6 +25,7 @@ var mountTests = []integration.Test{
 	testMountMetaArg,
 	testMountFromError,
 	testMountInvalid,
+	testMountTmpfsSize,
 }
 
 func init() {
@@ -452,3 +453,46 @@ RUN --mount=from=$ttt,type=cache,target=/tmp ls
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "'from' doesn't support variable expansion, define alias stage instead")
 }
+
+func testMountTmpfsSize(t *testing.T, sb integration.Sandbox) {
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+FROM busybox AS base
+RUN --mount=type=tmpfs,target=/dev/shm,size=128m mount | grep /dev/shm > /tmpfssize
+FROM scratch
+COPY --from=base /tmpfssize /
+`)
+
+	dir, err := tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	c, err := client.New(sb.Context(), 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(sb.Context(), 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)
+
+	dt, err := ioutil.ReadFile(filepath.Join(destDir, "tmpfssize"))
+	require.NoError(t, err)
+	require.Contains(t, string(dt), `size=131072k`)
+}
diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go
@@ -5291,7 +5291,7 @@ COPY --from=base /shmsize /
 
 	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
 		FrontendAttrs: map[string]string{
-			"shm-size": "131072",
+			"shm-size": "134217728",
 		},
 		LocalDirs: map[string]string{
 			builder.DefaultLocalNameDockerfile: dir,

diff --git a/frontend/dockerfile/instructions/commands_runmount.go b/frontend/dockerfile/instructions/commands_runmount.go
@@ -6,6 +6,7 @@ import (
 	"strconv"
 	"strings"
 
+	dockeropts "github.com/docker/docker/opts"
 	"github.com/moby/buildkit/util/suggest"
 	"github.com/pkg/errors"
 )
@@ -230,11 +231,11 @@ func parseMount(value string, expander SingleWordExpander) (*Mount, error) {
 			}
 		case "size":
 			if m.Type == "tmpfs" {
-				v, err := strconv.ParseInt(value, 10, 64)
-				if err != nil {
+				tmpfsSize := new(dockeropts.MemBytes)
+				if err := tmpfsSize.Set(value); err != nil {
 					return nil, errors.Errorf("invalid value for %s: %s", key, value)
 				}
-				m.SizeLimit = v
+				m.SizeLimit = tmpfsSize.Value()
 			} else {
 				return nil, errors.Errorf("unexpected key '%s' for mount type '%s'", key, m.Type)
 			}

diff --git a/solver/llbsolver/mounts/mount.go b/solver/llbsolver/mounts/mount.go
@@ -407,7 +407,7 @@ func (m *tmpfsMount) Mount() ([]mount.Mount, func() error, error) {
 	}
 	if m.opt != nil {
 		if m.opt.Size_ > 0 {
-			opt = append(opt, fmt.Sprintf("size=%dk", m.opt.Size_))
+			opt = append(opt, fmt.Sprintf("size=%d", m.opt.Size_))
 		}
 	}
 	return []mount.Mount{{