The current implementation sets all the environment variables passed in Process.Env in the current process, one by one, then uses os.Environ to read those back. As pointed out in [1], this is slow, as runc calls os.Setenv for every variable, and there may be a few thousands of those. Looking into how os.Setenv is implemented, it is indeed slow, especially when cgo is enabled. Looking into why it was implemented the way it is, I found commit 9744d72 and traced it to [2], which discusses the actual reasons. It boils down to these two: - HOME is not passed into container as it is set in setupUser by os.Setenv and has no effect on config.Env; - there is a need to deduplication of environment variables. Yet it was decided in [2] to not go ahead with this patch, but later [3] was opened with the carry of this patch, and merged. Now, from what I see: 1. Passing environment to exec is way faster than using os.Setenv and os.Environ (tests show ~20x speed improvement in a simple Go test, and ~3x improvement in real-world test, see below). 2. Setting environment variables in the runc context may result is some ugly side effects (think GODEBUG, LD_PRELOAD, or _LIBCONTAINER_*). 3. Nothing in runtime spec says that the environment needs to be deduplicated, or the order of preference (whether the first or the last value of a variable with the same name is to be used). We should stick to what we have in order to maintain backward compatibility. So, this patch: - switches to passing env directly to exec; - adds deduplication mechanism to retain backward compatibility; - takes care to set PATH from process.Env in the current process (so that supplied PATH is used to find the binary to execute), also to retain backward compatibility; - adds HOME to process.Env if not set; - ensures any StartContainer CommandHook entries with no environment set explicitly are run with the same environment as before. Thanks to @lifubang who noticed that peculiarity. The benchmark added by the previous commit shows ~3x improvement: │ before │ after │ │ sec/op │ sec/op vs base │ ExecInBigEnv-20 61.53m ± 1% 21.87m ± 16% -64.46% (p=0.000 n=10) [1]: opencontainers#1983 [2]: docker-archive/libcontainer#418 [3]: docker-archive/libcontainer#432 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Commits

Commits on Dec 23, 2024

libct/int: add BenchmarkExecInBigEnv

libct/int: improve TestExecInEnvironment

tests: add test to check StartContainer hook env

libct/configs: add HookList.SetDefaultEnv

libct: speedup process.Env handling