`INSTALL_DIND` UDC fails for `docker:dind` image

rrjjvv commented 1 year ago

TLDR: Compose v2 causes problems.

The title is misleading (I wasn't actually trying/wanting to use docker:dind), but it's a true statement, and the root of the actual problem I wanted to solve.

The recommended earthly/dind:alpine image solves 99% of my problems, but I have one project where it's preferrable to utilize the INSTALL_DIND UDC on top of an unrelated image. I was getting annoyed with the VERSION warning, so went to create a PR for it (already tracked in #5). It's a one-line fix, but I still ran the tests, and was surprised to see them fail.

The root cause of the test failures is a combination of two things:

the docker:dind image currently bundles a V2 compose (but with a docker-compose symlink, which satisfies the auto-install check)
the fact that the test (and probably real-world usages) place the compose file in the root directory

There are many ways to deal with this; if there was a clear "best" solution I would have submitted a PR. This is far from exhaustive (and some can be combined), but the 'easy' potential solutions:

remove docker:dind from the test suite (it's presence implies it's supported)
detect V2, and install V1 to trump it
add a -p to the compose invocations (which lets the compose file live in the root directory)
document that placing the compose file at the root directory is not supported
call write_compose_config and use the resulting config (not to conflate multiple issues, but doing this may address a bug-not-yet-reported, and the fact that it writes to a non-root directory is what solves the immediate problem)

The non-easily solutions basically amount to "natively support V2" (which could also pave the way for #6). That's further complicated due to "natively supporting V2" can mean supporting the V2 version (what we're dealing here, since there's a compatibility symlink), or supporting the V2 CLI syntax (i.e., docker compose vs. docker-compose).

Another possibility (but an obviously breaking change) would be to drop support for --compose entirely. That sounds crazy... but personally, if there was a --load docker-compose.yml, I doubt I would ever use --compose. In other words, the value (for me) is the loading of images, not in saving me from doing up myself (which is usually a hindrance). I may bring this up as feature request in the main earthly project, but figured I'd throw it out there in case it spurred other thoughts.

Again, this issue (as described in the title) does not actually impact me (though it may impact someone else). But it does impact the ability to add changes to this repo due to the failing tests.

Isolating it to the one failing test:

$ earthly -v
earthly version v0.6.24 20339818277375d1addc9cc20066a9890483761e linux/amd64; Ubuntu 20.04.5 LTS (Focal Fossa)

$ earthly -P --no-cache +test-install-dind-for-image --base_image=docker:dind
 1. Init 🚀
————————————————————————————————————————————————————————————————————————————————

           buildkitd | Found buildkit daemon as docker container (earthly-buildkitd)

 2. Build 🔧
————————————————————————————————————————————————————————————————————————————————

Warning: No version specified in ./Earthfile. Implying VERSION 0.5, which is not the latest available. Please note that in the future, the VERSION command will be required for all Earthfiles.
         docker:dind | --> Load metadata linux/amd64
         alpine:3.13 | --> Load metadata linux/amd64
+install-dind-script | --> FROM alpine:3.13
             context | --> local context .
+test-install-dind-for-image | base_image=docker:dind
+test-install-dind-for-image | --> FROM docker:dind
+install-dind-script | [██████████] 100% resolve docker.io/library/alpine:3.13@sha256:100448e45467d4f3838fc8d95faab2965e22711b6edf67bbd8ec9c07f612b553
+test-install-dind-for-image | [██████████] 100% resolve docker.io/library/docker:dind@sha256:1be6768d332a4aaed59b344e6525e1701c467c4ef8553d5d18bac086cbd82ef1
             ongoing | context (5 seconds ago)
             context | [          ]   0% transferring .:
             context | [██████████] 100% transferring .:
+install-dind-script | --> COPY ./install-dind.sh ./
+install-dind-script | --> SAVE ARTIFACT ./install-dind.sh +install-dind-script/install-dind.sh
+test-install-dind-for-image | --> COPY +install-dind-script/install-dind.sh /tmp/install-dind.sh
+test-install-dind-for-image | --> RUN /tmp/install-dind.sh
+test-install-dind-for-image | jq is missing. Attempting to install automatically.
+test-install-dind-for-image | fetch https://dl-cdn.alpinelinux.org/alpine/v3.16/main/x86_64/APKINDEX.tar.gz
+test-install-dind-for-image | fetch https://dl-cdn.alpinelinux.org/alpine/v3.16/community/x86_64/APKINDEX.tar.gz
+test-install-dind-for-image | (1/2) Installing oniguruma (6.9.8-r0)
+test-install-dind-for-image | (2/2) Installing jq (1.6-r1)
+test-install-dind-for-image | Executing busybox-1.35.0-r17.trigger
+test-install-dind-for-image | OK: 31 MiB in 57 packages
+test-install-dind-for-image | /usr/local/bin/dockerd
+test-install-dind-for-image | /usr/local/bin/docker-compose
+test-install-dind-for-image | base_image=docker:dind
+test-install-dind-for-image | --> RUN echo "
+test-install-dind-for-image | version: \"3\"
+test-install-dind-for-image | services:
+test-install-dind-for-image |     hello:
+test-install-dind-for-image |         image: hello-world:latest
+test-install-dind-for-image |     " >./docker-compose.yml
+test-install-dind-for-image | base_image=docker:dind
+test-install-dind-for-image | --> WITH DOCKER (install deps)
+test-install-dind-for-image | /usr/bin/jq
+test-install-dind-for-image | /usr/local/bin/dockerd
+test-install-dind-for-image | /usr/local/bin/docker-compose
+test-install-dind-for-image | base_image=docker:dind
+test-install-dind-for-image | --> WITH DOCKER (docker-compose config)
  hello-world:latest | --> Load metadata linux/amd64
  hello-world:latest | --> DOCKER PULL hello-world:latest
  hello-world:latest | [██████████] 100% resolve docker.io/library/hello-world:latest@sha256:62af9efd515a25f84961b70f973a798d2eca956b1b2b026d0a4a63a3b0b6a3f2
             ongoing | +test-install-dind-for-image (4 seconds ago)
             context | transferred 1 file(s) for context /tmp/earthly-docker-load1692874303 (11 kB, 1 file/dir stats)
+test-install-dind-for-image | base_image=docker:dind
+test-install-dind-for-image | --> WITH DOCKER RUN --privileged true
+test-install-dind-for-image | Starting dockerd with data root /var/earthly/dind/c22eb5c3ac654bc3a3ba123aea987989c7ab25b3dd74017ad3aa36871ac86309/tmp.nMEmBc
+test-install-dind-for-image | Loading images from BuildKit via tar files...
+test-install-dind-for-image | Loaded image: hello-world:latest
+test-install-dind-for-image | ...done
+test-install-dind-for-image | Network _default  Creating
+test-install-dind-for-image | Network _default  Created
+test-install-dind-for-image | Container -hello-1  Creating
+test-install-dind-for-image | Error response from daemon: Invalid container name (-hello-1), only [a-zA-Z0-9][a-zA-Z0-9_.-] are allowed
+test-install-dind-for-image | ERROR: Command exited with non-zero code: WITH DOCKER RUN --privileged true

============================ ❌ FAILURE [2. Build 🔧] ============================

Repeating the output of the command that caused the failure
+test-install-dind-for-image *failed* | base_image=docker:dind
+test-install-dind-for-image *failed* | --> WITH DOCKER RUN --privileged true
+test-install-dind-for-image *failed* | Starting dockerd with data root /var/earthly/dind/c22eb5c3ac654bc3a3ba123aea987989c7ab25b3dd74017ad3aa36871ac86309/tmp.nMEmBc
+test-install-dind-for-image *failed* | Loading images from BuildKit via tar files...
+test-install-dind-for-image *failed* | Loaded image: hello-world:latest
+test-install-dind-for-image *failed* | ...done
+test-install-dind-for-image *failed* | Network _default  Creating
+test-install-dind-for-image *failed* | Network _default  Created
+test-install-dind-for-image *failed* | Container -hello-1  Creating
+test-install-dind-for-image *failed* | Error response from daemon: Invalid container name (-hello-1), only [a-zA-Z0-9][a-zA-Z0-9_.-] are allowed
+test-install-dind-for-image *failed* | ERROR: Command exited with non-zero code: WITH DOCKER RUN --privileged true
Error: build target: build main: failed to solve: process "/bin/sh -c EARTHLY_DOCKERD_DATA_ROOT=\"/var/earthly/dind/c22eb5c3ac654bc3a3ba123aea987989c7ab25b3dd74017ad3aa36871ac86309\" EARTHLY_DOCKER_LOAD_FILES=\"/var/earthly/load-0/image.tar\" EARTHLY_IMAGES_WITH_DIGESTS=\"\" EARTHLY_START_COMPOSE=\"true\" EARTHLY_COMPOSE_FILES=\"docker-compose.yml\" EARTHLY_COMPOSE_SERVICES=\"\" DIND_COMMIT=42b1175eda071c0e9121e1d64345928384a93df1 DOCKER_BUILDX_VERSION=0.9.1 DOCKER_COMPOSE_VERSION=2.11.2 DOCKER_TLS_CERTDIR=/certs DOCKER_VERSION=20.10.18 EARTHLY_LOCALLY=false PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin base_image=docker:dind /var/earthly/dockerd-wrapper.sh execute /usr/bin/earth_debugger /bin/sh -c 'true'" did not complete successfully: exit code: 1

(A -p foo, or moving the compose file to /foo/, would both result in a valid container name of foo-hello-1.)

rrjjvv commented 1 year ago

Do you have any thoughts around the best/preferred solution? There is a subset of solutions that I'd be willing to implement in some free time.

Roughly speaking, most of the "work around docker:dind" and "work around v2" solutions I'm willing/comfortable implementing. However, the "support v2" solutions would require decisions from you around maintaining/breaking compatibility (so best left to you folks to implement).

(To save you a quick search: docs on compatibility. After migrating our CI infra to v2, we know of at least one other incompatibility not listed in that page, though it's probably not applicable to how Earthly would use it.)

alexcb commented 1 year ago

I like the idea of adding a --compose-load flag which would simply perform the image loads without the up.

Note that this would not install docker-compose, it would leave it up to the user to provide their own.

earthly / lib

`INSTALL_DIND` UDC fails for `docker:dind` image #7