Open kolyshkin opened 3 years ago
One thing runc exec
does and crun exec
does not is parsing /etc/passwd
and /etc/group
. I do not understand why it's needed, since we pass uid:gid as numbers (stringified but still) so the only need for passwd/group lookup is to $HOME value ... which is not used, unless not available from environment.
This needs to be fixed.
Is this a recent performance regression?
Is this a recent performance regression?
Good question. I don't believe it is, but let's check....
I compared with rc10, compiled by go 1.10.8, and the performance seems to be similar.
Choosing the best run among 10, here are the numbers.
git tip:
root@ubu2004:/home/kir/git/runc/tst# time for ((i=0; i<1000;i++)); do ../runc exec xx55 true; done
real 0m21.230s
user 0m10.349s
sys 0m13.599s
rc10:
root@ubu2004:/home/kir/git/runc/tst# time for ((i=0; i<1000;i++)); do ../runc-rc10 exec xx55 true; done
real 0m20.166s
user 0m8.895s
sys 0m16.125s
One thing runc exec does and crun exec does not is parsing /etc/passwd and /etc/group. I do not understand why it's needed
i think this is because runc supports passing username/groupname so we need to read the 2 files to get correct ids out. Does crun support that as well ?
i think this is because runc supports passing username/groupname so we need to read the 2 files to get correct ids out.
No, we (at least in CLI) only accept numeric IDs.
[kir@kir-rhat runc]$ runc exec --help | grep user
--user value, -u value UID (format: <uid>[:<gid>])
Docker supports passing username/groupname, but it reads them from inside the container, not from host which IMHO makes that feature useless (only reason I can think of for using names is matching host IDs for things that correspond to host, like device files).
@kolyshkin
One thing runc exec does and crun exec does not is parsing /etc/passwd and /etc/group. I do not understand why it's needed, since we pass uid:gid as numbers (stringified but still) so the only need for passwd/group lookup is to $HOME value ... which is not used, unless not available from environment.
We fetch the supplementary group IDs from the container even if the user ID specified is numeric, which Docker doesn't do for us IIRC. This is also something we do with the regular config so I'm not sure we can change the behaviour at this late stage. The whole stringification logic is a quirk from when libcontainer used by use directly by Docker, but we probably can't change that either because it's saved to the on-disk state format and changing that leads to very bad issues when upgrading runc.
I'm surprised that crun doesn't do this at all, because it's something AFAIK they need to do in order to have parity with what Docker expects of runc.
@eero-t
Docker supports passing username/groupname, but it reads them from inside the container, not from host which IMHO makes that feature useless (only reason I can think of for using names is matching host IDs for things that correspond to host, like device files).
It's not useless -- how else would you run a program as a non-root user that is configured inside the container (note: this is the mechanism by which the USER
directive in Dockerfiles works as well)? Require sudo be installed in every container? Arguably it makes less sense to use host users because the user configuration on the host is unrelated to the one in the container (outside of volume mounts -- which is related to the example usage you mentioned -- but that's a more generic issue which id mapped mounts might help solve).
@cyphar
It's not useless -- how else would you run a program as a non-root user that is configured inside the container (note: this is the mechanism by which the USER directive in Dockerfiles works as well)?
The point of using name for host mapping is that you do not know what are the numeric IDs, because those differ between distributions & individual host machines, and container orchestration systems are not in control of them. Whereas names for few common groups (e.g. render) and users (e.g. nobody) are fairly standard between distributions.
Unlike with hosts (which are "randomly" assigned by container orchestration), container users have full control of the container contents, they specify what is being run and can check (and often also override) them as needed, so using correct numeric values for container contents is trivial. Expecting that to be the case with hosts, is hopeless without full control of the hosts.
use " github.com/syndtr/gocapability" will call "os.Open("/proc/sys/kernel/cap_last_cap")" (it will run in init() ) even though we don't need it. I find we need it just run runc init. https://github.com/syndtr/gocapability/blob/42c35b4376354fd554efc7ad35e0b7f94e3a0ffb/capability/capability_linux.go#L37 If we can move it to runc "github.com/opencontainers/runc/libcontainer/capabilities" and rewite it, we will use less resource. this package been a long time since the last update.
Need to