OpenHands
OpenHands copied to clipboard
Update Makefile to skip sandbox Docker pull
Adding a build-nodocker option to make OpenDevin not pull the sandbox Docker. This needed in the dockerizing process for OpenDevin I have been working on here https://github.com/risingsunomi/OpenDevin-Docker
We cannot start a docker in a docker so will need to remove it.
I agree with the need, not fully onboard with the implementation.
If you make a copy of the existing make target and skip just the docker pull step, every time someone makes changes to make build they'll have to remember to make the same change to build-nodocker, and that's easy to miss.
Is there a way to make the regular make build skip a step? Like perhaps setting an env var.
making things up: $ SKIP_SANDBOX_IMAGE_PULL=1 make build
Maybe -
ifndef SKIP_SANDBOX_IMAGE_PULL
@$(MAKE) -s pull-docker-image
endif
(untested)
Maybe -
ifndef SKIP_SANDBOX_IMAGE_PULL @$(MAKE) -s pull-docker-image endif(untested)
I agree, this implementation is much better. Have it control by env we can possibly use the same env for other things. Will test this and see.
You can implement Docker-in-Docker with this project. But I as someone who has is not going to recommend not having control of when...just as a major correction about that statement, @risingsunomi . It can be done but it required major modification to the way the Docker image is built (in multiple steps, committing image changes from a builder container, and using a local registry), so what that ultimately means from my experiment is it must be done at container runtime and it is not recommended to pull it by default, and instead at runtime from /docker-entrypoint.sh with a Docker-in-Docker privileged solution. Which in itself is a good case for SKIP_SANDBOX_IMAGE_PULL to avoid the type of complexity I had to deal with.
I was able to get DinD working without modifying any of the repository code, but it did require restructuring of the way that the image is built (as a builder container must be able to run, and multistage builds with a container registry must be implemented, and have to avoid using docker compose until when everything is built and the runtime image is cooked successfully, a couple thousand lines of shell code had to be written, ouch). Privileged containers cannot happen at BuildKit time. Which is the entire reason that solution is extremely complex. I've done so.
Major restructure of the way the Docker image is built
- use a local container registry via
registry:2docker image.
docker run -it \
--name "registry" \
--network "opendevin_net" \
--port "5000:5000" \
--label "com.docker.stack.namespace=opendevin-admin" \
docker.io/registry:2 || return $?
- split the project image into 2 dockerfiles:
Dockerfile.builderandDockerfile.runtime.
FROM debian:bookworm-slim AS localhost:5000/opendevin-builder
- installing docker in
opendevin-builderimage in theDockerfile.builderby using the following RUN command (in a debian-bookworm image is where I tested this):
# I run in user mode, not as root, so I define a build argument that lets me set a username:
ARG APP_USER
ENV APP_USER ${APP_USER:-nobody}
# ... after installing base dependencies, creating user, and their home directory, etc. ...
# Setup Docker CE
RUN set -eux; \
curl -fsSL https://download.docker.com/linux/debian/gpg | gpg --dearmor -o /usr/share/keyrings/docker-archive-keyring.gpg; \
echo "deb [arch=amd64 signed-by=/usr/share/keyrings/docker-archive-keyring.gpg] https://download.docker.com/linux/debian $(lsb_release -cs) stable" | tee /etc/apt/sources.list.d/docker.list; \
apt-get update -qqy ;\
DEBIAN_FRONTEND=noninteractive apt-get install -qqy --no-install-recommends \
docker-ce; \
apt-get clean -y; \
rm -rf /var/lib/apt/lists/*; \
rm -rf /var/cache/apt; \
usermod -aG docker $(id -un); \
# issue: https://github.com/docker/cli/issues/4807#issuecomment-1903950217 \
sed -i 's/ulimit -Hn/# ulimit -Hn/g' /etc/init.d/docker; \
touch /var/run/docker.sock; \
chown ${APP_USER}:docker /var/run/docker.sock;\
chmod 4775 /var/run/docker.sock
- Then inherit from it in opendevin-runtime:
FROM localhost:5000/opendevin-builder AS opendevin-runtime
-
allowing
/docker-entrypoint.shto acceptbuildandrunarguments. This paste was too lengthy so I won't share, but I have a custom/docker-entrypoint.shthat does that here that supports building the project while the container is running, instead of at BuildKit time where docker in docker isn't supported and docker cannot run withservice docker start. -
passing
--priviledgedto thedocker runcommand in the runtime image -
using
--volumeto mount/var/run/docker.sock -
setting permissions on
/var/run/docker.sockto >=4755and having it owned by the runtime user:docker, which is what happens in the example RUN command I gave while installing docker.
Here's some code from my project.
docker build \
--file "./docker/Dockerfile.builder" \
--tag "localhost:5000/opendevin-builder:latest" \
--label "com.docker.stack.namespace=opendevin" \
"./docker" || return $?
docker build \
--file "./docker/OpenDevin/Dockerfile.runtime" \
--tag "localhost:5000/opendevin-runtime:latest" \
--label "com.docker.stack.namespace=opendevin" \
"./docker/OpenDevin" || return $?
docker run -it \
--name "opendevin-prod" \
--network "opendevin_net" \
--volume "/var/run/docker.sock:/var/run/docker.sock" \
--label "com.docker.stack.namespace=opendevin" \
localhost:5000/opendevin-runtime:latest \
build || return $?
The working proof of concept of a builder vs. a runtime image with a working Docker-in-Docker with zero modification to Makefile is here.
This required some extensive modification to the way the docker image itself is built.
I recommend the flag because of a different reason: control of when the Sandbox image actually gets pulled which totally would have saved me from writing a couple thousand lines of shell boilerplate and additional Dockerfile to work around the issue of not being able to DinD with --priviledged at BuildKit time. NOT because Docker-in-Docker isn't possible with this project. That is incorrect. It is, but will vastly inflate code complexity as in my experiment.
This doesn't cover all the lengths and footwork I myself had to do, but for obvious reasons related to not increasing complexity of docker solution to the level that I did, I vote yes on SKIP_SANDBOX_IMAGE_PULL, especially because I'm exhausted after my experiment doing without it!
The moral of the story is don't do what I did and a Makefile flag with a minor modification to /docker-entrypoint.sh is probably the cleanest solution I have seen to all the PR whiteout I have witnessed promoting a working docker image.
By the way I wish people would look before submitting PRs, we have #1097 #940 #377 #481 and #1023 . This is not directed at this particular PR, which makes a simple change and doesn't repeat what I've seen a lot of other PR's doing on this repository....
I think #1023 has the right idea staying a Draft and letting a group collaborate on it.... trying to implement docker for this project has become a bit of a mess! Please end the chaos! I highly recommend that if you want to work on Docker, currently the best place is in the Draft on #1023 that has been working collaboratively on one solution.
You can implement Docker-in-Docker with this project. But I as someone who has is not going to recommend not having control of when...just as a major correction about that statement, @risingsunomi . It can be done but it required major modification to the way the Docker image is built (in multiple steps, committing image changes from a builder container, and using a local registry), so what that ultimately means from my experiment is it must be done at container runtime and it is not recommended to pull it by default, and instead at runtime from
/docker-entrypoint.shwith a Docker-in-Docker privileged solution. Which in itself is a good case forSKIP_SANDBOX_IMAGE_PULLto avoid the type of complexity I had to deal with.I was able to get DinD working without modifying any of the repository code, but it did require restructuring of the way that the image is built (as a builder container must be able to run, and multistage builds with a container registry must be implemented, and have to avoid using
docker composeuntil when everything is built and the runtime image is cooked successfully, a couple thousand lines of shell code had to be written, ouch). Privileged containers cannot happen at BuildKit time. Which is the entire reason that solution is extremely complex. I've done so.Major restructure of the way the Docker image is built
- use a local container registry via
registry:2docker image.docker run -it \ --name "registry" \ --network "opendevin_net" \ --port "5000:5000" \ --label "com.docker.stack.namespace=opendevin-admin" \ docker.io/registry:2 || return $?
- split the project image into 2 dockerfiles:
Dockerfile.builderandDockerfile.runtime.FROM debian:bookworm-slim AS localhost:5000/opendevin-builder
- installing docker in
opendevin-builderimage in theDockerfile.builderby using the following RUN command (in a debian-bookworm image is where I tested this):# I run in user mode, not as root, so I define a build argument that lets me set a username: ARG APP_USER ENV APP_USER ${APP_USER:-nobody} # ... after installing base dependencies, creating user, and their home directory, etc. ... # Setup Docker CE RUN set -eux; \ curl -fsSL https://download.docker.com/linux/debian/gpg | gpg --dearmor -o /usr/share/keyrings/docker-archive-keyring.gpg; \ echo "deb [arch=amd64 signed-by=/usr/share/keyrings/docker-archive-keyring.gpg] https://download.docker.com/linux/debian $(lsb_release -cs) stable" | tee /etc/apt/sources.list.d/docker.list; \ apt-get update -qqy ;\ DEBIAN_FRONTEND=noninteractive apt-get install -qqy --no-install-recommends \ docker-ce; \ apt-get clean -y; \ rm -rf /var/lib/apt/lists/*; \ rm -rf /var/cache/apt; \ usermod -aG docker $(id -un); \ # issue: https://github.com/docker/cli/issues/4807#issuecomment-1903950217 \ sed -i 's/ulimit -Hn/# ulimit -Hn/g' /etc/init.d/docker; \ touch /var/run/docker.sock; \ chown ${APP_USER}:docker /var/run/docker.sock;\ chmod 4775 /var/run/docker.sock
- Then inherit from it in opendevin-runtime:
FROM localhost:5000/opendevin-builder AS opendevin-runtime
allowing
/docker-entrypoint.shto acceptbuildandrunarguments. This paste was too lengthy so I won't share, but I have a custom/docker-entrypoint.shthat does that here that supports building the project while the container is running, instead of at BuildKit time where docker in docker isn't supported and docker cannot run withservice docker start.passing
--priviledgedto thedocker runcommand in the runtime imageusing
--volumeto mount/var/run/docker.socksetting permissions on
/var/run/docker.sockto >=4755and having it owned by the runtime user:docker, which is what happens in the example RUN command I gave while installing docker.Here's some code from my project.
docker build \ --file "./docker/Dockerfile.builder" \ --tag "localhost:5000/opendevin-builder:latest" \ --label "com.docker.stack.namespace=opendevin" \ "./docker" || return $? docker build \ --file "./docker/OpenDevin/Dockerfile.runtime" \ --tag "localhost:5000/opendevin-runtime:latest" \ --label "com.docker.stack.namespace=opendevin" \ "./docker/OpenDevin" || return $? docker run -it \ --name "opendevin-prod" \ --network "opendevin_net" \ --volume "/var/run/docker.sock:/var/run/docker.sock" \ --label "com.docker.stack.namespace=opendevin" \ localhost:5000/opendevin-runtime:latest \ build || return $?The working proof of concept of a builder vs. a runtime image with a working Docker-in-Docker with zero modification to Makefile is here.
This required some extensive modification to the way the docker image itself is built.
I recommend the flag because of a different reason: control of when the Sandbox image actually gets pulled which totally would have saved me from writing a couple thousand lines of shell boilerplate and additional Dockerfile to work around the issue of not being able to DinD with
--priviledgedat BuildKit time. NOT because Docker-in-Docker isn't possible with this project. That is incorrect. It is, but will vastly inflate code complexity as in my experiment.This doesn't cover all the lengths and footwork I myself had to do, but for obvious reasons related to not increasing complexity of docker solution to the level that I did, I vote yes on
SKIP_SANDBOX_IMAGE_PULL, especially because I'm exhausted after my experiment doing without it!The moral of the story is don't do what I did and a Makefile flag with a minor modification to
/docker-entrypoint.shis probably the cleanest solution I have seen to all the PR whiteout I have witnessed promoting a working docker image.By the way I wish people would look before submitting PRs, we have #1097 #940 #377 #481 and #1023 . This is not directed at this particular PR, which makes a simple change and doesn't repeat what I've seen a lot of other PR's doing on this repository....
I think #1023 has the right idea staying a Draft and letting a group collaborate on it.... trying to implement docker for this project has become a bit of a mess! Please end the chaos! I highly recommend that if you want to work on Docker, currently the best place is in the Draft on #1023 that has been working collaboratively on one solution.
Thank you for the explanation and correction. Will make sure to make this more of a collab effort than all these different solutions. The docker stuff is kinda wild right now and possibly too many people working on it independently. Will consolidate the effort.
I think we can close this one now that we have a Dockerfile set up.