-
Notifications
You must be signed in to change notification settings - Fork 865
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
Environment Var For --allow-run-as-root #4451
Comments
I hear what you're saying, but I'm not sure it's the right approach. We put the Specifically: If Docker requires running MPI jobs as root, it seems like a better solution would be to fix that (i.e., enable Docker jobs to not run as root). Just my $0.02; sorry to be a grumpy old man... |
Thank you for the detailed answer. I understand your concerns and I fully share your thoughts on regular systems. Nevertheless, there is an important difference to be aware of when running something in a Docker container. Running containers are fully isolated (tm), which means it is not being started as hostsytem root (but from a user with user rights). Only internally, inside the container, the (container-)root user is active - which really is fine and save. The only thing that can happen is, that the "active copy" of an image, which when it is running is called container, self-destructs. But this does not affect the image it came from (which can be restarted just from that state) nor does it harm the host system.
Yes, it's a fundamental crazyness in Docker to be inside as a new user (root) and assumes something like full isolation really exists. Other solutions such as Singularity spin this way better, keeping the external user transparently as the in-container process owner and adding a transparent layer on top of the FS. Of course, I can also add a local user just to satisfy MPI inside docker (which is still another user), but this is adding no safety net (since it is already there) and is just a hard to maintain work-around. Long story short: for Docker |
@ax3l, when we've looked at best practices for running MPI in Docker containers (under Amazon ECS or AWS Batch), we didn't run into any real issues using a "normal" user to run MPI applications, and were much happier with the overall posture of the system. Yes, you're right that it's the container and not the host system running a root process, but there's still plenty of reason to be careful. For example, you do create some natural blast radius for filesystems mounted into your container (like an NFS/Lustre system with everyone's data on it). |
I did that as well as a work-around in the past but this complicates, e.g. the import of such Docker images into other solutions such as Singularity. Also, see the argument below about "who creates images" for a HPC workflow (the user).
Can you please elaborate a little on that? In what sense? In an ideal world, a (containerized) process should never have more rights (including FS access) than the user that executed it, independent of what userid it presents internally. In that sense, one would not mount a HPC file system directly inside docker but on the host system and then forward it with the same rights a user has.
I would not do that anyway (see above) but let FS mounts always be managed by the actual host system. Then only "docker mount" a user-visible view on a FS. One also needs to think about container workflows, where users should be allowed & flexible enough to adjust their containers and then could do anything to the FS if they control the direct mount / lustre clients / ... We would not want that. |
I'm not sure I understand your reply -- you seem to be saying that the container should be running with limited rights on the host. I was not under the impression that that was (currently) possible. E.g., if you are root in the container, you can be root on the underlying filesystem. Regardless, yes, we know that the Meaning / to be blunt: "it's awkward" is not a strong enough reason for us to implement another trapdoor to already-unsafe behavior. Is there something you can't do -- in any way -- without having |
err...allow-run-as-root is only available on the cmd line - there is no envar or config file option for controlling it. I believe that actually was the request |
I know. I'm asking if there's something that can't be done (via any mechanism) unless that functionality is invoked via an env var or config file. |
Yes, running simple mpi programs and test suites that just run Isn't that bad enough? I think a person that sets a central config file to allow run as root or even an environment variable should be aware of what he/she is doing, we don't need to make it awkward. |
I just ran into this issue as well. Requiring non-root in docker can be very inconvenient, since many existing dockerfiles and recipes assume a root user. Running as a non-root user often requires extra work. I understand that having some default safety features can be nice, but this particular method for implementing it is resulting in everyone using docker to either mix infrastructure details into their code, or modify the rest of their infrastructure which uses (or shares dockerfile images with code which runs) mpi to allow running as non-root. |
@ax3l To be clear: that was not my question. I asked if there was something that can't be done (via any mechanism) unless that functionality is provided via an env var or config file.
I'm trying to ask if there is a technical reason to require an environment variable or config file for some reason. The "I don't want to modify my Dockerfile" motivation is already established -- but that's not a technical reason. As I said above, we do provide an [intentionally] awkward way to get this functionality. Is there a technical reason to require that we provide this functionality via env var and/or config file option (other than "I don't want to modify my Dockerfile")? Additionally: is anyone leaning on Docker to fix this scenario? Forcing everything to run as root is not good practice, and is the real root of the problem (see what I did there? 😉). One thing I'm not clear on: even if we provide an env var or config file option for this functionality, you have to add that to the container somehow (i.e., modify/add this to the Dockerfile), right? Am I missing something? |
There isn't anything that can't currently be done inside of docker with the current code (by any mechanism). To be more clear about why we see this is an issue, having to modify a dockerfile at all is not the problem. The problem is that it requires making changes to components completely unrelated to mpi as running as root is a standard practice in docker. For example, if you already have a recipe for installing or configuring a package for use as root used throughout your infrastructure, you will no longer be able to include that recipe in your dockerfile without conditioning that recipe on whether or not mpi will also be used. Then you would also need to maintain a root and a non-root recipe for every component. Alternatively, you could modify every component in your system to support non-root, just to use mpi. This additional coupling between mpi 'config' and every other component of the system is the problem. Setting an environment variable on the other hand is not going to have any unintended consequences with the rest of the system. |
@zach-nervana Just to be clear: you can run Open MPI jobs as root, you simply have to specify
Isn't the same thing true with adding |
This is true, but as you've mentioned you don't really want your code to always have that flag set as it may be unsafe for many cases. The decision to run as root is a deployment level concern and so would be most naturally be set inside of a docker container/through an environment variable instead of baking it into your code, which might be deployed by different people in different contexts. |
You have to modify the Dockerfile to either set an environment variable or set this CLI flag, right? If so, I fail to see the difference. Additionally, you basically said above "everyone runs as root in a Docker", so you're somewhat contradicting yourself. However, if that's not true (i.e., you can sent an environment variable inside a Docker container from outside the Dockerfile), then why can't you do something like: # Outside the container
user$ export MY_MPIRUN_CLI_ARGS="--allow-run-as-root"
user$ start_the_container my_dockerfile
# Inside the container
root$ mpirun $MY_MPIRUN_CLI_ARGS ... Alternatively, one could avoid the environment variable issue altogether by just creating a user in the container: # Inside the container
root$ adduser mpiuser
root$ su -c "mpirun ..." mpiuser Clearly I am missing something in your argument... 😦 |
I just realized I missed another point from your previous reply: using Meaning: you can pass that CLI argument in all your Dockerfiles, regardless of whether they will be deployed as root- or non-root-runnable situations. It seems like there are 3 scenarios:
Taking a completely simplistic view: if you want to run as root, then embrace it and always use Am I missing something? |
Btw, is --allow-run-as-root OpenMPI only? That means it adds to the diversity of mpirun/mpiexec flags in the MPI eco system if not controlled by an env var (which other implementations would just ignore).
This is also something which is not very desirable from an application perspective to be honest :-)
|
Yes,
As mentioned above, |
Thank you for the feedback, I understand your argument. Nevertheless "deliberately intrusive" is intrusive in more places than you might think and where it is not required to be so. Let's take for example install routines and CMake scripts that include tests ("make test"), e.g. with CTest. They are in principle possible to work with small examples with any MPI implementation as long as What happens now when someone decides to perform this routine in an environment, where the "root" user is ok and save? It will fail with your clean error message for good. All right, so now they rethink their environment and come to the conclusion it is save to run the test as root user, e.g. be cause they are partitioning a docker container. Now they could set an environment variable and re-try if they are really sure it is save to proceed as root. The alternative is project-specific options, e.g. in An environment variable would unify the workflow by default. Set it, re-do But what if this env control does not exist? People, like we, will guess and add switch-cases at places where the MPI flavor is unnecessary to know - such as CMake scripts. And we/people hard-code work-arounds like this: that is making the install + make test more robust since the test will actually run (as long as OpenMPI is used). Now you can say: "huh, that's dangerous, you should make this an option!" - and you are right. So we are back to project-specific options for tests and launches because we developers wrap MPI calls. Instead, we could have of a common way to control it across the whole eco-system via an environment var. OpenMPI will not be alone with such an env var. Autotools react on |
This is unnecessary. Some modern container solutions such as singularity partition your container (configure - make - make test - make install) as container root user but run the container itself with a lightweight overlay of the filesystem as the current user exactly as you recommend it. Adding new users and unused directories is superfluous. (The point is: of course I do run full integration test in the end for a container with reduced user rights. But I won't have access to temporary "make test" stages of individual components anymore for component tests.) Also, most of the time one will just import a docker image - in both cases partitioning the container is done as "container root user". |
The OpenMPI devs aren't good at listening (open-mpi/ompi#4451).
Wow. This doesn't look healthy. I'm all for discussion but bashing people on commit message is just something else. I'm closing this as it seems to be resolved. |
Hi, OP author here. I am not supporting the personal language in the linked commit nor am I affiliated with it in any kind. I would like to continue the discussion based on objective feedback from various applications that benefit from OpenMPI. There are many people applying these work-arounds in scripts, CIs, etc. where they could control it better from the outside, e.g. when they test against various MPI applications. The issue is not resolved yet, please let us try to find a solution that does not divert MPI implementations for users, packagers and integrators. |
Adds a new recognized environment variable `OMPI_ALLOW_RUN_AS_ROOT` that can be set alternatively to passing `--allow-run-as-root` to orte. Implements proposal in open-mpi#4451 Alternative solutions are very welcome!
Hi, I will reopen this for discussion. I understand your frustration and I'm in no place to weight in on this but this is an interesting topic. So in our face to face meeting in March, we brought this issue up and I remember everyone is against having an env var. We have some catastrophic mistakes in the past when someone run mpirun with root privilege and we put some protection in place which is not really difficult but awkward to use (intentionally). It is very easy to detect Open MPI. You can have a script to locate the command Your ompi solution seems to work as well but I know you don't want to have your own custom ompi. |
My 2 cents: I'm testing (in a container) the code that someone else wrote but unit tests from the project are failing due to running |
The discussion is already long, but I'll add my $0.02 as well: We have the same issue in the deal.II and ASPECT projects (https://github.com/dealii/dealii, https://github.com/geodynamics/aspect): We have extensive testsuites that call What's particularly awkward is that if you already have a script that does this:
(In practice, this takes 500 lines and consequently is pre-packaged into a script that's handed around.) The problem here is that you need to (i) know which files From the perspective of the person who sets up the docker image, calling a script like the one above is atomic, or at the very least the individual commands inside it are atomic. You can't slice into these commands, because the person who sets up the container is (i) generally without the detailed knowledge of where to the files that need to be edited are, and (ii) even if they knew, they'd make themselves dependent on the source structure of the project. In the case of continuous integration services, you want to test patches. If a patch moves an automatically generated file somewhere else, you'd also have to adjust the docker image that's supposed to test the patch. That's not feasible in practice. I do understand your reluctance to make it easier to run as root. That's good practice. But I fail to see how people would accidentally shoot themselves in the foot with an environment variable.
We'd all be happy to jump through these hoops, and you make the procedure cumbersome enough that everyone who casually thinks about it has to at the very least read the documentation carefully. |
Hmmm...now that is actually a quite reasonable proposal. I have no objection to implementing that approach. |
@bangerth That's actually a very interesting idea (2 env variables). I think it might satisfy our requirement of "if you do this, you should really know what you are doing and not do it by accident and you accept all responsibility for what happens." It's not going to prevent people from copy-n-pasting this without realizing what they're doing, but it will help -- i.e., some people might actually read what they're pasting. |
Thanks, I am glad we found a compromise 👍 As a personal note, I think the solution is patronizing/nannying your users more than you might think. But I can live with that. As constructive criticism: instead of repeating twice that running as root is dangerous, rather document clearly what the design choices of OpenMPI are that make running as root insecure/potentially harmful. That is totally fine and users can then judge if these risks apply to their use case. Make it a chapter of your docs. Say once for an option it's dangerous in its doc string and refer to the chapter. |
I think you perhaps don't realize the situation, or maybe are only seeing it from your own perspective. This wasn't us being arbitrary for the sake of it. We had actual cases where users rendered their systems useless chunks of metal because of this problem, and we absorbed their anger. Documenting the risk made no difference to the situation - frankly, the people most at risk from making this mistake are also the ones who don't read the documentation. It is a rather common trait nowadays - I confess to rarely reading the manual myself. The more convinced someone is that they know what they are doing, the less likely they are to read the documentation or even the man page. Having someone indicate they intend to override the protection, and then confirm their choice, is actually a rather common practice. It is now standard on most potentially destructive operations (e.g., erase a partition), so I don't think we are being patronizing here. |
Thank you for your thoughts. I think it would help if the actual issues that were seen are referenced (mailing list threads? bug reports?) and he underlying architectural reasons are documented. All a user will find in the official docs seems to be this: which is as brief as any answer in this thread :-) Googling deeper (which user's probably won't) shows (fixed) bugs like these: But can you maybe add more insight to the docs/man pages? If one has a section on why running as root is risky, why not document the underlying reason for users? |
You cited the exact issue: https://svn.open-mpi.org/trac/ompi/ticket/4534 In brief: when MPI apps start up, they create a shared session directory tree. When they shut down, they remove the entire tree. There was a bug in Open MPI that caused the recursive directory removal to go higher than intended -- i.e., it ultimately could end up invoking Specifically: it's not that Open MPI has a design that is risky -- it's just that bugs happen. Bugs that occur when running as a regular user can be bad enough; bugs that occur when running as But the issue underscored the axiom that userspace applications should be run by regular users. We shouldn't have to defend this -- the whole POSIX system was designed around this philosophy. Indeed, not running applications as local administrator is an industry-wide best practice (indeed, run all applications with the fewest possible permissions). Some -- not all -- containers communities have chosen to go a different way. You may view our warnings as patronizing/nannying; we view them as fair warnings. Please -- let's end this discussion. Everyone has stated their opinion. No one is going to change their mind at this point. We have conceded and given you an option that we didn't want to. Please let that be enough. |
Thanks, that's the exact info I was looking for. |
Maybe you can do (from a Chinese page): orte/tools/orte-dvm/orte-dvm.c and delete the corresponding source code. Then you can get rid off "--allow-run-as-root" forever. |
Two non-obvious things about CI testing in Travis and CircleCI (and relatedly Docker testing infrastructure): MPI doesn't like being run as root. An example of the heated debate why: [1]. CircleCI has some issue reading from the home(?) directory (which it calls `working_directory`) when you are not root. The symptom was an inability to read the /root/project directory as a non-root user. In my local testing in Docker, this was never an issue. I worked around this by moving the build directory to /tmp/. [1] open-mpi/ompi#4451
Two non-obvious things about CI testing in Travis and CircleCI (and relatedly Docker testing infrastructure): MPI doesn't like being run as root. An example of the heated debate why: [1]. CircleCI has some issue reading from the home(?) directory (which it calls `working_directory`) when you are not root. The symptom was an inability to read the /root/project directory as a non-root user. In my local testing in Docker, this was never an issue. I worked around this by moving the build directory to /tmp/. [1] open-mpi/ompi#4451
Two non-obvious things about CI testing in Travis and CircleCI (and relatedly Docker testing infrastructure): MPI doesn't like being run as root. An example of the heated debate why: [1]. CircleCI has some issue reading from the home(?) directory (which it calls `working_directory`) when you are not root. The symptom was an inability to read the /root/project directory as a non-root user. In my local testing in Docker, this was never an issue. I worked around this by moving the build directory to /tmp/. [1] open-mpi/ompi#4451
Two non-obvious things about CI testing in Travis and CircleCI (and relatedly Docker testing infrastructure): MPI doesn't like being run as root. An example of the heated debate why: [1]. CircleCI has some issue reading from the home(?) directory (which it calls `working_directory`) when you are not root. The symptom was an inability to read the /root/project directory as a non-root user. In my local testing in Docker, this was never an issue. I worked around this by moving the build directory to /tmp/. [1] open-mpi/ompi#4451
Two non-obvious things about CI testing in Travis and CircleCI (and relatedly Docker testing infrastructure): MPI doesn't like being run as root. An example of the heated debate why: [1]. CircleCI has some issue reading from the home(?) directory (which it calls `working_directory`) when you are not root. The symptom was an inability to read the /root/project directory as a non-root user. In my local testing in Docker, this was never an issue. I worked around this by moving the build directory to /tmp/. [1] open-mpi/ompi#4451
Fix bugs in tools used to manage the gyselalibxx repo. In the static analysis a bug is fixed where directive quoting errors were reported on the wrong file. A second bug is fixed where the cppcheck output did not affect whether the test passed or failed. In the github CI tools environment variables are set to continue using OpenMPI despite the use of root in the docker file (necessary to avoid permission errors when creating/copying files). See open-mpi/ompi#4451 for more details See merge request gysela-developpers/gyselalibxx!655 --------------------------------------------
Fix bugs in tools used to manage the gyselalibxx repo. In the static analysis a bug is fixed where directive quoting errors were reported on the wrong file. A second bug is fixed where the cppcheck output did not affect whether the test passed or failed. In the github CI tools environment variables are set to continue using OpenMPI despite the use of root in the docker file (necessary to avoid permission errors when creating/copying files). See open-mpi/ompi#4451 for more details See merge request gysela-developpers/gyselalibxx!655 --------------------------------------------
Background information
What version of Open MPI are you using?
openmpi 3.0.0
Describe how Open MPI was installed (e.g., from a source/distribution tarball, from a git clone, from an operating system distribution package, etc.)
from source, via the spack package manager inside a Docker container.
Please describe the system on which you are running
Details of the problem
I would like to control the
--allow-run-as-root
flag via an environment variable (or config file) as well.Running as root in Docker containers is fine and partitioning by exporting an environment variable (e.g. autotools equivalent is
FORCE_UNSAFE_CONFIGURE=1
) would make it way easier to interact with mpirun/mpiexec for deeply integrated workflows (where directly editing the final mpi call might not always be possible).The text was updated successfully, but these errors were encountered: