buildkit icon indicating copy to clipboard operation
buildkit copied to clipboard

Add --parents opt-in flag for ADD/COPY (labs)

Open DYefimov opened this issue 2 years ago • 15 comments

partially fixes: #2917

.parents_test.sh:

#!/usr/bin/env sh
set -e

TMP_DIR=$(mktemp -dt parents_test.XXXXXXXX)

echo "Created \"$TMP_DIR\""
trap "echo \"Removing \\\"$TMP_DIR\\\"\"; rm -rf \"$TMP_DIR\"" EXIT

mkdir -p $TMP_DIR/src/dir1
mkdir -p $TMP_DIR/src/dir2/subdir2

touch $TMP_DIR/src/dir1/file1-1.py
touch $TMP_DIR/src/dir1/file1-2.go
touch $TMP_DIR/src/dir2/subdir2/file2-1.go
touch $TMP_DIR/src/dir2/subdir2/file2-2.js
touch $TMP_DIR/src/dir2/subdir2/file2-3.go

cat << DOCKERFILE > $TMP_DIR/dockerfile
# syntax=dyefimov/dockerfile:copy_parents-labs

FROM scratch

COPY ./src/*/*/*.go ./dst1/
COPY --parents ./src/*/*/*.go ./dst2/

CMD ["echo", "hello-world"]
DOCKERFILE

echo "======================================================="
SOCKET_DIR=$(realpath $(dirname $0))/.tmp
mkdir -p $SOCKET_DIR
sudo buildctl \
    --addr=unix://$SOCKET_DIR/buildkitd.socket \
    build \
        --frontend=dockerfile.v0 \
        --local context=$TMP_DIR \
        --local dockerfile=$TMP_DIR \
        --no-cache \
        --output=type=docker,name=parents_test | docker load

echo "======================================================="
echo "source:"
find $TMP_DIR -printf '%P\n' | grep -v '^[[:space:]]*$' | sort

echo "======================================================="
docker container rm -f parents_test > /dev/null 2>&1
docker container create --name=parents_test parents_test > /dev/null 2>&1
echo "COPY:"
docker container export parents_test | tar tf - | grep "dst1" | sed 's#^dst1/##' | grep -v '^[[:space:]]*$' | sort
echo "COPY --parents:"
docker container export parents_test | tar tf - | grep "dst2" | sed 's#^dst2/##' | grep -v '^[[:space:]]*$' | sort
docker container rm -f parents_test > /dev/null 2>&1

echo "======================================================="

.parents_test.sh output:

$ ./.parents_test.sh 
Created "/tmp/parents_test.KCR1qmQ3"
=======================================================
[+] Building 7.4s (8/8) FINISHED                                                                               
 => [internal] load build definition from Dockerfile                                                      0.0s
 => => transferring dockerfile: 196B                                                                      0.0s
 => [internal] load .dockerignore                                                                         0.0s
 => => transferring context: 2B                                                                           0.0s
 => resolve image config for docker.io/dyefimov/dockerfile:copy_parents-labs                              0.8s
 => CACHED docker-image://docker.io/dyefimov/dockerfile:copy_parents-labs@sha256:d610b759d921bc35515d468  0.0s
 => => resolve docker.io/dyefimov/dockerfile:copy_parents-labs@sha256:d610b759d921bc35515d468718f2609cd6  0.0s
 => [internal] load build context                                                                         0.0s
 => => transferring context: 185B                                                                         0.0s
 => [1/2] COPY ./src/*/*/*.go ./dst1/                                                                     2.6s
 => [2/2] COPY --parents ./src/*/*/*.go ./dst2/                                                           3.0s
 => exporting to oci image format                                                                         0.3s
 => => exporting layers                                                                                   0.3s
 => => exporting manifest sha256:24b4391d73d9094f6cd20457fdee6ac7fad2805988d095faabf6b9fe72ffacb6         0.0s
 => => exporting config sha256:5f6fa8bc19204a198bbb3b996f7fceb8a2674275b3182f7e804266ebc223c302           0.0s
 => => sending tarball                                                                                    0.0s
0fd58aaab3ea: Loading layer [==================================================>]     145B/145B
2fb6a3b6052f: Loading layer [==================================================>]     193B/193B
The image parents_test:latest already exists, renaming the old one with ID sha256:de30362eb4cd1c143121ff861bc844e83218abe87788889a4d707ef26209d363 to empty string
Loaded image: parents_test:latest
=======================================================
source:
dockerfile
src
src/dir1
src/dir1/file1-1.py
src/dir1/file1-2.go
src/dir2
src/dir2/subdir2
src/dir2/subdir2/file2-1.go
src/dir2/subdir2/file2-2.js
src/dir2/subdir2/file2-3.go
=======================================================
COPY:
file2-1.go
file2-3.go
COPY --parents:
src/
src/dir2/
src/dir2/subdir2/
src/dir2/subdir2/file2-1.go
src/dir2/subdir2/file2-3.go
=======================================================
Removing "/tmp/parents_test.KCR1qmQ3"

Signed-off-by: Dmitrii Efimov [email protected]

DYefimov avatar Aug 04 '22 14:08 DYefimov

fyi @aaronlehmann

tonistiigi avatar Aug 04 '22 18:08 tonistiigi

Excellent :tada: Thanks. Yep, I already has begun preparing the test suite.

DYefimov avatar Aug 06 '22 11:08 DYefimov

