From b812d4f7fae76679bb7e13b8508923aaf87a9794 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 12 Oct 2020 16:56:01 -0700 Subject: [PATCH 1/7] virtcontainers: add method for calculating cpuset for sandbox Calculate sandbox's CPUSet as the union of each of the container's CPUSets. Signed-off-by: Eric Ernst --- src/runtime/virtcontainers/sandbox.go | 22 +++++ src/runtime/virtcontainers/sandbox_test.go | 106 +++++++++++++++++++++ 2 files changed, 128 insertions(+) diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index a393e5c76bbf..5b23ce08da86 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -43,6 +43,7 @@ import ( vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" + "k8s.io/kubernetes/pkg/kubelet/cm/cpuset" ) const ( @@ -2275,3 +2276,24 @@ func (s *Sandbox) GetOOMEvent() (string, error) { func (s *Sandbox) GetAgentURL() (string, error) { return s.agent.getAgentURL() } + +// getSandboxCPUSet returns the union of each of the sandbox's containers' CPU sets +// as a string in canonical linux CPU list format +func (s *Sandbox) getSandboxCPUSet() (string, error) { + if s.config == nil { + return "", nil + } + + result := cpuset.NewCPUSet() + for _, ctr := range s.config.Containers { + if ctr.Resources.CPU != nil { + currSet, err := cpuset.Parse(ctr.Resources.CPU.Cpus) + if err != nil { + return "", fmt.Errorf("unable to parse CPUset for container %s", ctr.ID) + } + result = result.Union(currSet) + } + } + + return result.String(), nil +} diff --git a/src/runtime/virtcontainers/sandbox_test.go b/src/runtime/virtcontainers/sandbox_test.go index 4af47ffaec49..8345236f0fbd 100644 --- a/src/runtime/virtcontainers/sandbox_test.go +++ b/src/runtime/virtcontainers/sandbox_test.go @@ -1419,3 +1419,109 @@ func TestSandbox_SetupSandboxCgroup(t *testing.T) { }) } } + +func getContainerConfigWithCPUSet(cpuset string) ContainerConfig { + return ContainerConfig{ + Resources: specs.LinuxResources{ + CPU: &specs.LinuxCPU{ + Cpus: cpuset, + }, + }, + } +} + +func getSimpleSandbox(cpuset0, cpuset1, cpuset2 string) *Sandbox { + sandbox := Sandbox{} + + sandbox.config = &SandboxConfig{ + Containers: []ContainerConfig{ + getContainerConfigWithCPUSet(cpuset0), + getContainerConfigWithCPUSet(cpuset1), + getContainerConfigWithCPUSet(cpuset2), + }, + } + + return &sandbox +} + +func TestGetSandboxCpuSet(t *testing.T) { + + tests := []struct { + name string + cpuset0 string + cpuset1 string + cpuset2 string + result string + wantErr bool + }{ + { + "single, no cpuset", + "", + "", + "", + "", + false, + }, + { + "single cpuset", + "0", + "", + "", + "0", + false, + }, + { + "two duplicate cpuset", + "0", + "0", + "", + "0", + false, + }, + { + "3 cpusets", + "0-3", + "5-7", + "1", + "0-3,5-7", + false, + }, + { + "weird, but should be okay", + "0-3", + "99999", + "", + "0-3,99999", + false, + }, + { + "two, overlapping cpuset", + "0-3", + "1-2", + "", + "0-3", + false, + }, + { + "garbage, should fail", + "7 beard-seconds", + "Audrey + 7", + "Elliott - 17", + "", + true, + }, + } + for _, tt := range tests { + + t.Run(tt.name, func(t *testing.T) { + s := getSimpleSandbox(tt.cpuset0, tt.cpuset1, tt.cpuset2) + res, err := s.getSandboxCPUSet() + if (err != nil) != tt.wantErr { + t.Errorf("getSandboxCPUSet() error = %v, wantErr %v", err, tt.wantErr) + } + if res != tt.result { + t.Errorf("getSandboxCPUSet() result = %s, wanted result %s", res, tt.result) + } + }) + } +} From b6cf68a9857320c32399dd3b7829ce8d88334088 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 12 Oct 2020 17:12:26 -0700 Subject: [PATCH 2/7] cgroups: add ability to update CPUSet Add function for applying a cpuset change to a cgroup Signed-off-by: Eric Ernst --- src/runtime/virtcontainers/pkg/cgroups/manager.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/runtime/virtcontainers/pkg/cgroups/manager.go b/src/runtime/virtcontainers/pkg/cgroups/manager.go index fc3462c13ea0..e0f81777a12a 100644 --- a/src/runtime/virtcontainers/pkg/cgroups/manager.go +++ b/src/runtime/virtcontainers/pkg/cgroups/manager.go @@ -331,3 +331,16 @@ func (m *Manager) RemoveDevice(device string) error { m.Unlock() return fmt.Errorf("device %v not found in the cgroup", device) } + +func (m *Manager) SetCPUSet(cpuset string) error { + cgroups, err := m.GetCgroups() + if err != nil { + return err + } + + m.Lock() + cgroups.CpusetCpus = cpuset + m.Unlock() + + return m.Apply() +} From 12cc0ee168d49bb179bb9b65cbcf83ed2d4f6d87 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 12 Oct 2020 17:13:01 -0700 Subject: [PATCH 3/7] 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. Signed-off-by: Eric Ernst --- src/runtime/virtcontainers/container.go | 6 +++ src/runtime/virtcontainers/sandbox.go | 55 ++++++++++++++++++------- 2 files changed, 47 insertions(+), 14 deletions(-) diff --git a/src/runtime/virtcontainers/container.go b/src/runtime/virtcontainers/container.go index 449b84276e6d..2c49b14bc524 100644 --- a/src/runtime/virtcontainers/container.go +++ b/src/runtime/virtcontainers/container.go @@ -1139,6 +1139,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/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index 5b23ce08da86..73b2e28c6781 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -565,15 +565,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 { @@ -1216,7 +1226,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() @@ -1330,6 +1340,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 } @@ -1867,11 +1882,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") @@ -1976,9 +1992,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 } From 2d690536b850162ed134e7fa4b9efbdd3f00d6eb Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 12 Oct 2020 18:10:27 -0700 Subject: [PATCH 4/7] cpuset: add cpuset pkg Pulled from 1.18.4 Kubernetes, adding the cpuset pkg for managing CPUSet calculations on the host. Go mod'ing the original code from k8s.io/kubernetes was very painful, and this is very static, so let's just pull in what we need. Signed-off-by: Eric Ernst --- src/runtime/go.sum | 1 + .../virtcontainers/pkg/cpuset/cpuset.go | 296 +++++++++++++++ .../virtcontainers/pkg/cpuset/cpuset_test.go | 348 ++++++++++++++++++ src/runtime/virtcontainers/sandbox.go | 2 +- 4 files changed, 646 insertions(+), 1 deletion(-) create mode 100644 src/runtime/virtcontainers/pkg/cpuset/cpuset.go create mode 100644 src/runtime/virtcontainers/pkg/cpuset/cpuset_test.go diff --git a/src/runtime/go.sum b/src/runtime/go.sum index a6ab1c6c2856..ee967e706791 100644 --- a/src/runtime/go.sum +++ b/src/runtime/go.sum @@ -198,6 +198,7 @@ github.com/juju/errors v0.0.0-20180806074554-22422dad46e1/go.mod h1:W54LbzXuIE0b github.com/juju/loggo v0.0.0-20190526231331-6e530bcce5d8/go.mod h1:vgyd7OREkbtVEN/8IXZe5Ooef3LQePvuBm9UWj6ZL8U= github.com/juju/testing v0.0.0-20190613124551-e81189438503/go.mod h1:63prj8cnj0tU0S9OHjGJn+b1h0ZghCndfnbQolrYTwA= github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w= +github.com/kata-containers/kata-containers v0.0.0-20201013034856-c88820454d08 h1:yk9fzLKb9RmV9xuT5mkJw4owk/K0rX5cusm2ukEEDro= github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/konsorten/go-windows-terminal-sequences v1.0.1 h1:mweAR1A6xJ3oS2pRaGiHgQ4OO8tzTaLawm8vnODuwDk= diff --git a/src/runtime/virtcontainers/pkg/cpuset/cpuset.go b/src/runtime/virtcontainers/pkg/cpuset/cpuset.go new file mode 100644 index 000000000000..ee944c97143e --- /dev/null +++ b/src/runtime/virtcontainers/pkg/cpuset/cpuset.go @@ -0,0 +1,296 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +// Copyright (c) 2017 The Kubernetes Authors +// SPDX-License-Identifier: Apache-2.0 + +package cpuset + +import ( + "bytes" + "fmt" + "reflect" + "sort" + "strconv" + "strings" +) + +// Builder is a mutable builder for CPUSet. Functions that mutate instances +// of this type are not thread-safe. +type Builder struct { + result CPUSet + done bool +} + +// NewBuilder returns a mutable CPUSet builder. +func NewBuilder() Builder { + return Builder{ + result: CPUSet{ + elems: map[int]struct{}{}, + }, + } +} + +// Add adds the supplied elements to the result. Calling Add after calling +// Result has no effect. +func (b Builder) Add(elems ...int) { + if b.done { + return + } + for _, elem := range elems { + b.result.elems[elem] = struct{}{} + } +} + +// Result returns the result CPUSet containing all elements that were +// previously added to this builder. Subsequent calls to Add have no effect. +func (b Builder) Result() CPUSet { + b.done = true + return b.result +} + +// CPUSet is a thread-safe, immutable set-like data structure for CPU IDs. +type CPUSet struct { + elems map[int]struct{} +} + +// NewCPUSet returns a new CPUSet containing the supplied elements. +func NewCPUSet(cpus ...int) CPUSet { + b := NewBuilder() + for _, c := range cpus { + b.Add(c) + } + return b.Result() +} + +// Size returns the number of elements in this set. +func (s CPUSet) Size() int { + return len(s.elems) +} + +// IsEmpty returns true if there are zero elements in this set. +func (s CPUSet) IsEmpty() bool { + return s.Size() == 0 +} + +// Contains returns true if the supplied element is present in this set. +func (s CPUSet) Contains(cpu int) bool { + _, found := s.elems[cpu] + return found +} + +// Equals returns true if the supplied set contains exactly the same elements +// as this set (s IsSubsetOf s2 and s2 IsSubsetOf s). +func (s CPUSet) Equals(s2 CPUSet) bool { + return reflect.DeepEqual(s.elems, s2.elems) +} + +// Filter returns a new CPU set that contains all of the elements from this +// set that match the supplied predicate, without mutating the source set. +func (s CPUSet) Filter(predicate func(int) bool) CPUSet { + b := NewBuilder() + for cpu := range s.elems { + if predicate(cpu) { + b.Add(cpu) + } + } + return b.Result() +} + +// FilterNot returns a new CPU set that contains all of the elements from this +// set that do not match the supplied predicate, without mutating the source +// set. +func (s CPUSet) FilterNot(predicate func(int) bool) CPUSet { + b := NewBuilder() + for cpu := range s.elems { + if !predicate(cpu) { + b.Add(cpu) + } + } + return b.Result() +} + +// IsSubsetOf returns true if the supplied set contains all the elements +func (s CPUSet) IsSubsetOf(s2 CPUSet) bool { + result := true + for cpu := range s.elems { + if !s2.Contains(cpu) { + result = false + break + } + } + return result +} + +// Union returns a new CPU set that contains all of the elements from this +// set and all of the elements from the supplied set, without mutating +// either source set. +func (s CPUSet) Union(s2 CPUSet) CPUSet { + b := NewBuilder() + for cpu := range s.elems { + b.Add(cpu) + } + for cpu := range s2.elems { + b.Add(cpu) + } + return b.Result() +} + +// UnionAll returns a new CPU set that contains all of the elements from this +// set and all of the elements from the supplied sets, without mutating +// either source set. +func (s CPUSet) UnionAll(s2 []CPUSet) CPUSet { + b := NewBuilder() + for cpu := range s.elems { + b.Add(cpu) + } + for _, cs := range s2 { + for cpu := range cs.elems { + b.Add(cpu) + } + } + return b.Result() +} + +// Intersection returns a new CPU set that contains all of the elements +// that are present in both this set and the supplied set, without mutating +// either source set. +func (s CPUSet) Intersection(s2 CPUSet) CPUSet { + return s.Filter(func(cpu int) bool { return s2.Contains(cpu) }) +} + +// Difference returns a new CPU set that contains all of the elements that +// are present in this set and not the supplied set, without mutating either +// source set. +func (s CPUSet) Difference(s2 CPUSet) CPUSet { + return s.FilterNot(func(cpu int) bool { return s2.Contains(cpu) }) +} + +// ToSlice returns a slice of integers that contains all elements from +// this set. +func (s CPUSet) ToSlice() []int { + result := []int{} + for cpu := range s.elems { + result = append(result, cpu) + } + sort.Ints(result) + return result +} + +// ToSliceNoSort returns a slice of integers that contains all elements from +// this set. +func (s CPUSet) ToSliceNoSort() []int { + result := []int{} + for cpu := range s.elems { + result = append(result, cpu) + } + return result +} + +// String returns a new string representation of the elements in this CPU set +// in canonical linux CPU list format. +// +// See: http://man7.org/linux/man-pages/man7/cpuset.7.html#FORMATS +func (s CPUSet) String() string { + if s.IsEmpty() { + return "" + } + + elems := s.ToSlice() + + type rng struct { + start int + end int + } + + ranges := []rng{{elems[0], elems[0]}} + + for i := 1; i < len(elems); i++ { + lastRange := &ranges[len(ranges)-1] + // if this element is adjacent to the high end of the last range + if elems[i] == lastRange.end+1 { + // then extend the last range to include this element + lastRange.end = elems[i] + continue + } + // otherwise, start a new range beginning with this element + ranges = append(ranges, rng{elems[i], elems[i]}) + } + + // construct string from ranges + var result bytes.Buffer + for _, r := range ranges { + if r.start == r.end { + result.WriteString(strconv.Itoa(r.start)) + } else { + result.WriteString(fmt.Sprintf("%d-%d", r.start, r.end)) + } + result.WriteString(",") + } + return strings.TrimRight(result.String(), ",") +} + +// Parse CPUSet constructs a new CPU set from a Linux CPU list formatted string. +// +// See: http://man7.org/linux/man-pages/man7/cpuset.7.html#FORMATS +func Parse(s string) (CPUSet, error) { + b := NewBuilder() + + // Handle empty string. + if s == "" { + return b.Result(), nil + } + + // Split CPU list string: + // "0-5,34,46-48 => ["0-5", "34", "46-48"] + ranges := strings.Split(s, ",") + + for _, r := range ranges { + boundaries := strings.Split(r, "-") + if len(boundaries) == 1 { + // Handle ranges that consist of only one element like "34". + elem, err := strconv.Atoi(boundaries[0]) + if err != nil { + return NewCPUSet(), err + } + b.Add(elem) + } else if len(boundaries) == 2 { + // Handle multi-element ranges like "0-5". + start, err := strconv.Atoi(boundaries[0]) + if err != nil { + return NewCPUSet(), err + } + end, err := strconv.Atoi(boundaries[1]) + if err != nil { + return NewCPUSet(), err + } + // Add all elements to the result. + // e.g. "0-5", "46-48" => [0, 1, 2, 3, 4, 5, 46, 47, 48]. + for e := start; e <= end; e++ { + b.Add(e) + } + } + } + return b.Result(), nil +} + +// Clone returns a copy of this CPU set. +func (s CPUSet) Clone() CPUSet { + b := NewBuilder() + for elem := range s.elems { + b.Add(elem) + } + return b.Result() +} diff --git a/src/runtime/virtcontainers/pkg/cpuset/cpuset_test.go b/src/runtime/virtcontainers/pkg/cpuset/cpuset_test.go new file mode 100644 index 000000000000..433a702d229a --- /dev/null +++ b/src/runtime/virtcontainers/pkg/cpuset/cpuset_test.go @@ -0,0 +1,348 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +// Copyright (c) 2017 The Kubernetes Authors +// SPDX-License-Identifier: Apache-2.0 + +package cpuset + +import ( + "reflect" + "testing" +) + +func TestCPUSetBuilder(t *testing.T) { + b := NewBuilder() + elems := []int{1, 2, 3, 4, 5} + for _, elem := range elems { + b.Add(elem) + } + result := b.Result() + for _, elem := range elems { + if !result.Contains(elem) { + t.Fatalf("expected cpuset to contain element %d: [%v]", elem, result) + } + } + if len(elems) != result.Size() { + t.Fatalf("expected cpuset %s to have the same size as %v", result, elems) + } +} + +func TestCPUSetSize(t *testing.T) { + testCases := []struct { + cpuset CPUSet + expected int + }{ + {NewCPUSet(), 0}, + {NewCPUSet(5), 1}, + {NewCPUSet(1, 2, 3, 4, 5), 5}, + } + + for _, c := range testCases { + actual := c.cpuset.Size() + if actual != c.expected { + t.Fatalf("expected: %d, actual: %d, cpuset: [%v]", c.expected, actual, c.cpuset) + } + } +} + +func TestCPUSetIsEmpty(t *testing.T) { + testCases := []struct { + cpuset CPUSet + expected bool + }{ + {NewCPUSet(), true}, + {NewCPUSet(5), false}, + {NewCPUSet(1, 2, 3, 4, 5), false}, + } + + for _, c := range testCases { + actual := c.cpuset.IsEmpty() + if actual != c.expected { + t.Fatalf("expected: %t, IsEmpty() returned: %t, cpuset: [%v]", c.expected, actual, c.cpuset) + } + } +} + +func TestCPUSetContains(t *testing.T) { + testCases := []struct { + cpuset CPUSet + mustContain []int + mustNotContain []int + }{ + {NewCPUSet(), []int{}, []int{1, 2, 3, 4, 5}}, + {NewCPUSet(5), []int{5}, []int{1, 2, 3, 4}}, + {NewCPUSet(1, 2, 4, 5), []int{1, 2, 4, 5}, []int{0, 3, 6}}, + } + + for _, c := range testCases { + for _, elem := range c.mustContain { + if !c.cpuset.Contains(elem) { + t.Fatalf("expected cpuset to contain element %d: [%v]", elem, c.cpuset) + } + } + for _, elem := range c.mustNotContain { + if c.cpuset.Contains(elem) { + t.Fatalf("expected cpuset not to contain element %d: [%v]", elem, c.cpuset) + } + } + } +} + +func TestCPUSetEqual(t *testing.T) { + shouldEqual := []struct { + s1 CPUSet + s2 CPUSet + }{ + {NewCPUSet(), NewCPUSet()}, + {NewCPUSet(5), NewCPUSet(5)}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(1, 2, 3, 4, 5)}, + } + + shouldNotEqual := []struct { + s1 CPUSet + s2 CPUSet + }{ + {NewCPUSet(), NewCPUSet(5)}, + {NewCPUSet(5), NewCPUSet()}, + {NewCPUSet(), NewCPUSet(1, 2, 3, 4, 5)}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet()}, + {NewCPUSet(5), NewCPUSet(1, 2, 3, 4, 5)}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(5)}, + } + + for _, c := range shouldEqual { + if !c.s1.Equals(c.s2) { + t.Fatalf("expected cpusets to be equal: s1: [%v], s2: [%v]", c.s1, c.s2) + } + } + for _, c := range shouldNotEqual { + if c.s1.Equals(c.s2) { + t.Fatalf("expected cpusets to not be equal: s1: [%v], s2: [%v]", c.s1, c.s2) + } + } +} + +func TestCPUSetIsSubsetOf(t *testing.T) { + shouldBeSubset := []struct { + s1 CPUSet + s2 CPUSet + }{ + // A set is a subset of itself + {NewCPUSet(), NewCPUSet()}, + {NewCPUSet(5), NewCPUSet(5)}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(1, 2, 3, 4, 5)}, + + // Empty set is a subset of every set + {NewCPUSet(), NewCPUSet(5)}, + {NewCPUSet(), NewCPUSet(1, 2, 3, 4, 5)}, + + {NewCPUSet(5), NewCPUSet(1, 2, 3, 4, 5)}, + {NewCPUSet(1, 2, 3), NewCPUSet(1, 2, 3, 4, 5)}, + {NewCPUSet(4, 5), NewCPUSet(1, 2, 3, 4, 5)}, + {NewCPUSet(2, 3), NewCPUSet(1, 2, 3, 4, 5)}, + } + + shouldNotBeSubset := []struct { + s1 CPUSet + s2 CPUSet + }{} + + for _, c := range shouldBeSubset { + if !c.s1.IsSubsetOf(c.s2) { + t.Fatalf("expected s1 to be a subset of s2: s1: [%v], s2: [%v]", c.s1, c.s2) + } + } + for _, c := range shouldNotBeSubset { + if c.s1.IsSubsetOf(c.s2) { + t.Fatalf("expected s1 to not be a subset of s2: s1: [%v], s2: [%v]", c.s1, c.s2) + } + } +} + +func TestCPUSetUnionAll(t *testing.T) { + testCases := []struct { + s1 CPUSet + s2 CPUSet + s3 CPUSet + expected CPUSet + }{ + {NewCPUSet(), NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(4, 5), NewCPUSet(1, 2, 3, 4, 5)}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(), NewCPUSet(4), NewCPUSet(1, 2, 3, 4, 5)}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(1, 5), NewCPUSet(1, 2, 3, 4, 5)}, + } + for _, c := range testCases { + s := []CPUSet{} + s = append(s, c.s2) + s = append(s, c.s3) + result := c.s1.UnionAll(s) + if !result.Equals(c.expected) { + t.Fatalf("expected the union of s1 and s2 to be [%v] (got [%v]), s1: [%v], s2: [%v]", c.expected, result, c.s1, c.s2) + } + } +} + +func TestCPUSetUnion(t *testing.T) { + testCases := []struct { + s1 CPUSet + s2 CPUSet + expected CPUSet + }{ + {NewCPUSet(), NewCPUSet(), NewCPUSet()}, + + {NewCPUSet(), NewCPUSet(5), NewCPUSet(5)}, + {NewCPUSet(5), NewCPUSet(), NewCPUSet(5)}, + {NewCPUSet(5), NewCPUSet(5), NewCPUSet(5)}, + + {NewCPUSet(), NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(1, 2, 3, 4, 5)}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(), NewCPUSet(1, 2, 3, 4, 5)}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(1, 2, 3, 4, 5)}, + + {NewCPUSet(5), NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(1, 2, 3, 4, 5)}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(5), NewCPUSet(1, 2, 3, 4, 5)}, + + {NewCPUSet(1, 2), NewCPUSet(3, 4, 5), NewCPUSet(1, 2, 3, 4, 5)}, + {NewCPUSet(1, 2, 3), NewCPUSet(3, 4, 5), NewCPUSet(1, 2, 3, 4, 5)}, + } + + for _, c := range testCases { + result := c.s1.Union(c.s2) + if !result.Equals(c.expected) { + t.Fatalf("expected the union of s1 and s2 to be [%v] (got [%v]), s1: [%v], s2: [%v]", c.expected, result, c.s1, c.s2) + } + } +} + +func TestCPUSetIntersection(t *testing.T) { + testCases := []struct { + s1 CPUSet + s2 CPUSet + expected CPUSet + }{ + {NewCPUSet(), NewCPUSet(), NewCPUSet()}, + + {NewCPUSet(), NewCPUSet(5), NewCPUSet()}, + {NewCPUSet(5), NewCPUSet(), NewCPUSet()}, + {NewCPUSet(5), NewCPUSet(5), NewCPUSet(5)}, + + {NewCPUSet(), NewCPUSet(1, 2, 3, 4, 5), NewCPUSet()}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(), NewCPUSet()}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(1, 2, 3, 4, 5)}, + + {NewCPUSet(5), NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(5)}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(5), NewCPUSet(5)}, + + {NewCPUSet(1, 2), NewCPUSet(3, 4, 5), NewCPUSet()}, + {NewCPUSet(1, 2, 3), NewCPUSet(3, 4, 5), NewCPUSet(3)}, + } + + for _, c := range testCases { + result := c.s1.Intersection(c.s2) + if !result.Equals(c.expected) { + t.Fatalf("expected the intersection of s1 and s2 to be [%v] (got [%v]), s1: [%v], s2: [%v]", c.expected, result, c.s1, c.s2) + } + } +} + +func TestCPUSetDifference(t *testing.T) { + testCases := []struct { + s1 CPUSet + s2 CPUSet + expected CPUSet + }{ + {NewCPUSet(), NewCPUSet(), NewCPUSet()}, + + {NewCPUSet(), NewCPUSet(5), NewCPUSet()}, + {NewCPUSet(5), NewCPUSet(), NewCPUSet(5)}, + {NewCPUSet(5), NewCPUSet(5), NewCPUSet()}, + + {NewCPUSet(), NewCPUSet(1, 2, 3, 4, 5), NewCPUSet()}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(), NewCPUSet(1, 2, 3, 4, 5)}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(1, 2, 3, 4, 5), NewCPUSet()}, + + {NewCPUSet(5), NewCPUSet(1, 2, 3, 4, 5), NewCPUSet()}, + {NewCPUSet(1, 2, 3, 4, 5), NewCPUSet(5), NewCPUSet(1, 2, 3, 4)}, + + {NewCPUSet(1, 2), NewCPUSet(3, 4, 5), NewCPUSet(1, 2)}, + {NewCPUSet(1, 2, 3), NewCPUSet(3, 4, 5), NewCPUSet(1, 2)}, + } + + for _, c := range testCases { + result := c.s1.Difference(c.s2) + if !result.Equals(c.expected) { + t.Fatalf("expected the difference of s1 and s2 to be [%v] (got [%v]), s1: [%v], s2: [%v]", c.expected, result, c.s1, c.s2) + } + } +} + +func TestCPUSetToSlice(t *testing.T) { + testCases := []struct { + set CPUSet + expected []int + }{ + {NewCPUSet(), []int{}}, + {NewCPUSet(5), []int{5}}, + {NewCPUSet(1, 2, 3, 4, 5), []int{1, 2, 3, 4, 5}}, + } + + for _, c := range testCases { + result := c.set.ToSlice() + if !reflect.DeepEqual(result, c.expected) { + t.Fatalf("expected set as slice to be [%v] (got [%v]), s: [%v]", c.expected, result, c.set) + } + } +} + +func TestCPUSetString(t *testing.T) { + testCases := []struct { + set CPUSet + expected string + }{ + {NewCPUSet(), ""}, + {NewCPUSet(5), "5"}, + {NewCPUSet(1, 2, 3, 4, 5), "1-5"}, + {NewCPUSet(1, 2, 3, 5, 6, 8), "1-3,5-6,8"}, + } + + for _, c := range testCases { + result := c.set.String() + if result != c.expected { + t.Fatalf("expected set as string to be %s (got \"%s\"), s: [%v]", c.expected, result, c.set) + } + } +} + +func TestParse(t *testing.T) { + testCases := []struct { + cpusetString string + expected CPUSet + }{ + {"", NewCPUSet()}, + {"5", NewCPUSet(5)}, + {"1,2,3,4,5", NewCPUSet(1, 2, 3, 4, 5)}, + {"1-5", NewCPUSet(1, 2, 3, 4, 5)}, + {"1-2,3-5", NewCPUSet(1, 2, 3, 4, 5)}, + } + + for _, c := range testCases { + result, err := Parse(c.cpusetString) + if err != nil { + t.Fatalf("expected error not to have occurred: %v", err) + } + if !result.Equals(c.expected) { + t.Fatalf("expected string \"%s\" to parse as [%v] (got [%v])", c.cpusetString, c.expected, result) + } + } +} diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index 73b2e28c6781..24e440ee35f6 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -39,11 +39,11 @@ import ( "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/annotations" vccgroups "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/cgroups" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/compatoci" + "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/cpuset" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/rootless" vcTypes "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/types" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" - "k8s.io/kubernetes/pkg/kubelet/cm/cpuset" ) const ( From 77a463e57a928344665d59cc3cfba3b4a1074d7f Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 12 Oct 2020 21:15:21 -0700 Subject: [PATCH 5/7] cpuset: support setting mems for sandbox CPUSet cgroup allows for pinning the memory associated with a cpuset to a given numa node. Similar to cpuset.cpus, we should take cpuset.mems into account for the sandbox-cgroup that Kata creates. Signed-off-by: Eric Ernst --- .../virtcontainers/pkg/cgroups/manager.go | 3 +- src/runtime/virtcontainers/sandbox.go | 33 ++++---- src/runtime/virtcontainers/sandbox_test.go | 82 +++++++++++-------- 3 files changed, 71 insertions(+), 47 deletions(-) diff --git a/src/runtime/virtcontainers/pkg/cgroups/manager.go b/src/runtime/virtcontainers/pkg/cgroups/manager.go index e0f81777a12a..23ca1d827975 100644 --- a/src/runtime/virtcontainers/pkg/cgroups/manager.go +++ b/src/runtime/virtcontainers/pkg/cgroups/manager.go @@ -332,7 +332,7 @@ func (m *Manager) RemoveDevice(device string) error { return fmt.Errorf("device %v not found in the cgroup", device) } -func (m *Manager) SetCPUSet(cpuset string) error { +func (m *Manager) SetCPUSet(cpuset, memset string) error { cgroups, err := m.GetCgroups() if err != nil { return err @@ -340,6 +340,7 @@ func (m *Manager) SetCPUSet(cpuset string) error { m.Lock() cgroups.CpusetCpus = cpuset + cgroups.CpusetMems = memset m.Unlock() return m.Apply() diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index 24e440ee35f6..b87566b8f81c 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -1995,15 +1995,13 @@ func (s *Sandbox) cgroupsUpdate() error { // in the Kata sandbox cgroup (inherited). Check to see if sandbox cpuset needs to be // updated. if s.config.SandboxCgroupOnly { - cpuset, err := s.getSandboxCPUSet() + cpuset, memset, err := s.getSandboxCPUSet() if err != nil { return err } - if cpuset != "" { - if err := s.cgroupMgr.SetCPUSet(cpuset); err != nil { - return err - } + if err := s.cgroupMgr.SetCPUSet(cpuset, memset); err != nil { + return err } return nil @@ -2304,23 +2302,30 @@ func (s *Sandbox) GetAgentURL() (string, error) { return s.agent.getAgentURL() } -// getSandboxCPUSet returns the union of each of the sandbox's containers' CPU sets -// as a string in canonical linux CPU list format -func (s *Sandbox) getSandboxCPUSet() (string, error) { +// getSandboxCPUSet returns the union of each of the sandbox's containers' CPU sets' +// cpus and mems as a string in canonical linux CPU/mems list format +func (s *Sandbox) getSandboxCPUSet() (string, string, error) { if s.config == nil { - return "", nil + return "", "", nil } - result := cpuset.NewCPUSet() + cpuResult := cpuset.NewCPUSet() + memResult := cpuset.NewCPUSet() for _, ctr := range s.config.Containers { if ctr.Resources.CPU != nil { - currSet, err := cpuset.Parse(ctr.Resources.CPU.Cpus) + currCpuSet, err := cpuset.Parse(ctr.Resources.CPU.Cpus) + if err != nil { + return "", "", fmt.Errorf("unable to parse CPUset.cpus for container %s: %v", ctr.ID, err) + } + cpuResult = cpuResult.Union(currCpuSet) + + currMemSet, err := cpuset.Parse(ctr.Resources.CPU.Mems) if err != nil { - return "", fmt.Errorf("unable to parse CPUset for container %s", ctr.ID) + return "", "", fmt.Errorf("unable to parse CPUset.mems for container %s: %v", ctr.ID, err) } - result = result.Union(currSet) + memResult = memResult.Union(currMemSet) } } - return result.String(), nil + return cpuResult.String(), memResult.String(), nil } diff --git a/src/runtime/virtcontainers/sandbox_test.go b/src/runtime/virtcontainers/sandbox_test.go index 8345236f0fbd..09b762a07809 100644 --- a/src/runtime/virtcontainers/sandbox_test.go +++ b/src/runtime/virtcontainers/sandbox_test.go @@ -1420,24 +1420,25 @@ func TestSandbox_SetupSandboxCgroup(t *testing.T) { } } -func getContainerConfigWithCPUSet(cpuset string) ContainerConfig { +func getContainerConfigWithCPUSet(cpuset, memset string) ContainerConfig { return ContainerConfig{ Resources: specs.LinuxResources{ CPU: &specs.LinuxCPU{ Cpus: cpuset, + Mems: memset, }, }, } } -func getSimpleSandbox(cpuset0, cpuset1, cpuset2 string) *Sandbox { +func getSimpleSandbox(cpusets, memsets [3]string) *Sandbox { sandbox := Sandbox{} sandbox.config = &SandboxConfig{ Containers: []ContainerConfig{ - getContainerConfigWithCPUSet(cpuset0), - getContainerConfigWithCPUSet(cpuset1), - getContainerConfigWithCPUSet(cpuset2), + getContainerConfigWithCPUSet(cpusets[0], memsets[0]), + getContainerConfigWithCPUSet(cpusets[1], memsets[1]), + getContainerConfigWithCPUSet(cpusets[2], memsets[2]), }, } @@ -1447,80 +1448,97 @@ func getSimpleSandbox(cpuset0, cpuset1, cpuset2 string) *Sandbox { func TestGetSandboxCpuSet(t *testing.T) { tests := []struct { - name string - cpuset0 string - cpuset1 string - cpuset2 string - result string - wantErr bool + name string + cpusets [3]string + memsets [3]string + cpuResult string + memResult string + wantErr bool }{ { "single, no cpuset", - "", - "", + [3]string{"", "", ""}, + [3]string{"", "", ""}, "", "", false, }, { "single cpuset", + [3]string{"0", "", ""}, + [3]string{"", "", ""}, "0", "", - "", - "0", false, }, { "two duplicate cpuset", - "0", + [3]string{"0", "0", ""}, + [3]string{"", "", ""}, "0", "", - "0", false, }, { "3 cpusets", - "0-3", - "5-7", - "1", + [3]string{"0-3", "5-7", "1"}, + [3]string{"", "", ""}, "0-3,5-7", + "", false, }, + { "weird, but should be okay", - "0-3", - "99999", - "", + [3]string{"0-3", "99999", ""}, + [3]string{"", "", ""}, "0-3,99999", + "", false, }, { "two, overlapping cpuset", + [3]string{"0-3", "1-2", ""}, + [3]string{"", "", ""}, "0-3", - "1-2", "", - "0-3", false, }, { "garbage, should fail", - "7 beard-seconds", - "Audrey + 7", - "Elliott - 17", + [3]string{"7 beard-seconds", "Audrey + 7", "Elliott - 17"}, + [3]string{"", "", ""}, + "", "", true, }, + { + "cpuset and memset", + [3]string{"0-3", "1-2", ""}, + [3]string{"0", "1", "0-1"}, + "0-3", + "0-1", + false, + }, + { + "memset", + [3]string{"0-3", "1-2", ""}, + [3]string{"0", "3", ""}, + "0-3", + "0,3", + false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s := getSimpleSandbox(tt.cpuset0, tt.cpuset1, tt.cpuset2) - res, err := s.getSandboxCPUSet() + s := getSimpleSandbox(tt.cpusets, tt.memsets) + res, _, err := s.getSandboxCPUSet() if (err != nil) != tt.wantErr { t.Errorf("getSandboxCPUSet() error = %v, wantErr %v", err, tt.wantErr) } - if res != tt.result { - t.Errorf("getSandboxCPUSet() result = %s, wanted result %s", res, tt.result) + if res != tt.cpuResult { + t.Errorf("getSandboxCPUSet() result = %s, wanted result %s", res, tt.cpuResult) } }) } From 88cd71287617ef3514a8cf9749839f18c4912ae5 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 12 Oct 2020 21:15:55 -0700 Subject: [PATCH 6/7] sandbox: consider cpusets if quota is not enforced CPUSet cgroup allows for pinning the memory associated with a cpuset to a given numa node. Similar to cpuset.cpus, we should take cpuset.mems into account for the sandbox-cgroup that Kata creates. Signed-off-by: Eric Ernst --- src/runtime/virtcontainers/sandbox.go | 27 ++++++++++++++++++---- src/runtime/virtcontainers/sandbox_test.go | 15 +++++++++--- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index b87566b8f81c..16db127ea1c7 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -1897,7 +1897,10 @@ func (s *Sandbox) updateResources() error { return fmt.Errorf("sandbox config is nil") } - sandboxVCPUs := s.calculateSandboxCPUs() + sandboxVCPUs, err := s.calculateSandboxCPUs() + if err != nil { + return err + } // Add default vcpus for sandbox sandboxVCPUs += s.hypervisor.hypervisorConfig().NumVCPUs @@ -1959,8 +1962,9 @@ func (s *Sandbox) calculateSandboxMemory() int64 { return memorySandbox } -func (s *Sandbox) calculateSandboxCPUs() uint32 { +func (s *Sandbox) calculateSandboxCPUs() (uint32, error) { mCPU := uint32(0) + cpusetCount := int(0) for _, c := range s.config.Containers { // Do not hot add again non-running containers resources @@ -1974,9 +1978,22 @@ func (s *Sandbox) calculateSandboxCPUs() uint32 { mCPU += utils.CalculateMilliCPUs(*cpu.Quota, *cpu.Period) } + set, err := cpuset.Parse(cpu.Cpus) + if err != nil { + return 0, nil + } + cpusetCount += set.Size() } } - return utils.CalculateVCpusFromMilliCpus(mCPU) + + // If we aren't being constrained, then we could have two scenarios: + // 1. BestEffort QoS: no proper support today in Kata. + // 2. We could be constrained only by CPUSets. Check for this: + if mCPU == 0 && cpusetCount > 0 { + return uint32(cpusetCount), nil + } + + return utils.CalculateVCpusFromMilliCpus(mCPU), nil } // GetHypervisorType is used for getting Hypervisor name currently used. @@ -2313,11 +2330,11 @@ func (s *Sandbox) getSandboxCPUSet() (string, string, error) { memResult := cpuset.NewCPUSet() for _, ctr := range s.config.Containers { if ctr.Resources.CPU != nil { - currCpuSet, err := cpuset.Parse(ctr.Resources.CPU.Cpus) + currCPUSet, err := cpuset.Parse(ctr.Resources.CPU.Cpus) if err != nil { return "", "", fmt.Errorf("unable to parse CPUset.cpus for container %s: %v", ctr.ID, err) } - cpuResult = cpuResult.Union(currCpuSet) + cpuResult = cpuResult.Union(currCPUSet) currMemSet, err := cpuset.Parse(ctr.Resources.CPU.Mems) if err != nil { diff --git a/src/runtime/virtcontainers/sandbox_test.go b/src/runtime/virtcontainers/sandbox_test.go index 09b762a07809..6e6dd752c1a2 100644 --- a/src/runtime/virtcontainers/sandbox_test.go +++ b/src/runtime/virtcontainers/sandbox_test.go @@ -106,12 +106,18 @@ func TestCreateMockSandbox(t *testing.T) { func TestCalculateSandboxCPUs(t *testing.T) { sandbox := &Sandbox{} sandbox.config = &SandboxConfig{} + unconstrained := newTestContainerConfigNoop("cont-00001") - constrained := newTestContainerConfigNoop("cont-00001") + constrained := newTestContainerConfigNoop("cont-00002") + unconstrainedCpusets0_1 := newTestContainerConfigNoop("cont-00003") + unconstrainedCpusets2 := newTestContainerConfigNoop("cont-00004") + constrainedCpusets0_7 := newTestContainerConfigNoop("cont-00005") quota := int64(4000) period := uint64(1000) constrained.Resources.CPU = &specs.LinuxCPU{Period: &period, Quota: "a} - + unconstrainedCpusets0_1.Resources.CPU = &specs.LinuxCPU{Cpus: "0-1"} + unconstrainedCpusets2.Resources.CPU = &specs.LinuxCPU{Cpus: "2"} + constrainedCpusets0_7.Resources.CPU = &specs.LinuxCPU{Period: &period, Quota: "a, Cpus: "0-7"} tests := []struct { name string containers []ContainerConfig @@ -123,11 +129,14 @@ func TestCalculateSandboxCPUs(t *testing.T) { {"2-constrained", []ContainerConfig{constrained, constrained}, 8}, {"3-mix-constraints", []ContainerConfig{unconstrained, constrained, constrained}, 8}, {"3-constrained", []ContainerConfig{constrained, constrained, constrained}, 12}, + {"unconstrained-1-cpuset", []ContainerConfig{unconstrained, unconstrained, unconstrainedCpusets0_1}, 2}, + {"unconstrained-2-cpuset", []ContainerConfig{unconstrainedCpusets0_1, unconstrainedCpusets2}, 3}, + {"constrained-cpuset", []ContainerConfig{constrainedCpusets0_7}, 4}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { sandbox.config.Containers = tt.containers - got := sandbox.calculateSandboxCPUs() + got, _ := sandbox.calculateSandboxCPUs() assert.Equal(t, got, tt.want) }) } From d8a8fe47fbb14ab5ce1f08235caa5bdf9d6198a4 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 12 Oct 2020 21:18:08 -0700 Subject: [PATCH 7/7] cpuset: don't set cpuset.mems in the guest Kata doesn't map any numa topologies in the guest. Let's make sure we clear the Cpuset fields before passing container updates to the guest. Note, in the future we may want to have a vCPU to guest CPU mapping and still include the cpuset.Cpus. Until we have this support, clear this as well. Fixes: #932 Signed-off-by: Eric Ernst --- src/runtime/virtcontainers/container.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/runtime/virtcontainers/container.go b/src/runtime/virtcontainers/container.go index 2c49b14bc524..21f18f35d714 100644 --- a/src/runtime/virtcontainers/container.go +++ b/src/runtime/virtcontainers/container.go @@ -1165,6 +1165,14 @@ func (c *Container) update(resources specs.LinuxResources) error { } } + // There currently isn't a notion of cpusets.cpus or mems being tracked + // inside of the guest. Make sure we clear these before asking agent to update + // the container's cgroups. + if resources.CPU != nil { + resources.CPU.Mems = "" + resources.CPU.Cpus = "" + } + return c.sandbox.agent.updateContainer(c.sandbox, *c, resources) }