-
Notifications
You must be signed in to change notification settings - Fork 19
config: switch PidsLimit to *int64 #48
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
base: main
Are you sure you want to change the base?
Conversation
6aa36c2 to
6a5011f
Compare
We now remap <0 to -1 and correctly handle some other annoying cases (TasksMax=0).
|
@kolyshkin This now passes in runc's CI, but since systemd doesn't support |
I guess so, yes. Otherwise setting it to |
|
Okay, let's go with that then. I will add an implementors note to the runtime-spec... |
Previously we would treat a value of "0" as meaning "no-op", which lead to quite a bit of confusion. The runtime-spec has been updated to make the "pids.limit" value a pointer, and explicitly state that: * Values >= 0 should be treated as normal values; and * < 0 (usually -1) indicates "max". In practice this means we should switch PidsLimit to an *int64. Luckily, this is actually backwards-compatible with our previous JSON -- an old value of "0" would be omitted from the output, which will now be parsed as "nil". The handling by our cgroup code would be identical but the latter now correctly reflects the guidance by the runtime-spec. Signed-off-by: Aleksa Sarai <[email protected]>
Co-developed-by: Kir Kolyshkin <[email protected]> Signed-off-by: Kir Kolyshkin <[email protected]> Signed-off-by: Aleksa Sarai <[email protected]>
|
Done! |
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.
LGTM
Previously we would treat a value of "0" as meaning "no-op", which lead
to quite a bit of confusion. The runtime-spec has been updated to make
the "pids.limit" value a pointer, and explicitly state that:
In practice this means we should switch PidsLimit to an *int64. Luckily,
this is actually backwards-compatible with our previous JSON -- an old
value of "0" would be omitted from the output, which will now be parsed
as "nil". The handling by our cgroup code would be identical but the
latter now correctly reflects the guidance by the runtime-spec.
Ref: opencontainers/runc#4014
Ref: opencontainers/runtime-spec#1279
Signed-off-by: Aleksa Sarai [email protected]