How would you like --parents to behave in remote case you mentioned? (or URL/git cases? Not sure about current git behavior yet though - git looks very complicated inside) Right now --parents is applied only in local/tar scenarios. Guess we should treat URL/git as a separate decoupled "transport (or src) layer" and apply --parents indepently afterwards?

DYefimov avatar Aug 08 '22 14:08 DYefimov

URLs shouldn't be the determining factor here I don't think - the complexity comes with tars/gits/etc.

For a tar, I'd expect that it behave exactly like a directory, so with --parents, the entire contents being unpacked into the destination directory. But actually, I think that's how without --parents works, so I think it should be safe to discard. Same applies for .git.

jedevc avatar Aug 08 '22 15:08 jedevc

Looks like this needs a rebase

thaJeztah avatar Aug 16 '22 09:08 thaJeztah

Looks like this needs a rebase

Just came back from vacation yesterday, rebase, some fixes and test suite coming shortly.

DYefimov avatar Aug 16 '22 18:08 DYefimov

I think you may have rebased with an outdated local state (I see some unrelated commits made their way in); let me know if you need help with rebasing 👍

thaJeztah avatar Aug 24 '22 14:08 thaJeztah

I think you may have rebased with an outdated local state (I see some unrelated commits made their way in); let me know if you need help with rebasing +1

Yep, thanks. The diff to HEAD still gives the correct change set. Guess that's not a big deal, if I'll squash all but relevant and force push with the next rebase?

DYefimov avatar Aug 24 '22 14:08 DYefimov

:wave: @DYefimov just wanted to follow up and check to see you weren't blocked on anything?

jedevc avatar Sep 07 '22 12:09 jedevc

@jedevc In fact I'm kind of do. Failing in getting my head around some weird mounting problem. Can't get TestParentsAddURL test compiling, and for that matter just any test using TestServer inside is failing with this strange mounting error. I tried without container-in-container scheme with the same outcome. Other tests I've added are working just fine. Maybe I'm using the test suite the wrong way? TEST_KEEP_CACHE=1 DOCKERFILE_RELEASES=labs TESTFLAGS="--run /TestParents*/worker=oci$/ -v" ./hack/test dockerfile

