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/permissions #550

Merged
merged 5 commits into from
Jun 22, 2023
Merged

Docker/permissions #550

merged 5 commits into from
Jun 22, 2023

Conversation

PGijsbers
Copy link
Collaborator

@PGijsbers PGijsbers commented Jun 22, 2023

Removes the logic about users and permissions for docker introduced in #495.
Instead, introduces a docker.run_as option which can run the docker container as a specific user (root, current unix user, or a specific other user).

The problem with the previous implementation was that it did not work if the docker images were meant to be shared, as the creators uid/gid were embedded in it. I tried to look for a general solution that would work in both cases, but in doing so I found that docker is inconsistent. I had two users, both on ubuntu 22.04.02 and docker cb74dfc, run:

mkdir output
docker run -v $(pwd)/output:/output -u 0:0 --entrypoint=/bin/bash ubuntu:22.04 -c "touch /output/a$(date +%H%M%S)"
docker run -v $(pwd)/output:/output -u $(id -u):$(id -g) --entrypoint=/bin/bash ubuntu:22.04 -c "touch /output/b$(date +%H%M%S)"

One had the root file listed under her own name on the host machine while the second command gave a permission error.
The other had the root file listed under root on the host machine while the second command created it under his name.
Help is welcome, but until then we'll leave it configurable.

Additionally:

  • shrink the docker images by leaving out some additional directories.
  • no longer force docker run as root in aws explicitly, but use the 'default'.

Since the USER is overwritten by `-u` for non-Windows platforms,
which creates issues when the account running the docker image
is not the same as the one that created it.
This is required, because otherwise in docker mode a non-existent
directory is mounted, which is by default locked to `root`
permissions. This in turn makes the benchmark app unable to create
the subdirectories when the image is run as user.
The run_as option is then configurable so that it can be enabled
for people who run into issues. Unfortunately, I observed
different behavior from two systems with the same OS and docker
versions installed. So for now I give up on one unified solution.
@PGijsbers PGijsbers added the container Issues when running in container (docker, singularity) label Jun 22, 2023
@PGijsbers PGijsbers merged commit 429488c into master Jun 22, 2023
@PGijsbers PGijsbers deleted the docker/permissions branch June 22, 2023 19:31
PGijsbers added a commit that referenced this pull request Jun 22, 2023
* Remove ownership changing and starting as user for docker images

Since the USER is overwritten by `-u` for non-Windows platforms,
which creates issues when the account running the docker image
is not the same as the one that created it.

* Dont run docker as root since images no longer have associated user

* Ignore some additional files not needed to run the benchmark

* Create root dir if it does not exist

This is required, because otherwise in docker mode a non-existent
directory is mounted, which is by default locked to `root`
permissions. This in turn makes the benchmark app unable to create
the subdirectories when the image is run as user.

* Further remove user info from docker build and add run_as option

The run_as option is then configurable so that it can be enabled
for people who run into issues. Unfortunately, I observed
different behavior from two systems with the same OS and docker
versions installed. So for now I give up on one unified solution.
PGijsbers added a commit that referenced this pull request Jun 22, 2023
* Update AutoGluon `max_memory` from 0.1 to 0.4 in persist_models (#543)

* Add `optimize_for_deployment` for AutoGluon_hq (#544)

* Reduce training time by 10% if a high_quality preset is used (#546)

* Reduce training time by 10% if a high_quality preset is used

High quality presets perform a post-fit step which takes 10~15%
of total time (by Nick's estimate). To ensure comparisons stay
reasonably fair we pre-emptively tell AutoGluon to use less time,
so that all frameworks' models are based on "max_total_time"
amount of effort.

* Allow preset to be str or list and still reduce if hq or gq

* Add identical markers to identify fit/inferencetime/predict stages (#548)

* Add start_time, stop_time and log_time to failure.csv (#547)

This helps more quickly identify at what stage the failure took place. E.g., if it's just a few minutes in, it is probably setup failure (such as connectivity issues).

* Docker/permissions (#550)

* Remove ownership changing and starting as user for docker images

Since the USER is overwritten by `-u` for non-Windows platforms,
which creates issues when the account running the docker image
is not the same as the one that created it.

* Dont run docker as root since images no longer have associated user

* Ignore some additional files not needed to run the benchmark

* Create root dir if it does not exist

This is required, because otherwise in docker mode a non-existent
directory is mounted, which is by default locked to `root`
permissions. This in turn makes the benchmark app unable to create
the subdirectories when the image is run as user.

* Further remove user info from docker build and add run_as option

The run_as option is then configurable so that it can be enabled
for people who run into issues. Unfortunately, I observed
different behavior from two systems with the same OS and docker
versions installed. So for now I give up on one unified solution.

* Update GAMA for v23.0.0 (#551)

---------

Co-authored-by: Nick Erickson <innixma@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
container Issues when running in container (docker, singularity)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant