Skip to content

Builder: print relative path if COPY/ADD source path was not found#41327

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:improve_build_errors
Aug 18, 2020
Merged

Builder: print relative path if COPY/ADD source path was not found#41327
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:improve_build_errors

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Aug 7, 2020

relates to #34986

Before this change, the error returned to the user would include the physical
path inside the tmp dir on the daemon host. These paths should be considered
an implementation detail, and provide no value to the user. Printing the tmp
path can confuse users, and will be even more confusing if the daemon is running
remotely (or in a VM, such as on Docker Desktop), in which case the path in the
error message does not exist on the local machine;

echo -e "FROM busybox\nCOPY /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- .

Sending build context to Docker daemon   1.57kB
Step 1/2 : FROM busybox
 ---> 1c35c4412082
Step 2/2 : COPY /some/non-existing/file.txt .
COPY failed: stat /var/lib/docker/tmp/docker-builder405687992/some/non-existing/file.txt: no such file or directory

When copying files from an image or a build stage, using --from, the error
is similarly confusing:

echo -e "FROM busybox\nCOPY --from=busybox /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- .
Sending build context to Docker daemon  4.671kB
Step 1/2 : FROM busybox
 ---> 018c9d7b792b
Step 2/2 : COPY --from=busybox /some/non-existing/file.txt .
COPY failed: stat /var/lib/docker/overlay2/ef34239c80526c779b7afaeaedbf11c1b201d7f7681d45613102c4541da0e156/merged/some/non-existing/file.txt: no such file or directory

This patch updates the error messages to be more user-friendly. Changes are slightly
different, depending on if the source was a local path, or an image (or build-stage),
using --from.

If --from is used, only the path is updated, and we print the relative path
instead of the full path;

echo -e "FROM busybox\nCOPY --from=busybox /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- .
Sending build context to Docker daemon  1.583kB
Step 1/2 : FROM busybox
 ---> 018c9d7b792b
Step 2/2 : COPY --from=busybox /some/non-existing/file.txt .
COPY failed: stat some/non-existing/file.txt: file does not exist

In other cases, additional information is added to mention "build context" and
".dockerignore", which could provide the user some hints to find the problem:

echo -e "FROM busybox\nCOPY /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- .
Sending build context to Docker daemon  1.583kB
Step 1/2 : FROM busybox
 ---> 018c9d7b792b
Step 2/2 : COPY /some/non-existing/file.txt .
COPY failed: file not found in build context or excluded by .dockerignore: stat some/non-existing/file.txt: file does not exist

echo -e "FROM busybox\nADD /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- .
Sending build context to Docker daemon  1.583kB
Step 1/2 : FROM busybox
 ---> 018c9d7b792b
Step 2/2 : ADD /some/non-existing/file.txt .
ADD failed: file not found in build context or excluded by .dockerignore: stat some/non-existing/file.txt: file does not exist

This patch only improves the error for the classic builder. Similar changes could
be made for BuildKit, which produces equally, or even more confusing errors;

echo -e "FROM busybox\nCOPY /some/non-existing/file.txt ." | DOCKER_BUILDKIT=1 docker build -f- .
[+] Building 1.2s (6/6) FINISHED
 => [internal] load build definition from Dockerfile                 0.0s
 => => transferring dockerfile: 85B                                  0.0s
 => [internal] load .dockerignore                                    0.0s
 => => transferring context: 2B                                      0.0s
 => [internal] load metadata for docker.io/library/busybox:latest    1.2s
 => [internal] load build context                                    0.0s
 => => transferring context: 2B                                      0.0s
 => CACHED [1/2] FROM docker.io/library/busybox@sha256:4f47c01...    0.0s
 => ERROR [2/2] COPY /some/non-existing/file.txt .                   0.0s
------
 > [2/2] COPY /some/non-existing/file.txt .:
------
failed to compute cache key: failed to walk /var/lib/docker/tmp/buildkit-mount181923793/some/non-existing:
lstat /var/lib/docker/tmp/buildkit-mount181923793/some/non-existing: no such file or directory

echo -e "FROM busybox\nCOPY --from=busybox /some/non-existing/file.txt ." | DOCKER_BUILDKIT=1 docker build -f- .
[+] Building 2.5s (6/6) FINISHED
 => [internal] load build definition from Dockerfile                        0.0s
 => => transferring dockerfile: 100B                                        0.0s
 => [internal] load .dockerignore                                           0.0s
 => => transferring context: 2B                                             0.0s
 => [internal] load metadata for docker.io/library/busybox:latest           1.2s
 => FROM docker.io/library/busybox:latest                                   1.2s
 => => resolve docker.io/library/busybox:latest                             1.2s
 => CACHED [stage-0 1/2] FROM docker.io/library/busybox@sha256:4f47c01...   0.0s
 => ERROR [stage-0 2/2] COPY --from=busybox /some/non-existing/file.txt .   0.0s
------
 > [stage-0 2/2] COPY --from=busybox /some/non-existing/file.txt .:
------
failed to compute cache key: failed to walk /var/lib/docker/overlay2/2a796d91e46fc038648c6010f062bdfd612ee62b0e8fe77bc632688e3fba32d9/merged/some/non-existing:
lstat /var/lib/docker/overlay2/2a796d91e46fc038648c6010f062bdfd612ee62b0e8fe77bc632688e3fba32d9/merged/some/non-existing: no such file or directory

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added area/builder Build status/2-code-review kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. labels Aug 7, 2020
@thaJeztah thaJeztah requested a review from tonistiigi as a code owner August 7, 2020 19:35
@thaJeztah
Copy link
Copy Markdown
Member Author

@tonistiigi PTAL

I know you've been looking at errors from buildkit (wondering if you have ideas how to improve this error in the buildkit case)

Also note that this only addresses the COPY/ADD situation for "file not found". There's possibly other code paths that print the full path, but this was the most notable.

@tonistiigi
Copy link
Copy Markdown
Member

We can definitely fix the buildkit side. You can copy these cases to that repo with examples and how you think the error should look like. I haven't cleaned up the "rpc" prefix as well although that is possible now.

@thaJeztah thaJeztah force-pushed the improve_build_errors branch 2 times, most recently from 458d31e to 350820e Compare August 12, 2020 11:43
@thaJeztah
Copy link
Copy Markdown
Member Author

@tonistiigi made some changes; PTAL

@thaJeztah
Copy link
Copy Markdown
Member Author

Arf; looks like I need to update a test;

=== RUN   TestDockerSuite/TestBuildAddFileNotFound
    --- FAIL: TestDockerSuite/TestBuildAddFileNotFound (0.16s)
        docker_cli_build_test.go:2201: assertion failed: 
            Command:  /usr/local/cli/docker build -t testbuildaddnotfound .
            ExitCode: 1
            Error:    exit status 1
            Stdout:   Sending build context to Docker daemon  3.072kB

            Step 1/2 : FROM scratch
             ---> 
            Step 2/2 : ADD foo /usr/local/bar
            
            Stderr:   ADD failed: file not found in build context or excluded by .dockerignore: stat foo: file does not exist
            
            
            Failures:
            Expected stderr to contain "foo: no such file or directory"

Before this change, the error returned to the user would include the physical
path inside the tmp dir on the daemon host. These paths should be considered
an implementation detail, and provide no value to the user. Printing the tmp
path can confuse users, and will be even more confusing if the daemon is running
remotely (or in a VM, such as on Docker Desktop), in which case the path in the
error message does not exist on the local machine;

    echo -e "FROM busybox\nCOPY /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- .

    Sending build context to Docker daemon   1.57kB
    Step 1/2 : FROM busybox
     ---> 1c35c4412082
    Step 2/2 : COPY /some/non-existing/file.txt .
    COPY failed: stat /var/lib/docker/tmp/docker-builder405687992/some/non-existing/file.txt: no such file or directory

When copying files from an image or a build stage, using `--from`, the error
is similarly confusing:

    echo -e "FROM busybox\nCOPY --from=busybox /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- .
    Sending build context to Docker daemon  4.671kB
    Step 1/2 : FROM busybox
     ---> 018c9d7b792b
    Step 2/2 : COPY --from=busybox /some/non-existing/file.txt .
    COPY failed: stat /var/lib/docker/overlay2/ef34239c80526c779b7afaeaedbf11c1b201d7f7681d45613102c4541da0e156/merged/some/non-existing/file.txt: no such file or directory

This patch updates the error messages to be more user-friendly. Changes are slightly
different, depending on if the source was a local path, or an image (or build-stage),
using `--from`.

