moby / buildkit

concurrent, cache-efficient, and Dockerfile-agnostic builder toolkit
https://github.com/moby/moby/issues/34227
Apache License 2.0
8.21k stars 1.16k forks source link

bug: buildkit does not consider ONBUILD COPY --from #816

Closed astorath closed 1 month ago

astorath commented 5 years ago

Hi, we make heavy use of ONBUILD option in our environment.

Our typical Dockerfile looks like this:

FROM my.company.com/ci/dotnet:v1-build as build
FROM my.company.com/ci/dotnet:v1-runtime

This works fine in legacy docker build, but turning on BUILDKIT option activates some optimizations, so docker tries to build these containers in parallel. The problem is containers are dependent:

  1. my.company.com/ci/dotnet:v1-build:

    FROM base:v1
    WORKDIR /src
    ONBUILD COPY . /src
    ONBUILD make clean all
  2. my.company.com/ci/dotnet:v1-runtime:

    FROM runtime:v1
    WORKDIR /app
    ONBUILD COPY --from=build /src/target /app

Running DOCKER_BUILDKIT=1 docker build . results in:

DOCKER_BUILDKIT=1 docker build .
[+] Building 0.8s (8/8) FINISHED
...
 => ERROR [stage-2 3/1] COPY --from=build /src/target/ /app 0.0s
------
 > [stage-2 3/1] COPY --from=build /src/target/ /app:
------
 not found: not found

My questions are:

  1. Can I turn off these optimizations to run builds sequentially?
  2. Is there a way to mark these stages as dependent explicitly?
tonistiigi commented 5 years ago

This is a very weird (and inventive) way to use COPY --from.

  • Can I turn off these optimizations to run builds sequentially?
  • Is there a way to mark these stages as dependent explicitly?

No, we don't want to add any special behavior for this. Either we consider this as bug and just fix the image resolution or we document that this is invalid usage (and make it error with a proper message).

A problem with implementing this is that we can't start to process all stages in parallel like we do now because we only know about the dependant images after we have pulled the config of the base. Still doable, just adds complexity. Once we have determined the correct dependencies of the stages it would build in regular concurrent manner.

@AkihiroSuda @tiborvass @ijc @thaJeztah get your votes in

AkihiroSuda commented 5 years ago

IIRC ONBUILD was deprecated and unrecommended?

thaJeztah commented 5 years ago

It's not deprecated, but I think the official images stopped creating onbuild variants, due to the behaviour being confusing.

So, IIUC, in buildkit the problem is that the stages are executed in parallel, thus (e.g.)

FROM foo AS ubuntu

FROM something:onbuild AS final

Where something:onbuild has

ONBUILD COPY --from=ubuntu /foo /foo

