openml / automlbenchmark

OpenML AutoML Benchmarking Framework
https://openml.github.io/automlbenchmark
MIT License
391 stars 130 forks source link

Fixed user and sudo under docker #495

Closed alanwilter closed 1 year ago

alanwilter commented 1 year ago

This fixed several issues I created with my previous RP re docker.py.

Now it runs under $USER, grant permissions and allow sudo.

sebhrusen commented 1 year ago

@alanwilter, to be clear, this fixes issues introduced with https://github.com/openml/automlbenchmark/pull/464, correct?

Could you please detail a bit more the kind of issues that were introduced by this? For us to understand the changes, for other users that may face the same problems… thanks.

PGijsbers commented 1 year ago

@alanwilter I picked up this PR since it went stale. I made sure Windows support was not broken. Additionally, I added HDF5 libraries required to install tables. After the patches building and running in docker mode works for me on both MacOS and Windows. Could you confirm that after these patches the original program you fixed is still resolved with your setup?

PGijsbers commented 1 year ago

Thanks @alanwilter for fixing the initial problem!

alanwilter commented 1 year ago

You're welcome. I still need to find time to check the out https://github.com/openml/automlbenchmark/pull/495#issuecomment-1453378049 But I have no idea when I will do that.

PGijsbers commented 1 year ago

Since there was no response for 3 weeks, I wasn't sure if you were still interested in trying so I went ahead and merged. If you do ever find the time to check, we would appreciate a quick confirmation :) But there's no rush, we'll operate under the assumption the problem is fixed (because, as far as I can tell, it is).

PGijsbers commented 1 year ago

This introduced an issue when building public images: the image is set up for the builder, but the user will start the container under a different uid, which leads to permission errors. I think we should maybe remove any user information from the docker container, and instead create/assign permissions on startup with something like:

set -e

if [ $UID = 0 ]; then
  echo "Docker started as root, not changing file permissions."
  exit 0
fi

user_id=$UID
echo "root" | su -c "adduser --disabled-password --gecos '' -uid $user_id amlb"
echo "root" | su -c "echo '%sudo ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers"
echo "root" | su -c "chown -R amlb:amlb /home/amlb"
echo "root" | su -c "hown -R amlb:amlb /bench"
echo "root" | su -c "passwd -d amlb"
su "amlb"
PGijsbers commented 1 year ago

There's probably cleanier ways to set this up (though whatever UID is inserted is unlikely to be in de sudo file), but my experience lacks here.

PGijsbers commented 1 year ago

@alanwilter something like that should still solve your issue, right?

alanwilter commented 1 year ago

Unfortunately I'm not working on related projects anymore so I'd say do what is better for you guys. If I'll come back one day then I will check and test all that.

PGijsbers commented 1 year ago

Thanks for letting us know!