cgroup: ignore cpuset if can not be applied. #447

jcvenegas · 2019-01-28T22:24:10Z

The way that kata works is hotplugging cpus to the VM
based in cpu and qouta, cpuset request may not match
with the cpus requested from the host. Apply
only if possible.

Cpuset cgroup.

From Kernel documentation:

" Cpusets provide a mechanism for assigning a set of CPUs and Memory Nodes to
a set of tasks."

The Kata agent brings compatibility the cgroup cpuset CPU on the guest side.

The cpuset CPU cgroup will be applied on two events:

containers creation.
container update.

When the runtime request to apply cpuset cgroup to the agent, the amout of
vcpus available may be not the same to the required in the request.

This is becasue the request from the agent client (the Kata runtime) pass
cpusets that were requested to be placed on the host. This is to isolate the
container workload on some specific host CPUs. The the runtime pass the
requested cpuset to the agent, that will try to apply the cgroup cpuset on the
guest.

Because the runtime will only hot-plug and amount the CPUs calculated based in
the container period and quota, the VM wont have the same amount CPUs than the host.
guest.

Example:

docker run -ti --cpus 2 --cpuset 0,1 busybox

The expected result is that the container is limited to the time of 2 CPUs, but
is only allowed to be scheduled on CPUs 0 and 1

In a similar case and a valid traditional container is the following:

docker run -ti --cpus 2 --cpuset 2,3,4 busybox

Where the container is limited to 2 CPUs and can be scheduled on CPU 2,3,4.

In the case of the Kata. The Kata runtime will only hotplug 2 cpus, making
imposible to request to the guest kernel to schedule the workload on vcpu
3,4.

cpuset best effort application.

Kata will try agent will evaluate this request and see if is possible to
apply the cpuset request on the guest.

If the cpuset request includes CPUs that are not available in the guest it
will ignore the request.
If the request has the CPUs requested in are available the request be
applied in the guest.

Fixes: #446

Signed-off-by: Jose Carlos Venegas Munoz [email protected]

devimc · 2019-01-29T15:03:02Z

cgroup.go

+		return "", err
+	}
+
+	cpusetGuestList, err := parsers.ParseUintList(strings.TrimSpace(string(cpusetGuest)))


cpusetGuest is a string, why is string() needed?

Good catch, I did not removed when factored out getCpusetGuest()

devimc · 2019-01-29T15:13:09Z

cgroup.go