output
    dockerfile_parents_test.go:229: 
                Error Trace:    dockerfile_parents_test.go:229
                                                        run.go:85
                                                        run.go:184
                Error:          Received unexpected error:
                                lstat /tmp/bktest_buildkitd2939727001/tmp/buildkit-mount378098859/tmp/bktest_buildkitd2939727001/tmp/buildkit-mount378098859: no such file or directory
                                github.com/moby/buildkit/util/stack.Enable
                                        /src/util/stack/stack.go:77
                                github.com/moby/buildkit/util/grpcerrors.FromGRPC
                                        /src/util/grpcerrors/grpcerrors.go:196
                                github.com/moby/buildkit/util/grpcerrors.UnaryClientInterceptor
                                        /src/util/grpcerrors/intercept.go:41
                                google.golang.org/grpc.(*ClientConn).Invoke
                                        /src/vendor/google.golang.org/grpc/call.go:35
                                github.com/moby/buildkit/api/services/control.(*controlClient).Solve
                                        /src/api/services/control/control.pb.go:1535
                                github.com/moby/buildkit/client.(*Client).solve.func2
                                        /src/client/solve.go:231
                                golang.org/x/sync/errgroup.(*Group).Go.func1
                                        /src/vendor/golang.org/x/sync/errgroup/errgroup.go:57
                                runtime.goexit
                                        /usr/local/go/src/runtime/asm_amd64.s:1571
                                failed to solve
                                github.com/moby/buildkit/client.(*Client).solve.func2
                                        /src/client/solve.go:244
                                golang.org/x/sync/errgroup.(*Group).Go.func1
                                        /src/vendor/golang.org/x/sync/errgroup/errgroup.go:57
                                runtime.goexit
                                        /usr/local/go/src/runtime/asm_amd64.s:1571
                Test:           TestIntegration/TestParentsAddUrl/worker=oci/frontend=gateway
    sandbox.go:234: stdout: /usr/bin/buildkitd
    sandbox.go:234: stderr: /usr/bin/buildkitd
    sandbox.go:237: > startCmd 2022-09-07 14:38:38.14992294 +0000 UTC m=+7.105587618 [buildkitd --oci-worker=true --containerd-worker=false --oci-worker-gc=false --oci-worker-labels=org.mobyproject.buildkit.worker.sandbox=true --config=/tmp/bktest_config3250988747/buildkitd.toml --root /tmp/bktest_buildkitd2939727001 --addr unix:///tmp/bktest_buildkitd2939727001/buildkitd.sock --debug]
    sandbox.go:237: time="2022-09-07T14:38:40Z" level=info msg="auto snapshotter: using overlayfs"
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=info msg="found worker \"l7i4zpx8mh85wo1txmozkpz58\", labels=map[org.mobyproject.buildkit.worker.executor:oci org.mobyproject.buildkit.worker.hostname:705f791a78df org.mobyproject.buildkit.worker.network:cni org.mobyproject.buildkit.worker.oci.process-mode:sandbox org.mobyproject.buildkit.worker.sandbox:true org.mobyproject.buildkit.worker.snapshotter:overlayfs], platforms=[linux/amd64 linux/amd64/v2 linux/amd64/v3 linux/386]"
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=info msg="found 1 workers, default=\"l7i4zpx8mh85wo1txmozkpz58\""
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=warning msg="currently, only the default worker can be used."
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=info msg="running server on /tmp/bktest_buildkitd2939727001/buildkitd.sock"
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=debug msg="session started"
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=debug msg=resolving host="localhost:36511"
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=debug msg="do request" host="localhost:36511" request.header.accept="application/vnd.docker.distribution.manifest.v2+json, application/vnd.docker.distribution.manifest.list.v2+json, application/vnd.oci.image.manifest.v1+json, application/vnd.oci.image.index.v1+json, */*" request.header.user-agent=buildkit/v0.10-dev request.method=HEAD url="http://localhost:36511/v2/buildkit_test/6mwgccd4ycjb9fmef2jm7icr3/manifests/latest?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=debug msg="fetch response received" host="localhost:36511" response.header.content-length=507 response.header.content-type=application/vnd.oci.image.manifest.v1+json response.header.date="Wed, 07 Sep 2022 14:38:41 GMT" response.header.docker-content-digest="sha256:c5f8c4651185c37ebb4a54be11d545b9469bb9c60d6d6b40c1bfe200416f1fa2" response.header.docker-distribution-api-version=registry/2.0 response.header.etag="\"sha256:c5f8c4651185c37ebb4a54be11d545b9469bb9c60d6d6b40c1bfe200416f1fa2\"" response.status="200 OK" url="http://localhost:36511/v2/buildkit_test/6mwgccd4ycjb9fmef2jm7icr3/manifests/latest?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=debug msg=resolved desc.digest="sha256:c5f8c4651185c37ebb4a54be11d545b9469bb9c60d6d6b40c1bfe200416f1fa2" host="localhost:36511"
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=debug msg=fetch digest="sha256:c5f8c4651185c37ebb4a54be11d545b9469bb9c60d6d6b40c1bfe200416f1fa2" mediatype=application/vnd.oci.image.manifest.v1+json size=507
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=debug msg="do request" digest="sha256:c5f8c4651185c37ebb4a54be11d545b9469bb9c60d6d6b40c1bfe200416f1fa2" mediatype=application/vnd.oci.image.manifest.v1+json request.header.accept="application/vnd.oci.image.manifest.v1+json, */*" request.header.user-agent=buildkit/v0.10-dev request.method=GET size=507 url="http://localhost:36511/v2/buildkit_test/6mwgccd4ycjb9fmef2jm7icr3/manifests/sha256:c5f8c4651185c37ebb4a54be11d545b9469bb9c60d6d6b40c1bfe200416f1fa2?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=debug msg="fetch response received" digest="sha256:c5f8c4651185c37ebb4a54be11d545b9469bb9c60d6d6b40c1bfe200416f1fa2" mediatype=application/vnd.oci.image.manifest.v1+json response.header.content-length=507 response.header.content-type=application/vnd.oci.image.manifest.v1+json response.header.date="Wed, 07 Sep 2022 14:38:41 GMT" response.header.docker-content-digest="sha256:c5f8c4651185c37ebb4a54be11d545b9469bb9c60d6d6b40c1bfe200416f1fa2" response.header.docker-distribution-api-version=registry/2.0 response.header.etag="\"sha256:c5f8c4651185c37ebb4a54be11d545b9469bb9c60d6d6b40c1bfe200416f1fa2\"" response.status="200 OK" size=507 url="http://localhost:36511/v2/buildkit_test/6mwgccd4ycjb9fmef2jm7icr3/manifests/sha256:c5f8c4651185c37ebb4a54be11d545b9469bb9c60d6d6b40c1bfe200416f1fa2?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=debug msg=fetch digest="sha256:15346172750ff1cd7d1645f8f66b3f91399d569a4ad2ff718c0a6c18da904523" mediatype=application/vnd.oci.image.config.v1+json size=1843
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=debug msg="do request" digest="sha256:15346172750ff1cd7d1645f8f66b3f91399d569a4ad2ff718c0a6c18da904523" mediatype=application/vnd.oci.image.config.v1+json request.header.accept="application/vnd.oci.image.config.v1+json, */*" request.header.user-agent=buildkit/v0.10-dev request.method=GET size=1843 url="http://localhost:36511/v2/buildkit_test/6mwgccd4ycjb9fmef2jm7icr3/blobs/sha256:15346172750ff1cd7d1645f8f66b3f91399d569a4ad2ff718c0a6c18da904523?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=debug msg="fetch response received" digest="sha256:15346172750ff1cd7d1645f8f66b3f91399d569a4ad2ff718c0a6c18da904523" mediatype=application/vnd.oci.image.config.v1+json response.header.accept-ranges=bytes response.header.cache-control="max-age=31536000" response.header.content-length=1843 response.header.content-type=application/octet-stream response.header.date="Wed, 07 Sep 2022 14:38:41 GMT" response.header.docker-content-digest="sha256:15346172750ff1cd7d1645f8f66b3f91399d569a4ad2ff718c0a6c18da904523" response.header.docker-distribution-api-version=registry/2.0 response.header.etag="\"sha256:15346172750ff1cd7d1645f8f66b3f91399d569a4ad2ff718c0a6c18da904523\"" response.status="200 OK" size=1843 url="http://localhost:36511/v2/buildkit_test/6mwgccd4ycjb9fmef2jm7icr3/blobs/sha256:15346172750ff1cd7d1645f8f66b3f91399d569a4ad2ff718c0a6c18da904523?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=debug msg=fetch digest="sha256:c5f8c4651185c37ebb4a54be11d545b9469bb9c60d6d6b40c1bfe200416f1fa2" mediatype=application/vnd.oci.image.manifest.v1+json size=507
    sandbox.go:237: time="2022-09-07T14:38:41Z" level=debug msg=fetch digest="sha256:15346172750ff1cd7d1645f8f66b3f91399d569a4ad2ff718c0a6c18da904523" mediatype=application/vnd.oci.image.config.v1+json size=1843
    sandbox.go:237: time="2022-09-07T14:38:42Z" level=debug msg=fetch digest="sha256:419a7037d29b1f42df90debfd2b4902306076f23f7d831da04489f4cf858e4cc" mediatype=application/vnd.oci.image.layer.v1.tar+gzip size=10572963
    sandbox.go:237: time="2022-09-07T14:38:42Z" level=debug msg="do request" digest="sha256:419a7037d29b1f42df90debfd2b4902306076f23f7d831da04489f4cf858e4cc" mediatype=application/vnd.oci.image.layer.v1.tar+gzip request.header.accept="application/vnd.oci.image.layer.v1.tar+gzip, */*" request.header.user-agent=containerd/1.6.6+unknown request.method=GET size=10572963 url="http://localhost:36511/v2/buildkit_test/6mwgccd4ycjb9fmef2jm7icr3/blobs/sha256:419a7037d29b1f42df90debfd2b4902306076f23f7d831da04489f4cf858e4cc?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:42Z" level=debug msg="fetch response received" digest="sha256:419a7037d29b1f42df90debfd2b4902306076f23f7d831da04489f4cf858e4cc" mediatype=application/vnd.oci.image.layer.v1.tar+gzip response.header.accept-ranges=bytes response.header.cache-control="max-age=31536000" response.header.content-length=10572963 response.header.content-type=application/octet-stream response.header.date="Wed, 07 Sep 2022 14:38:42 GMT" response.header.docker-content-digest="sha256:419a7037d29b1f42df90debfd2b4902306076f23f7d831da04489f4cf858e4cc" response.header.docker-distribution-api-version=registry/2.0 response.header.etag="\"sha256:419a7037d29b1f42df90debfd2b4902306076f23f7d831da04489f4cf858e4cc\"" response.status="200 OK" size=10572963 url="http://localhost:36511/v2/buildkit_test/6mwgccd4ycjb9fmef2jm7icr3/blobs/sha256:419a7037d29b1f42df90debfd2b4902306076f23f7d831da04489f4cf858e4cc?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:42Z" level=debug msg="unpigz not found, falling back to go gzip" error="exec: \"unpigz\": executable file not found in $PATH"
    sandbox.go:237: time="2022-09-07T14:38:42Z" level=debug msg="diff applied" d=241.26513ms digest="sha256:419a7037d29b1f42df90debfd2b4902306076f23f7d831da04489f4cf858e4cc" media=application/vnd.oci.image.layer.v1.tar+gzip size=10572963
    sandbox.go:237: time="2022-09-07T14:38:42Z" level=debug msg="serving grpc connection"
    sandbox.go:237: time="2022-09-07T14:38:42Z" level=debug msg="> creating fv9sy8pt0khy44hi1f70mh532 [/bin/dockerfile-frontend]"
    sandbox.go:237: time="2022-09-07T14:38:43Z" level=debug msg="new ref for local: sz0fgqxdj0z7hremziol9tbte" span="[internal] load build definition from Dockerfile"
    sandbox.go:237: time="2022-09-07T14:38:43Z" level=debug msg="diffcopy took: 5.141658ms" span="[internal] load build definition from Dockerfile"
    sandbox.go:237: time="2022-09-07T14:38:43Z" level=debug msg="new ref for local: sfezqludgl413om5nz5es7i7n" span="[internal] load .dockerignore"
    sandbox.go:237: time="2022-09-07T14:38:43Z" level=debug msg="diffcopy took: 7.630458ms" span="[internal] load .dockerignore"
    sandbox.go:237: time="2022-09-07T14:38:43Z" level=debug msg="saved sz0fgqxdj0z7hremziol9tbte as dockerfile:dockerfile:" span="[internal] load build definition from Dockerfile"
    sandbox.go:237: time="2022-09-07T14:38:43Z" level=debug msg="saved sfezqludgl413om5nz5es7i7n as context:context-.dockerignore:" span="[internal] load .dockerignore"
    sandbox.go:237: time="2022-09-07T14:38:44Z" level=debug msg=resolving host="localhost:36511"
    sandbox.go:237: time="2022-09-07T14:38:44Z" level=debug msg="do request" host="localhost:36511" request.header.accept="application/vnd.docker.distribution.manifest.v2+json, application/vnd.docker.distribution.manifest.list.v2+json, application/vnd.oci.image.manifest.v1+json, application/vnd.oci.image.index.v1+json, */*" request.header.user-agent=buildkit/v0.10-dev request.method=HEAD url="http://localhost:36511/v2/library/busybox/manifests/latest?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:44Z" level=debug msg="fetch response received" host="localhost:36511" response.header.content-length=527 response.header.content-type=application/vnd.docker.distribution.manifest.v2+json response.header.date="Wed, 07 Sep 2022 14:38:44 GMT" response.header.docker-content-digest="sha256:98de1ad411c6d08e50f26f392f3bc6cd65f686469b7c22a85c7b5fb1b820c154" response.header.docker-distribution-api-version=registry/2.0 response.header.etag="\"sha256:98de1ad411c6d08e50f26f392f3bc6cd65f686469b7c22a85c7b5fb1b820c154\"" response.status="200 OK" url="http://localhost:36511/v2/library/busybox/manifests/latest?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:44Z" level=debug msg=resolved desc.digest="sha256:98de1ad411c6d08e50f26f392f3bc6cd65f686469b7c22a85c7b5fb1b820c154" host="localhost:36511"
    sandbox.go:237: time="2022-09-07T14:38:44Z" level=debug msg=fetch digest="sha256:98de1ad411c6d08e50f26f392f3bc6cd65f686469b7c22a85c7b5fb1b820c154" mediatype=application/vnd.docker.distribution.manifest.v2+json size=527
    sandbox.go:237: time="2022-09-07T14:38:44Z" level=debug msg="do request" digest="sha256:98de1ad411c6d08e50f26f392f3bc6cd65f686469b7c22a85c7b5fb1b820c154" mediatype=application/vnd.docker.distribution.manifest.v2+json request.header.accept="application/vnd.docker.distribution.manifest.v2+json, */*" request.header.user-agent=buildkit/v0.10-dev request.method=GET size=527 url="http://localhost:36511/v2/library/busybox/manifests/sha256:98de1ad411c6d08e50f26f392f3bc6cd65f686469b7c22a85c7b5fb1b820c154?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:44Z" level=debug msg="fetch response received" digest="sha256:98de1ad411c6d08e50f26f392f3bc6cd65f686469b7c22a85c7b5fb1b820c154" mediatype=application/vnd.docker.distribution.manifest.v2+json response.header.content-length=527 response.header.content-type=application/vnd.docker.distribution.manifest.v2+json response.header.date="Wed, 07 Sep 2022 14:38:44 GMT" response.header.docker-content-digest="sha256:98de1ad411c6d08e50f26f392f3bc6cd65f686469b7c22a85c7b5fb1b820c154" response.header.docker-distribution-api-version=registry/2.0 response.header.etag="\"sha256:98de1ad411c6d08e50f26f392f3bc6cd65f686469b7c22a85c7b5fb1b820c154\"" response.status="200 OK" size=527 url="http://localhost:36511/v2/library/busybox/manifests/sha256:98de1ad411c6d08e50f26f392f3bc6cd65f686469b7c22a85c7b5fb1b820c154?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:44Z" level=debug msg=fetch digest="sha256:7a80323521ccd4c2b4b423fa6e38e5cea156600f40cd855e464cc52a321a24dd" mediatype=application/vnd.docker.container.image.v1+json size=1457
    sandbox.go:237: time="2022-09-07T14:38:44Z" level=debug msg="do request" digest="sha256:7a80323521ccd4c2b4b423fa6e38e5cea156600f40cd855e464cc52a321a24dd" mediatype=application/vnd.docker.container.image.v1+json request.header.accept="application/vnd.docker.container.image.v1+json, */*" request.header.user-agent=buildkit/v0.10-dev request.method=GET size=1457 url="http://localhost:36511/v2/library/busybox/blobs/sha256:7a80323521ccd4c2b4b423fa6e38e5cea156600f40cd855e464cc52a321a24dd?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:44Z" level=debug msg="fetch response received" digest="sha256:7a80323521ccd4c2b4b423fa6e38e5cea156600f40cd855e464cc52a321a24dd" mediatype=application/vnd.docker.container.image.v1+json response.header.accept-ranges=bytes response.header.cache-control="max-age=31536000" response.header.content-length=1457 response.header.content-type=application/octet-stream response.header.date="Wed, 07 Sep 2022 14:38:44 GMT" response.header.docker-content-digest="sha256:7a80323521ccd4c2b4b423fa6e38e5cea156600f40cd855e464cc52a321a24dd" response.header.docker-distribution-api-version=registry/2.0 response.header.etag="\"sha256:7a80323521ccd4c2b4b423fa6e38e5cea156600f40cd855e464cc52a321a24dd\"" response.status="200 OK" size=1457 url="http://localhost:36511/v2/library/busybox/blobs/sha256:7a80323521ccd4c2b4b423fa6e38e5cea156600f40cd855e464cc52a321a24dd?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:44Z" level=debug msg=fetch digest="sha256:98de1ad411c6d08e50f26f392f3bc6cd65f686469b7c22a85c7b5fb1b820c154" mediatype=application/vnd.docker.distribution.manifest.v2+json size=527
    sandbox.go:237: time="2022-09-07T14:38:44Z" level=debug msg=fetch digest="sha256:7a80323521ccd4c2b4b423fa6e38e5cea156600f40cd855e464cc52a321a24dd" mediatype=application/vnd.docker.container.image.v1+json size=1457
    sandbox.go:237: time="2022-09-07T14:38:45Z" level=debug msg=fetch digest="sha256:50783e0dfb64b73019e973e7bce2c0d5a882301b781327ca153b876ad758dbd3" mediatype=application/vnd.docker.image.rootfs.diff.tar.gzip size=773262
    sandbox.go:237: time="2022-09-07T14:38:45Z" level=debug msg="do request" digest="sha256:50783e0dfb64b73019e973e7bce2c0d5a882301b781327ca153b876ad758dbd3" mediatype=application/vnd.docker.image.rootfs.diff.tar.gzip request.header.accept="application/vnd.docker.image.rootfs.diff.tar.gzip, */*" request.header.user-agent=containerd/1.6.6+unknown request.method=GET size=773262 url="http://localhost:36511/v2/library/busybox/blobs/sha256:50783e0dfb64b73019e973e7bce2c0d5a882301b781327ca153b876ad758dbd3?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:45Z" level=debug msg="fetch response received" digest="sha256:50783e0dfb64b73019e973e7bce2c0d5a882301b781327ca153b876ad758dbd3" mediatype=application/vnd.docker.image.rootfs.diff.tar.gzip response.header.accept-ranges=bytes response.header.cache-control="max-age=31536000" response.header.content-length=773262 response.header.content-type=application/octet-stream response.header.date="Wed, 07 Sep 2022 14:38:45 GMT" response.header.docker-content-digest="sha256:50783e0dfb64b73019e973e7bce2c0d5a882301b781327ca153b876ad758dbd3" response.header.docker-distribution-api-version=registry/2.0 response.header.etag="\"sha256:50783e0dfb64b73019e973e7bce2c0d5a882301b781327ca153b876ad758dbd3\"" response.status="200 OK" size=773262 url="http://localhost:36511/v2/library/busybox/blobs/sha256:50783e0dfb64b73019e973e7bce2c0d5a882301b781327ca153b876ad758dbd3?ns=docker.io"
    sandbox.go:237: time="2022-09-07T14:38:45Z" level=debug msg="diff applied" d=47.494367ms digest="sha256:50783e0dfb64b73019e973e7bce2c0d5a882301b781327ca153b876ad758dbd3" media=application/vnd.docker.image.rootfs.diff.tar.gzip size=773262
    sandbox.go:237: time="2022-09-07T14:38:45Z" level=error msg="/moby.buildkit.v1.Control/Solve returned error: rpc error: code = Unknown desc = lstat /tmp/bktest_buildkitd2939727001/tmp/buildkit-mount378098859/tmp/bktest_buildkitd2939727001/tmp/buildkit-mount378098859: no such file or directory"
    sandbox.go:237: lstat /tmp/bktest_buildkitd2939727001/tmp/buildkit-mount378098859/tmp/bktest_buildkitd2939727001/tmp/buildkit-mount378098859: no such file or directory
    sandbox.go:237: 6419 v0.10.0-491-g81a2997b buildkitd --oci-worker=true --containerd-worker=false --oci-worker-gc=false --oci-worker-labels=org.mobyproject.buildkit.worker.sandbox=true --config=/tmp/bktest_config3250988747/buildkitd.toml --root /tmp/bktest_buildkitd2939727001 --addr unix:///tmp/bktest_buildkitd2939727001/buildkitd.sock --debug
    sandbox.go:237: github.com/moby/buildkit/solver.(*edge).execOp
    sandbox.go:237:     /src/solver/edge.go:904
    sandbox.go:237: github.com/moby/buildkit/solver/internal/pipe.NewWithFunction.func2
    sandbox.go:237:     /src/solver/internal/pipe/pipe.go:82
    sandbox.go:237: runtime.goexit
    sandbox.go:237:     /usr/local/go/src/runtime/asm_amd64.s:1571
    sandbox.go:237: 
    sandbox.go:237: 6419 v0.10.0-491-g81a2997b buildkitd --oci-worker=true --containerd-worker=false --oci-worker-gc=false --oci-worker-labels=org.mobyproject.buildkit.worker.sandbox=true --config=/tmp/bktest_config3250988747/buildkitd.toml --root /tmp/bktest_buildkitd2939727001 --addr unix:///tmp/bktest_buildkitd2939727001/buildkitd.sock --debug
    sandbox.go:237: main.unaryInterceptor.func1
    sandbox.go:237:     /src/cmd/buildkitd/main.go:570
    sandbox.go:237: github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1.1.1
    sandbox.go:237:     /src/vendor/github.com/grpc-ecosystem/go-grpc-middleware/chain.go:25
    sandbox.go:237: github.com/grpc-ecosystem/go-grpc-middleware.ChainUnaryServer.func1
    sandbox.go:237:     /src/vendor/github.com/grpc-ecosystem/go-grpc-middleware/chain.go:34
    sandbox.go:237: github.com/moby/buildkit/api/services/control._Control_Solve_Handler
    sandbox.go:237:     /src/api/services/control/control.pb.go:1718
    sandbox.go:237: google.golang.org/grpc.(*Server).processUnaryRPC
    sandbox.go:237:     /src/vendor/google.golang.org/grpc/server.go:1283
    sandbox.go:237: google.golang.org/grpc.(*Server).handleStream
    sandbox.go:237:     /src/vendor/google.golang.org/grpc/server.go:1620
    sandbox.go:237: google.golang.org/grpc.(*Server).serveStreams.func1.2
    sandbox.go:237:     /src/vendor/google.golang.org/grpc/server.go:922
    sandbox.go:237: runtime.goexit
    sandbox.go:237:     /usr/local/go/src/runtime/asm_amd64.s:1571
    sandbox.go:237: 
    sandbox.go:237: time="2022-09-07T14:38:45Z" level=debug msg="session finished: <nil>"
--- FAIL: TestIntegration (7.09s)
    --- FAIL: TestIntegration/TestParentsAddUrl/worker=oci/frontend=gateway (7.61s)
    --- PASS: TestIntegration/TestParentsAddArchive/worker=oci/frontend=gateway (9.14s)
    --- PASS: TestIntegration/TestParentsAddBasic/worker=oci/frontend=gateway (20.64s)
    --- PASS: TestIntegration/TestParentsCopyBasic/worker=oci/frontend=gateway (21.51s)
FAIL

DYefimov avatar Sep 07 '22 14:09 DYefimov

Looks like the test suite is running the right way, that's how I run individual tests. I also just tried a test using TestServer, TestHTTPDockerfile, which seems to run perfectly fine.

The issue looks like a file that the test attempts to access doesn't exist at that path - could you maybe push what you have, I'm happy to take a look.

jedevc avatar Sep 08 '22 08:09 jedevc

Thanks. Found it! Was a tough one, and totally on me. The only thing left to do is to make URL behave properly (apart from the mess I made with git) and I'll rebase and push, hopefully in a day or two, finally. Sorry for it taking so long :facepalm:

DYefimov avatar Sep 09 '22 10:09 DYefimov

If you're getting stuck on the "mess you made with git", and need help / instructions how to resolve, let us know; happy to help

thaJeztah avatar Sep 09 '22 11:09 thaJeztah

Gave a try to new git tools and it didn't go as expected... Funny enough, I'm using git oneflow ideology for the most part - it is "based on rebasing", and now we got here :) @thaJeztah, thanks, an advice/instructions would be much appreciated. So I made two mistakes: accidentally pulled on rebase instead of force pushing commits, and synced master to my feature branch. To my knowledge, I should rewrite the history now like this:

  • pull the master
  • switch to the feature branch
  • do an interactive rebase on top of the master (git rebase -i master)
  • drop all the commits slipped from the master (not squash right? as they were pushed already to the feature branch?)
  • force push the feature branch

