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

Set Delegate to true for cgroups transient units #648

Merged
merged 1 commit into from
Mar 16, 2016

Conversation

mrunalp
Copy link
Contributor

@mrunalp mrunalp commented Mar 15, 2016

This is required because we manage some of the cgroups ourselves.
This recommendation came from talking with systemd devs about
some of the issues that we see when using the systemd cgroups driver.

Signed-off-by: Mrunal Patel mrunalp@gmail.com

@crosbymichael
Copy link
Member

LGTM

@cyphar
Copy link
Member

cyphar commented Mar 15, 2016

@mrunalp Do you know which version of systemd added support for Delegate = true inside transient units? I can see it in the documentation, but I'm not sure how recently support for this was added.

runcom pushed a commit to projectatomic/docker that referenced this pull request Mar 16, 2016
Upstream reference: opencontainers/runc#648

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
@dqminh
Copy link
Contributor

dqminh commented Mar 16, 2016

@cyphar i think Delegate=is not supported for systemd < 218. Im not sure if this will result in an error for those systems.

LGTM if this works with systemd < 218 😛

runcom pushed a commit to projectatomic/docker that referenced this pull request Mar 16, 2016
Upstream reference: opencontainers/runc#648

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
@mrunalp
Copy link
Contributor Author

mrunalp commented Mar 16, 2016

I think we should be fine merging this. Fedora 23 has 222. RHEL 7 has 219. Ubuntu 15.10 has 225 (The last LTS 14.04 did not have it and next 16.04 should have a newer version).
If some older distro wants to package this up, they can patch this out. I am adding a comment to that effect.

This is required because we manage some of the cgroups ourselves.
This recommendation came from talking with systemd devs about
some of the issues that we see when using the systemd cgroups driver.

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
@cyphar
Copy link
Member

cyphar commented Mar 16, 2016

And AFAIK openSUSE Tumblweed and Leap 42.2 will all have systemd >= 218. Thanks for checking.

LGTM.

@mrunalp
Copy link
Contributor Author

mrunalp commented Mar 16, 2016

@dqminh @cyphar Thanks!

mrunalp pushed a commit that referenced this pull request Mar 16, 2016
Set Delegate to true for cgroups transient units
@mrunalp mrunalp merged commit 53ca128 into opencontainers:master Mar 16, 2016
runcom pushed a commit to runcom/docker that referenced this pull request Jun 8, 2016
Upstream reference: opencontainers/runc#648

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
crawford pushed a commit to crawford/docker that referenced this pull request Jun 14, 2016
Upstream reference: opencontainers/runc#648

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
stefanberger pushed a commit to stefanberger/runc that referenced this pull request Sep 8, 2017
Allow negative value for some resource fields
stefanberger pushed a commit to stefanberger/runc that referenced this pull request Sep 8, 2017
This partially revert opencontainers#648 , after a second thought, I think we
should use specs value the same as kernel API input, see:
opencontainers/runtime-spec#692 (comment)

For memory and hugetlb limits *.limit_in_bytes, cgroup APIs take the values
as string, but the parsed values are unsigned long, see:
https://github.com/torvalds/linux/blob/v4.10/mm/page_counter.c#L175-L193

For `cpu.cfs_quota_us` and `cpu.rt_runtime_us`, cgroup APIs take the input
value as signed long long, while `cpu.cfs_period_us` and `cpu.rt_periof_us`
take the input value as unsigned long long.

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
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

5 participants