Merge pull request #1346 from jcvenegas/resource-calc-multi-container

virtcontainers: move resource calculation to its own function

virtcontainers/sandbox.go

@@ -7,6 +7,7 @@ package virtcontainers
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -1608,36 +1609,21 @@ func (s *Sandbox) AddDevice(info config.DeviceInfo) (api.Device, error) {
 func (s *Sandbox) updateResources() error {
 	// the hypervisor.MemorySize is the amount of memory reserved for
 	// the VM and contaniners without memory limit
-	sumResources := specs.LinuxResources{
+
-		Memory: &specs.LinuxMemory{
+	if s == nil {
-			Limit: new(int64),
+		return errors.New("sandbox is nil")
-		},
-		CPU: &specs.LinuxCPU{
-			Period: new(uint64),
-			Quota:  new(int64),
-		},
 	}
 
-	var mCPU uint32
+	if s.config == nil {
-
+		return fmt.Errorf("sandbox config is nil")
-	// Calculate running total of memory and mCPUs requested
-	for _, c := range s.config.Containers {
-		if m := c.Resources.Memory; m != nil && m.Limit != nil {
-			*sumResources.Memory.Limit += *m.Limit
-		}
-		if cpu := c.Resources.CPU; cpu != nil {
-			if cpu.Period != nil && cpu.Quota != nil {
-				mCPU += utils.CalculateMilliCPUs(*cpu.Quota, *cpu.Period)
-			}
-
-		}
 	}
 
-	sandboxVCPUs := utils.CalculateVCpusFromMilliCpus(mCPU)
+	sandboxVCPUs := s.calculateSandboxCPUs()
+	// Add default vcpus for sandbox
 	sandboxVCPUs += s.hypervisor.hypervisorConfig().NumVCPUs
 
 	sandboxMemoryByte := int64(s.hypervisor.hypervisorConfig().MemorySize) << utils.MibToBytesShift
-	sandboxMemoryByte += *sumResources.Memory.Limit
+	sandboxMemoryByte += s.calculateSandboxMemory()
 
 	// Update VCPUs
 	s.Logger().WithField("cpus-sandbox", sandboxVCPUs).Debugf("Request to hypervisor to update vCPUs")
@@ -1666,3 +1652,27 @@ func (s *Sandbox) updateResources() error {
 	}
 	return nil
 }
+
+func (s *Sandbox) calculateSandboxMemory() int64 {
+	memorySandbox := int64(0)
+	for _, c := range s.config.Containers {
+		if m := c.Resources.Memory; m != nil && m.Limit != nil {
+			memorySandbox += *m.Limit
+		}
+	}
+	return memorySandbox
+}
+
+func (s *Sandbox) calculateSandboxCPUs() uint32 {
+	mCPU := uint32(0)
+
+	for _, c := range s.config.Containers {
+		if cpu := c.Resources.CPU; cpu != nil {
+			if cpu.Period != nil && cpu.Quota != nil {
+				mCPU += utils.CalculateMilliCPUs(*cpu.Quota, *cpu.Period)
+			}
+
+		}
+	}
+	return utils.CalculateVCpusFromMilliCpus(mCPU)
+}

virtcontainers/sandbox_test.go

@@ -19,14 +19,14 @@ import (
 	"syscall"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
-
 	"github.com/kata-containers/runtime/virtcontainers/device/config"
 	"github.com/kata-containers/runtime/virtcontainers/device/drivers"
 	"github.com/kata-containers/runtime/virtcontainers/device/manager"
 	"github.com/kata-containers/runtime/virtcontainers/pkg/annotations"
 	"github.com/kata-containers/runtime/virtcontainers/store"
 	"github.com/kata-containers/runtime/virtcontainers/types"
+	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/stretchr/testify/assert"
 	"golang.org/x/sys/unix"
 )
 
@@ -107,6 +107,67 @@ func TestCreateMockSandbox(t *testing.T) {
 	defer cleanUp()
 }
 
+func TestCalculateSandboxCPUs(t *testing.T) {
+	sandbox := &Sandbox{}
+	sandbox.config = &SandboxConfig{}
+	unconstrained := newTestContainerConfigNoop("cont-00001")
+	constrained := newTestContainerConfigNoop("cont-00001")
+	quota := int64(4000)
+	period := uint64(1000)
+	constrained.Resources.CPU = &specs.LinuxCPU{Period: &period, Quota: &quota}
+
+	tests := []struct {
+		name       string
+		containers []ContainerConfig
+		want       uint32
+	}{
+		{"1-unconstrained", []ContainerConfig{unconstrained}, 0},
+		{"2-unconstrained", []ContainerConfig{unconstrained, unconstrained}, 0},
+		{"1-constrained", []ContainerConfig{constrained}, 4},
+		{"2-constrained", []ContainerConfig{constrained, constrained}, 8},
+		{"3-mix-constraints", []ContainerConfig{unconstrained, constrained, constrained}, 8},
+		{"3-constrained", []ContainerConfig{constrained, constrained, constrained}, 12},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			sandbox.config.Containers = tt.containers
+			if got := sandbox.calculateSandboxCPUs(); got != tt.want {
+				t.Errorf("calculateSandboxCPUs() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestCalculateSandboxMem(t *testing.T) {
+	sandbox := &Sandbox{}
+	sandbox.config = &SandboxConfig{}
+	unconstrained := newTestContainerConfigNoop("cont-00001")
+	constrained := newTestContainerConfigNoop("cont-00001")
+	limit := int64(4000)
+	constrained.Resources.Memory = &specs.LinuxMemory{Limit: &limit}
+
+	tests := []struct {
+		name       string
+		containers []ContainerConfig
+		want       int64
+	}{
+		{"1-unconstrained", []ContainerConfig{unconstrained}, 0},
+		{"2-unconstrained", []ContainerConfig{unconstrained, unconstrained}, 0},
+		{"1-constrained", []ContainerConfig{constrained}, limit},
+		{"2-constrained", []ContainerConfig{constrained, constrained}, limit * 2},
+		{"3-mix-constraints", []ContainerConfig{unconstrained, constrained, constrained}, limit * 2},
+		{"3-constrained", []ContainerConfig{constrained, constrained, constrained}, limit * 3},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			sandbox.config.Containers = tt.containers
+			if got := sandbox.calculateSandboxMemory(); got != tt.want {
+				t.Errorf("calculateSandboxMemory() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestCreateSandboxEmptyID(t *testing.T) {
 	hConfig := newHypervisorConfig(nil, nil)
 
@@ -1574,3 +1635,48 @@ func TestSandboxCreationFromConfigRollbackFromCreateSandbox(t *testing.T) {
 	err = checkSandboxRemains()
 	assert.NoError(err)
 }
+
+func TestSandboxUpdateResources(t *testing.T) {
+	contConfig1 := newTestContainerConfigNoop("cont-00001")
+	contConfig2 := newTestContainerConfigNoop("cont-00002")
+	hConfig := newHypervisorConfig(nil, nil)
+
+	defer cleanUp()
+	// create a sandbox
+	s, err := testCreateSandbox(t,
+		testSandboxID,
+		MockHypervisor,
+		hConfig,
+		NoopAgentType,
+		NetworkConfig{},
+		[]ContainerConfig{contConfig1, contConfig2},
+		nil)
+
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = s.updateResources()
+	if err != nil {
+		t.Fatal(err)
+	}
+	containerMemLimit := int64(1000)
+	containerCPUPeriod := uint64(1000)
+	containerCPUQouta := int64(5)
+	for _, c := range s.config.Containers {
+		c.Resources.Memory = &specs.LinuxMemory{
+			Limit: new(int64),
+		}
+		c.Resources.CPU = &specs.LinuxCPU{
+			Period: new(uint64),
+			Quota:  new(int64),
+		}
+		c.Resources.Memory.Limit = &containerMemLimit
+		c.Resources.CPU.Period = &containerCPUPeriod
+		c.Resources.CPU.Quota = &containerCPUQouta
+	}
+	err = s.updateResources()
+	if err != nil {
+		t.Fatal(err)
+	}
+}