Of cause, my local state has no merge op in it. And I should not drop duplicates of my own commits.

DYefimov avatar Sep 09 '22 13:09 DYefimov

I tend to not worry too much about the state of the git history while I'm working in "draft mode" - at least while the code's final shape is unclear, it's quite frustrating to try and have a neat history.

Once it's all done, I either:

  • git rebase -i master and squash down to a single commit if it's a simple enough change, amending it later to have a nice tidy message (this is what I'd do here, since it's a fairly self-contained change)
  • git reset master and then manually create new detailed commits with selectively staged lines if it's a more complex change.

The steps you've outlined look good to me, when rebasing you just to only pick the commits you've written as part of this PR, everything else will already be in master.

jedevc avatar Sep 12 '22 08:09 jedevc

Thanks, followed your instructions and squashed/amended it.

Apart from documentation, there are several issues left with ADD gitref case.

  1. Failed ADDing individual file from gitref, like so: ADD https://github.com/DYefimov/buildkit.git#master:frontend/dockerfile/dockerfile.go ./dst4/ Is it an intended behavior? It seems to be failing in here: https://github.com/moby/buildkit/blob/dabd4678ce074ff7c8bf922adfc0b3f76f9166b0/source/git/gitsource.go#L564

  2. Right now ADD gitref truncates SubDir by itself somewhere inside, and passes "/" path to the CopyOp. It looks inconsistent with other cases, and it felt wrong adding --parents to the gitsource protobuf. It might be beneficial not to truncate SubDir in git case, and patch it first, by leting CopyOp consistently decide whether to flatten or not the directory structure. Which way would you prefer it to work? And it feels wrong too to mix --parents + "gitref consistency fix" in one PR, as it will somewhat ruin the revert-with-one-swipe.