If `--from` is used, only the path is updated, and we print the relative path
instead of the full path;

    echo -e "FROM busybox\nCOPY --from=busybox /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- .
    Sending build context to Docker daemon  1.583kB
    Step 1/2 : FROM busybox
     ---> 018c9d7b792b
    Step 2/2 : COPY --from=busybox /some/non-existing/file.txt .
    COPY failed: stat some/non-existing/file.txt: file does not exist

In other cases, additional information is added to mention "build context" and
".dockerignore", which could provide the user some hints to find the problem:

    echo -e "FROM busybox\nCOPY /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- .
    Sending build context to Docker daemon  1.583kB
    Step 1/2 : FROM busybox
     ---> 018c9d7b792b
    Step 2/2 : COPY /some/non-existing/file.txt .
    COPY failed: file not found in build context or excluded by .dockerignore: stat some/non-existing/file.txt: file does not exist

    echo -e "FROM busybox\nADD /some/non-existing/file.txt ." | DOCKER_BUILDKIT=0 docker build -f- .
    Sending build context to Docker daemon  1.583kB
    Step 1/2 : FROM busybox
     ---> 018c9d7b792b
    Step 2/2 : ADD /some/non-existing/file.txt .
    ADD failed: file not found in build context or excluded by .dockerignore: stat some/non-existing/file.txt: file does not exist

This patch only improves the error for the classic builder. Similar changes could
be made for BuildKit, which produces equally, or even more confusing errors;

    echo -e "FROM busybox\nCOPY /some/non-existing/file.txt ." | DOCKER_BUILDKIT=1 docker build -f- .
    [+] Building 1.2s (6/6) FINISHED
     => [internal] load build definition from Dockerfile                 0.0s
     => => transferring dockerfile: 85B                                  0.0s
     => [internal] load .dockerignore                                    0.0s
     => => transferring context: 2B                                      0.0s
     => [internal] load metadata for docker.io/library/busybox:latest    1.2s
     => [internal] load build context                                    0.0s
     => => transferring context: 2B                                      0.0s
     => CACHED [1/2] FROM docker.io/library/busybox@sha256:4f47c01...    0.0s
     => ERROR [2/2] COPY /some/non-existing/file.txt .                   0.0s
    ------
     > [2/2] COPY /some/non-existing/file.txt .:
    ------
    failed to compute cache key: failed to walk /var/lib/docker/tmp/buildkit-mount181923793/some/non-existing:
    lstat /var/lib/docker/tmp/buildkit-mount181923793/some/non-existing: no such file or directory

    echo -e "FROM busybox\nCOPY --from=busybox /some/non-existing/file.txt ." | DOCKER_BUILDKIT=1 docker build -f- .
    [+] Building 2.5s (6/6) FINISHED
     => [internal] load build definition from Dockerfile                        0.0s
     => => transferring dockerfile: 100B                                        0.0s
     => [internal] load .dockerignore                                           0.0s
     => => transferring context: 2B                                             0.0s
     => [internal] load metadata for docker.io/library/busybox:latest           1.2s
     => FROM docker.io/library/busybox:latest                                   1.2s
     => => resolve docker.io/library/busybox:latest                             1.2s
     => CACHED [stage-0 1/2] FROM docker.io/library/busybox@sha256:4f47c01...   0.0s
     => ERROR [stage-0 2/2] COPY --from=busybox /some/non-existing/file.txt .   0.0s
    ------
     > [stage-0 2/2] COPY --from=busybox /some/non-existing/file.txt .:
    ------
    failed to compute cache key: failed to walk /var/lib/docker/overlay2/2a796d91e46fc038648c6010f062bdfd612ee62b0e8fe77bc632688e3fba32d9/merged/some/non-existing:
    lstat /var/lib/docker/overlay2/2a796d91e46fc038648c6010f062bdfd612ee62b0e8fe77bc632688e3fba32d9/merged/some/non-existing: no such file or directory

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the improve_build_errors branch from 350820e to 010adee Compare August 17, 2020 10:03
@thaJeztah
Copy link
Copy Markdown
Member Author

Failure is on building the e2e image, and unrelated

@thaJeztah thaJeztah merged commit 0db8310 into moby:master Aug 18, 2020
@thaJeztah thaJeztah deleted the improve_build_errors branch August 18, 2020 07:50
@thaJeztah thaJeztah added this to the 20.03.0 milestone Aug 18, 2020
@thaJeztah
Copy link
Copy Markdown
Member Author

Opened moby/buildkit#1647 to track enhancing the BuildKit errors for this situation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/builder Build kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants