Skip to content
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

docker: no longer chown /opt #558

Merged
merged 6 commits into from
Jun 15, 2021

Conversation

KrisThielemans
Copy link
Member

At least on some systems, changing ownership of /opt/* took a long time when the container was created. Instead, we just give everyone write permissions, but do this when creating the images.

Fixes #555

@KrisThielemans KrisThielemans marked this pull request as draft June 7, 2021 05:49
@KrisThielemans
Copy link
Member Author

container start-up time is now indeed very fast, while creation time is very long...

We should try to be more selective probably, and definitely not do the whole of /opt again at the end of the service image creation (where it should only have to do whatever was installed as extra in /opt/pyenv)

@KrisThielemans
Copy link
Member Author

Ah well. I thought I had it fixed with my new strategy: use umask 000 when calling user_*sh. Indeed, this fixes permissions of /opt/pyenv and /opt/SIRF-SuperBuild as I expected to be read/writable for everyone:

$ ls -l /opt/
total 24
drwxr-xr-x 2 root root 4096 Jun  5 17:20 bin
drwxrwxrwx 1 root root 4096 Jun  7 21:16 ccache
lrwxrwxrwx 1 root root   25 Jun  5 17:20 cmake -> cmake-3.13.4-Linux-x86_64
drwxr-xr-x 6 root root 4096 Jun  5 17:20 cmake-3.13.4-Linux-x86_64
drwxrwxrwx 1 root root 4096 Jun  7 22:02 pyvenv
drwxrwxrwx 1 root root 4096 Jun  7 22:02 SIRF-SuperBuild

Result: image creation time same as on master, but container creation time 1 second! Yay!

Except... it doesn't work. Files in /opt/SIRF-SuperBuild/sources/SIRFand.../builds/SIRF/build(and all others) are not writable bysirfuser, and a conda install coverage` fails with a permission problem.

It seems that setting umask 000 gets ignored by same programs. Oh well.

@KrisThielemans
Copy link
Member Author

So, the question is then if we can live with sirfuser not being able to write to most of /opt. Consequences

  • conda install fails
  • sudo conda install fails as it's not in the path (fixable)
  • sudo /opt/pyvenv/bin/conda install seems to work
  • pip install works
  • updating the /opt/SIRF-SuperBuild would need sudo

Presumably it would be safest to chmod .../sources/SIRF/data

Of course, this would have to be documented.

It's not ideal, but shaving 10 mins of start-time isn't small...

Or is there another solution?

@paskino
Copy link
Contributor

paskino commented Jun 8, 2021

Is it not possible to use sirfuser to run some stuff, like building SuperBuild, install conda etc?

There's no reason why conda should be installed by the superuser.

@KrisThielemans
Copy link
Member Author

Is it not possible to use sirfuser to run some stuff, like building SuperBuild, install conda etc?

The point is that at image build time (i.e. whatever Dockerfile does), you don't know yet what the UID:GID has to be. You could do it with 1000:1000 and hope to be lucky, but we'd still have to cope with other values.

(Note that the username/groupname don't mean anything, it's the numbers that matter).

@KrisThielemans
Copy link
Member Author

Haroon Chughtai suggests

another option could be to create and grant permissions to the needed folders to a group during the build. Then when the image is pulled and run, the created user just needs to be added to that group to have the right access?

I guess this might work as:

  • Dockerfile
    • create group (with crazy gid)
    • (I think) create user (with crazy uid)
    • run the user_sh as this user
  • entrypoint.sh
    • add sirfuser to the above group

First check in a VM (or running docker) that the above strategy actually allows sirfuser to write to the relevant directories.

@KrisThielemans
Copy link
Member Author

@paskino I've taken some of your lines in #569 and incorporated them in the "regular" stuff. I've kept @casperdcl's overall structure, but do make another user (part of users group), and make sirfuser also part of users, as suggested by Haroon. This way, we don't need to chown most files anymore. Image build time is a bit longer (as it still has to do some chown), but container build time is then very fast.

We have therefore 2 users: jovyan (builds) and sirfuser (runs and mounts /devel).

However, conda is run by jovyan, while bash (in sirf:latest) or jupyter (in service) are run by sirfuser , which could/will lead to confusion.

Note that with this set-up, it should be easy (?) to copy some of your lines at the end of the current Dockerfile for a new jupyterhub service.

I hope this was clear enough, but probably not!

@KrisThielemans KrisThielemans marked this pull request as ready for review June 14, 2021 22:51
@KrisThielemans KrisThielemans requested a review from paskino June 14, 2021 22:51
@KrisThielemans
Copy link
Member Author

@paskino I tried this together with #574 It seems to work quite well. Reasonably fast build time (some chown stuff still), and very fast container start-up. I see no down-sides (aside from the extra complication...).

It should also be almost ready for the jupyterhub changes.

I believe we should merge these PRs. (There's a conflict, which I'll sort out).

@KrisThielemans KrisThielemans force-pushed the docker_opt_permissions branch from 5910cc2 to eb429a1 Compare June 15, 2021 16:03
To avoid having root owning too many files, and to prepare for jupyterhub
(which doesn't want root), we create the user* files using a new user
(currently defaulting to be called jovyan).

entrypoint.sh and sirf-compose will still create sirfuser such that it can
read/write to the mounted volume.

This way, we don't have to change permissions/ownership if most of the files, saving
time. However, it does mean that conda needs to be run by jovyan, which is undesirable.

[ci skip]
add some brackets...

[ci skip]
@KrisThielemans KrisThielemans force-pushed the docker_opt_permissions branch from eb429a1 to 2c7f966 Compare June 15, 2021 16:12
@KrisThielemans KrisThielemans merged commit 5d565e6 into SyneRBI:master Jun 15, 2021
@KrisThielemans
Copy link
Member Author

@paskino I've cleaned up the history and merged it. Let's see what the Travis build says for the generated dockers...

@KrisThielemans KrisThielemans deleted the docker_opt_permissions branch March 9, 2024 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docker: slow startup of container the very first time
2 participants