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

Avoid repeated os.SetEnv calls in container init. #1983

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

brandon-mabey
Copy link

In a Kubernetes cluster with a large number of services (>5000, causing a large amount of environment variables for the container) with guaranteed pods, the runc init process can take a long time to run. This is caused by two issues:

  1. runc init calls os.SetEnv for every environment variable in the bundle, which can be expensive due to the underlying mutex locking and syscall logic.
  2. The cgroup is limited before the runc init process runs. This is outside the perview of runc I believe.

This PR targets the former by removing the os.SetEnv calls, which causes the runc init process to run much faster in cases where there are a large number of configured environment variables in the bundle.

For containers with a large number of configured environment variables
(>5000), os.SetEnv can make container creation take a much longer time
than in normal circumstances. This is due to os.SetEnv locking and
unlocking a mutex on every environment variable added.

By removing the os.SetEnv calls during
container creation, and instead controlling the list of environment
variables manually, this overhead can be removed.

Signed-off-by: Brandon Mabey <brandonmabey@google.com>

// Certain functions that are used later, such as exec.LookPath, require that the PATH
// environment variable is setup with the container's PATH.
os.Setenv("PATH", mappedEnv["PATH"])
Copy link
Member

Choose a reason for hiding this comment

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

Could you be more specific? Wondering it may cause security issues.

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

Successfully merging this pull request may close these issues.

None yet

2 participants