-
Notifications
You must be signed in to change notification settings - Fork 625
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
update:fix update pids-limit=0 error #3781
Conversation
ce0b396
to
857501d
Compare
857501d
to
a0c5c95
Compare
fix: #3784 |
@AkihiroSuda can you review this pr ,thank you? |
Can we have a test? |
9fb9ccb
to
8327370
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
f22c6fc
to
b7e52ab
Compare
|
Signed-off-by: ningmingxiao <[email protected]>
b7e52ab
to
75c8066
Compare
func TestIssue3781(t *testing.T) { | ||
t.Parallel() | ||
testCase := nerdtest.Setup() | ||
testCase.Require = test.Not(nerdtest.Docker) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because i will use containerd client to get container id and use container.Spec(ctx) to get spec, this may fail with docker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than depending on client.LoadContainer
, can we just exec cat /sys/fs/cgroup/pids.max
in the container?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
old nerdctl cat /sys/fs/cgroup/pids.max will not report error when use goshim. runc will ignore pids.max=0 see:opencontainers/runc#4566
It only effect rustshim. https://github.com/containerd/rust-extensions. rustshim wirte cgroup by rustshim itself.(go shim call runc to update source)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
when try nerdctl -n k8s.io update --cpuset-cpus 0-2 ffe
this code
nerdctl/cmd/nerdctl/container/container_update.go
Lines 327 to 329 in c41b394
will make spec.Linux.Resources.Pids =0 it will not work on rust-shim(will cause rshim set pidlimit=0 goshim works well.).
https://github.com/containerd/rust-extensions
will cause