Tidy up development images

mantidproject / dockerfiles

Docker images relating to Mantid

https://hub.docker.com/u/mantidproject/

GNU General Public License v3.0

3 stars 7 forks source link

Tidy up development images #5

Closed DanNixon closed 5 years ago

DanNixon commented 5 years ago

This does the following things:

Default to a non-root user
Ensures user environment is set correctly (including USER environment variable)
Allows RW to host filesystem as your host user
Includes ParaView src and build in more sensible lcoations

This fixes:

Two failing unit tests
Issues where you have trouble with root filesystem access (cc @soininen )

To test:

Try the new images out (I have copies to test on my Docker Hub account) and make sure they work. See the updated documentation, running them is slightly different now (you must pass in your host user and group IDs)
See that once built, all unit tests pass inside the container
See that build time, disk usage and kernel CPU usage are not overly affected

DanNixon commented 5 years ago

I've pushed an updated set of images that include the sudo changes.

DanNixon commented 5 years ago

@martyngigg That's odd, at that point everything should be running as the abc user and groupmod and usermod should have already changed ownership of the home directory.

Edit: I don't get those errors.

Was this on a source build as I have not pushed updates to Docker Hub yet.

martyngigg commented 5 years ago

@DanNixon I thought that was odd too. I pulled the latest image from your account as mentioned in the original comment.

DanNixon commented 5 years ago

@martyngigg Ah, that may be it. I've pushed the latest versions now.

martyngigg commented 5 years ago

Thanks for pushing the updates. I don't appear to get the warnings any more.

One thing that just occurred to me that would make them slightly easier to use also would be if the data volume was mounted at $HOME/MantidExternalData as it's the default if no -D option is provided. It would then remove the need to specify -DMANTID_DATA_STORE=/mantid_data on new cmake builds. Just a thought but not required.

DanNixon commented 5 years ago

@martyngigg Yes, that would be nicer, however it could drastically slow down the container start up time as usermod (see entrypoint.sh) will attempt to do a recursive owner change (currently it is only done for the directory itself) if the data directory was under $HOME.

Given that there are already several parameters to specify I'm fine with having to specify this manually.

martyngigg commented 5 years ago

Ok.

Just out of interest, would a symbolic link work?

DanNixon commented 5 years ago

Possibly, not sure if usermod would try to follow it or not.

The default of chown is to dereference links so I'd assume it wouldn't make a difference (usermod doesn't specify how links are handled).

I'll give it a try anyway.

martyngigg commented 5 years ago

Thanks. Shall I merge this and that can be a new PR? This is a good set of improvements already.

DanNixon commented 5 years ago

Yes, that's probably best.

Cheers