Skip to content

Build package with local UID/GID and fast-retrieve app/package information #168

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

Merged
merged 1 commit into from
Nov 23, 2021

Conversation

gigony
Copy link
Collaborator

@gigony gigony commented Oct 9, 2021

MAP is created with the default UID/GID (1000) by default.
However, if the user has UID which is not 1000 such as 1001, the local input/output folder is mounted with the host's input folder permission (1001) by App Runner and causes the following message when MAP is executed.

...
Reading MONAI App Package manifest...
 > export '/var/run/monai/export/' detected
fatal: Access to the path '/var/run/monai/export/config/pkg.json' is denied.
ERROR: Failed to fetch MAP manifest.
Execution Aborted

This patch builds a Docker image with local UID/GID to resolve the issue.

Also, it retrieves app/package information with the faster method if the OS is not Windows.
(copying app.json/pkg.json through docker cp command is faster than launching container to download config file).

        if sys.platform == "win32":
            cmd = f"docker run --rm -a STDOUT -a STDERR -v {info_dir}:/var/run/monai/export/config {map_name}"
        else:
            cmd = f"""docker_id=$(docker create {map_name})
docker cp $docker_id:/etc/monai/app.json {info_dir}/app.json
docker cp $docker_id:/etc/monai/pkg.json {info_dir}/pkg.json
docker rm -v $docker_id > /dev/null
"""

The bug is confirmed and tested by creating AWS EC2 instance:

AMI: ubuntu/images/hvm-ssd/ubuntu-focal-20.04-amd64-server-20210430
Instance type: t2.large
Storage: 20 GB 

Due to large base image size, only 1.3GB was left after exercise the Tutorial 1 (examples/apps/simple_imaging_app):

(monai) gbae@ip-172-31-0-144:~/monai-deploy-app-sdk$ df
Filesystem     1K-blocks     Used Available Use% Mounted on
/dev/root       20263484 18888788   1358312  94% /
devtmpfs         4069584        0   4069584   0% /dev
tmpfs            4075372        0   4075372   0% /dev/shm
tmpfs             815076      832    814244   1% /run
tmpfs               5120        0      5120   0% /run/lock
tmpfs            4075372        0   4075372   0% /sys/fs/cgroup
/dev/loop0         56832    56832         0 100% /snap/core18/1997
/dev/loop1         34176    34176         0 100% /snap/amazon-ssm-agent/3552
/dev/loop2         33152    33152         0 100% /snap/snapd/11588
/dev/loop3         72192    72192         0 100% /snap/lxd/19647
tmpfs             815072        0    815072   0% /run/user/1001

This patch is verified in the AWS EC2 instance by using the following test package:

pip install -i https://test.pypi.org/simple/ monai-deploy-app-sdk==0.0.165

Resolves #165

P.S. It might be better to use 'docker' client library instead of executing docker with shell command. We will address the issue later.

Signed-off-by: Gigon Bae <gbae@nvidia.com>
@gigony gigony added the bug Something isn't working label Oct 9, 2021
@gigony gigony self-assigned this Oct 9, 2021
@MMelQin
Copy link
Collaborator

MMelQin commented Oct 9, 2021

Yep, I requested Docker SDK be used at the start of the project.

docker_build_cmd = ["docker", "build", "-f", docker_file_path, "-t", tag, temp_dir]
docker_build_cmd = f'''docker build -f "{docker_file_path}" -t {tag} "{temp_dir}"'''
if sys.platform != "win32":
docker_build_cmd += """ --build-arg MONAI_UID=$(id -u) --build-arg MONAI_GID=$(id -g)"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, the user ID and group of the current logged-on user is used to as MONAI_UID in the Docker that can potentially be run on any other server or in another logged-on user's env. Does not sound right to me.

General security principal is that you never do this, instead, a service account needs to be defined, and at deployment time, the account is configured/assigned to the app. It will be important to declare the intensions in this code.

This will work if the same user creates the MAP and then runs the MAP in the same env. What happens if this MAP is run by any other user (say 1000 creates it, 1003 runs it, and 1003 could be in a completely different set of groups)?

if sys.platform == "win32":
cmd = f'docker run --rm -a STDOUT -a STDERR -v "{info_dir}":/var/run/monai/export/config {map_name}'
else:
cmd = f"""docker_id=$(docker create {map_name})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole set of commands can be encapsulated in a method, and Docker SDK used to make it more robust and easier to handle exception.

In any case, It is a good change; don't run docker when one only needs to copy files from within the Docker image.

Copy link
Collaborator

@MMelQin MMelQin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix correctly gets the current UID/GID to create Docker image, and the image will run fine with the current user running it, but what happens if anther use runs the MAP?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] ERROR: Failed to fetch MAP manifest.
2 participants