+	for k := range cpusetListReq {
+		if !cpusetGuestList[k] {
+			agentLog.Warnf("cpu %d is not in guest cpuset, using %s cpuset", k, cpusetGuest)
+			return cpusetGuest, nil


getAvailableCpusetList returns all the cpus when a cpu in the list of cpuset is not available in the guest, for example if cpuset is 7, but the available cpus in the guest are 0-6, this function will return 0-6 even when only one cpu is requested. I wonder if this implementation is correct

For me it is difficult to decide if correct or not. Because it is traying to make more compatible a broken use case in Kata that is a current limitation because we run the workload in VM with a different set of CPUs

I prefer to not apply the cpuset at all instead of apply only where just a subset of them, why?

the workload is still limited to use limited cpu time based on cpu cgroup (cpu and qouta) so still be safe not consuming cpu time of other process.

for example if cpuset is 7, but the available cpus in the guest are 0-6, this function will return 0-6 even when only one cpu is requested

Lets translate to a docker example.

docker run -ti --runtime kata-runtime --cpuset-cpus 7 --cpus 6 --net none --rm busybox / # cat /proc/self/status | grep Cpus_allowed_list Cpus_allowed_list: 0-5

On the host the VM will run only on this cpu, so the workload as well with virtcontainers: reimplement sandbox cgroup runtime#1189

On the guest workload, the cpuset will allow to run the workload in all the vcpus (think cpuset on the host is still isolating our workload to jump to other process.

From user perspective on the host this is still correct because the VM is only using only the CPU 7

Another case:

docker run -tid --cpuset-cpu 1,5-9 --cpus 4 mycontainer

The cpuset list is valid on the host and thanks to virtcontainers: reimplement sandbox cgroup runtime#1189 this is going to be limited on the host and the VM is allow to have its 4cpu time on any of the 1,5-9 host cpus (we isolate the workload)

In the guest we will have 4 cpus, and cpuset list of 1,5-9, where only cpu 1 is available.

There was a few options.

Create a mapping for each cpu and vcpu, in this case is not even possible because host has at least 10 cpus and the guest hast 4.

Apply only the ones that are available, and ignore the rest, in this case the only cpu available is 1 so the workload would be able to run only on vcpu 1 but there rest of them wont be used by it, this mean waste of vcpus and the workload limited from the cpuset we partially apply to not get the cpu time of 4 cpus.

If the request has cpus that are not part of it, just ignore it. The cpuset is not applied on the guest and workload will be jumping around the guest vcpus but the host cpuset will prevent the VM going to not allowed host cpus, and the workload will be still limited to use 4 cpus because its cpu cgroup ( period and quota).

So I went for the last option where it if is possible the cpuset is applied. But if not there is still isolation at host level. The solution is not perfect but solve a very basic docker use case and works with cpu cgroup and cpuset host cgroup to work in a more compatible way.

docker run -tid --cpuset-cpu 2-4 --cpus 2 mycontainer

And should be the same for kubernetes when it isolate workloads to use a set of cpus. I still need to check this use case.

The cpuset itself needs to be translated before passing into the guest, imo. As it is provided, it is only valid for physical CPUs. Once cpuset is done on the host (kata-containers/runtime#1189), then this PR becomes relevant. IMO, after the runtime pins on the host, the cpuset information needs to be translated to map from physical CPUs to virtual CPUs in the guest. We shouldn't just pass the original request in.

@egernst agree. I work on follow up PRs to do this, this is only allowing to run Kata on K8s static policy and docker.

devimc · 2019-01-29T15:15:48Z

documentation/features/cpuset.md

+
+This is becasue the request from the agent client (the Kata runtime) pass
+cpusets that were requested to be placed on the host. This is to isolate the
+container workload on some specific host CPUs. The the runtime pass the


The the runtime

jcvenegas · 2019-02-01T20:45:20Z

Updated PR to fix issue found testing commit with Kuberentes (multi containers in a pod). This adds fix the agent when try to update cgroup parents with with the request. The parents now are updated with the max set of cpus in the guest.

grahamwhaley · 2019-02-04T13:59:53Z

Just going to try and tickle pullapprove on this, as we are testing it... and I need something not on the proxy repo. Going temporarily push a DNM label on it..

jodh-intel · 2019-02-04T14:07:42Z

🐟

klynnrif

Suggested changes from a grammar/structure/flow review. Thanks!

documentation/features/cpuset.md

jcvenegas · 2019-02-06T18:25:00Z

@klynnrif thank you! now looks better and easier to understand :)

jcvenegas · 2019-02-06T18:56:18Z

/test

jcvenegas · 2019-02-14T18:07:59Z

/test

jcvenegas · 2019-02-14T22:52:52Z

/test

ttx · 2019-02-15T07:53:57Z

/zuul-recheck

devimc · 2019-02-15T17:18:31Z

cgroup.go

+
+	for k := range cpusetListReq {
+		if !cpusetGuestList[k] {
+			agentLog.Warnf("cpuset '%s': cpu %d is not in guest cpu list, using guest cpus %s", cpusetReq, k, cpusetGuest)


WithFields.. please 😄

devimc · 2019-02-15T17:19:11Z

cgroup.go

+	}
+
+	// All the cpus are valid keep the same cpuset string
+	agentLog.Debugf("the requested cpuset '%s' is valid using it", cpusetReq)


WithFields

done, running again CI

jcvenegas · 2019-02-15T17:29:11Z

/test

devimc

thanks @jcvenegas

sboeuf

A few comments, but looks good!

sboeuf · 2019-02-15T19:25:07Z

documentation/features/cpuset.md

-Kata will try agent will evaluate this request and see if is possible to 
-apply the cpuset request on the guest.
+The Kata agent evaluates the request to see if it is possible to apply the
+cpuset request onto the guest.


Can't you fix this directly on the first commit introducing this new documentation?

sboeuf · 2019-02-15T19:25:31Z

cgroup_test.go

-		{"1", fakeGuestCpuset},
-		{"1,3", "2,13"},
+		{"1", "1"},
+		{"1,3", "1,3"},


Can't you fix this directly on the first commit introducing those tests?

I reorganized the commits

sboeuf · 2019-02-15T19:30:45Z

grpc.go

@@ -179,7 +179,7 @@ func updateContainerCpuset(cgroupPath string, newCpuset string, cookies cookie)
 		// Don't use c.container.Set because of it will modify container's config.
 		// c.container.Set MUST BE used only on update.
 		cpusetCpusPath := filepath.Join(cpusetPath, "cpuset.cpus")
-		agentLog.WithField("path", cpusetPath).Debug("updating cpuset cgroup")


why did you remove this log?

Probably miss to re-add when I move the code, I added back

The way that kata works is hotplugging cpus to the VM based in cpu and qouta, cpuset request may not match with the cpus requested from the host. Apply only if possible. Fixes: kata-containers#446 Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>

When the agent is requested to update cpuset, it also updates the cpuset for the parents with the cpuset requested. This is an issue when more than one container in the pod are running. Because containers may share the cgroup parent. To avoid those issues update the parents with max cpus. And containers cpusets whith the value requested. Signed-off-by: Jose Carlos Venegas Munoz <[email protected]>

jcvenegas · 2019-02-15T20:00:42Z

/test

sboeuf

Thanks @jcvenegas, lgtm

jcvenegas requested review from devimc, sboeuf and gnawux January 28, 2019 22:24

devimc reviewed Jan 29, 2019

View reviewed changes

jcvenegas force-pushed the cpuset-ignore branch from 27a6ec2 to 4d1f54e Compare February 1, 2019 20:36

jcvenegas requested a review from a team as a code owner February 1, 2019 20:36

jcvenegas force-pushed the cpuset-ignore branch 2 times, most recently from a70ea5b to 3ed31d7 Compare February 1, 2019 20:40

grahamwhaley added do-not-merge and removed do-not-merge labels Feb 4, 2019

klynnrif suggested changes Feb 5, 2019

View reviewed changes

jcvenegas force-pushed the cpuset-ignore branch 3 times, most recently from 9560c23 to e321f77 Compare February 6, 2019 18:23

klynnrif previously approved these changes Feb 11, 2019

View reviewed changes

jcvenegas dismissed klynnrif’s stale review via 28724bb February 14, 2019 15:11

jcvenegas force-pushed the cpuset-ignore branch 4 times, most recently from 049cff5 to fee39b1 Compare February 14, 2019 17:57

jcvenegas force-pushed the cpuset-ignore branch 2 times, most recently from a0d4fcc to 836c04c Compare February 14, 2019 22:02

ttx added wip and removed wip labels Feb 15, 2019

devimc reviewed Feb 15, 2019

View reviewed changes

jcvenegas force-pushed the cpuset-ignore branch from 836c04c to 31a745a Compare February 15, 2019 17:28

devimc previously approved these changes Feb 15, 2019

View reviewed changes

sboeuf reviewed Feb 15, 2019

View reviewed changes

jcvenegas added 2 commits February 15, 2019 13:43

jcvenegas dismissed devimc’s stale review via 67b2559 February 15, 2019 19:58

jcvenegas force-pushed the cpuset-ignore branch from 31a745a to 67b2559 Compare February 15, 2019 19:58

sboeuf approved these changes Feb 16, 2019

View reviewed changes

devimc approved these changes Feb 18, 2019

View reviewed changes

devimc merged commit e8ca07f into kata-containers:master Feb 18, 2019

egernst mentioned this pull request Feb 26, 2019

Stable 1.5.1 rc #465

Merged

jcvenegas deleted the cpuset-ignore branch February 13, 2020 13:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cgroup: ignore cpuset if can not be applied. #447

cgroup: ignore cpuset if can not be applied. #447

jcvenegas commented Jan 28, 2019

devimc Jan 29, 2019

jcvenegas Jan 29, 2019

devimc Jan 29, 2019

jcvenegas Jan 29, 2019

egernst Jan 29, 2019 •

edited

Loading

jcvenegas Feb 1, 2019

devimc Jan 29, 2019

jcvenegas Feb 1, 2019

jcvenegas commented Feb 1, 2019

grahamwhaley commented Feb 4, 2019

jodh-intel commented Feb 4, 2019

klynnrif left a comment

jcvenegas commented Feb 6, 2019

jcvenegas commented Feb 6, 2019

jcvenegas commented Feb 14, 2019

jcvenegas commented Feb 14, 2019

ttx commented Feb 15, 2019

devimc Feb 15, 2019

devimc Feb 15, 2019

jcvenegas Feb 15, 2019

jcvenegas commented Feb 15, 2019

devimc left a comment

sboeuf left a comment

sboeuf Feb 15, 2019

sboeuf Feb 15, 2019

jcvenegas Feb 15, 2019

sboeuf Feb 15, 2019

jcvenegas Feb 15, 2019

jcvenegas commented Feb 15, 2019

sboeuf left a comment

cgroup: ignore cpuset if can not be applied. #447

cgroup: ignore cpuset if can not be applied. #447

Conversation

jcvenegas commented Jan 28, 2019

Cpuset cgroup.

cpuset best effort application.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

egernst Jan 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcvenegas commented Feb 1, 2019

grahamwhaley commented Feb 4, 2019

jodh-intel commented Feb 4, 2019

klynnrif left a comment

Choose a reason for hiding this comment

jcvenegas commented Feb 6, 2019

jcvenegas commented Feb 6, 2019

jcvenegas commented Feb 14, 2019

jcvenegas commented Feb 14, 2019

ttx commented Feb 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcvenegas commented Feb 15, 2019

devimc left a comment

Choose a reason for hiding this comment

sboeuf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcvenegas commented Feb 15, 2019

sboeuf left a comment

Choose a reason for hiding this comment

egernst Jan 29, 2019 •

edited

Loading