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-compose with minimal PID namespace and non-root permissions #2397

Merged
merged 5 commits into from
Jul 19, 2021
Merged

Conversation

mariuspod
Copy link
Contributor

@mariuspod mariuspod commented Jul 18, 2021

What I did

  1. I reduced the number of processes visible in both containers to only erigon and rpcdaemon which is sufficient
  2. I created an erigon user in the thorax/erigon image and run all containers with that user for the benefit of consistent data permissions

How I did it

  1. I used the erigon container PID namespace instead of the entire host namespace.
  2. After some digging I found out that when the volumes are initially created by docker-compose they will be owned by root which is causing problems in various scenarios. Therefore make docker-compose now takes care of creating the initial docker volume sub-directories erigon, erigon-grafana and erigon-prometheus
    I've also created a erigon user in the Dockerfile and using the same UID:GID combination for the prometheus and grafana containers.

Results

  • volume directories are always owned by the erigon user defined in Dockerfile
  • running dockerized erigon from scratch works with or without XDG_DATA_HOME set
  • running dockerized erigon with existing data works in most common cases
  • tested on linux/mac

I hope this will be useful in 90% of the cases 😂

@mariuspod
Copy link
Contributor Author

I just realized another thing, please don't merge yet.

…rs in favour of erigon user in Dockerfile. Adapted Makefile to pre-create mounted volumes to avoid permission issues at runtime.
@AskAlexSharov
Copy link
Collaborator

Do we need? https://github.com/yorickdowne/eth-docker/blob/main/erigon/Dockerfile.source#L22

@mariuspod mariuspod changed the title Feat: only share the erigon container PID namespace with rpcdaemon in… docker-compose with minimal PID namespace and non-root permissions Jul 18, 2021
…(1000:1000) regardless of the user that invoked the make command.
@AskAlexSharov
Copy link
Collaborator

I told with MDBX author. He said:

  • just don’t open DB from different processes with same PID (what happening in different docker containers). It using PID - to mark locks and if another process dies - main process can clear all this locks (by looking at PID).

can you try idea “processes in different containers must have different PID”?

@AskAlexSharov
Copy link
Collaborator

As I remember - using non-root user in Docker makes it very complicated to get inside container. Is it true?

@mariuspod
Copy link
Contributor Author

mariuspod commented Jul 18, 2021

Right now we have:

$ docker exec -it mariuspod_erigon_erigon_1 ps
PID   USER     TIME  COMMAND
    1 erigon    3:54 erigon --datadir=/var/lib/erigon --metrics --metrics.addr=
   21 erigon    0:00 rpcdaemon --datadir /var/lib/erigon --private.api.addr=eri
   64 erigon    0:00 ps

This looks like when running without docker except there are only the three listed processes.

So basically erigon and rpcdaemon share the same process namespace and both processes have different PIDs although running in separate containers. erigon is started first so it obtains the file lock. It looks like rpcdaemon is getting a shared-lock but I need to debug it more to understand how this could break now.
Do you know where the PID is saved for the locking in mdbx ?

Re non-root user: you can always override it with docker run -it --user root thorax/erigon sh

@AskAlexSharov
Copy link
Collaborator

Do you know where the PID is saved for the locking in mdbx ? No, but probably in .lock file - in same dir with mdbx.dat

@AskAlexSharov
Copy link
Collaborator

“ can always override it” - yes, just unclear when need override and when don’t

@mariuspod
Copy link
Contributor Author

What do you mean by that ?
I guess it depends a bit if the binary inside the docker can also run as non-privileged user.
I'm running it since several hours now from my fork as unprivileged erigon user and my node is synced and the grafana dashboard is showing the charts 👍

What would you like me to test ?

@AskAlexSharov
Copy link
Collaborator

All good. Will test mac tomorrow

@mariuspod
Copy link
Contributor Author

@AskAlexSharov
I broke it on mac 😢
The good thing is I have a test machine now.
Please consider this as draft now until I found a fix.

@mariuspod
Copy link
Contributor Author

@AskAlexSharov fixed it 🎆
it's running fine for me on linux and mac.

Makefile Outdated
@@ -22,8 +23,12 @@ docker:
docker build -t turbo-geth:latest --build-arg git_commit='${GIT_COMMIT}' --build-arg git_branch='${GIT_BRANCH}' --build-arg git_tag='${GIT_TAG}' .

docker-compose:
# Uses host's PID,UID,GID. It required to open Erigon's DB from another process (RPCDaemon local-mode)
UID_GID=$(shell id -u):$(shell id -g) docker-compose up
@if test -n "$(XDG_DATA_HOME)"; then \
Copy link
Collaborator

Choose a reason for hiding this comment

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

are you sure we need create folders manually?
On mac folders auto-created under my user (not root).

Copy link
Contributor Author

@mariuspod mariuspod Jul 19, 2021

Choose a reason for hiding this comment

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

e.g. https://github.com/mariuspod/erigon/blob/devel/docker-compose.yml#L10
The left part of the second colon is the local volume path which is expanded and created as root user on linux.

This is causing permission issues later on, that's why I'm pre-creating them with the non-root user on the host so that the volume can just pick it up and not change the permissions at runtime.

@AskAlexSharov
Copy link
Collaborator

@mariuspod , any profit from supporting ERIGON_HOME if we already support XDG_DATA_HOME?

@mariuspod
Copy link
Contributor Author

@AskAlexSharov
you're absolutely right, I removed it. I just had it there because I needed a Makefile variable.
Moved the check down near the docker-compose rule. Unfortunately it's not possible to set a variable inside the Makefile rule without applying eval tricks 😅

@AskAlexSharov AskAlexSharov merged commit d085bf9 into erigontech:devel Jul 19, 2021
@AskAlexSharov
Copy link
Collaborator

perfecto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants