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

cgroups: Use only pod cgroup#1880

Merged
egernst merged 12 commits into
kata-containers:masterfrom
jcvenegas:pod-cgroup-only
Sep 9, 2019
Merged

cgroups: Use only pod cgroup#1880
egernst merged 12 commits into
kata-containers:masterfrom
jcvenegas:pod-cgroup-only

Conversation

@jcvenegas
Copy link
Copy Markdown
Member

@jcvenegas jcvenegas commented Jul 12, 2019

  • config add option to use only pod cgroup (SandboxCgroupOnly)
  • Get the sandbox Cgroup path based on cgroupsPath from Sandbox Container
  • Join the Sandbox Cgroup before create any process
  • All the kata processes are inside the Sandbox Cgroup
  • The cgroup is not restricted by the runtime but the caller is free to limit it.

Depends-on: github.com/kata-containers/tests#1824

Fixes: #1879
Signed-off-by: Jose Carlos Venegas Munoz jose.carlos.venegas.munoz@intel.com

Comment thread virtcontainers/container.go Outdated
@jcvenegas jcvenegas force-pushed the pod-cgroup-only branch 2 times, most recently from 62224f5 to 3c57d9b Compare July 15, 2019 22:28
@jcvenegas
Copy link
Copy Markdown
Member Author

/test

1 similar comment
@jcvenegas
Copy link
Copy Markdown
Member Author

/test

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 16, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@bc15e44). Click here to learn what that means.
The diff coverage is 23.45%.

@@            Coverage Diff            @@
##             master    #1880   +/-   ##
=========================================
  Coverage          ?   52.32%           
=========================================
  Files             ?      108           
  Lines             ?    14025           
  Branches          ?        0           
=========================================
  Hits              ?     7338           
  Misses            ?     5813           
  Partials          ?      874

1 similar comment
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 16, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@bc15e44). Click here to learn what that means.
The diff coverage is 23.45%.

@@            Coverage Diff            @@
##             master    #1880   +/-   ##
=========================================
  Coverage          ?   52.32%           
=========================================
  Files             ?      108           
  Lines             ?    14025           
  Branches          ?        0           
=========================================
  Hits              ?     7338           
  Misses            ?     5813           
  Partials          ?      874

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 16, 2019

Codecov Report

Merging #1880 into master will decrease coverage by 0.17%.
The diff coverage is 58.33%.

@@            Coverage Diff             @@
##           master    #1880      +/-   ##
==========================================
- Coverage   51.89%   51.71%   -0.18%     
==========================================
  Files         107      107              
  Lines       14456    14589     +133     
==========================================
+ Hits         7502     7545      +43     
- Misses       6061     6140      +79     
- Partials      893      904      +11

Comment thread virtcontainers/sandbox.go Outdated
}

// Use the parent cgroup of the container sandbox as the sandbox cgroup
s.state.CgroupPath = filepath.Dir(c.state.CgroupPath) + "/kata-sandbox/"
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We are creating a cgroup, we should should remove it on delete (even if the caller will delete all the parent cgroup)
If we use a common name al the pods will use the same cgroup if the cgroup parent was not set just for the pod, e.g. docker. So probably need to add an additional sufix (container.id) or just drop kata in the parent cgroup instead of create one.

@jcvenegas
Copy link
Copy Markdown
Member Author

/test

@jcvenegas
Copy link
Copy Markdown
Member Author

All passed with the flag not enabled. So all good in the default/current way to do cgroups.

Re-ran with the flag enabled,

fedora failed with (this job does not run docker)

15:26:11 ok 24 ctr oom # skip This is not working (Issue https://github.com/kata-containers/tests/issues/714)
15:26:29 not ok 25 ctr /etc/resolv.conf rw/ro mode
15:26:29 # (in test file ctr.bats, line 882)
15:26:29 #   `[ "$status" -eq 0 ]' failed

Restarting to get if is non related error.

Ubuntu job failed with (this job does not run docker)

16:30:11 not ok 33 ctr /etc/resolv.conf rw/ro mode
16:30:11 # (in test file ctr.bats, line 1217)
16:30:11 #   `[ "$status" -eq 0 ]' failed
16:30:11 # time="2019-07-16T21:29:09Z" level=error msg="File descriptor 3 (pipe:[156253]) leaked on pvdisplay invocation. Parent PID 109987: /tmp/jenkins/workspace/kata-con
16:30:11 # File descriptor 6 (/dev/mapper/control) leaked on pvdisplay invocation. Parent PID 109987: /tmp/jenkins/workspace/kata-con
16:30:11 #   Failed to find physical volume "/dev/sdb".
16:30:11 # " error="exit status 5" 

The rest of jobs that failed with

15:23:10 [1] • Failure [13.542 seconds]
15:23:10 [1] Checking CPU cgroups in the host
15:23:10 [1] /tmp/jenkins/workspace/kata-containers-runtime-ubuntu-nemu/go/src/github.com/kata-containers/tests/integration/docker/cgroups_test.go:110
15:23:10 [1]   checking whether cgroups can be deleted
15:23:10 [1]   /tmp/jenkins/workspace/kata-containers-runtime-ubuntu-nemu/go/src/github.com/kata-containers/tests/integration/docker/cgroups_test.go:130
15:23:10 [1]     with a running process
15:23:10 [1]     /tmp/jenkins/workspace/kata-containers-runtime-ubuntu-nemu/go/src/github.com/kata-containers/tests/integration/docker/cgroups_test.go:131
15:23:10 [1]       should be deleted [It]
15:23:10 [1]       /tmp/jenkins/workspace/kata-containers-runtime-ubuntu-nemu/go/src/github.com/kata-containers/tests/integration/docker/cgroups_test.go:132
15:23:10 [1] 
15:23:10 [1]       Expected
15:23:10 [1]           <string>: /sys/fs/cgroup/cpu/docker/kata_ae38701e4bc415ee6876f4dba2d1841949f0528f4bbc336ac7526cd4634445e8
15:23:10 [1]       to be a directory: stat /sys/fs/cgroup/cpu/docker/kata_ae38701e4bc415ee6876f4dba2d1841949f0528f4bbc336ac7526cd4634445e8: no such file or directory

Expected as those run docker test

jcvenegas pushed a commit to jcvenegas/kata-test that referenced this pull request Jul 17, 2019
- Add scripts to enable it if needed.
- Add script to rolback config to enable it.
- Check in docker test if option is enabled to not check cgroups in the
host.

Depends-on: github.com/kata-containers/runtime#1880

Fixes: kata-containers#1810

Signed-off-by: Jose Carlos Venegas Munoz <jcvenega@jcvenega-nuc.zpn.intel.com>
jcvenegas pushed a commit to jcvenegas/kata-test that referenced this pull request Jul 18, 2019
- Add scripts to enable it if needed.
- Add script to rolback config to enable it.
- Check in docker test if option is enabled to not check cgroups in the
host.

Depends-on: github.com/kata-containers/runtime#1880

Fixes: kata-containers#1810

Signed-off-by: Jose Carlos Venegas Munoz <jcvenega@jcvenega-nuc.zpn.intel.com>
@caoruidong
Copy link
Copy Markdown
Member

/test

1 similar comment
@jcvenegas
Copy link
Copy Markdown
Member Author

/test

@jcvenegas
Copy link
Copy Markdown
Member Author

/test

1 similar comment
@jcvenegas
Copy link
Copy Markdown
Member Author

/test

jcvenegas pushed a commit to jcvenegas/kata-test that referenced this pull request Jul 25, 2019
- Add scripts to enable it if needed.
- Add script to rolback config to enable it.
- Check in docker test if option is enabled to not check cgroups in the
host.

Depends-on: github.com/kata-containers/runtime#1880

Fixes: kata-containers#1810

Signed-off-by: Jose Carlos Venegas Munoz <jcvenega@jcvenega-nuc.zpn.intel.com>
@jcvenegas
Copy link
Copy Markdown
Member Author

/test

@jcvenegas jcvenegas force-pushed the pod-cgroup-only branch 2 times, most recently from f9ec4fb to ce4d1dc Compare July 26, 2019 16:32
jcvenegas pushed a commit to jcvenegas/kata-test that referenced this pull request Jul 26, 2019
- Add scripts to enable it if needed.
- Add script to rolback config to enable it.
- Check in docker test if option is enabled to not check cgroups in the
host.

Depends-on: github.com/kata-containers/runtime#1880

Fixes: kata-containers#1810

Signed-off-by: Jose Carlos Venegas Munoz <jcvenega@jcvenega-nuc.zpn.intel.com>
@jcvenegas
Copy link
Copy Markdown
Member Author

/test

Comment thread virtcontainers/container.go Outdated
@jcvenegas
Copy link
Copy Markdown
Member Author

@egernst please take a look to the last updates.

prefix cgroup related methods with cgroups,
make easy to group together in auto-generated docs.

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
Move sandbox related methods to its own file.

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
rename to allow group in auto-generated docs.

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
constrainHypervisor -> constrainHypervisorVCPUs

Document and rename function.

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
Some errors propagate with printing showing a cgroup path.
If for some reason this is empty is difficult to know looking
at the logs.

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
The container CgroupsPath is optional acording to OCI.

If for some reason the runtime decide to not define one.
just skip cgroup operations.

This is going to be useful for upcoming, sandbox cgroup only
cgroup managment feature.

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
add option to eneable only pod cgroup (SandboxCgroupOnly)

Depends-on: github.com/kata-containers/tests#1824

Fixes: kata-containers#1879
Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
No call cgroup operations for containers in host
if SandboxCgroupOnly is enabled.

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
When a new sandbox is created, join to its cgroup path
this will create all proxy, shim, etc in the sandbox cgroup.

