-
Notifications
You must be signed in to change notification settings - Fork 5
Add support for rootless containers #8
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
Conversation
87dbec1 to
035b0e6
Compare
|
While I would prefer f-strings (or |
|
First of all: thanks for your PR! I work with rootless Docker containers on a daily basis but unfortunately do not use rootless PodMan containers at the moment. With regards to your PR: the daemonless nature of PodMan indeed makes it hard to check rootless containers. Unfortunately this approach has some shortcomings we still need to cover before merging. I mostly work in the RHEL-ecosystem so I tested your wrapping-command on RHEL 7/8/9 platforms. Regarding pylint's What's the reasoning behind the additional |
035b0e6 to
5f478a8
Compare
|
I've had some more thoughts on your PR. The way you currently implemented the feature it will introduce a breaking change and decrease compatibility across OS platforms. How about making the Probably something like this: def get_args():
# [...]
parser.add_argument("--rootless", required=False, action='store_true',
help="Check rootless container (via systemd-run)",
type=bool, default=False, dest='rootless')
parser.add_argument("-u", "--user", required=False,
help="Username of container (only rootless)",
type=str, dest='user')
# [...]
def get_container_stats(args, docker_env):
# [...]
# systemd-run wrap-command for checking rootless podman container
rl_wrap_cmd: list = ['/usr/bin/systemd-run', f'--machine={ args.user }@', '--quiet', '--user',
'--collect', '--pipe', '--wait']
pm_stat_cmd: list = ['podman', 'stats', args.container_name, '--no-stream', '--format',
'"{{.Name}},{{.ID}},{{.CPUPerc}},{{.MemUsage}},{{.NetIO}},{{.BlockIO}},{{.PIDs}}"']
if args.rootless is True:
# Prepend "podman stats" with "systemd-run" wrapper
pm_stat_cmd = rl_wrap_cmd + pm_stat_cmd
result = subprocess.run(pm_stat_cmd,
shell=False,
check=False,
env=docker_env,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
# [...]
def get_container_pslist(args, docker_env):
# same principle as get_container_stats() |
|
have an error message for nrpe + rootless container montitoring, ideas? details: #7 (comment) diags on rocky linux 9.1 (/usr/bin/systemd-run --machine=mysql-user@ --quiet --user --collect --pipe --wait whoami) /usr/bin/systemd-run --machine=nrpe@ --quiet --user --collect --pipe --wait whoami |
|
Apologies @m-erhardt I think a notification was lost and this drifted off of my radar. I agree making it optional is probably the best approach, although I was trying to avoid that and find a one-size-fits-all solution. Regarding |
|
I'll need to do some more testing, but it looks like using cgroupsv2 and |
|
I know you weren't thrilled about importing |
Adds support for providing a username to access rootless containers. Defaults to root.
Fixes #7