-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Fix net.ipv4.ping_group_range with userns #8779
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
|
Hi @rata. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
I'll create a backport for 1.7 when this is merged, if you all agree :) |
d7ae6c5 to
bc9b45f
Compare
userns.RunningInUserNS() checks if the code calling that function is
running inside a user namespace. But we need to check if the container
we will create will use a user namespace, in that case we need to
disable the sysctl too (or we would need to take the userns mapping into
account to set the IDs).
This was added in PR:
containerd#6170
And the param documentation says it is not enabled when user namespaces
are in use:
https://github.com/containerd/containerd/pull/6170/files#diff-91d0a4c61f6d3523b5a19717d1b40b5fffd7e392d8fe22aed7c905fe195b8902R118
I'm not sure if the intention was to disable this if containerd is
running inside a userns (rootless, if that is even supported) or just
when the pod has user namespaces.
Out of an abundance of caution, I'm keeping the userns.RunningInUserNS()
so it is still not used if containerd runs inside a user namespace.
With this patch and "enable_unprivileged_icmp = true" in the config,
running containerd as root on the host, pods with user namespaces start
just fine. Without this patch they fail with:
... failed to create containerd task: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: w
/proc/sys/net/ipv4/ping_group_range: invalid argument: unknown
Thanks a lot to Andy on the k8s slack for reporting the issue. He also
mentions he hits this with k3s on a default installation (the param
is off by default on containerd, but k3s turns that on by default it
seems). He also debugged which part of the stack was setting that
sysctl, found the PR that added this code in containerd and a workaround
(to turn the bool off).
Signed-off-by: Rodrigo Campos <[email protected]>
Signed-off-by: Rodrigo Campos <[email protected]>
abfdbd9 to
c17d3bd
Compare
|
CI seems like a flake/something broken in main and unrelated to this PR.
|
userns.RunningInUserNS() checks if the code calling that function is running inside a user namespace. But we need to check if the container we will create will use a user namespace, in that case we need to disable the sysctl too (or we would need to take the userns mapping into account to set the IDs).
This was added in PR:
#6170
And the param documentation says it is not enabled when user namespaces are in use:
https://github.com/containerd/containerd/pull/6170/files#diff-91d0a4c61f6d3523b5a19717d1b40b5fffd7e392d8fe22aed7c905fe195b8902R118
I'm not sure if the intention was to disable this if containerd is running inside a userns (rootless, if that is even supported) or just when the pod has user namespaces.
Out of an abundance of caution, I'm keeping the userns.RunningInUserNS() so it is still not used if containerd runs inside a user namespace.
With this patch and "enable_unprivileged_icmp = true" in the config, running containerd as root on the host, pods with user namespaces start just fine. Without this patch they fail with:
Thanks a lot to Andy on the k8s slack for reporting the issue. He also mentions he hits this with k3s on a default installation (the param is off by default on containerd, but k3s turns that on by default it seems). He also debugged which part of the stack was setting that sysctl, found the PR that added this code in containerd and a workaround (to turn the bool off). It was very valuable and a great report :)