ROCm / ROCm-docker

Dockerfiles for the various software layers defined in the ROCm software platform
MIT License
432 stars 65 forks source link

add rocm-user to video group #75

Closed MathiasMagnus closed 3 years ago

MathiasMagnus commented 3 years ago

The docs on the landing page demonstrate the recommended way of launching rocm-terminal, however the option used there (--group-add) is unavailable when used in tandem with GitLab runners. The [runners.docker] section of /etc/gitlab-runner/config.toml doesn't expose this docker option, hence in all of our CI scripts, any command actually using the GPUs need to be prefixed with sudo. This burns engineer time every time someone stumbles upon this every 3-4 months by seeing errors in CI of devices not being present.

I see no reason why rocm-user couldn't be made a member of the video group when the image is created; I can't think of a use case where one wishes to run this image purposfully wanting to run device code with elevated priviliges and only be able to run them that way.

Quote from SO:

If --group-add option alone is specified without --user and the image does have the user declared( via USER instruction in Dockerfile from which the image got created), group modifications happen to the declared user in the container.

This addition to the recommended command-line has no effect on the interplay of the container and the host system, it is totally self-confined. If there's no use case behind rocm-user being absent from the video group, please accept the PR, as it's burning the time of users for no good reason.

Tested, it works as intended.

sunway513 commented 3 years ago

Hi @MathiasMagnus , your assessment is correct, we'll take this PR. Thanks for the contribution!