Could either result in the COPY copying from ubuntu:latest or from the first build-stage (depending on if it's been evaluated first)?

astorath commented 5 years ago

@tonistiigi

This is a very weird (and inventive) way to use COPY --from.

Well, this is not my invention, https://engineering.busbud.com/2017/05/21/going-further-docker-multi-stage-builds/ - this is one of the first google search results. So I don't think I'm alone here.

@thaJeztah

It's not deprecated, but I think the official images stopped creating unbuild variants, due to the behaviour being confusing.

Maybe this is confusing for official images, but for multistage internal images designed for that purpose - ONBUILD is a revelation...

Down the link above is a nice solution to the problem: if we could use something like:

ONBUILD FROM runtime:v1

we won't need this strange hack.

tonistiigi commented 5 years ago

Quote from https://engineering.busbud.com/2017/05/21/going-further-docker-multi-stage-builds/:

I’m not sure if it’s a bug, a feature, or a undefined behavior

As the author of the multi-stage PR I can confirm I was not clever enough to see it as a possible feature.

benvan commented 5 years ago

Just chiming in here - we do the same as @astorath at our company. It's been a godsend for enabling extremely efficient / modular builds and dockerfiles

For context: We discovered this issue after trying to use (new) build secrets (with buildkit)

DavG commented 4 years ago

Same problem here.

I cannot reference another stage, or even another image, with ONBUILD COPY --from :(

sirlatrom commented 4 years ago

Same problem here.

I cannot reference another stage, or even another image, with ONBUILD COPY --from :(

You can if you add a dummy COPY --from=<previous layer> /dummy /dummy (assuming `/dummy exists in source image). That will add an explicit dependency. It's a workaround, even if inconvenient.

DavG commented 4 years ago

I know this trick to force some intermediate stages to build in "weird cases", but I don't see how it is related to the problem here.

DavG commented 4 years ago

It even does not work with a external image.

Minimum example to reproduce :

template/Dockerfile

FROM php:7.4.2-fpm-buster
ONBUILD COPY --from=composer:1.9.2 /usr/bin/composer /usr/bin/composer

project/Dockerfile

FROM bug_template

build.sh

DOCKER_BUILDKIT=1 docker build ./template --tag bug_template
DOCKER_BUILDKIT=1 docker build ./project

@sirlatrom maybe i didn't get your trick ? do you see a solution here ?

sirlatrom commented 4 years ago

The "trick" is to add a step in the bug image that copies a known-to-exist file from the composer image. That could also be done by creating a prior stage (from the composer image) in the bug image that adds such a dummy file so you can control which it is.

DavG commented 4 years ago

Ok, let's try it :

template/Dockerfile

FROM php:7.4.2-fpm-buster
COPY --from=composer:1.9.2 /etc/passwd /etc/passwd_composer
ONBUILD COPY --from=composer:1.9.2 /usr/bin/composer /usr/bin/composer

Same error.

sirlatrom commented 4 years ago

I meant the new copy instruction should be placed in the last, most downstream image

DavG commented 4 years ago

Could you provide a working fix based on my minimal example ?

sirlatrom commented 4 years ago

Could you provide a working fix based on my minimal example ?

Note that the use of images instead of stage names in COPY --from is not documented, so you should use probably stage aliases as shown.

./template/Dockerfile:

FROM php:7.4.2-fpm-buster
ONBUILD COPY --from=template /usr/bin/composer /usr/bin/composer

./project/Dockerfile:

FROM composer:1.9.2 AS template
RUN ["touch", "/tmp/dummy"]

FROM bug_template
COPY --from=template /tmp/dummy /tmp/dummy
DOCKER_BUILDKIT=1 docker build ./template --tag bug_template
DOCKER_BUILDKIT=1 docker build ./project
tuananh170489 commented 4 years ago

Same error @astorath

EricHripko commented 4 years ago

Facing the same issue as what @astorath described - we have an ONBUILD build and ONBUILD runtime stages to streamline the Dockerfiles. Lack of this feature in buildkit is big roadblock to adoption for us πŸ™ˆ

EricHripko commented 4 years ago

Hey folks πŸ‘‹ Docker Desktop 2.4.0.0 enables BuildKit by default now, which will potentially break any Dockerfiles dependent on this functionality. I'm happy to work with maintainers to get this addressed if capacity is an issue here (will possibly need some pointers on where to start though). It looks like @tonistiigi's suggestion could be a way forward:

bruth commented 4 years ago

I ran in to this problem and stumbled upon this issue. What is needed to move this fix forward?

tonistiigi commented 4 years ago

Two approaches to implement this:

If you are hitting this I strongly also advise you to look if this is actually a correct solution for your problem. Pointing to a non-existant stage is against Dockerfile design where every Dockerfile should be complete and not just a snippet that only works inside another specific Dockerfile. Maybe what you need instead are imports to connect Dockerfiles.

tonistiigi commented 4 years ago

This is also a bit of a security issue. If an image that is used in a Dockerfile gets updated to include ONBUILD COPY --from= it could point to an image that user considers private and does not want to leak, without the user having any way to verify that this kind of referencing happens.

bruth commented 4 years ago

@tonistiigi Thanks.

If you are hitting this I strongly also advise you to look if this is actually a correct solution for your problem.

Yes I am still evaluating whether this is a necessary approach. However, like the OP, what led me to think it was a bug was that it worked without buildkit.

The use case I have is:

FROM my-image-build:latest as build
FROM my-image-package:latest as package

It fails even though (I thought) the ONBUILD COPY --from=build .. should resolve properly since it is identified here. But the way it is resolving, it seems like the --from=.. resolution is not considering this build stage. Whereas the non-buildkit Docker build works.

EricHripko commented 4 years ago

If you are hitting this I strongly also advise you to look if this is actually a correct solution for your problem

I think this solution is born out of the need to create Dockerfiles for a lot of similarly structured projects combined with an attempt to follow best practices. Let's say you have a setup where:

This is, of course, trivial to achieve with multi-stage and that's the recommended approach (keeping build and runtime environments separate). However, this doesn't really scale well, as you'd need to repeat this same Dockerfile for 10, 20 or even 50 projects (not uncommon for systems powered by microservices).

Before BuildKit, (arguably) the neatest way to achieve this was with multi-stage ONBUILD files. It enables developers to write extremely modular/reusable Dockerfiles, whilst keeping Docker workflows native (docker/docker-compose build just works).

Given BuildKit feature set, I agree that the solution no longer seems like a great fit. @tonistiigi is the vision to fill this gap with custom BuildKit frontends?

If the path forward is via BuildKit custom frontends, it'd be great to make dockerfile.Build more easily extensible. Creating a custom frontend is quite an undertaking today, as it requires a lot of specialist knowledge (LLB) and boiler plate code (not to mention additional duplication if the end goal is to use Dockerfile syntax). It'd be awesome if there was a simpler API for this to match the learning curve of multi-stage Dockerfiles. For example, what I can imagine doing is:

FROM jre COPY --from=build /src/build /bin

Any additional (project-specific) customisations specified in source

COPY ... RUN ...

- If there's an easy way to load files from build context, this is even more powerful; You'd now be able to have a generic frontend that can combine Java/Python/etc functionality in a single streamlined entrypoint:
```Dockerfile
# syntax = dockerfiles/microservice
# Any additional (project-specific) customisations can be specified via familiar Dockerfile syntax
COPY ... 
RUN ... 

What are you thoughts on this?

EricHripko commented 3 years ago

I've written up a blog post to document some alternatives to ONBUILD COPY --from. These vary in flexibility (and, as a result, complexity) but are all compatible with BuildKit πŸŽ‰ Would love feedback from folks here on whether this works for their use cases and if it could be improved πŸ™‚

boonware commented 2 years ago

Is there any update on a solution for this? I've just discovered this limitation while building out some reusable images for a large project. I have multi-stage images which declare instructions such as ONBUILD COPY --from=other-image /foo /bar and also use BuiltKit secrets mechanism in ONBUILD instructions: ONBUILD RUN --mount=type=secret,id=my-secret,uid=101 source /run/secrets/my-secret && install-dependencies. The instructions are not being run in downstream image builds, which is a serious limitation. If it worked it would allow me to reduce the amount of boiler plate and have a set of reusable layers for application images across a large project.

EDIT: As I wrote the above I realized that I only use BuildKit for the secrets mechanism. If I can find an alternative I may drop BuildKit due to this limitation.

softworm commented 2 years ago

Is there any update on a solution for this? I've just discovered this limitation while building out some reusable images for a large project. I have multi-stage images which declare instructions such as ONBUILD COPY --from=other-image /foo /bar and also use BuiltKit secrets mechanism in ONBUILD instructions: ONBUILD RUN --mount=type=secret,id=my-secret,uid=101 source /run/secrets/my-secret && install-dependencies. The instructions are not being run in downstream image builds, which is a serious limitation. If it worked it would allow me to reduce the amount of boiler plate and have a set of reusable layers for application images across a large project.

EDIT: As I wrote the above I realized that I only use BuildKit for the secrets mechanism. If I can find an alternative I may drop BuildKit due to this limitation.

Here is the switch https://github.com/companieshouse/ch.gov.uk/pull/192

Just set environment arg DOCKER_BUILDKIT=0 before docker build

boonware commented 2 years ago

@softworm This does not solve the problem. Turning off BuiltKit means you cannot use the secrets mechanism, which is the only decent approach for passing secrets into a build.

c-ameron commented 2 years ago

So this is a bug right? I have a build image

ONBUILD COPY Gemfile* /application/
ONBUILD RUN bundle install

and then a multi-stage build, however if I change the contents of the Gemfile, it won't pick it up

FROM build-image:foo as builder

FROM base-image

I can confirm that running a build with buildkit off, it works and will pick up the changes

c-ameron commented 2 years ago

It also looks like someone else is having this problem https://stackoverflow.com/questions/66952378/docker-multistage-onbuild-not-executing-first-stage-onbuild-commands

gautaz commented 2 years ago

Just for the sake of adding another usage example of this (now missing) feature: https://github.com/r2d2bzh/docker-build-nodejs

This project started internally 3 years ago. At this time it was designed this way after reading the article pointed by @astorath in a previous comment and it also closely relates to the situation described by @EricHripko.

Many of us are still not using buildkit. This means that, for now, we have to support both the pre-buildkit and post-buildkit environments. The only way to do that ATM is to provide two different build methods, one for non-buildkit users and another for buildkit users.

What is also clearly odd for someone using the docker compose plugin is that these errors start popping between versions 2.3.3 and 2.3.4 of the plugin. It took me at least an hour to relate these behaviors to this issue, this is OK but perhaps having a warning on this particular subject in the Dockerfile reference (or somewhere else) might be helpful to spot or avoid the issue more easily. I can help documenting this, provided the proper guidance.

tonistiigi commented 2 years ago

I've marked this with "help wanted" label but if you are planning to take this on please do verify the design here first(also look at https://github.com/moby/buildkit/issues/816#issuecomment-727524865).

gautaz commented 2 years ago

Thanks @tonistiigi for the input :thumbsup:.

Just looking back at your latest comment, it feels like a link is missing:

please do verify the design here first

Did you intend to point to some particular documentation on the "here first" or did you just intend to point to the buildkit documentation?

I have also read again your proposals to resolve the issue, the second option (support by the dockerfile frontend for the async llb graph generation) seems in fact sexier. Is there already some work started on this particular subject? I guess the help wanted tag means there is no such thing but just to be sure...

imsamurai commented 2 years ago

Hi, so there going to be some fix or you're just breaking backward compatibility by literally saying "you always do it wrong way, our way is better! Just do refactoring of all your dockerfiles and ci/cd"? )

chloe-zen commented 2 years ago

Hi, so there going to be some fix or you're just breaking backward compatibility by literally saying "you always do it wrong way, our way is better! Just do refactoring of all your dockerfiles and ci/cd"? )

This ^

(I would have simply left a "this" emoji reaction but there is none.)

vdboor commented 1 year ago

For anyone that loves to work on this issue, I've included steps to reproduce the issue in moby/moby#42845 and kept those as simple as possible.

filip-aipl commented 1 year ago

Any update on that? Would love to use it. Or maybe there is some workaround for that?

The 3 alternatives listed above are not the best options.

Nearoo commented 1 year ago

Ditto, when is this getting fixed?

ericqt commented 1 year ago

Seeing as how this issue is related to concurrent builds of images I saw that we can create a builder with a config file that sets the max-parallelism to 0. I followed this page to create a builder instance: https://docs.docker.com/engine/reference/commandline/buildx_create/ and supplied it with a parameter for config file using the --config parameter. My config file looks like this:

debug = true

[worker.oci]
  enabled = true
  rootless = false
  max-parallelism = 0

but it seems like it's not working since the build failure is still persisting.

Another thing I've noticed is if we take this simple example from here: https://github.com/moby/buildkit/issues/816#issuecomment-1117591353

and I changed the the order of the imports from:

FROM test/base as base-image
FROM test/runtime

RUN cat /hello && echo "final"

to

FROM test/runtime
FROM test/base as base-image

RUN cat /hello && echo "final"

The build now passes. Seems like the builds start from the bottom?


Also, this issue is also blocking my org as we rely on ONBUILD ARG to determine how to build our images when the downstream processes consume our base images.

chloe-zen commented 1 year ago

@ericqt how about max-parallelism = 1?

ericqt commented 1 year ago

@chloe-zen I tried that as well and no go :(

re-mscho commented 1 year ago

Any update? As we now see:

DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
            BuildKit is currently disabled; enable it by removing the DOCKER_BUILDKIT=0
            environment-variable.

This will kill all our builds.

laoshancun commented 11 months ago

any update? same issue on multi-stage ONBUILD copy.

drewhemm commented 11 months ago

I ran into this issue and decided to think about it as a dependency management issue rather than a bug. Consider this code:

Base image Dockerfile, tagged as base:latest:

FROM alpine:latest
WORKDIR /foo
ONBUILD COPY bar.txt . 
ONBUILD RUN cat bar.txt

Application Dockerfile:

# Stage 1 
FROM base:latest AS me

# Stage 2
FROM alpine:latest
COPY --from me /foo/bar.txt /hello/bar.txt
RUN cat /hello/bar.txt

This example will likely fail with buildkit since the COPY instruction in app build stage 2 might happen before the ONBUILD executed when stage 1 is built, resulting in a 'file not found' error. The solution is really quite simple and does not require a huge refactor:

Base image Dockerfile, built as base:latest:

FROM alpine:latest
WORKDIR /foo

Application Dockerfile:

# Stage 1 
FROM base:latest AS me
COPY bar.txt . 
RUN cat bar.txt

# Stage 2
FROM alpine:latest
COPY --from me bar.txt /hello/bar.txt
RUN cat /hello/bar.txt

The secret is to ensure that any instructions in application stage one that are depended on by stage two are defined in the application Dockerfile rather than the base image Dockerfile. Any ONBUILD instructions in the base image Dockerfile must not be dependencies of later build stages. This means we can still use ONBUILD COPY and ONBUILD RUN to do things like install/update packages, copy files etc, i.e. things that are needed at runtime .

This is obviously a very cut down example and is not very useful, but in the real-world project I am currently working on, the method of the solution was identical and means I am able to achieve the same original intent, with minor changes to code. Yes, it means the COPY and RUN instructions are no longer centralised in the base image repo and they now need to be replicated across n application repos, but for two lines of code per Dockerfile, I can live with it.

nightlark commented 7 months ago

I just encountered this bug when trying to set up a multi-stage build with an early "builder" stage that uses a FROM image with ONBUILD to create an installer, and then a final stage that uses a FROM image with ONBUILD instructions to copy the installer from the builder and do the steps needed to install it. This involves quite a bit of duplicated instructions repeated across Dockerfiles in 50+ repositories, unlike the previous commenter who only has 2 fairly short lines that get duplicated.

From the docs (https://docs.docker.com/reference/dockerfile/#onbuild), The trigger will be executed in the context of the downstream build, as if it had been inserted immediately after the FROM instruction in the downstream Dockerfile implies that ONBUILD COPY executed as part of the image used for the final stage FROM image, should have identical behavior to sticking a COPY instruction immediately after the final stage FROM image in my Dockerfile -- but as many have pointed out here, it does not.

tonistiigi commented 1 month ago

This has been implemented in https://github.com/moby/buildkit/pull/5357 . You can test with #syntax=tonistiigi/buildkit:pr5357 on top of the Dockerfile.