Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v0.18] cherry-picks for v0.18.2 #5601

Merged
merged 7 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# syntax=docker/dockerfile-upstream:master

ARG RUNC_VERSION=v1.2.2
ARG RUNC_VERSION=v1.2.3
ARG CONTAINERD_VERSION=v2.0.0
# CONTAINERD_ALT_VERSION_... defines fallback containerd version for integration tests
ARG CONTAINERD_ALT_VERSION_17=v1.7.23
Expand All @@ -20,7 +20,7 @@ ARG DELVE_VERSION=v1.23.1

ARG GO_VERSION=1.23
ARG ALPINE_VERSION=3.20
ARG XX_VERSION=1.5.0
ARG XX_VERSION=1.6.1
ARG BUILDKIT_DEBUG

# minio for s3 integration tests
Expand Down
1 change: 0 additions & 1 deletion client/llb/definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
// For example, after marshalling a LLB state and sending over the wire, the
// LLB state can be reconstructed from the definition.
type DefinitionOp struct {
MarshalCache
mu sync.Mutex
ops map[digest.Digest]*pb.Op
defs map[digest.Digest][]byte
Expand Down
9 changes: 6 additions & 3 deletions client/llb/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

type DiffOp struct {
MarshalCache
cache MarshalCache
lower Output
upper Output
output Output
Expand All @@ -31,7 +31,10 @@ func (m *DiffOp) Validate(ctx context.Context, constraints *Constraints) error {
}

func (m *DiffOp) Marshal(ctx context.Context, constraints *Constraints) (digest.Digest, []byte, *pb.OpMetadata, []*SourceLocation, error) {
if dgst, dt, md, srcs, err := m.Load(constraints); err == nil {
cache := m.cache.Acquire()
defer cache.Release()

if dgst, dt, md, srcs, err := cache.Load(constraints); err == nil {
return dgst, dt, md, srcs, nil
}
if err := m.Validate(ctx, constraints); err != nil {
Expand Down Expand Up @@ -72,7 +75,7 @@ func (m *DiffOp) Marshal(ctx context.Context, constraints *Constraints) (digest.
return "", nil, nil, nil, err
}

return m.Store(dt, md, m.constraints.SourceLocations, constraints)
return cache.Store(dt, md, m.constraints.SourceLocations, constraints)
}

func (m *DiffOp) Output() Output {
Expand Down
14 changes: 10 additions & 4 deletions client/llb/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type mount struct {
}

type ExecOp struct {
MarshalCache
cache MarshalCache
proxyEnv *ProxyEnv
root Output
mounts []*mount
Expand All @@ -63,6 +63,9 @@ type ExecOp struct {
}

func (e *ExecOp) AddMount(target string, source Output, opt ...MountOption) Output {
cache := e.cache.Acquire()
defer cache.Release()

m := &mount{
target: target,
source: source,
Expand All @@ -84,7 +87,7 @@ func (e *ExecOp) AddMount(target string, source Output, opt ...MountOption) Outp
}
m.output = o
}
e.Store(nil, nil, nil, nil)
cache.Store(nil, nil, nil, nil)
e.isValidated = false
return m.output
}
Expand Down Expand Up @@ -128,7 +131,10 @@ func (e *ExecOp) Validate(ctx context.Context, c *Constraints) error {
}

func (e *ExecOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, []byte, *pb.OpMetadata, []*SourceLocation, error) {
if dgst, dt, md, srcs, err := e.Load(c); err == nil {
cache := e.cache.Acquire()
defer cache.Release()

if dgst, dt, md, srcs, err := cache.Load(c); err == nil {
return dgst, dt, md, srcs, nil
}

Expand Down Expand Up @@ -446,7 +452,7 @@ func (e *ExecOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, []
if err != nil {
return "", nil, nil, nil, err
}
return e.Store(dt, md, e.constraints.SourceLocations, c)
return cache.Store(dt, md, e.constraints.SourceLocations, c)
}

func (e *ExecOp) Output() Output {
Expand Down
7 changes: 5 additions & 2 deletions client/llb/fileop.go
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,10 @@ func (ms *marshalState) add(fa *FileAction, c *Constraints) (*fileActionState, e
}

func (f *FileOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, []byte, *pb.OpMetadata, []*SourceLocation, error) {
if dgst, dt, md, srcs, err := f.Load(c); err == nil {
cache := f.Acquire()
defer cache.Release()

if dgst, dt, md, srcs, err := cache.Load(c); err == nil {
return dgst, dt, md, srcs, nil
}

Expand Down Expand Up @@ -816,7 +819,7 @@ func (f *FileOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, []
if err != nil {
return "", nil, nil, nil, err
}
return f.Store(dt, md, f.constraints.SourceLocations, c)
return cache.Store(dt, md, f.constraints.SourceLocations, c)
}

func normalizePath(parent, p string, keepSlash bool) string {
Expand Down
13 changes: 13 additions & 0 deletions client/llb/fileop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/moby/buildkit/solver/pb"
digest "github.com/opencontainers/go-digest"
"github.com/stretchr/testify/require"
"golang.org/x/sync/errgroup"
)

func TestFileMkdir(t *testing.T) {
Expand Down Expand Up @@ -737,3 +738,15 @@ func TestFileOpMarshalConsistency(t *testing.T) {
prevDef = def.Def
}
}

func TestParallelMarshal(t *testing.T) {
st := Scratch().File(Mkfile("/tmp", 0644, []byte("tmp 1")))
eg, ctx := errgroup.WithContext(context.Background())
for i := 0; i < 100; i++ {
eg.Go(func() error {
_, err := st.Marshal(ctx)
return err
})
}
require.NoError(t, eg.Wait())
}
9 changes: 6 additions & 3 deletions client/llb/llbbuild/llbbuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func NewBuildOp(source llb.Output, opt ...BuildOption) llb.Vertex {
}

type build struct {
llb.MarshalCache
cache llb.MarshalCache
source llb.Output
info *BuildInfo
constraints llb.Constraints
Expand All @@ -47,7 +47,10 @@ func (b *build) Validate(context.Context, *llb.Constraints) error {
}

func (b *build) Marshal(ctx context.Context, c *llb.Constraints) (digest.Digest, []byte, *pb.OpMetadata, []*llb.SourceLocation, error) {
if dgst, dt, md, srcs, err := b.Load(c); err == nil {
cache := b.cache.Acquire()
defer cache.Release()

if dgst, dt, md, srcs, err := cache.Load(c); err == nil {
return dgst, dt, md, srcs, nil
}

Expand Down Expand Up @@ -85,7 +88,7 @@ func (b *build) Marshal(ctx context.Context, c *llb.Constraints) (digest.Digest,
if err != nil {
return "", nil, nil, nil, err
}
return b.Store(dt, md, b.constraints.SourceLocations, c)
return cache.Store(dt, md, b.constraints.SourceLocations, c)
}

func (b *build) Output() llb.Output {
Expand Down
29 changes: 22 additions & 7 deletions client/llb/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,30 +117,45 @@ func MarshalConstraints(base, override *Constraints) (*pb.Op, *pb.OpMetadata) {
}

type MarshalCache struct {
cache sync.Map
mu sync.Mutex
cache map[*Constraints]*marshalCacheResult
}

func (mc *MarshalCache) Load(c *Constraints) (digest.Digest, []byte, *pb.OpMetadata, []*SourceLocation, error) {
v, ok := mc.cache.Load(c)
type MarshalCacheInstance struct {
*MarshalCache
}

func (mc *MarshalCache) Acquire() *MarshalCacheInstance {
mc.mu.Lock()
return &MarshalCacheInstance{mc}
}

func (mc *MarshalCacheInstance) Load(c *Constraints) (digest.Digest, []byte, *pb.OpMetadata, []*SourceLocation, error) {
res, ok := mc.cache[c]
if !ok {
return "", nil, nil, nil, cerrdefs.ErrNotFound
}

res := v.(*marshalCacheResult)
return res.digest, res.dt, res.md, res.srcs, nil
}

func (mc *MarshalCache) Store(dt []byte, md *pb.OpMetadata, srcs []*SourceLocation, c *Constraints) (digest.Digest, []byte, *pb.OpMetadata, []*SourceLocation, error) {
func (mc *MarshalCacheInstance) Store(dt []byte, md *pb.OpMetadata, srcs []*SourceLocation, c *Constraints) (digest.Digest, []byte, *pb.OpMetadata, []*SourceLocation, error) {
res := &marshalCacheResult{
digest: digest.FromBytes(dt),
dt: dt,
md: md,
srcs: srcs,
}
mc.cache.Store(c, res)
if mc.cache == nil {
mc.cache = make(map[*Constraints]*marshalCacheResult)
}
mc.cache[c] = res
return res.digest, res.dt, res.md, res.srcs, nil
}

func (mc *MarshalCacheInstance) Release() {
mc.mu.Unlock()
}

type marshalCacheResult struct {
digest digest.Digest
dt []byte
Expand Down
9 changes: 6 additions & 3 deletions client/llb/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

type MergeOp struct {
MarshalCache
cache MarshalCache
inputs []Output
output Output
constraints Constraints
Expand All @@ -32,7 +32,10 @@ func (m *MergeOp) Validate(ctx context.Context, constraints *Constraints) error
}

func (m *MergeOp) Marshal(ctx context.Context, constraints *Constraints) (digest.Digest, []byte, *pb.OpMetadata, []*SourceLocation, error) {
if dgst, dt, md, srcs, err := m.Load(constraints); err == nil {
cache := m.cache.Acquire()
defer cache.Release()

if dgst, dt, md, srcs, err := cache.Load(constraints); err == nil {
return dgst, dt, md, srcs, nil
}

Expand All @@ -59,7 +62,7 @@ func (m *MergeOp) Marshal(ctx context.Context, constraints *Constraints) (digest
return "", nil, nil, nil, err
}

return m.Store(dt, md, m.constraints.SourceLocations, constraints)
return cache.Store(dt, md, m.constraints.SourceLocations, constraints)
}

func (m *MergeOp) Output() Output {
Expand Down
9 changes: 6 additions & 3 deletions client/llb/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
)

type SourceOp struct {
MarshalCache
cache MarshalCache
id string
attrs map[string]string
output Output
Expand Down Expand Up @@ -49,7 +49,10 @@ func (s *SourceOp) Validate(ctx context.Context, c *Constraints) error {
}

func (s *SourceOp) Marshal(ctx context.Context, constraints *Constraints) (digest.Digest, []byte, *pb.OpMetadata, []*SourceLocation, error) {
if dgst, dt, md, srcs, err := s.Load(constraints); err == nil {
cache := s.cache.Acquire()
defer cache.Release()

if dgst, dt, md, srcs, err := cache.Load(constraints); err == nil {
return dgst, dt, md, srcs, nil
}

Expand Down Expand Up @@ -82,7 +85,7 @@ func (s *SourceOp) Marshal(ctx context.Context, constraints *Constraints) (diges
return "", nil, nil, nil, err
}

return s.Store(dt, md, s.constraints.SourceLocations, constraints)
return cache.Store(dt, md, s.constraints.SourceLocations, constraints)
}

func (s *SourceOp) Output() Output {
Expand Down
2 changes: 1 addition & 1 deletion frontend/dockerfile/cmd/dockerfile-frontend/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

ARG GO_VERSION=1.23
ARG ALPINE_VERSION=3.20
ARG XX_VERSION=1.5.0
ARG XX_VERSION=1.6.1

# xx is a helper for cross-compilation
FROM --platform=$BUILDPLATFORM tonistiigi/xx:${XX_VERSION} AS xx
Expand Down
3 changes: 3 additions & 0 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
}
}
allDispatchStates.addState(ds)
ds.base = nil // reset base set by addState
continue
}
}
Expand Down Expand Up @@ -1065,6 +1066,8 @@ func (ds *dispatchState) init() {
ds.state = ds.base.state
ds.platform = ds.base.platform
ds.image = clone(ds.base.image)
// onbuild triggers to not carry over from base stage
ds.image.Config.OnBuild = nil
ds.baseImg = cloneX(ds.base.baseImg)
// Utilize the same path index as our base image so we propagate
// the paths we use back to the base image.
Expand Down
1 change: 1 addition & 0 deletions frontend/dockerfile/dockerfile2llb/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ func clone(src dockerspec.DockerOCIImage) dockerspec.DockerOCIImage {
img.Config.Env = append([]string{}, src.Config.Env...)
img.Config.Cmd = append([]string{}, src.Config.Cmd...)
img.Config.Entrypoint = append([]string{}, src.Config.Entrypoint...)
img.Config.OnBuild = append([]string{}, src.Config.OnBuild...)
return img
}

Expand Down
41 changes: 41 additions & 0 deletions frontend/dockerfile/dockerfile_mount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var mountTests = integration.TestFuncs(
testMountTmpfsSize,
testMountDuplicate,
testCacheMountUser,
testCacheMountParallel,
)

func init() {
Expand Down Expand Up @@ -536,3 +537,43 @@ COPY --from=base /combined.txt /
test("foo\n")
test("updated\n")
}

// moby/buildkit#5566
func testCacheMountParallel(t *testing.T, sb integration.Sandbox) {
integration.SkipOnPlatform(t, "windows")
f := getFrontend(t, sb)

dockerfile := []byte(`
FROM alpine AS b1
RUN --mount=type=cache,target=/foo/bar --mount=type=cache,target=/foo/bar/baz echo 1

FROM alpine AS b2
RUN --mount=type=cache,target=/foo/bar --mount=type=cache,target=/foo/bar/baz echo 2

FROM scratch
COPY --from=b1 /etc/passwd p1
COPY --from=b2 /etc/passwd p2
`)

dir := integration.Tmpdir(
t,
fstest.CreateFile("Dockerfile", dockerfile, 0600),
)

c, err := client.New(sb.Context(), sb.Address())
require.NoError(t, err)
defer c.Close()

for i := 0; i < 20; i++ {
_, err = f.Solve(sb.Context(), c, client.SolveOpt{
FrontendAttrs: map[string]string{
"no-cache": "",
},
LocalMounts: map[string]fsutil.FS{
dockerui.DefaultLocalNameDockerfile: dir,
dockerui.DefaultLocalNameContext: dir,
},
}, nil)
require.NoError(t, err)
}
}
Loading
Loading