-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix devcontainer setup on non-rootless docker #3
base: switch-to-poetry
Are you sure you want to change the base?
Fix devcontainer setup on non-rootless docker #3
Conversation
This should fix issues described in RDFLib#2187 (comment). |
Dockerfile.devcontainer
Outdated
|
||
RUN \ | ||
mkdir -p \ | ||
/srv/workspace/.venv \ | ||
/srv/workspace/.tox \ | ||
/srv/workspace/.mypy_cache \ | ||
/srv/workspace/.pytest_cache | ||
|
||
RUN \ | ||
chown -R 1000:1000 \ | ||
/srv/workspace/.venv \ | ||
/srv/workspace/.tox \ | ||
/srv/workspace/.mypy_cache \ | ||
/srv/workspace/.pytest_cache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Under normal operation this should be mounted from outside the docker container - but I will check tonight what is happening and ensure it works as well as it can with rootful (i.e. non-rootless) docker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be relevant docker/compose#3270
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it working correctly for you on the main branch? Also what docker do you use, docker desktop on windows or are you in a VM? And how do you invoke things that cause problems, do you just use it from vscode or do you use docker compose run
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@edmondchuc I think I'm just trying to be too clever with what I did here.
The reasoning for adding these volumes were so that usage of the devcontainer does not contaminate your host environment while still keeping state outside of the container. This works with rootless docker on Linux, but I'm not sure how it works on MacOS and Windows - so I think best is maybe just to not try and be too clever. if it works for you with the following lines removed then we should maybe just remove them instead:
- dot-venv:/srv/workspace/.venv
- dot-tox:/srv/workspace/.tox
- dot-mypy-cache:/srv/workspace/.mypy_cache
- dot-pytest-cache:/srv/workspace/.pytest_cache
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recall trying with Docker Desktop on Windows with WSL2, macOS and also on an Ubuntu VM. I also get the same issue when I run it in GitHub Codespaces.
I agree, I think for running a dev container using Visual Studio Code, it's not necessary to have the docker compose volumes.
Do you need the volume mounts because you're running one-off commands in the containers? E.g. docker-compose run --rm devcontainer task validate
?
Maybe we can keep the current docker-compose.yml
so that you can continue running one-off commands and we can have another docker-compose file for the Visual Studio Code dev container?
https://github.com/RDFLib/rdflib/blob/9b778b39a7759a9c94d5437e1d5f02196bdc9c51/.devcontainer.json#L2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need the volume mounts because you're running one-off commands in the containers? E.g.
docker-compose run --rm devcontainer task validate
?
The default behaviour will actually work fine, the only problem is that the venv created by the devcontainer may not be to the liking of the host system, and vice versa.
If I do task configure
and task validate
on my host system, it will create a .venv (I run with POETRY_VIRTUALENVS_IN_PROJECT=1
), and pytest, mypy and other caches in my source directory. If I then run the same commands in the devcontainer with vscode or with docker compose, the .venv and cache folders from my host system will be visible in the container.
This would be fine in some cases, but in others poetry just does not like what is happening, the venvs seem to be somewhat platform specific.
But either way, we don't need it.
Maybe we can keep the current docker-compose.yml so that you can continue running one-off commands and we can have another docker-compose file for the Visual Studio Code dev container?
We can actually just have a separate service in the same docker-compose.yml
file as we can specify which service to run for devcontainer, which may be a bit more terse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can actually just have a separate service in the same docker-compose.yml file as we can specify which service to run for devcontainer, which may be a bit more terse.
Good idea. I've gone back to using just the one docker-compose.yml
file with the devcontainer
service assigned to the devcontainer and the run
service dedicated to running one-off commands with named volumes in commit f3eb90f.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me 👍
This reverts commit a4da002.
…o install poetry with all extras flag.
…mmands inside the devcontainer with docker volumes from the host
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks for the review @aucampia! |
I think the broken run ( |
Thanks @edmondchuc for the contribution and followup, and @aucampia for working out the details! I just tried this on my own laptop (MacOS) and found that when I do this step:
(that "Recreating" seems to be serious - I think it wipes out whatever was there and starts fresh, since I end up with nothing installed into that venv and I need to re-run "poetry install" to get things back). It would be much less disruptive if the "configure" task didn't modify my working checked-out copy of the repo. |
(bit of a verbose explanation and suggestion) We are trying to compensate for docker container storage being ephemeral which means if we store state inside the container (like pytest, tox or mypy cache or venv) things will get very slow every time the container is created. We have two options for this:
The reason why you get this folder appearing outside is because there has to be something at the mount point, and docker just creates it if it does not exist. For the However volumes does not work well with devcontainers so there we need another solution. Possibly the best is to just instruct tox, mypy, pytest and poetry to put their state in the source directories but use different names, possibly So our options as I see them:
I think 2,i (mounting volumes outside I will look at this tonight (CET). |
Done now in: Seems to work for me. |
Adding chown in the devcontainer's Dockerfile. By default, the vscode user uid is 1000.Reference: https://code.visualstudio.com/remote/advancedcontainers/add-nonroot-userpostCreateCommand
run
to run one-off commands with named volumesdevcontainer
service