Run Docker container with a specific user

[humitos]

Pass --user (DOCKER_USER) attribute when creating the container.

This has no effect in production since we are using the same user and
group than the one defined inside the Dockerfile image (docs:docs).
Although, this allow us to avoid permissions conflicts when running
the build with Docker locally (development) since we can pass our
current user.

That way, every file created/modified inside the container will be
done using the current UID and GID defined by the developer.

This can be done as,

# local_settings.py
DOCKER_USER = f'{os.geteuid()}:{os.getegid()}'

With this change, there is no need to re-build the Docker image used
in production with our own custom USER instruction.

https://docs.docker.com/engine/reference/run/#user

@raulcd helped me to find out this and make it work properly considering production and development environment

Connected to #4608
Related #2692 #3245 #3003 #3152 #1133

[humitos]

Once this gets merged, we can just pull the docker image from Docker Hub and run our instance without doing additional steps.

1. clone the repo
2. create venv
3. pull the docker image
4. update the config in local settings
5. run the instance

No problems with messed permissions or owners of the files, etc.

[humitos]

DOCKER_USER = f'{os.geteuid()}:{os.getegid()}'

This line can be added to settings/dev.py if you consider it's better, to avoid this extra step of adding it to the local settings file.

[stsewd]

This line can be added to settings/dev.py if you consider it's better, to avoid this extra step of adding it to the local settings file.

I'm +1 with this

[humitos]

@ericholscher tested this in MacOS and he said it worked properly.

After some discussion I tested different versions of Python (installed via pyenv) in the Docker image and they worked.

Although, when I tested a build with conda it failed. The reason is because conda is installed as docs user inside the Docker image and some paths are configured for that user, conda info:

          package cache : /home/docs/.conda/pkgs
                          /.conda/pkgs
       envs directories : /.conda/envs
                          /home/docs/.conda/envs

So, when running the Docker container with a user that has a different UID:GID which does not have write permissions on at least one of those directories listed, we have a problem and the build fails.

NotWritableError: The current user does not have write permissions to a required path.
  path: /.conda/pkgs/urls.txt
  uid: 1000
  gid: 100

I'm not sure yet how to solve this, but it may need changes on the Dockerfile. I'm marking this PR as blocked until we get there.

[humitos]

A simple way to reproduce the issue locally for this,

$ docker run --rm -it --user $(id -u):$(id -g) readthedocs/build:latest
I have no name!@c1d78f3c9931:/$ conda create -n test

NotWritableError: The current user does not have write permissions to a required path.
  path: /.conda/pkgs/urls.txt
  uid: 1000
  gid: 100

If you feel that permissions on this path are set incorrectly, you can manually
change them by executing

  $ sudo chown 1000:100 /.conda/pkgs/urls.txt

In general, it's not advisable to use 'sudo conda'.


I have no name!@c1d78f3c9931:/$ 

The way I think this can be done is by setting a different paths with conda config in the Docker image or change the permissions for those directories to allow rw access to all users.

[humitos]

We said that we can still merge this and promote this to start contributing and alert the developer that conda won't work for this case. In the end it will be still better than what we have but it won't work for 100% cases.

If the developer wants to test a conda build or change on it, we can refer them to a guide that explains how to re-compile the Docker image with the proper user permissions.

[stsewd]

We should check that conda works in production :)

[astrojuanlu]

Comment from the peanut gallery: I never saw the need to use conda inside Docker (as long as you have permission to install or compile any system dependency, of course)

[stsewd]

@Juanlu001 we use docker to encapsulate each build in production, we use docker in dev for testing and it's easy instead of installing all the stuff

[humitos]

I added a warning note under the "Build Environments" page that follows the "Installation" guide mentioning that in case that you need to build projects with Conda you may need to follow another guide to re-build our Docker image.

I think it covers all the cases:

1. MacOS + all cases, no modification needed
2. Linux + no conda, no modification needed
3. Linux + conda, point to where the guide is being written

Being 1) and 2) most of the cases I'm happy to deploy this. Since 3) is a special and uncommon case, we can attend it separately and keep working on @agjohnson's PR #4608 with more time.

[agjohnson]

