powerapi-ng / hwpc-sensor

Hardware Performance Counters monitoring agent for containers.
BSD 3-Clause "New" or "Revised" License
14 stars 16 forks source link

Container name resolution fails in docker #26

Closed PierreRustOrange closed 2 years ago

PierreRustOrange commented 2 years ago

Now that the DockerFile uses the powerapi instead of root using CAP_SYS_ADMIN (and thanks for that !!) , the name resolution fails.

Indeed, in target_docker.c the name is extracted from the /var/lib/docker/containers/%.*s/config.v2.json config file, which cannot be read by the user powerapi.

While it does not really bothers me for my use cases, I suspect it might be troublesome for some users who rely on that name to recognize the power consumption of each container.

gfieni commented 2 years ago

Hello, Yes, this is expected as the containers directory is owned by root and we switch to the powerapi user in the container. To quickly fix this problem, one can run the container as root while dropping all capabilities. docker run --cap-drop ALL --cap-add CAP_SYS_ADMIN --user root powerapi/hwpc-sensor For this use case, the much more restrictive CAP_PERFMON capability would be more appropriate. (if supported)

Switching to the powerapi user by default poses more problems than expected for the name resolution. Maybe reverting to the container run as root by default, and let the user choose (with the --user parameter, or equivalent for other deployments) could be more convenient. We could also always run the container as root and changes the user at startup, which will be easier for other deployments.

PierreRustOrange commented 2 years ago

hi, thanks for the answer.

I agree that switching to powerapi by default is probably an issue for most users, it would be better to set the user from the command line. I suggest the following:

Create powerapi group and user in the DockerFile remove the USER powerapi line at the end of the file:

RUN groupadd -g 1000 powerapi && \
    useradd -d /opt/powerapi -m powerapi --uid 1000 --gid 1000 && \

When running the container:

I've just tested this approach and it works fine in both cases.

BTW, I definitely agree with your comment on CAP_PERFMON but is not supported widely enough yet to require it for the moment imho.

gfieni commented 2 years ago

Fixed in bc4df8a2a6fc4655e289aec4718c80b6fec59049