diff --git a/Dockerfile b/Dockerfile index 3e674530280a..abbb6bde57be 100644 --- a/Dockerfile +++ b/Dockerfile @@ -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 @@ -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 diff --git a/client/llb/definition.go b/client/llb/definition.go index 430ccca13402..3c9ee7320736 100644 --- a/client/llb/definition.go +++ b/client/llb/definition.go @@ -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 diff --git a/client/llb/diff.go b/client/llb/diff.go index 96c60dd62c7a..789e1e1c4d90 100644 --- a/client/llb/diff.go +++ b/client/llb/diff.go @@ -8,7 +8,7 @@ import ( ) type DiffOp struct { - MarshalCache + cache MarshalCache lower Output upper Output output Output @@ -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 { @@ -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 { diff --git a/client/llb/exec.go b/client/llb/exec.go index 1e0ca3ffab3b..a10fb94a26d6 100644 --- a/client/llb/exec.go +++ b/client/llb/exec.go @@ -51,7 +51,7 @@ type mount struct { } type ExecOp struct { - MarshalCache + cache MarshalCache proxyEnv *ProxyEnv root Output mounts []*mount @@ -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, @@ -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 } @@ -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 } @@ -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 { diff --git a/client/llb/fileop.go b/client/llb/fileop.go index eabbc0f15847..1fa793576982 100644 --- a/client/llb/fileop.go +++ b/client/llb/fileop.go @@ -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 } @@ -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 { diff --git a/client/llb/fileop_test.go b/client/llb/fileop_test.go index ac0acc264863..e5c4c7eb0660 100644 --- a/client/llb/fileop_test.go +++ b/client/llb/fileop_test.go @@ -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) { @@ -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()) +} diff --git a/client/llb/llbbuild/llbbuild.go b/client/llb/llbbuild/llbbuild.go index 6e9bd075de3d..8acd4fe10552 100644 --- a/client/llb/llbbuild/llbbuild.go +++ b/client/llb/llbbuild/llbbuild.go @@ -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 @@ -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 } @@ -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 { diff --git a/client/llb/marshal.go b/client/llb/marshal.go index e5fda5323684..d23a83b118c8 100644 --- a/client/llb/marshal.go +++ b/client/llb/marshal.go @@ -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 diff --git a/client/llb/merge.go b/client/llb/merge.go index 289c84c4f2ae..880dc1e68ca5 100644 --- a/client/llb/merge.go +++ b/client/llb/merge.go @@ -9,7 +9,7 @@ import ( ) type MergeOp struct { - MarshalCache + cache MarshalCache inputs []Output output Output constraints Constraints @@ -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 } @@ -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 { diff --git a/client/llb/source.go b/client/llb/source.go index cae3b2fcc5db..b08027e1df52 100644 --- a/client/llb/source.go +++ b/client/llb/source.go @@ -20,7 +20,7 @@ import ( ) type SourceOp struct { - MarshalCache + cache MarshalCache id string attrs map[string]string output Output @@ -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 } @@ -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 { diff --git a/frontend/dockerfile/cmd/dockerfile-frontend/Dockerfile b/frontend/dockerfile/cmd/dockerfile-frontend/Dockerfile index 768ecb9f7cf3..e60aede7ef31 100644 --- a/frontend/dockerfile/cmd/dockerfile-frontend/Dockerfile +++ b/frontend/dockerfile/cmd/dockerfile-frontend/Dockerfile @@ -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 diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index e0e49dd047b6..934c3a338e3d 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -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 } } @@ -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. diff --git a/frontend/dockerfile/dockerfile2llb/image.go b/frontend/dockerfile/dockerfile2llb/image.go index b6b589e77654..0f10542afc4f 100644 --- a/frontend/dockerfile/dockerfile2llb/image.go +++ b/frontend/dockerfile/dockerfile2llb/image.go @@ -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 } diff --git a/frontend/dockerfile/dockerfile_mount_test.go b/frontend/dockerfile/dockerfile_mount_test.go index 56e4031b2595..0423145eabd5 100644 --- a/frontend/dockerfile/dockerfile_mount_test.go +++ b/frontend/dockerfile/dockerfile_mount_test.go @@ -28,6 +28,7 @@ var mountTests = integration.TestFuncs( testMountTmpfsSize, testMountDuplicate, testCacheMountUser, + testCacheMountParallel, ) func init() { @@ -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) + } +} diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index e777690d578d..d9d70191e65c 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -127,6 +127,7 @@ var allTests = integration.TestFuncs( testEnvEmptyFormatting, testCacheMultiPlatformImportExport, testOnBuildCleared, + testOnBuildWithChildStage, testOnBuildInheritedStageRun, testOnBuildInheritedStageWithFrom, testOnBuildNewDeps, @@ -4986,6 +4987,96 @@ ONBUILD RUN mkdir \out && echo 11>> \out\foo require.Equal(t, integration.UnixOrWindows("11", "11\r\n"), string(dt)) } +// testOnBuildWithChildStage tests that ONBUILD rules from the parent image do +// not run again if another stage inherits from current stage. +// moby/buildkit#5578 +func testOnBuildWithChildStage(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows") + workers.CheckFeatureCompat(t, sb, workers.FeatureDirectPush) + f := getFrontend(t, sb) + + registry, err := sb.NewRegistry() + if errors.Is(err, integration.ErrRequirements) { + t.Skip(err.Error()) + } + require.NoError(t, err) + + dockerfile := []byte(` +FROM busybox +ONBUILD RUN mkdir -p /out && echo -n yes >> /out/didrun +`) + + dir := integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + + c, err := client.New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + target := registry + "/buildkit/testonbuildstage:base" + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + Exports: []client.ExportEntry{ + { + Type: client.ExporterImage, + Attrs: map[string]string{ + "push": "true", + "name": target, + }, + }, + }, + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + }, nil) + require.NoError(t, err) + + dockerfile = []byte(fmt.Sprintf(` +FROM %s AS base +RUN [ -f /out/didrun ] && touch /step1 +RUN rm /out/didrun +RUN [ ! -f /out/didrun ] && touch /step2 + +FROM base AS child +RUN [ ! -f /out/didrun ] && touch /step3 + +FROM scratch +COPY --from=child /step* / + `, target)) + + dir = integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + destDir := t.TempDir() + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + Exports: []client.ExportEntry{ + { + Type: client.ExporterLocal, + OutputDir: destDir, + }, + }, + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + }, nil) + require.NoError(t, err) + + _, err = os.Stat(filepath.Join(destDir, "step1")) + require.NoError(t, err) + + _, err = os.Stat(filepath.Join(destDir, "step2")) + require.NoError(t, err) + + _, err = os.Stat(filepath.Join(destDir, "step3")) + require.NoError(t, err) +} + func testOnBuildNamedContext(t *testing.T, sb integration.Sandbox) { integration.SkipOnPlatform(t, "windows") workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter, workers.FeatureOCILayout) @@ -7019,6 +7110,44 @@ COPY --from=base /env_foobar / dt, err = os.ReadFile(filepath.Join(destDir, "env_path")) require.NoError(t, err) require.Contains(t, string(dt), "/foobar:") + + // this case checks replacing stage that is based on another stage. + // moby/buildkit#5578-2539397486 + + dockerfile = []byte(` +FROM busybox AS parent +FROM parent AS base +RUN echo base > /out +FROM base +RUN [ -f /etc/alpine-release ] +RUN [ ! -f /out ] +`) + + dir = integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + + f = getFrontend(t, sb) + + destDir = t.TempDir() + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + FrontendAttrs: map[string]string{ + "context:base": "docker-image://" + target, + }, + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + Exports: []client.ExportEntry{ + { + Type: client.ExporterLocal, + OutputDir: destDir, + }, + }, + }, nil) + require.NoError(t, err) } func testNamedImageContextPlatform(t *testing.T, sb integration.Sandbox) { diff --git a/hack/dockerfiles/archutil.Dockerfile b/hack/dockerfiles/archutil.Dockerfile index 114e86d474c9..5f080a1fd550 100644 --- a/hack/dockerfiles/archutil.Dockerfile +++ b/hack/dockerfiles/archutil.Dockerfile @@ -120,6 +120,8 @@ RUN --mount=type=bind,target=.,rw \ if [ "$(ls -A /generated-files)" ]; then cp -rf /generated-files/* ./util/archutil fi + # loong64 is not stable atm + git checkout -- util/archutil/loong64_binary.go diff=$(git status --porcelain -- util/archutil) if [ -n "$diff" ]; then echo >&2 'ERROR: The result of archutil differs. Please update with "make archutil"' diff --git a/hack/dockerfiles/lint.Dockerfile b/hack/dockerfiles/lint.Dockerfile index e99f6be1207b..cd9c6100ff1a 100644 --- a/hack/dockerfiles/lint.Dockerfile +++ b/hack/dockerfiles/lint.Dockerfile @@ -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 ARG PROTOLINT_VERSION=0.50.5 ARG GOLANGCI_LINT_VERSION=1.61.0 ARG GOPLS_VERSION=v0.26.0