DYefimov avatar Sep 26 '22 11:09 DYefimov

Have left a couple comments, when you're happy with the state of this, I think it's good to come out of draft :tada:

  1. Yup, ADDing individual files isn't supported, as you've noted. It's only for sub-directories, see here. Maybe we should also support individual files at some point? I can't find an issue for it, but it definitely falls outside of the scope of this PR.

  2. Ugh, sorry I hadn't realized that using a sub-directory for GitSource unpacks that sub-directory to the root of the filesystem. Good call, we should definitely make that change later, I'd be happy for now with an explicit error message of the form "--parents flag is not currently support for git source".

    To change that, we'd need to make a change to the gateway API, probably by allowing an additional option to the git source that allows specifying what location to unpack into, similar to how we have llb.Filename for llb.HTTP. Then if --parents is specified, we could set that option appropriately, falling back to the error message if buildkit doesn't have the appropriate caps set.

jedevc avatar Sep 29 '22 09:09 jedevc

@DYefimov all good? Just wanted to checkup to see if there's anything else or other clarification you need?

jedevc avatar Oct 10 '22 10:10 jedevc

Great! Integrated suggested changes, fixed some linting, and rebased just in case. Coming out of draft.

What about the documentation? Guess it should be placed right after this one: https://github.com/moby/buildkit/blob/master/frontend/dockerfile/docs/reference.md#copy---link Do you want me to write a skeleton for it?

