grpc: honour CPU constraints in Kubernetes #242
Conversation
| // Each cpuset cgroup MUST BE updated with the actual number of vCPUs. | ||
| cpusetPath := sysfsCpusetPath | ||
| cgroupsPaths := strings.Split(contConfig.Cgroups.Path, "/") | ||
| for _, path := range cgroupsPaths { |
There was a problem hiding this comment.
Does cgroupsPaths only contain cpu cgroup path?
There was a problem hiding this comment.
From cgroup_linux.go
specifies the path to cgroups that are created and/or joined by the container.
The path is assumed to be relative to the host system cgroup mountpoint.
For example, if contConfig.Cgroups.Path is docker/3201e3829cc65216f462da39363e8f36a10d80c63ece7f55818c9932b7a899a4, then each cgroup (memory, cpu, pids, cpuset, etc) contains a directory with this path
$ ls /sys/fs/cgroup/cpuset
cgroup.clone_children cpuset.memory_pressure_enabled
cgroup.procs cpuset.memory_spread_page
cgroup.sane_behavior cpuset.memory_spread_slab
cpuset.cpu_exclusive cpuset.mems
cpuset.cpus cpuset.sched_load_balance
cpuset.effective_cpus cpuset.sched_relax_domain_level
cpuset.effective_mems docker
cpuset.mem_exclusive notify_on_release
cpuset.mem_hardwall release_agent
cpuset.memory_migrate tasks
cpuset.memory_pressure
$ ls /sys/fs/cgroup/cpuset/docker
3201e3829cc65216f462da39363e8f36a10d80c63ece7f55818c9932b7a899a4
cgroup.clone_children
cgroup.procs
cpuset.cpu_exclusive
cpuset.cpus
cpuset.effective_cpus
cpuset.effective_mems
cpuset.mem_exclusive
cpuset.mem_hardwall
cpuset.memory_migrate
cpuset.memory_pressure
cpuset.memory_spread_page
cpuset.memory_spread_slab
cpuset.mems
cpuset.sched_load_balance
cpuset.sched_relax_domain_level
notify_on_release
tasks
|
see failure in tests: |
There was a problem hiding this comment.
Do we have tests (or atleast a https://github.com/kata-containers/tests issue to create new tests) for this change?
| @@ -57,7 +56,7 @@ var ( | |||
| sysfsCPUOnlinePath = "/sys/devices/system/cpu" | |||
| sysfsMemOnlinePath = "/sys/devices/system/memory" | |||
| sysfsConnectedCPUsPath = filepath.Join(sysfsCPUOnlinePath, "online") | |||
| sysfsDockerCpusetPath = "/sys/fs/cgroup/cpuset/docker/cpuset.cpus" | |||
| sysfsCpusetPath = "/sys/fs/cgroup/cpuset" | |||
grpc.go
Outdated
|
|
||
| agentLog.Debugf("Number of vCPUs to connect: %d", req.NbCpus) |
There was a problem hiding this comment.
Nit: stylistically, I prefer to use WithField() or WithFields() for this message and others below:
agentLog.WithField("vcpus-to-connect", req.NbCpus).Debug("connecting vCPUs)
It also makes the logs easier+safer to parse as those fields are less likely to change.
|
@jodh-intel I filed an issue kata-containers/tests#309 |
devimc
force-pushed
the
cpu/ks8FixConstraints
branch
2 times, most recently
from
08727a6 to
7fd38b0
Compare
Codecov Report
@@ Coverage Diff @@
## master #242 +/- ##
==========================================
+ Coverage 41.8% 42.79% +0.98%
==========================================
Files 13 13
Lines 1935 2033 +98
==========================================
+ Hits 809 870 +61
- Misses 1027 1056 +29
- Partials 99 107 +8
Continue to review full report at Codecov.
|
Once all vCPUs have been connected, cpuset cgroup MUST BE updated, to achieve that, each cpuset cgroup parent of the container MUST BE updated with the actual range of vCPUs. fixes kata-containers#239 fixes kata-containers#232 Signed-off-by: Julio Montes <[email protected]>
devimc
force-pushed
the
cpu/ks8FixConstraints
branch
from
7fd38b0 to
4bad43e
Compare
|
@jodh-intel @egernst please take a look |
|
lgtm |
Once all vCPUs have been connected, cpuset cgroup MUST BE updated,
to achieve that, each cpuset cgroup parent of the container
MUST BE updated with the actual range of vCPUs.
fixes #239
Signed-off-by: Julio Montes [email protected]