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

overconstraining sandbox in case of k8s #2792

Closed
egernst opened this issue Jun 24, 2020 · 3 comments
Closed

overconstraining sandbox in case of k8s #2792

egernst opened this issue Jun 24, 2020 · 3 comments
Labels
bug Incorrect behaviour needs-review Needs to be assessed by the team.

Comments

@egernst
Copy link
Member

egernst commented Jun 24, 2020

Description of problem

#2409 introduced an undesirable behavior in the scenario where Kata is being integrated with Kubernetes. In this case, kubelet manages the sandbox cgroup, including memory and cpu subsystems. By applying these cgroups, we are over-constraining the sandbox cgroup

Expected result

CPU and Memory subsystems are left untouched by default.

Actual result

It looks like the cgroup would be set based on pause container in case of k8s, or at least doesn't appear to take into consideration the sum of container requests. Further, even if it was considering container summation, it is not able to apply an overhead for running in a sandbox.

I'd recommend reverting 2409 and hiding this behavior change behind a non-default flag.
@egernst egernst added bug Incorrect behaviour needs-review Needs to be assessed by the team. labels Jun 24, 2020
@egernst egernst changed the title By default, we should not apply mem/cpu subsystem cgroups for sandbox on host overconstraining sandbox in case of k8s Jun 24, 2020
@egernst
Copy link
Member Author

egernst commented Jun 24, 2020

heads up @devimc @jcvenegas

@jcvenegas
Copy link
Member

Thanks for open the issue, so now given that sandbox_cgroup_only=true additionally an annotation has to be given from the CRI runtime used?

Correct me if I am wrong but this is a consequence of the fragmentation on how Kata is expected to manage resources. PodLavel vs Container Level. Based on our previous checks, trying to do assumptions of how resources should be contain for a Pod makes difficult. I still missing context, but #2408 documents that sandbox_cgroup_only=true + docker was not applying limits. Something we could do is suggest if folks want docker limited host level keep using sandbox_cgroup_only=false.

Anyway, some questions may help us to understand.

  1. Does it CRI runtime annotation is not passed when running in K8s ?
  2. Do we want a context-dependent configuration? If yes we need some good testing to avoid hide this kind of issue.

@egernst
Copy link
Member Author

egernst commented Jun 26, 2020 •
edited

There is no annotation from CRI to indicate K8S.

K8S is and should be the default configuration for Kata Containers (and is the priority). The prior PR breaks this. I would recommend creating a flag in toml for indicating that the runtime should manage cpu/mem group subsystems on the host, and this should be disabled by default. This would be set if folks want to constrain and are using Docker or Podman.

In any case - we should be careful not to break kata+k8s...

egernst pushed a commit that referenced this issue Jun 29, 2020
cgroups: don't constrain cpus,mem for sandbox, only cpuset
193a960 
There is a mix of handling in the runtime code around cgroups.

Let's go ahead and allow for constraining the cpuset. 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,
started or updated, or when containers are removed.

To facilitate this without rewriting the 'non constrained cgroup'
handling, let's go ahead and add to the Sandbox's updateResources
function.

Fixes: #2792

Signed-off-by: Eric Ernst <eric@amperecomputing.com>
egernst pushed a commit that referenced this issue Jun 29, 2020
cgroups: don't constrain cpus,mem for sandbox, only cpuset,devices
f465867 
There is a mix of handling in the runtime code around cgroups.

Let's go ahead and 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,
started or updated, or when containers are removed.

To facilitate this without rewriting the 'non constrained cgroup'
handling, let's go ahead and add to the Sandbox's updateResources
function.

Fixes: #2792

Signed-off-by: Eric Ernst <eric@amperecomputing.com>
egernst pushed a commit that referenced this issue Jul 1, 2020
cgroups: don't constrain cpus,mem for sandbox, only cpuset,devices
0e16b4e 
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 go ahead and add to the Sandbox's cgroupsUpdat
function.

Fixes: #2792

Signed-off-by: Eric Ernst <eric@amperecomputing.com>
egernst pushed a commit that referenced this issue Jul 1, 2020
cgroups: don't constrain cpus,mem for sandbox, only cpuset,devices
d0cffb4 
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 go ahead and add to the Sandbox's cgroupsUpdat
function.

Fixes: #2792

Signed-off-by: Eric Ernst <eric@amperecomputing.com>
egernst pushed a commit that referenced this issue Jul 1, 2020
cgroups: don't constrain cpus,mem for sandbox, only cpuset,devices
97b41ce 
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 go ahead and add to the Sandbox's cgroupsUpdat
function.

Fixes: #2792

Signed-off-by: Eric Ernst <eric@amperecomputing.com>
egernst pushed a commit that referenced this issue Jul 1, 2020
cgroups: don't constrain cpus,mem for sandbox, only cpuset,devices
73412a8 
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 go ahead and add to the Sandbox's cgroupsUpdat
function.

Fixes: #2792

Signed-off-by: Eric Ernst <eric@amperecomputing.com>
egernst pushed a commit that referenced this issue Jul 1, 2020
cgroups: don't constrain cpus,mem for sandbox, only cpuset,devices
1017646 
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 go ahead and add to the Sandbox's cgroupsUpdat
function.

Fixes: #2792

Signed-off-by: Eric Ernst <eric@amperecomputing.com>
egernst pushed a commit that referenced this issue Jul 1, 2020
sandbox: don't constrain cpus, mem only cpuset, devices
aa4a07f 
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 go ahead and add to the Sandbox's cgroupsUpdat
function.

Fixes: #2792

Signed-off-by: Eric Ernst <eric@amperecomputing.com>
egernst pushed a commit that referenced this issue Jul 8, 2020
sandbox: don't constrain cpus, mem only cpuset, devices
23e60b8 
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 go ahead and add to the Sandbox's cgroupsUpdat
function.

Fixes: #2792

Signed-off-by: Eric Ernst <eric@amperecomputing.com>
egernst pushed a commit that referenced this issue Jul 8, 2020
sandbox: don't constrain cpus, mem only cpuset, devices
430b826 
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 go ahead and add to the Sandbox's cgroupsUpdat
function.

Fixes: #2792

Signed-off-by: Eric Ernst <eric@amperecomputing.com>
egernst pushed a commit that referenced this issue Jul 8, 2020
sandbox: don't constrain cpus, mem only cpuset, devices
b420ae0 
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 go ahead and add to the Sandbox's cgroupsUpdat
function.

Fixes: #2792

Signed-off-by: Eric Ernst <eric@amperecomputing.com>
egernst pushed a commit that referenced this issue Jul 8, 2020
sandbox: don't constrain cpus, mem only cpuset, devices
f86a1a8 
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 pushed a commit that referenced this issue Jul 16, 2020
sandbox: don't constrain cpus, mem only cpuset, devices
52af634 
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>
@jodh-intel jodh-intel mentioned this issue Aug 26, 2020
Open
fidencio pushed a commit to fidencio/kata-runtime that referenced this issue Sep 7, 2020
@fidencio
sandbox: don't constrain cpus, mem only cpuset, devices
b4d0a62 
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: kata-containers#2792

Signed-off-by: Eric Ernst <eric@amperecomputing.com>
(cherry picked from commit 8b4c299)
fidencio pushed a commit to fidencio/kata-runtime that referenced this issue Sep 11, 2020
@fidencio
sandbox: don't constrain cpus, mem only cpuset, devices
bea6af5 
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: kata-containers#2792

Signed-off-by: Eric Ernst <eric@amperecomputing.com>
(cherry picked from commit 8b4c299)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Incorrect behaviour needs-review Needs to be assessed by the team.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@egernst @jcvenegas