Constrain the host sandbox cgroup for devices and cpusets by default when sandbox-cgroup-only selected #2793

egernst · 2020-06-24T22:18:55Z

This PR pulls in package from Kubernetes for working with CPUSets.

This will calculate the union of the containers to create a sandbox-level CPUSet.

auto-comment · 2020-06-24T22:18:58Z

Thank you for raising your pull request. Please note that the main development of Kata Containers has moved to the 2.0-dev branch of https://github.com/kata-containers/kata-containers repository. The kata-containers/runtime repository is kept for 1.x release maintenance. Please check twice if your change should go to the 2.0-dev branch directly.

If it is strongly required for adding the change to Kata Containers 1.x releases, please ping @kata-containers/runtime to assign a dedicated developer to be responsible for porting the change to 2.0-dev branch. Thanks!

jodh-intel

Thanks @egernst. Please could you change the log calls to remove your name :)

virtcontainers/pkg/cgroups/manager.go

jodh-intel · 2020-06-25T06:53:22Z

virtcontainers/pkg/cgroups/manager.go

+		}).Debug("EGERNST")
+	}
+	m.Lock()
+	cgroups.CpusetCpus = cpuset


No validation on cpuset? Can it be blank? Can we check the expected format, etc?

An empty string is normal/expected. Validation is done @ the sandbox via CPUSets package, and I expect we'll just have an error, if the format somehow was broken, when we try to 'apply' the cgroup (see err handling there).

virtcontainers/pkg/cgroups/manager.go

egernst · 2020-06-29T16:16:28Z

@devimc @amshinde @jodh-intel @jcvenegas updated - PTAL. This is verified using cgroupfs. I will begin testing for systemd based group handling, but that will involve more cgroup handling updates outside the scope of 'fixing' the sandbox constraints.

devimc

thanks @egernst - but I think that for security reasons the device cgroup is a must

virtcontainers/sandbox.go

virtcontainers/pkg/cgroups/manager.go

egernst · 2020-06-29T16:33:00Z

(still WIP until I have tests running in k8s)

egernst · 2020-06-29T22:56:20Z

Tested with containerd/k8s and with Docker CLI with cgroupfs. This should be about ready from my perspective.

egernst · 2020-07-01T14:14:56Z

@jodh-intel @bergwolf @devimc PTAL.

devimc

thanks, I left some questions

virtcontainers/sandbox.go

devimc · 2020-07-01T14:23:07Z

virtcontainers/sandbox.go

-		resources = *spec.Linux.Resources
+		// engine by default. The exception is for devices whitelist as well as sandbox-level
+		// CPUSet.
+		resources.Devices = spec.Linux.Resources.Devices


okay, I understand why cpu and memory are not being honoured, but what about blockIO ?

There are a couple issues open for this ability now, and there's work in progress on K8S side to add this. Are you okay with holding off until that discussion (both on Kata, [1], and k8s side) complete to address this subsystem?

[1] - #2160

egernst · 2020-07-01T17:53:45Z

Updated to address feedback. PTAL @devimc et al

devimc · 2020-07-01T18:10:57Z

Ci is not happy

Detected TravisCI Environment

Found 4 commits between commit 97b41cefa00bf41326262f076c93689852cf2fb1 and branch master

ERROR: Commit 63dab32080bc2dac749b080e55080e0239c545b8: Failed to find subsystem in subject: "add method to get resulting cpuset for a sandbox"

ERROR: checkcommits failed. See the document below for help on formatting

commits for the project.

egernst · 2020-07-08T19:32:15Z

/test

egernst · 2020-07-08T22:34:50Z

@chavafg @GabyCT any insights into this failure? http://jenkins.katacontainers.io/blue/organizations/jenkins/kata-containers-runtime-opensuse-15-PR/detail/kata-containers-runtime-opensuse-15-PR/1219/pipeline#log-3049 on SuSe test?

I expect my changes to only by exercised when dealing with sandbox group only flag set.

chavafg · 2020-07-09T14:31:35Z

not really, have restarted it

egernst · 2020-07-09T21:45:59Z

@devimc @amshinde still need reviews if you have time....

amshinde · 2020-07-09T23:58:32Z

@jodh-intel Can you take another look?

amshinde · 2020-07-10T17:41:32Z

@bergwolf @jcvenegas Can you take a look at this PR?

jcvenegas · 2020-07-10T18:02:04Z

It is looking good probably update kata docs to document what is expected and why around cpusets would be great to point to users.

amshinde · 2020-07-10T20:57:06Z

@chavafg @GabyCT I had restarted the failing CI, but still see some of them failing.
Are these expected to fail..can you take a look?

chavafg · 2020-07-10T21:30:38Z

took a look and seems that the opensuse CI is only failing with this PR.
The arm failure seems to be not related to this PR, /cc @Pennyzct
vfio ci not sure, but seems that could be related to this PR. I see that vfio CI of tests repo is passing and looks very stable. /cc @cmaf

bergwolf

lgtm!

bergwolf · 2020-07-13T02:48:37Z

@chavafg The PR should only have impacts on cpuset/devices cgroups related tests.

egernst · 2020-07-14T03:50:08Z

@chavafg if you agree, can you help get this merged (skip the travis)?

chavafg · 2020-07-14T18:27:32Z

So I looked at the configuration for the opensuse CI and I see that we use SandboxCgroupOnly = true. This seems to be the issue as we are using this flag in the VFIO CI which also fails.

This is the error I see in the kata-runtime logs of the CI job:

Jul 10 18:33:53 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:53.8716997Z" level=debug msg="Request to hypervisor to update vCPUs" arch=amd64 command=run container=4xsQsD4p3c2dojkSwCg4 cpus-
sandbox=1 name=kata-runtime pid=54761 sandbox=4xsQsD4p3c2dojkSwCg4 source=virtcontainers subsystem=sandbox
Jul 10 18:33:53 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:53.8806391Z" level=debug msg="Sandbox CPUs: 1" arch=amd64 command=run container=4xsQsD4p3c2dojkSwCg4 name=kata-runtime pid=54761
 sandbox=4xsQsD4p3c2dojkSwCg4 source=virtcontainers subsystem=sandbox
