Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

cli: add systemd-cgroup option #747

Merged
merged 1 commit into from Sep 20, 2018

Conversation

devimc
Copy link

@devimc devimc commented Sep 18, 2018

Add support for cgroup driver systemd.
systemd cgroup is not applied in the VM since in some cases like initrd images
there is no systemd running and nobody can update a systemd cgroup using
systemctl.

fixes #596

Signed-off-by: Julio Montes julio.montes@intel.com

@katacontainersbot
Copy link
Contributor

PSS Measurement:
Qemu: 165517 KB
Proxy: 4088 KB
Shim: 8793 KB

Memory inside container:
Total Memory: 2043460 KB
Free Memory: 2006580 KB

@katacontainersbot
Copy link
Contributor

PSS Measurement:
Qemu: 168160 KB
Proxy: 4114 KB
Shim: 8834 KB

Memory inside container:
Total Memory: 2043460 KB
Free Memory: 2006704 KB

@katacontainersbot
Copy link
Contributor

PSS Measurement:
Qemu: 165506 KB
Proxy: 4069 KB
Shim: 8866 KB

Memory inside container:
Total Memory: 2043460 KB
Free Memory: 2006736 KB

Add support for cgroup driver systemd.
systemd cgroup is not applied in the VM since in some cases like initrd images
there is no systemd running and nobody can update a systemd cgroup using
systemctl.

fixes kata-containers#596

Signed-off-by: Julio Montes <julio.montes@intel.com>
@katacontainersbot
Copy link
Contributor

PSS Measurement:
Qemu: 165862 KB
Proxy: 4136 KB
Shim: 8908 KB

Memory inside container:
Total Memory: 2043460 KB
Free Memory: 2006760 KB

@codecov
Copy link

codecov bot commented Sep 18, 2018

Codecov Report

Merging #747 into master will increase coverage by 0.02%.
The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           master     #747      +/-   ##
==========================================
+ Coverage   65.41%   65.43%   +0.02%     
==========================================
  Files          87       87              
  Lines       10532    10541       +9     
==========================================
+ Hits         6889     6897       +8     
- Misses       2951     2952       +1     
  Partials      692      692

// - Initrd image doesn't have systemd.
// - Nobody will be able to modify the resources of a specific container by using systemctl set-property.
// - docker is not running in the VM.
if systemdCgroup {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we can do the conversion in the cli package instead?

Copy link
Author

Choose a reason for hiding this comment

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

I thought the same, the problem is that systemd cgroup should be applied in the host (if systemd is the init process and where docker is running), so I prefer to modify the grpcSpec before sending it to the agent and don't modify the original ociSpec to let virtcontainer apply it in the host

Copy link
Member

Choose a reason for hiding this comment

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

Agree with @devimc

Copy link
Member

Choose a reason for hiding this comment

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

hmm, sound good to me. Thanks @devimc .

@WeiZhang555
Copy link
Member

I'm adding host cgroup support currently: #734

Wondering if this should also apply to host cgroup? When user/docker specify systemd cgroup, which layer do they want to apply? Host Cgroup or Guest Cgroup?

@devimc
Copy link
Author

devimc commented Sep 19, 2018

@WeiZhang555

Wondering if this should also apply to host cgroup?

Systemd cgroup should be applied in the host, not in the guest

When user/docker specify systemd cgroup, which layer do they want to apply? Host Cgroup or Guest Cgroup?

#596 (comment)

@WeiZhang555
Copy link
Member

WeiZhang555 commented Sep 20, 2018

@devimc Oh, I mis-read your description, this is good to me. I'll take this into consideration while doing host cgroup support, but this depends on the flag provided by your PR.

LGTM

Approved with PullApprove

@bergwolf bergwolf merged commit 76b0c3c into kata-containers:master Sep 20, 2018
@devimc devimc deleted the topic/systemdCgroup branch April 8, 2019 14:51
egernst pushed a commit to egernst/runtime that referenced this pull request Feb 9, 2021
…tdin

agent: don't receive signals from stdin
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docker: cgroupdriver: runtime does not support docker cgroupdriver argument
4 participants