Fixes: kata-containers#1879

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
Add containers does not need to check the cgroup path
this is done in a different function

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
Use all subsystems for SandboxOnly option to make sure
all cgroups are deleted.

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
jcvenegas pushed a commit to jcvenegas/kata-test that referenced this pull request Aug 30, 2019
- Add scripts to enable it if needed.
- Add script to rolback config to enable it.
- Add ci Job case for pod cgroup
- Run again kuberentes test with pod cgroup enabled for
K8S_CONTAINERD_JOB
- Check in docker test if option is enabled to not check cgroups in the
host.

Depends-on: github.com/kata-containers/runtime#1880

Fixes: kata-containers#1810

Signed-off-by: Jose Carlos Venegas Munoz <jcvenega@jcvenega-nuc.zpn.intel.com>
jcvenegas pushed a commit to jcvenegas/kata-test that referenced this pull request Aug 30, 2019
- Add scripts to enable it if needed.
- Add script to rolback config to enable it.
- Add ci Job case for pod cgroup
- Run again kuberentes test with pod cgroup enabled for
K8S_CONTAINERD_JOB
- Check in docker test if option is enabled to not check cgroups in the
host.

Depends-on: github.com/kata-containers/runtime#1880

Fixes: kata-containers#1810

Signed-off-by: Jose Carlos Venegas Munoz <jcvenega@jcvenega-nuc.zpn.intel.com>
jcvenegas pushed a commit to jcvenegas/kata-test that referenced this pull request Aug 30, 2019
- Add scripts to enable it if needed.
- Add script to rolback config to enable it.
- Add ci Job case for pod cgroup
- Run again kuberentes test with pod cgroup enabled for
K8S_CONTAINERD_JOB
- Check in docker test if option is enabled to not check cgroups in the
host.

Depends-on: github.com/kata-containers/runtime#1880

Fixes: kata-containers#1810

Signed-off-by: Jose Carlos Venegas Munoz <jcvenega@jcvenega-nuc.zpn.intel.com>
jcvenegas pushed a commit to jcvenegas/kata-test that referenced this pull request Aug 30, 2019
- Add scripts to enable it if needed.
- Add script to rolback config to enable it.
- Add ci Job case for pod cgroup
- Run again kuberentes test with pod cgroup enabled for
K8S_CONTAINERD_JOB
- Check in docker test if option is enabled to not check cgroups in the
host.

Depends-on: github.com/kata-containers/runtime#1880

Fixes: kata-containers#1810

Signed-off-by: Jose Carlos Venegas Munoz <jcvenega@jcvenega-nuc.zpn.intel.com>
jcvenegas pushed a commit to jcvenegas/kata-test that referenced this pull request Aug 30, 2019
- Add scripts to enable it if needed.
- Add script to rolback config to enable it.
- Add ci Job case for pod cgroup
- Run again kuberentes test with pod cgroup enabled for
K8S_CONTAINERD_JOB
- Check in docker test if option is enabled to not check cgroups in the
host.

Depends-on: github.com/kata-containers/runtime#1880

Fixes: kata-containers#1810

Signed-off-by: Jose Carlos Venegas Munoz <jcvenega@jcvenega-nuc.zpn.intel.com>
@jcvenegas
Copy link
Copy Markdown
Member Author

/test

Copy link
Copy Markdown
Member

@egernst egernst left a comment

Choose a reason for hiding this comment

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

looking good. Submitted one more patch which I think simplifies things.

@egernst egernst dismissed their stale review September 5, 2019 20:48

erics review is old

@egernst
Copy link
Copy Markdown
Member

egernst commented Sep 5, 2019

/test

Simplify the tests and the code by combining the create and join
functions into a single function.

Signed-off-by: Eric Ernst <eric.ernst@intel.com>
Copy link
Copy Markdown
Member

@bergwolf bergwolf left a comment

Choose a reason for hiding this comment

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

LGTM. A side question how do we handle guest resource calculation w.r.t. podoverhead? I mean, if we allocate the requested cpu/memory based on container cpu/memory limits, and hand all of them to the guest and container process, there is actually no reserved resource of the kata components.

@egernst
Copy link
Copy Markdown
Member

egernst commented Sep 7, 2019

@bergwolf : the overhead is assumed to be just on host, we don’t size the VM any larger to account for it (today, anyway). So, the container sizing is the same as what is requested. Perhaps the default cpu/memory can be considered for this (recall that we start with a minimal cpu/memory, and then add in whatever is requested by the workload).

@egernst egernst merged commit 282d858 into kata-containers:master Sep 9, 2019
@jcvenegas jcvenegas deleted the pod-cgroup-only branch January 23, 2020 19:44
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.

cgroups: Use only pod cgroup, not create container cgroup in the host.

7 participants