Jul 10 18:33:53 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:53.890858Z" level=debug msg="Request to hypervisor to update memory" arch=amd64 command=run container=4xsQsD4p3c2dojkSwCg4 memor
y-sandbox-size-byte=2147483648 name=kata-runtime pid=54761 sandbox=4xsQsD4p3c2dojkSwCg4 source=virtcontainers subsystem=sandbox
Jul 10 18:33:53 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:53.9044327Z" level=info msg="{\"QMP\": {\"version\": {\"qemu\": {\"micro\": 0, \"minor\": 0, \"major\": 5}, \"package\": \"kata-
static\"}, \"capabilities\": [\"oob\"]}}" arch=amd64 command=run container=4xsQsD4p3c2dojkSwCg4 name=kata-runtime pid=54761 source=virtcontainers subsystem=qmp
Jul 10 18:33:53 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:53.9125341Z" level=info msg="{\"execute\":\"qmp_capabilities\"}" arch=amd64 command=run container=4xsQsD4p3c2dojkSwCg4 name=kata
-runtime pid=54761 source=virtcontainers subsystem=qmp
Jul 10 18:33:53 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:53.9223659Z" level=info msg="{\"return\": {}}" arch=amd64 command=run container=4xsQsD4p3c2dojkSwCg4 name=kata-runtime pid=54761
 source=virtcontainers subsystem=qmp
Jul 10 18:33:53 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:53.9304178Z" level=debug msg="Sandbox memory size: 2048 MB" arch=amd64 command=run container=4xsQsD4p3c2dojkSwCg4 name=kata-runt
ime pid=54761 sandbox=4xsQsD4p3c2dojkSwCg4 source=virtcontainers subsystem=sandbox
Jul 10 18:33:53 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:53.937722Z" level=info msg="New client" arch=amd64 command=run container=4xsQsD4p3c2dojkSwCg4 name=kata-runtime pid=54761 proxy=
54784 source=virtcontainers subsystem=kata_agent url="unix:///run/vc/sbs/4xsQsD4p3c2dojkSwCg4/proxy.sock"
Jul 10 18:33:53 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:53.9461859Z" level=debug msg="sending request" arch=amd64 command=run container=4xsQsD4p3c2dojkSwCg4 name=grpc.OnlineCPUMemReque
st pid=54761 req= source=virtcontainers subsystem=kata_agent
Jul 10 18:33:53 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:53.9595868Z" level=error msg="fatal error" arch=amd64 name=kata-runtime panic="runtime error: invalid memory address or nil poin
ter dereference" pid=54761 source=runtime
Jul 10 18:33:54 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:54.0764491Z" level=error msg="heap profile: 0: 0 [2: 576] @ heap/1048576" arch=amd64 name=kata-runtime pid=54761 source=runtime
Jul 10 18:33:54 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:54.0992272Z" level=error msg="0: 0 [0: 0] @ 0x5563c04c360f 0x5563c04c445d 0x5563c04c01db 0x5563c04bdf53 0x5563c04bdabc 0x5563c04
c13bf 0x5563c04c10de 0x5563c04bdf95 0x5563c04bdabc 0x5563c04c4159 0x5563c04c445d 0x5563c04c01db 0x5563c04bdf53 0x5563c04bdabc 0x5563c04c4159 0x5563c04c445d 0x5563c04c01db 0x5563c04bdf53 0x5563c04bdabc 0x5563c04c
4159 0x5563c04c445d 0x5563c04c01db 0x5563c04bdf53 0x5563c04bdabc 0x5563c04c162f 0x5563c04bdecf 0x5563c04bdabc 0x5563c04c4159 0x5563c04c445d 0x5563c04c01db 0x5563c04bdf53 0x5563c04bdabc" arch=amd64 name=kata-runt
ime pid=54761 source=runtime
Jul 10 18:33:54 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:54.1061465Z" level=error msg="#\t0x5563c04c360e\tencoding/json.typeFields+0xb2e\t\t/usr/local/go/src/encoding/json/encode.go:117
6" arch=amd64 name=kata-runtime pid=54761 source=runtime
Jul 10 18:33:54 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:54.1129271Z" level=error msg="#\t0x5563c04c445c\tencoding/json.cachedTypeFields+0xec\t/usr/local/go/src/encoding/json/encode.go:
1278" arch=amd64 name=kata-runtime pid=54761 source=runtime
Jul 10 18:33:54 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:54.121212Z" level=error msg="#\t0x5563c04c01da\tencoding/json.newStructEncoder+0x3a\t/usr/local/go/src/encoding/json/encode.go:6
74" arch=amd64 name=kata-runtime pid=54761 source=runtime
Jul 10 18:33:54 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:54.1324095Z" level=error msg="#\t0x5563c04bdf52\tencoding/json.newTypeEncoder+0x372\t/usr/local/go/src/encoding/json/encode.go:4
25" arch=amd64 name=kata-runtime pid=54761 source=runtime
Jul 10 18:33:54 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:54.1393119Z" level=error msg="#\t0x5563c04bdabb\tencoding/json.typeEncoder+0x19b\t\t/usr/local/go/src/encoding/json/encode.go:38
1" arch=amd64 name=kata-runtime pid=54761 source=runtime
Jul 10 18:33:54 opensuse15-azuread71a0 kata-runtime[54761]: time="2020-07-10T18:33:54.1466495Z" level=error msg="#\t0x5563c04c13be\tencoding/json.newArrayEncoder+0x4e\t/usr/local/go/src/encoding/json/encode.go:7
97" arch=amd64 name=kata-runtime pid=54761 source=runtime

You can see the full log by downloading the kata-runtime from the artifacts of the CI job: http://jenkins.katacontainers.io/job/kata-containers-runtime-opensuse-15-PR/1221/artifact/artifacts/kata-runtime_06.gz

You should be able to reproduce in your environment using this flag and running the functional tests from the tests repo:

sudo -E PATH=$PATH make functional

jodh-intel

Thanks @egernst.

lgtm

Calculate sandbox's CPUSet as the union of each of the container's CPUSets. Signed-off-by: Eric Ernst <eric@amperecomputing.com>

added for calculating union of cpusets Signed-off-by: Eric Ernst <eric@amperecomputing.com>

add function for applying a CPUset change to a cgroup Signed-off-by: Eric Ernst <eric@amperecomputing.com>

Allow for constraining the cpuset as well as the devices-whitelist . Revert sandbox constraints for cpu/memory, as they break the K8S use case. Can re-add behind a non-default flag in the future. The sandbox CPUSet should be updated every time a container is created, updated, or removed. To facilitate this without rewriting the 'non constrained cgroup' handling, let's add to the Sandbox's cgroupsUpdate function. Fixes: #2792 Signed-off-by: Eric Ernst <eric@amperecomputing.com>

egernst · 2020-07-16T05:17:10Z

addressed issue.

/test

egernst · 2020-07-16T16:01:28Z

@chavafg PTAL?

chavafg · 2020-07-16T16:52:27Z

Thanks @egernst, ARM CI failure is unrelated and SLES CI is also having unrelated issues. We can merge this.

ariel-adam · 2020-12-08T16:46:09Z

Fixes #2811

egernst force-pushed the add-cpuset-calc-func branch from dc66f8f to 88c1a04 Compare June 24, 2020 22:24

egernst requested a review from a team as a code owner June 25, 2020 05:36

jodh-intel previously requested changes Jun 25, 2020

View reviewed changes

egernst commented Jun 25, 2020

View reviewed changes

virtcontainers/pkg/cgroups/manager.go Show resolved Hide resolved

egernst changed the title ~~sandbox: add function to calcule sandbox level cpuset~~ WIP/RFC: sandbox: add function to calcule sandbox level cpuset Jun 26, 2020

egernst force-pushed the add-cpuset-calc-func branch 3 times, most recently from 9d442d3 to 78ed479 Compare June 29, 2020 16:14

egernst force-pushed the add-cpuset-calc-func branch from 78ed479 to 26fc577 Compare June 29, 2020 16:17

devimc previously requested changes Jun 29, 2020

View reviewed changes

virtcontainers/sandbox.go Outdated Show resolved Hide resolved

virtcontainers/pkg/cgroups/manager.go Outdated Show resolved Hide resolved

egernst force-pushed the add-cpuset-calc-func branch from 26fc577 to 1eade24 Compare June 29, 2020 17:00

amshinde self-requested a review June 29, 2020 21:23

egernst requested review from jodh-intel and devimc June 29, 2020 22:54

egernst changed the title ~~WIP/RFC: sandbox: add function to calcule sandbox level cpuset~~ Constrain the host sandbox cgroup for devices and cpusets by default when sandbox-cgroup-only selected Jun 29, 2020

egernst force-pushed the add-cpuset-calc-func branch from d3a828b to a4a42b9 Compare June 29, 2020 22:57

devimc reviewed Jul 1, 2020

View reviewed changes

egernst force-pushed the add-cpuset-calc-func branch 3 times, most recently from d0cffb4 to 97b41ce Compare July 1, 2020 17:51

egernst force-pushed the add-cpuset-calc-func branch from 97b41ce to 73412a8 Compare July 1, 2020 18:19

egernst requested a review from devimc July 1, 2020 18:19

egernst force-pushed the add-cpuset-calc-func branch from 73412a8 to 1017646 Compare July 1, 2020 19:28

egernst mentioned this pull request Jul 8, 2020

Fixup systemd cgroup handling #2817

Merged

2 tasks

amshinde approved these changes Jul 9, 2020

View reviewed changes

jcvenegas approved these changes Jul 10, 2020

View reviewed changes

bergwolf approved these changes Jul 13, 2020

View reviewed changes

jodh-intel approved these changes Jul 15, 2020

View reviewed changes

egernst force-pushed the add-cpuset-calc-func branch from f86a1a8 to 52af634 Compare July 16, 2020 05:15

Eric Ernst added 4 commits July 15, 2020 22:16

virtcontainers: add method for calculating cpuset for sandbox

1aa0cec

Calculate sandbox's CPUSet as the union of each of the container's CPUSets. Signed-off-by: Eric Ernst <eric@amperecomputing.com>

vendor: add cpuset package from kubernetes

9fa2bf1

added for calculating union of cpusets Signed-off-by: Eric Ernst <eric@amperecomputing.com>

cgroups: add ability to update CPUSet

093aaa8

add function for applying a CPUset change to a cgroup Signed-off-by: Eric Ernst <eric@amperecomputing.com>

egernst force-pushed the add-cpuset-calc-func branch from 52af634 to 8b4c299 Compare July 16, 2020 05:16

chavafg approved these changes Jul 16, 2020

View reviewed changes

chavafg merged commit 1d7a37e into master Jul 16, 2020

jodh-intel mentioned this pull request Aug 26, 2020

[forward port]: Constrain the host sandbox cgroup for devices and cpusets by default when sandbox-cgroup-only selected kata-containers/kata-containers#596

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Constrain the host sandbox cgroup for devices and cpusets by default when sandbox-cgroup-only selected #2793

Constrain the host sandbox cgroup for devices and cpusets by default when sandbox-cgroup-only selected #2793

egernst commented Jun 24, 2020

auto-comment bot commented Jun 24, 2020

jodh-intel left a comment

jodh-intel Jun 25, 2020

egernst Jun 25, 2020

egernst commented Jun 29, 2020

devimc left a comment

egernst commented Jun 29, 2020

egernst commented Jun 29, 2020

egernst commented Jul 1, 2020

devimc left a comment

devimc Jul 1, 2020

egernst Jul 1, 2020 •

edited

egernst commented Jul 1, 2020

devimc commented Jul 1, 2020

egernst commented Jul 8, 2020

egernst commented Jul 8, 2020 •

edited

chavafg commented Jul 9, 2020 •

edited

egernst commented Jul 9, 2020

amshinde commented Jul 9, 2020

amshinde commented Jul 10, 2020

jcvenegas commented Jul 10, 2020

amshinde commented Jul 10, 2020

chavafg commented Jul 10, 2020

bergwolf left a comment

bergwolf commented Jul 13, 2020

egernst commented Jul 14, 2020

chavafg commented Jul 14, 2020

jodh-intel left a comment

egernst commented Jul 16, 2020

egernst commented Jul 16, 2020

chavafg commented Jul 16, 2020

ariel-adam commented Dec 8, 2020

Constrain the host sandbox cgroup for devices and cpusets by default when sandbox-cgroup-only selected #2793

Constrain the host sandbox cgroup for devices and cpusets by default when sandbox-cgroup-only selected #2793

Conversation

egernst commented Jun 24, 2020

auto-comment bot commented Jun 24, 2020

jodh-intel left a comment

Choose a reason for hiding this comment

jodh-intel Jun 25, 2020

Choose a reason for hiding this comment

egernst Jun 25, 2020

Choose a reason for hiding this comment

egernst commented Jun 29, 2020

devimc left a comment

Choose a reason for hiding this comment

egernst commented Jun 29, 2020

egernst commented Jun 29, 2020

egernst commented Jul 1, 2020

devimc left a comment

Choose a reason for hiding this comment

devimc Jul 1, 2020

Choose a reason for hiding this comment

egernst Jul 1, 2020 • edited

Choose a reason for hiding this comment

egernst commented Jul 1, 2020

devimc commented Jul 1, 2020

egernst commented Jul 8, 2020

egernst commented Jul 8, 2020 • edited

chavafg commented Jul 9, 2020 • edited

egernst commented Jul 9, 2020

amshinde commented Jul 9, 2020

amshinde commented Jul 10, 2020

jcvenegas commented Jul 10, 2020

amshinde commented Jul 10, 2020

chavafg commented Jul 10, 2020

bergwolf left a comment

Choose a reason for hiding this comment

bergwolf commented Jul 13, 2020

egernst commented Jul 14, 2020

chavafg commented Jul 14, 2020

jodh-intel left a comment

Choose a reason for hiding this comment

egernst commented Jul 16, 2020

egernst commented Jul 16, 2020

chavafg commented Jul 16, 2020

ariel-adam commented Dec 8, 2020

egernst Jul 1, 2020 •

edited

egernst commented Jul 8, 2020 •

edited

chavafg commented Jul 9, 2020 •

edited