-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[Fix #3087] Permission denied in building with docker on local VM #3152
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
Conversation
This looks great. Docker is hard to test, but I do worry about changing something specific like this, so I'll need to test it locally before merging it. |
I would assume this approach wouldn't work for most cases. The host UID probably doesn't exist in the build container, and more than likely isn't the |
@agjohnson I think, this fixed for most of the cases. Most of the time, the host userid is |
I have tested with a random |
Does a build happen successfully with a random UID? I can create the container, but I have no permissions:
|
In the build, we mount the host environment there. As the host |
The build will not successfull with random UID, but if its host UID, it must be successful as I have tested. |
I get problems with permissions with a mounted volume as well -- I'm on OSX and running docker in a vm, with shares of my home path to the VM, which is likely the problem here. For example: |
@agjohnson Before applying the patch, it was working? Can you give me a steps to reproduce? and what error you are getting? I have tested in Ubunutu and it works fine there. |
So I have tried in my Macbook pro running OS X with the safwans-MacBook-Pro:kuma safwan$ docker run --rm -t -u $UID -v ~/test:/test -i ubuntu /bin/bash
I have no name!@66c1d3e58a39:/$ cd test
I have no name!@66c1d3e58a39:/test$ ls
kubernetes-django mozilla-django-oidc origin
I have no name!@66c1d3e58a39:/test$ mkdir temp
I have no name!@66c1d3e58a39:/test$ ls
kubernetes-django mozilla-django-oidc origin temp
I have no name!@66c1d3e58a39:/test$ exit |
My builds work fine without this patch, but because Vagrant remaps the permissions, and files are shared with the vm as I think the most correct fix is similar to @humitos fix at: I think the proper fix is using a custom Dockerfile to change the
|
We have a hardcoded user in the image though, so testing |
I am not actually comfortable to have device specific docker image. The image should be universal and the contributor should not build image in his host machine.
I strongly believe, this should be fixed first. The docker image should be platform independent. Can you speify where we are assuming so I can look over them and fix them. |
Whats the reason behind it? |
Agreed, I'd also prefer that this isn't the case, but an extension image is likely the quickest/easiest option. Realistically, we don't have a lot of bandwidth to help with overhauling our build image for local development. It should only take 1 extra steps for development setup with a custom extension image, setting the configuration to use the local extension image. We require running as a Because of the complexity here, I lean towards the easy solution of a custom extension dockerfile. If you wanted to alter our base dockerfile, we'd need a good amount of QA across projects -- test conda projects, multiple python versions, and project options around pip/conda. We had talked a while back about some form of development docker image, but efforts there stalled and seem to be superseded by an extension image for local development |
@agjohnson I understand your point. I will investigate more to find a way to fix it up by using the |
@agjohnson So I have found a solution about changing the UID of But for this, we need to upgrade our Can I know the Docker and Docker API version installed in the production? I have working on a patch, can you check @agjohnson and let me know if you have any concern about the way? |
After testing this a little I'm on the fence whether this is a good approach or not. The operation in your example is exactly what we need to, however the operation takes a significant amount of time. I'm testing local setup on a box now and it's been running for a while -- too long to be acceptable for each build. Because of this, we likely don't want to do this in production either. It should be a noop, as the UID isn't changing in production, but better to be explicit here. I'm still leaning towards a custom image, compilation of the image would only happen once. This is the image I'm building now:
This would need to be configurable for users though. A new dev set up might be:
We can likely automate the custom configuration step by assuming the image name is |
@agjohnson I have also inspected it and found that its also taking too much time. I am going to investigate more to find why its taking too much time. |
I'm happy that this problem came up again and there is another person trying to solve it :) I think this issue is kind of complicated in terms of being sure that any change will work in all the situations that we need to work on, but I think it worth to make the effort if we want to open the door to new contributors. Probably, it will need contributors for testing and QA in these different cases, also. I have been thinking on this last week and I think the idea that @safwanrahman is proposing is the way to go. I mean, having a generic docker image that works on production and on development without making any changes.
@agjohnson why do we need permissions under the
@agjohnson can you point me one of these to understand it better? This post explains exactly the problem that we are having here: https://medium.com/@mccode/understanding-how-uid-and-gid-work-in-docker-containers-c37a01d01cf
After reading that post, I think this is not really important since you can execute a command with the UID from the host without needed to exist in the container. Another similar idea to the one proposed here, instead of using
I think that most of the contributors will have the |
@agjohnson I modify your Dockerfile like this:
This way, any new contributor can build their own docker image with their own UID and GID. I commented the If we are going on this way, I think we should include this Dockerfile in one of RTD repositories and add it in the documentation as a tip or something like that (as I did in my PR) maybe explaining the why and linking to these issues for a deeper understanding. |
Its not permissions, its the home path. We can't ensure tools relying on the user's home path will execute in the same way. We set up a number of things on the
This is not a huge hurdle, but we don't use UID 1000 in production either, so this introduces friction to getting a simple change out. We'd need to bring down each build server, alter the UID across TBs of docs and re-provision everything. UID 1000 also isn't the default minimum on RHEL/Centos, OSX, and a few others, so changing this on the build image would only b a partial fix still.
Agreed. I still think this is the most simple solution. There is probably some application logic we can use to raise a warning that the UID doesn't match. The simplest version of this seems to be a build configurable Dockerfile that we can store in This doesn't make any more work for the developer -- that is, our built image is still downloaded and there is no compilation required. The additional step to alter the UID took a few minutes at most. This isn't even any additional steps, it just replaces:
with
Also, thanks for continue to track this down everyone, all of the ideas above are great ✨ Addressing this with more application logic is probably the most correct thing to do. But I think a custom Dockerfile like above is the best thing to do here -- it's simple, doesn't create more work, and offers the same solution to developers. |
@agjohnson So we can set permission to the |
This would affect run the commands as the docs UID though, so wouldn't be enough. |
Seems like maybe this can be closed? I think there was a lot of discussion, but it didn't seem like this was the right solution. |
Yeah, think we can close this. |
This is a common problem of Docker container. The build in docker container is running with root permission and creating the file as root. So the local user does not have permission to access the directory.
passing
user
while creating the container should fix the issue. So in the container, all the command will run as theuid
of host user, and the host user will have the permission to access the directory.@agjohnson r?