Regarding this:

To change that, we'd need to make a change to the gateway API, probably by allowing an additional option to the git source that allows specifying what location to unpack into, similar to how we have llb.Filename for llb.HTTP. Then if --parents is specified, we could set that option appropriately, falling back to the error message if buildkit doesn't have the appropriate caps set.

Didn't exactly follow you on this one. Is it really that complicated of a change? Please, check the placeholder I left here: frontend/dockerfile/dockerfile2llb/convert.go From what I understood, that should be a minor non-breaking change - just teach GitSource not to truncate parent directories, and CopyOp will do the rest automatically.

DYefimov avatar Oct 13 '22 12:10 DYefimov

What about the documentation? Guess it should be placed right after this one: https://github.com/moby/buildkit/blob/master/frontend/dockerfile/docs/reference.md#copy---link Do you want me to write a skeleton for it?

:eyes: that would be wonderful :heart:

Didn't exactly follow you on this one. Is it really that complicated of a change?

...yup, I think you're right - not sure what confused me before. The snippet you have isn't quite right though, I think the part in the condition would need to be f = filepath.Join("/", gitRef.SubDir) instead of with src (which would be the whole git URL). Since the Git operation always puts the subdirectory into the root (stripping the parent information away), we need to include that subdirectory in the CopyOp.

