From 8b4c299eb6128720ce2b4c1d350ae6bd04886ce5 Mon Sep 17 00:00:00 2001
From: Eric Ernst <eric@amperecomputing.com>
Date: Wed, 24 Jun 2020 23:53:16 +0000
Subject: [PATCH] sandbox: don't constrain cpus, mem only cpuset, devices

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>
---
 virtcontainers/container.go |  6 ++++
 virtcontainers/sandbox.go   | 55 +++++++++++++++++++++++++++----------
 2 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/virtcontainers/container.go b/virtcontainers/container.go
index 9e2d1e94b4..2f5279c866 100644
--- a/virtcontainers/container.go
+++ b/virtcontainers/container.go
@@ -1249,6 +1249,12 @@ func (c *Container) update(resources specs.LinuxResources) error {
 		if q := cpu.Quota; q != nil && *q != 0 {
 			c.config.Resources.CPU.Quota = q
 		}
+		if cpu.Cpus != "" {
+			c.config.Resources.CPU.Cpus = cpu.Cpus
+		}
+		if cpu.Mems != "" {
+			c.config.Resources.CPU.Mems = cpu.Mems
+		}
 	}
 
 	if c.config.Resources.Memory == nil {
diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go
index db9ec1920d..8c907b8091 100644
--- a/virtcontainers/sandbox.go
+++ b/virtcontainers/sandbox.go
@@ -632,15 +632,25 @@ func (s *Sandbox) createCgroupManager() error {
 	}
 
 	spec := s.GetPatchedOCISpec()
-	if spec != nil {
+	if spec != nil && spec.Linux != nil {
 		cgroupPath = spec.Linux.CgroupsPath
 
 		// Kata relies on the cgroup parent created and configured by the container
-		// engine, but sometimes the sandbox cgroup is not configured and the container
-		// may have access to all the resources, hence the runtime must constrain the
-		// sandbox and update the list of devices with the devices hotplugged in the
-		// hypervisor.
-		resources = *spec.Linux.Resources
+		// engine by default. The exception is for devices whitelist as well as sandbox-level
+		// CPUSet.
+		if spec.Linux.Resources != nil {
+			resources.Devices = spec.Linux.Resources.Devices
+
+			if spec.Linux.Resources.CPU != nil {
+				resources.CPU = &specs.LinuxCPU{
+					Cpus: spec.Linux.Resources.CPU.Cpus,
+				}
+			}
+		}
+
+		//TODO: in Docker or Podman use case, it is reasonable to set a constraint. Need to add a flag
+		// to allow users to configure Kata to constrain CPUs and Memory in this alternative
+		// scenario. See https://github.com/kata-containers/runtime/issues/2811
 	}
 
 	if s.devManager != nil {
@@ -1171,7 +1181,7 @@ func (s *Sandbox) CreateContainer(contConfig ContainerConfig) (VCContainer, erro
 		}
 	}()
 
-	// Sandbox is reponsable to update VM resources needed by Containers
+	// Sandbox is responsible to update VM resources needed by Containers
 	// Update resources after having added containers to the sandbox, since
 	// container status is requiered to know if more resources should be added.
 	err = s.updateResources()
@@ -1285,6 +1295,11 @@ func (s *Sandbox) DeleteContainer(containerID string) (VCContainer, error) {
 		}
 	}
 
+	// update the sandbox cgroup
+	if err = s.cgroupsUpdate(); err != nil {
+		return nil, err
+	}
+
 	if err = s.storeSandbox(); err != nil {
 		return nil, err
 	}
@@ -1814,11 +1829,12 @@ func (s *Sandbox) AddDevice(info config.DeviceInfo) (api.Device, error) {
 	return b, nil
 }
 
-// updateResources will calculate the resources required for the virtual machine, and
-// adjust the virtual machine sizing accordingly. For a given sandbox, it will calculate the
-// number of vCPUs required based on the sum of container requests, plus default CPUs for the VM.
-// Similar is done for memory. If changes in memory or CPU are made, the VM will be updated and
-// the agent will online the applicable CPU and memory.
+// updateResources will:
+// - calculate the resources required for the virtual machine, and adjust the virtual machine
+// sizing accordingly. For a given sandbox, it will calculate the number of vCPUs required based
+// on the sum of container requests, plus default CPUs for the VM. Similar is done for memory.
+// If changes in memory or CPU are made, the VM will be updated and the agent will online the
+// applicable CPU and memory.
 func (s *Sandbox) updateResources() error {
 	if s == nil {
 		return errors.New("sandbox is nil")
@@ -1921,9 +1937,20 @@ func (s *Sandbox) GetHypervisorType() string {
 func (s *Sandbox) cgroupsUpdate() error {
 
 	// If Kata is configured for SandboxCgroupOnly, the VMM and its processes are already
-	// in the Kata sandbox cgroup (inherited). No need to move threads/processes, and we should
-	// rely on parent's cgroup CPU/memory values
+	// in the Kata sandbox cgroup (inherited). Check to see if sandbox cpuset needs to be
+	// updated.
 	if s.config.SandboxCgroupOnly {
+		cpuset, err := s.getSandboxCPUSet()
+		if err != nil {
+			return err
+		}
+
+		if cpuset != "" {
+			if err := s.cgroupMgr.SetCPUSet(cpuset); err != nil {
+				return err
+			}
+		}
+
 		return nil
 	}