Perhaps we should just work on merging #4608, if we can avoid breaking conda testing on linux installs -- given this impacts 3/5 of the core team.

[agjohnson]

I'm probably -1 on this PR given it introduces a big blind spot for local conda testing. I think #4608 is closest to a working solution. I'm going to block this while we discuss the other PR more.

[humitos]

I'm probably -1 on this PR given it introduces a big blind spot for local conda testing

Well, it is not a blind spot since it hard fail the builds with a permission error.

IMO, asking contributors to build their own Docker image to fix a small issue adds a big barrier which probably makes to loose the contribution. Which is not a good.

[humitos]

Perhaps we should just work on merging #4608, if we can avoid breaking conda testing on linux installs -- given this impacts 3/5 of the core team.

I want both.

• Core Team must re build Docker images.
• Contributors does not, but may want to do it.

I see this as a very easy way to start contributing. Then, if you find that your builds are failing, you will refer to the documentation and will find out the solution: re build your Docker image. Although, from the first build (and contribution) to a failing build because with conda, we could already received many contributors and ease their lives :)

[stsewd]

I'm +1 in just merging #4608, having a blind spot can cause surprises, and this failing with conda makes me think there are more things that could break?

And, most of the contributors don't use docker, they use the local builder.

[raulcd]

I know you probably don't care too much about my point of view. As a new contributor having a docker environment with docker-compose being able to do docker-compose up as with anything that uses docker-compose and having "almost everything" up and ready vs needing to follow a long list of instructions and create a local builder (which to be fair no-one who is not working long time on the project understand what really is) is a huge stop for me to contribute.

[stsewd]

@raulcd that's a different problem, using docker-compose or dockerize the project. This is about running the builds inside docker vs running the builds in the local machine. Current installation instructions should work without caring so much about where the builds happen.

[agjohnson]

Well, it is not a blind spot since it hard fail the builds with a permission error.

@humitos sorry, i meant it introduces a feature/section of code that won't be testable by contributors. we get an error, but you need to have the background in that error to know this is an issue with docker permissions.

IMO, asking contributors to build their own Docker image to fix a small issue adds a big barrier which probably makes to loose the contribution.

I sort of agree there and understand that this PR is probably okay in a good chunk of cases, but the main thing I don't like is that conda just breaks. Contributors besides us won't know that the conda error is really a docker permission error because the image needs to be rebuilt. Reducing the areas where dev and prod differ is one of our short term goals, so this seems to move in the opposite direction.

The good news is that the process for building an image on top of readthedocs/build:latest is insignificant in relation even to the time it takes to docker pull readthedocs/build:latest (docker-build.sh takes ~1m on my machine for the new layers). If the user is to the point where they are switching to docker, the extra step to compile an image shouldn't be a huge barrier as long as we are communicating the process well.

[humitos]

I sort of agree there and understand that this PR is probably okay in a good chunk of cases, but the main thing I don't like is that conda just breaks

We can spend some time to make the original Docker image to install conda properly (defining PATHs properly) so it does not break. Then, keep using the code from this PR. That's probably the best direction to take since there won't be needed to rebuild the image.

[agjohnson]

True, but I'd rather not invest more time in this right now though. Given our decision to deprioritize pointing new contributors at this code base, I don't think we need to further increase our focus on the new contributor here.

For now, working with docker is going to be a prohibitively large hurdle either way, and we'll have a solution that works for 100% of contributors working at this level (with docker) without any further work. In the future, we can revisit this PR and make improvements to docker onboarding if we decide we want to prioritize getting contributions to our main product repo.

[saadmk11]

I was trying to build docs using DockerBuildEnvironment for Vertualenv locally. Even though I was using readthedocs/build-dev:latest I was getting permission denied error. So, I tried the changes from this PR and it worked perfectly. Thanks 👍

[stsewd]

@saadmk11 you can check https://docs.readthedocs.io/en/stable/development/buildenvironments.html

[saadmk11]

@saadmk11 you can check https://docs.readthedocs.io/en/stable/development/buildenvironments.html

Yes I have followed this at first but was still getting permission denied

[humitos]

I'm closing this PR in favor of #6295 which already include the commits from this PR. Thank you all for the help on this. Appreciated!