jedevc avatar Oct 13 '22 12:10 jedevc

The snippet you have isn't quite right though...

Meant pseudo, but you are right, polished it a bit for not to confuse our future selves. I'll leave it there as a TODO if you don't mind.

One thing confused me a bit in the documentation: ADD <git ref> says This flag defaults to false for the --keep-git-dir flag. What does it mean exactly "defaults" - like if it is omitted completely or the =<boolean> part is omitted? Logically if we don't provide the flag at all, then it is false by definition, =<boolean> means an explicit assignment, no =<boolean> means the default value. Not sure, but for parents it defaults to true? Tried to put it clear for --parents with an explicit examples.

Can you, please, help with the CI pipeline for the coverage. Am I missing some important step to pass the rest of the checks?

DYefimov avatar Oct 13 '22 14:10 DYefimov

One thing confused me a bit in the documentation: ADD says This flag defaults to false for the --keep-git-dir flag. What does it mean exactly "defaults" - like if it is omitted completely or the = part is omitted?

This means that if it's not explicitly enabled by including the flag, then the git directory will not be kept.

jedevc avatar Oct 17 '22 08:10 jedevc

Can you, please, help with the CI pipeline for the coverage. Am I missing some important step to pass the rest of the checks?

It looks like TestDockerfileAddArchiveWildcard fails consistently - I'd recommend running it locally and trying to fix the issues from there.

