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

Stop dropping root in segmenter container #22

Merged
merged 7 commits into from
Feb 27, 2024

Conversation

ethanjli
Copy link
Member

This PR reverts a change introduced in #17, which had dropped the user from root to pi for security reasons. However, that caused problems because bind mounts of nonexistent directories on the host are created by the Docker daemon (which we're running with root privileges) with root ownership, so that the segmenter container itself is unable to access such directories; and because subdirectories of /home/pi/data/img are created on the host by the Python hardware controller with root ownership, so that the segmenter container is unable to create done.txt files under those subdirectories (as reported by @tpollina in https://planktoscope.slack.com/archives/C015K99AJAE/p1708530438903539).

A longer-term solution would be to switch from Docker to Podman (so that we can run all these containers as a non-root user) and/or to make the Python hardware controller run without root. For now, this PR just reverts the segmenter to run as root so that it won't break in unexpected ways on files/directories created with root ownership.

@ethanjli
Copy link
Member Author

I don't encounter any errors in basic and full testing (i.e. with a real dataset) of the modified segmenter, so I'll merge in this PR now.

@ethanjli ethanjli added this pull request to the merge queue Feb 27, 2024
Merged via the queue into main with commit ebd357f Feb 27, 2024
4 checks passed
@ethanjli ethanjli deleted the hotfix/segmenter-file-permissions branch February 27, 2024 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

1 participant