Browse code

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

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>

Sebastiaan van Stijn authored on 2020/08/08 04:31:31
Showing 2 changed files
... ...
@@ -242,6 +242,8 @@ func (o *copier) calcCopyInfo(origPath string, allowWildcards bool) ([]copyInfo,
242 242
 	// Deal with the single file case
243 243
 	copyInfo, err := copyInfoForFile(o.source, origPath)
244 244
 	switch {
245
+	case imageSource == nil && errors.Is(err, os.ErrNotExist):
246
+		return nil, errors.Wrapf(err, "file not found in build context or excluded by .dockerignore")
245 247
 	case err != nil:
246 248
 		return nil, err
247 249
 	case copyInfo.hash != "":
... ...
@@ -315,6 +317,10 @@ func (o *copier) copyWithWildcards(origPath string) ([]copyInfo, error) {
315 315
 func copyInfoForFile(source builder.Source, path string) (copyInfo, error) {
316 316
 	fi, err := remotecontext.StatAt(source, path)
317 317
 	if err != nil {
318
+		if errors.Is(err, os.ErrNotExist) {
319
+			// return the relative path in the error, which is more user-friendly than the full path to the tmp-dir
320
+			return copyInfo{}, errors.WithStack(&os.PathError{Op: "stat", Path: path, Err: os.ErrNotExist})
321
+		}
318 322
 		return copyInfo{}, err
319 323
 	}
320 324
 
... ...
@@ -2189,18 +2189,13 @@ func (s *DockerSuite) TestBuildEntrypointRunCleanup(c *testing.T) {
2189 2189
 
2190 2190
 func (s *DockerSuite) TestBuildAddFileNotFound(c *testing.T) {
2191 2191
 	name := "testbuildaddnotfound"
2192
-	expected := "foo: no such file or directory"
2193
-
2194
-	if testEnv.OSType == "windows" {
2195
-		expected = "foo: The system cannot find the file specified"
2196
-	}
2197 2192
 
2198 2193
 	buildImage(name, build.WithBuildContext(c,
2199 2194
 		build.WithFile("Dockerfile", `FROM `+minimalBaseImage()+`
2200 2195
         ADD foo /usr/local/bar`),
2201 2196
 		build.WithFile("bar", "hello"))).Assert(c, icmd.Expected{
2202 2197
 		ExitCode: 1,
2203
-		Err:      expected,
2198
+		Err:      "stat foo: file does not exist",
2204 2199
 	})
2205 2200
 }
2206 2201