From 8f38abb350e884e54e2df763e60dccabbfc214bf Mon Sep 17 00:00:00 2001 From: Connor Doyle Date: Tue, 22 Aug 2017 10:44:01 -0700 Subject: [PATCH 1/3] Add cpuset helper library. --- pkg/kubelet/cm/BUILD | 1 + pkg/kubelet/cm/cpuset/BUILD | 36 +++ pkg/kubelet/cm/cpuset/OWNERS | 5 + pkg/kubelet/cm/cpuset/cpuset.go | 260 +++++++++++++++++++++ pkg/kubelet/cm/cpuset/cpuset_test.go | 330 +++++++++++++++++++++++++++ 5 files changed, 632 insertions(+) create mode 100644 pkg/kubelet/cm/cpuset/BUILD create mode 100644 pkg/kubelet/cm/cpuset/OWNERS create mode 100644 pkg/kubelet/cm/cpuset/cpuset.go create mode 100644 pkg/kubelet/cm/cpuset/cpuset_test.go diff --git a/pkg/kubelet/cm/BUILD b/pkg/kubelet/cm/BUILD index cfd2f6e60c8..e352bd354a6 100644 --- a/pkg/kubelet/cm/BUILD +++ b/pkg/kubelet/cm/BUILD @@ -109,6 +109,7 @@ filegroup( name = "all-srcs", srcs = [ ":package-srcs", + "//pkg/kubelet/cm/cpuset:all-srcs", "//pkg/kubelet/cm/util:all-srcs", ], tags = ["automanaged"], diff --git a/pkg/kubelet/cm/cpuset/BUILD b/pkg/kubelet/cm/cpuset/BUILD new file mode 100644 index 00000000000..84f1beebd15 --- /dev/null +++ b/pkg/kubelet/cm/cpuset/BUILD @@ -0,0 +1,36 @@ +package(default_visibility = ["//visibility:public"]) + +licenses(["notice"]) + +load( + "@io_bazel_rules_go//go:def.bzl", + "go_library", + "go_test", +) + +go_test( + name = "go_default_test", + srcs = ["cpuset_test.go"], + library = ":go_default_library", + tags = ["automanaged"], +) + +go_library( + name = "go_default_library", + srcs = ["cpuset.go"], + tags = ["automanaged"], + deps = ["//vendor/github.com/golang/glog:go_default_library"], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], +) diff --git a/pkg/kubelet/cm/cpuset/OWNERS b/pkg/kubelet/cm/cpuset/OWNERS new file mode 100644 index 00000000000..02f72bf6cc1 --- /dev/null +++ b/pkg/kubelet/cm/cpuset/OWNERS @@ -0,0 +1,5 @@ +approvers: +- derekwaynecarr +- vishh +- ConnorDoyle +- sjenning diff --git a/pkg/kubelet/cm/cpuset/cpuset.go b/pkg/kubelet/cm/cpuset/cpuset.go new file mode 100644 index 00000000000..7021f80d288 --- /dev/null +++ b/pkg/kubelet/cm/cpuset/cpuset.go @@ -0,0 +1,260 @@ +/* +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. +*/ + +package cpuset + +import ( + "bytes" + "fmt" + "github.com/golang/glog" + "reflect" + "sort" + "strconv" + "strings" +) + +// CPUSet is a set-like data structure for CPU IDs. +type CPUSet map[int]struct{} + +// NewCPUSet return CPUSet based on provided cpu id's +func NewCPUSet(cpus ...int) CPUSet { + res := CPUSet{} + for _, c := range cpus { + res.Add(c) + } + return res +} + +// Size returns the number of elements in this set. +func (s CPUSet) Size() int { + return len(s) +} + +// 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[cpu] + return found +} + +// Add mutates this set to contain the supplied elements. +func (s CPUSet) Add(cpus ...int) { + for _, cpu := range cpus { + s[cpu] = struct{}{} + } +} + +// Remove mutates this set to not contain the supplied elements, if they +// exists. +func (s CPUSet) Remove(cpus ...int) { + for _, cpu := range cpus { + delete(s, cpu) + } +} + +// 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, s2) +} + +// 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 { + result := NewCPUSet() + for cpu := range s { + if predicate(cpu) { + result.Add(cpu) + } + } + return 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 { + result := NewCPUSet() + for cpu := range s { + if !predicate(cpu) { + result.Add(cpu) + } + } + return 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 { + 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 { + result := NewCPUSet() + for cpu := range s { + result.Add(cpu) + } + for cpu := range s2 { + result.Add(cpu) + } + return 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) }) +} + +// AsSlice returns a slice of integers that contains all elements from +// this set. +func (s CPUSet) AsSlice() []int { + result := []int{} + for cpu := range s { + result = append(result, cpu) + } + sort.Ints(result) + 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.AsSlice() + sort.Ints(elems) + + 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(), ",") +} + +// MustParse CPUSet constructs a new CPU set from a Linux CPU list formatted +// string. Unlike Parse, it does not return an error but rather panics if the +// input cannot be used to construct a CPU set. +func MustParse(s string) CPUSet { + res, err := Parse(s) + if err != nil { + glog.Fatalf("unable to parse [%s] as CPUSet: %v", s, err) + } + return res +} + +// 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) { + result := NewCPUSet() + + // Handle empty string. + if s == "" { + return 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 nil, err + } + result.Add(elem) + } else if len(boundaries) == 2 { + // Handle multi-element ranges like "0-5". + start, err := strconv.Atoi(boundaries[0]) + if err != nil { + return nil, err + } + end, err := strconv.Atoi(boundaries[1]) + if err != nil { + return nil, 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++ { + result.Add(e) + } + } + } + return result, nil +} + +// Clone returns a copy of this CPU set. +func (s CPUSet) Clone() CPUSet { + res := NewCPUSet() + for k, v := range s { + res[k] = v + } + return res +} diff --git a/pkg/kubelet/cm/cpuset/cpuset_test.go b/pkg/kubelet/cm/cpuset/cpuset_test.go new file mode 100644 index 00000000000..f17530c52a7 --- /dev/null +++ b/pkg/kubelet/cm/cpuset/cpuset_test.go @@ -0,0 +1,330 @@ +/* +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. +*/ + +package cpuset + +import ( + "reflect" + "testing" +) + +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 TestCPUSetAdd(t *testing.T) { + result := NewCPUSet() + for _, elem := range []int{1, 2, 3, 4, 5} { + result.Add(elem) + if !result.Contains(elem) { + t.Fatalf("expected cpuset to contain element %d: [%v]", elem, result) + } + } +} + +func TestCPUSetRemove(t *testing.T) { + result := NewCPUSet(1, 2, 3, 4, 5) + for _, elem := range []int{1, 2, 3, 4, 5} { + result.Remove(elem) + if result.Contains(elem) { + t.Fatalf("expected cpuset to not contain element %d: [%v]", elem, result) + } + } + if !result.IsEmpty() { + t.Fatalf("expected cpuset to be empty: [%v]", result) + } +} + +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 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 TestCPUSetAsSlice(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.AsSlice() + 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) + } + } +} From e686ecb6eacce507a8cbfb1ff374ea09642b10e6 Mon Sep 17 00:00:00 2001 From: Connor Doyle Date: Tue, 22 Aug 2017 21:21:26 -0700 Subject: [PATCH 2/3] Renamed CPUSet.AsSlice() => CPUSet.ToSlice() --- pkg/kubelet/cm/cpuset/cpuset.go | 6 +++--- pkg/kubelet/cm/cpuset/cpuset_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/kubelet/cm/cpuset/cpuset.go b/pkg/kubelet/cm/cpuset/cpuset.go index 7021f80d288..1a388c2880d 100644 --- a/pkg/kubelet/cm/cpuset/cpuset.go +++ b/pkg/kubelet/cm/cpuset/cpuset.go @@ -140,9 +140,9 @@ func (s CPUSet) Difference(s2 CPUSet) CPUSet { return s.FilterNot(func(cpu int) bool { return s2.Contains(cpu) }) } -// AsSlice returns a slice of integers that contains all elements from +// ToSlice returns a slice of integers that contains all elements from // this set. -func (s CPUSet) AsSlice() []int { +func (s CPUSet) ToSlice() []int { result := []int{} for cpu := range s { result = append(result, cpu) @@ -160,7 +160,7 @@ func (s CPUSet) String() string { return "" } - elems := s.AsSlice() + elems := s.ToSlice() sort.Ints(elems) type rng struct { diff --git a/pkg/kubelet/cm/cpuset/cpuset_test.go b/pkg/kubelet/cm/cpuset/cpuset_test.go index f17530c52a7..6652eb0165c 100644 --- a/pkg/kubelet/cm/cpuset/cpuset_test.go +++ b/pkg/kubelet/cm/cpuset/cpuset_test.go @@ -269,7 +269,7 @@ func TestCPUSetDifference(t *testing.T) { } } -func TestCPUSetAsSlice(t *testing.T) { +func TestCPUSetToSlice(t *testing.T) { testCases := []struct { set CPUSet expected []int @@ -280,7 +280,7 @@ func TestCPUSetAsSlice(t *testing.T) { } for _, c := range testCases { - result := c.set.AsSlice() + 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) } From 515d86faa03503e84ee124eeab9d210b7c16f46a Mon Sep 17 00:00:00 2001 From: Connor Doyle Date: Tue, 22 Aug 2017 21:22:37 -0700 Subject: [PATCH 3/3] Add CPUSetBuilder, make CPUSet immutable. --- pkg/kubelet/cm/cpuset/cpuset.go | 130 +++++++++++++++------------ pkg/kubelet/cm/cpuset/cpuset_test.go | 40 ++++----- 2 files changed, 92 insertions(+), 78 deletions(-) diff --git a/pkg/kubelet/cm/cpuset/cpuset.go b/pkg/kubelet/cm/cpuset/cpuset.go index 1a388c2880d..ccf577d819c 100644 --- a/pkg/kubelet/cm/cpuset/cpuset.go +++ b/pkg/kubelet/cm/cpuset/cpuset.go @@ -26,21 +26,57 @@ import ( "strings" ) -// CPUSet is a set-like data structure for CPU IDs. -type CPUSet map[int]struct{} +// 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 +} -// NewCPUSet return CPUSet based on provided cpu id's -func NewCPUSet(cpus ...int) CPUSet { - res := CPUSet{} - for _, c := range cpus { - res.Add(c) +// NewBuilder returns a mutable CPUSet builder. +func NewBuilder() Builder { + return Builder{ + result: CPUSet{ + elems: map[int]struct{}{}, + }, } - return res +} + +// 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) + return len(s.elems) } // IsEmpty returns true if there are zero elements in this set. @@ -50,60 +86,45 @@ func (s CPUSet) IsEmpty() bool { // Contains returns true if the supplied element is present in this set. func (s CPUSet) Contains(cpu int) bool { - _, found := s[cpu] + _, found := s.elems[cpu] return found } -// Add mutates this set to contain the supplied elements. -func (s CPUSet) Add(cpus ...int) { - for _, cpu := range cpus { - s[cpu] = struct{}{} - } -} - -// Remove mutates this set to not contain the supplied elements, if they -// exists. -func (s CPUSet) Remove(cpus ...int) { - for _, cpu := range cpus { - delete(s, cpu) - } -} - // 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, s2) + 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 { - result := NewCPUSet() - for cpu := range s { + b := NewBuilder() + for cpu := range s.elems { if predicate(cpu) { - result.Add(cpu) + b.Add(cpu) } } - return result + 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 { - result := NewCPUSet() - for cpu := range s { + b := NewBuilder() + for cpu := range s.elems { if !predicate(cpu) { - result.Add(cpu) + b.Add(cpu) } } - return result + 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 { + for cpu := range s.elems { if !s2.Contains(cpu) { result = false break @@ -116,14 +137,14 @@ func (s CPUSet) IsSubsetOf(s2 CPUSet) bool { // set and all of the elements from the supplied set, without mutating // either source set. func (s CPUSet) Union(s2 CPUSet) CPUSet { - result := NewCPUSet() - for cpu := range s { - result.Add(cpu) + b := NewBuilder() + for cpu := range s.elems { + b.Add(cpu) } - for cpu := range s2 { - result.Add(cpu) + for cpu := range s2.elems { + b.Add(cpu) } - return result + return b.Result() } // Intersection returns a new CPU set that contains all of the elements @@ -144,7 +165,7 @@ func (s CPUSet) Difference(s2 CPUSet) CPUSet { // this set. func (s CPUSet) ToSlice() []int { result := []int{} - for cpu := range s { + for cpu := range s.elems { result = append(result, cpu) } sort.Ints(result) @@ -161,7 +182,6 @@ func (s CPUSet) String() string { } elems := s.ToSlice() - sort.Ints(elems) type rng struct { start int @@ -210,11 +230,11 @@ func MustParse(s string) CPUSet { // // See: http://man7.org/linux/man-pages/man7/cpuset.7.html#FORMATS func Parse(s string) (CPUSet, error) { - result := NewCPUSet() + b := NewBuilder() // Handle empty string. if s == "" { - return result, nil + return b.Result(), nil } // Split CPU list string: @@ -227,34 +247,34 @@ func Parse(s string) (CPUSet, error) { // Handle ranges that consist of only one element like "34". elem, err := strconv.Atoi(boundaries[0]) if err != nil { - return nil, err + return NewCPUSet(), err } - result.Add(elem) + 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 nil, err + return NewCPUSet(), err } end, err := strconv.Atoi(boundaries[1]) if err != nil { - return nil, err + 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++ { - result.Add(e) + b.Add(e) } } } - return result, nil + return b.Result(), nil } // Clone returns a copy of this CPU set. func (s CPUSet) Clone() CPUSet { - res := NewCPUSet() - for k, v := range s { - res[k] = v + b := NewBuilder() + for elem := range s.elems { + b.Add(elem) } - return res + return b.Result() } diff --git a/pkg/kubelet/cm/cpuset/cpuset_test.go b/pkg/kubelet/cm/cpuset/cpuset_test.go index 6652eb0165c..e2d2410bbca 100644 --- a/pkg/kubelet/cm/cpuset/cpuset_test.go +++ b/pkg/kubelet/cm/cpuset/cpuset_test.go @@ -21,6 +21,23 @@ import ( "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 @@ -82,29 +99,6 @@ func TestCPUSetContains(t *testing.T) { } } -func TestCPUSetAdd(t *testing.T) { - result := NewCPUSet() - for _, elem := range []int{1, 2, 3, 4, 5} { - result.Add(elem) - if !result.Contains(elem) { - t.Fatalf("expected cpuset to contain element %d: [%v]", elem, result) - } - } -} - -func TestCPUSetRemove(t *testing.T) { - result := NewCPUSet(1, 2, 3, 4, 5) - for _, elem := range []int{1, 2, 3, 4, 5} { - result.Remove(elem) - if result.Contains(elem) { - t.Fatalf("expected cpuset to not contain element %d: [%v]", elem, result) - } - } - if !result.IsEmpty() { - t.Fatalf("expected cpuset to be empty: [%v]", result) - } -} - func TestCPUSetEqual(t *testing.T) { shouldEqual := []struct { s1 CPUSet