https://github.com/moby/buildkit/actions/runs/3243169399/jobs/5362299452#step:6:1355

Also, I would expect the git test to fail in it's current state - since that behavior is left unimplemented.

jedevc avatar Oct 17 '22 08:10 jedevc

Removing "partially fixes: #2917" from the PR headline. Assuming that is something we do later when getting out of the -labs channel.

It is a little weird that we accept a boolean here as a flag argument --parents=true is the same as --parents and --parents=false is the same as having nothing. I think it's mostly for consistency, but for now let's stick to the existing style.

This might be useful for setting such flags programmatically via variable expansion (either by some global variable or by some "abstract outside tool"). But the "default" thing is definitely a bit confusing :) Fixed.

It looks like TestDockerfileAddArchiveWildcard fails consistently - I'd recommend running it locally and trying to fix the issues from there.

Oh my, thanks, completely overlooked it - just found out that I was inspecting a completely different pipeline informing me of a successful check stage. Fixed now too.

FYI, there are two copies of testDockerfileAddArchiveWildcard inside the dockerfile_test.go allTests array: https://github.com/moby/buildkit/blob/6bff1509/frontend/dockerfile/dockerfile_test.go#L92 https://github.com/moby/buildkit/blob/6bff1509/frontend/dockerfile/dockerfile_test.go#L95

DYefimov avatar Oct 18 '22 14:10 DYefimov

PTAL @tonistiigi @crazy-max @thaJeztah

jedevc avatar Oct 18 '22 14:10 jedevc

This looks very promising and it seems like this is a highly requested feature. Are there any plan on pushing to get this in? @DYefimov @jedevc

agrinh avatar Feb 10 '23 14:02 agrinh

@agrinh To the best of my knowledge, PR is ready and just awaits some final reviews. Guess the merge put on hold due to milestone household issues? Anyway, I monitor the PR in case more work have to be done.

DYefimov avatar Feb 10 '23 15:02 DYefimov

Thanks for the update @DYefimov. Any chance we can get this reviewed and hopefully merged soon @jedevc @tonistiigi?

agrinh avatar Feb 21 '23 12:02 agrinh

Apologies, this has been on my backlog forever - unfortunately this has missed the deadline for dockerfile 1.5 labs, but hopefully we should have plenty of time to make dockerfile 1.6 labs (I pushed a commit to update the docs for this).

So - correct me if I'm wrong, but I think the switch to use IncludePatterns here is fairly significant? Because we change the source directory as part of this, we have to make sure that IncludePatterns and ExcludePatterns are modified to make sure we get the right files in the source directory, so that the options can be used in combination? (we also shouldn't overwrite IncludePatterns, we should append).

If we're using IncludePatterns, I don't think that we need to make any LLB changes at all (except in the HTTP source)? We could make all the changes in Dockerfile, without needing to introduce any new LLB. I'd prefer this approach, since then it would work with new Dockerfile and older buildkit. Hopefully that makes sense :smile:

@DYefimov - really sorry I've let this sit for a while. If asking for a rework like this is something you don't have time for, just let me know and I'm happy to carry it to the finish line.

jedevc avatar Feb 21 '23 13:02 jedevc

NP at all, @jedevc :)

I believe I got your idea - you basically want to move the control logic into the frontend and keep backend changes to a bare minimum. That definitely makes sense!

Regarding the "append part". Yep - I though of possible consequences, and didn't find any direct use of Include/ExcludePatterns functionality anywhere else in the buildkit that might collide with --parents. But you are right - that can and most likely will interfere in the future. And, due to the (very hacky) way we achieve our desired behavior, it might fail horribly in a very unexpected way, IMHO. But well...

That some thoughts from the top of my head. Let me look into the code on Thursday.

DYefimov avatar Feb 21 '23 17:02 DYefimov