Skip to content

Commit a047e8a

Browse files
AlexeyPerevalovAlexeyPerevalov
committed
move to cadvisor.MachineInfo
This patch removes GetNUMANodeInfo, cadvisor.MachineInfo will be used instead of it. GetNUMANodeInfo was introduced due to difference of meaning of MachineInfo.Topology. On the arm it was NUMA nodes, but on the x86 it represents sockets (since reading from /proc/cpuinfo). Now it unified and MachineInfo.Topology represents NUMA node. Signed-off-by: Alexey Perevalov <[email protected]>
1 parent e33ba9e commit a047e8a

File tree

12 files changed

+65
-154
lines changed

12 files changed

+65
-154
lines changed

‎pkg/kubelet/cm/BUILD‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ go_library(
5151
"//pkg/apis/core/v1/helper/qos:go_default_library",
5252
"//pkg/kubelet/cadvisor:go_default_library",
5353
"//pkg/kubelet/cm/containermap:go_default_library",
54-
"//pkg/kubelet/cm/cpumanager/topology:go_default_library",
5554
"//pkg/kubelet/cm/devicemanager:go_default_library",
5655
"//pkg/kubelet/cm/util:go_default_library",
5756
"//pkg/kubelet/events:go_default_library",
@@ -103,7 +102,6 @@ go_library(
103102
"//pkg/apis/core/v1/helper/qos:go_default_library",
104103
"//pkg/kubelet/cadvisor:go_default_library",
105104
"//pkg/kubelet/cm/containermap:go_default_library",
106-
"//pkg/kubelet/cm/cpumanager/topology:go_default_library",
107105
"//pkg/kubelet/cm/devicemanager:go_default_library",
108106
"//pkg/kubelet/cm/util:go_default_library",
109107
"//pkg/kubelet/events:go_default_library",

‎pkg/kubelet/cm/container_manager_linux.go‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ import (
5252
"k8s.io/kubernetes/pkg/kubelet/cadvisor"
5353
"k8s.io/kubernetes/pkg/kubelet/cm/containermap"
5454
"k8s.io/kubernetes/pkg/kubelet/cm/cpumanager"
55-
cputopology "k8s.io/kubernetes/pkg/kubelet/cm/cpumanager/topology"
5655
"k8s.io/kubernetes/pkg/kubelet/cm/devicemanager"
5756
"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager"
5857
cmutil "k8s.io/kubernetes/pkg/kubelet/cm/util"
@@ -238,13 +237,6 @@ func NewContainerManager(mountUtil mount.Interface, cadvisorInterface cadvisor.I
238237
if err != nil {
239238
return nil, err
240239
}
241-
// Correct NUMA information is currently missing from cadvisor's
242-
// MachineInfo struct, so we use the CPUManager's internal logic for
243-
// gathering NUMANodeInfo to pass to components that care about it.
244-
numaNodeInfo, err := cputopology.GetNUMANodeInfo()
245-
if err != nil {
246-
return nil, err
247-
}
248240
capacity := cadvisor.CapacityFromMachineInfo(machineInfo)
249241
for k, v := range capacity {
250242
internalCapacity[k] = v
@@ -300,7 +292,7 @@ func NewContainerManager(mountUtil mount.Interface, cadvisorInterface cadvisor.I
300292

301293
if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.TopologyManager) {
302294
cm.topologyManager, err = topologymanager.NewManager(
303-
numaNodeInfo,
295+
machineInfo.Topology,
304296
nodeConfig.ExperimentalTopologyManagerPolicy,
305297
)
306298

@@ -315,7 +307,7 @@ func NewContainerManager(mountUtil mount.Interface, cadvisorInterface cadvisor.I
315307

316308
klog.Infof("Creating device plugin manager: %t", devicePluginEnabled)
317309
if devicePluginEnabled {
318-
cm.deviceManager, err = devicemanager.NewManagerImpl(numaNodeInfo, cm.topologyManager)
310+
cm.deviceManager, err = devicemanager.NewManagerImpl(machineInfo.Topology, cm.topologyManager)
319311
cm.topologyManager.AddHintProvider(cm.deviceManager)
320312
} else {
321313
cm.deviceManager, err = devicemanager.NewManagerStub()
@@ -330,7 +322,6 @@ func NewContainerManager(mountUtil mount.Interface, cadvisorInterface cadvisor.I
330322
nodeConfig.ExperimentalCPUManagerPolicy,
331323
nodeConfig.ExperimentalCPUManagerReconcilePeriod,
332324
machineInfo,
333-
numaNodeInfo,
334325
nodeConfig.NodeAllocatableConfig.ReservedSystemCPUs,
335326
cm.GetNodeAllocatableReservation(),
336327
nodeConfig.KubeletRootDir,

‎pkg/kubelet/cm/cpumanager/cpu_manager.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func (s *sourcesReadyStub) AddSource(source string) {}
126126
func (s *sourcesReadyStub) AllReady() bool { return true }
127127

128128
// NewManager creates new cpu manager based on provided policy
129-
func NewManager(cpuPolicyName string, reconcilePeriod time.Duration, machineInfo *cadvisorapi.MachineInfo, numaNodeInfo topology.NUMANodeInfo, specificCPUs cpuset.CPUSet, nodeAllocatableReservation v1.ResourceList, stateFileDirectory string, affinity topologymanager.Store) (Manager, error) {
129+
func NewManager(cpuPolicyName string, reconcilePeriod time.Duration, machineInfo *cadvisorapi.MachineInfo, specificCPUs cpuset.CPUSet, nodeAllocatableReservation v1.ResourceList, stateFileDirectory string, affinity topologymanager.Store) (Manager, error) {
130130
var topo *topology.CPUTopology
131131
var policy Policy
132132

@@ -137,7 +137,7 @@ func NewManager(cpuPolicyName string, reconcilePeriod time.Duration, machineInfo
137137

138138
case PolicyStatic:
139139
var err error
140-
topo, err = topology.Discover(machineInfo, numaNodeInfo)
140+
topo, err = topology.Discover(machineInfo)
141141
if err != nil {
142142
return nil, err
143143
}

‎pkg/kubelet/cm/cpumanager/cpu_manager_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,7 @@ func TestCPUManagerGenerate(t *testing.T) {
629629
}
630630
defer os.RemoveAll(sDir)
631631

632-
mgr, err := NewManager(testCase.cpuPolicyName, 5*time.Second, machineInfo, nil, cpuset.NewCPUSet(), testCase.nodeAllocatableReservation, sDir, topologymanager.NewFakeManager())
632+
mgr, err := NewManager(testCase.cpuPolicyName, 5*time.Second, machineInfo, cpuset.NewCPUSet(), testCase.nodeAllocatableReservation, sDir, topologymanager.NewFakeManager())
633633
if testCase.expectedError != nil {
634634
if !strings.Contains(err.Error(), testCase.expectedError.Error()) {
635635
t.Errorf("Unexpected error message. Have: %s wants %s", err.Error(), testCase.expectedError.Error())

‎pkg/kubelet/cm/cpumanager/topology/BUILD‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,5 @@ go_test(
3333
name = "go_default_test",
3434
srcs = ["topology_test.go"],
3535
embed = [":go_default_library"],
36-
deps = [
37-
"//pkg/kubelet/cm/cpuset:go_default_library",
38-
"//vendor/github.com/google/cadvisor/info/v1:go_default_library",
39-
],
36+
deps = ["//vendor/github.com/google/cadvisor/info/v1:go_default_library"],
4037
)

‎pkg/kubelet/cm/cpumanager/topology/topology.go‎

Lines changed: 7 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ package topology
1818

1919
import (
2020
"fmt"
21-
"io/ioutil"
22-
"strings"
2321

2422
cadvisorapi "github.com/google/cadvisor/info/v1"
2523
"k8s.io/klog/v2"
@@ -218,34 +216,28 @@ func (d CPUDetails) CPUsInCores(ids ...int) cpuset.CPUSet {
218216
}
219217

220218
// Discover returns CPUTopology based on cadvisor node info
221-
func Discover(machineInfo *cadvisorapi.MachineInfo, numaNodeInfo NUMANodeInfo) (*CPUTopology, error) {
219+
func Discover(machineInfo *cadvisorapi.MachineInfo) (*CPUTopology, error) {
222220
if machineInfo.NumCores == 0 {
223221
return nil, fmt.Errorf("could not detect number of cpus")
224222
}
225223

226224
CPUDetails := CPUDetails{}
227225
numPhysicalCores := 0
228226

229-
for _, socket := range machineInfo.Topology {
230-
numPhysicalCores += len(socket.Cores)
231-
for _, core := range socket.Cores {
227+
for _, node := range machineInfo.Topology {
228+
numPhysicalCores += len(node.Cores)
229+
for _, core := range node.Cores {
232230
if coreID, err := getUniqueCoreID(core.Threads); err == nil {
233231
for _, cpu := range core.Threads {
234-
numaNodeID := 0
235-
for id, cset := range numaNodeInfo {
236-
if cset.Contains(cpu) {
237-
numaNodeID = id
238-
}
239-
}
240232
CPUDetails[cpu] = CPUInfo{
241233
CoreID: coreID,
242-
SocketID: socket.Id,
243-
NUMANodeID: numaNodeID,
234+
SocketID: core.SocketID,
235+
NUMANodeID: node.Id,
244236
}
245237
}
246238
} else {
247239
klog.Errorf("could not get unique coreID for socket: %d core %d threads: %v",
248-
socket.Id, core.Id, core.Threads)
240+
core.SocketID, core.Id, core.Threads)
249241
return nil, err
250242
}
251243
}
@@ -280,49 +272,3 @@ func getUniqueCoreID(threads []int) (coreID int, err error) {
280272

281273
return min, nil
282274
}
283-
284-
// GetNUMANodeInfo uses sysfs to return a map of NUMANode id to the list of
285-
// CPUs associated with that NUMANode.
286-
//
287-
// TODO: This is a temporary workaround until cadvisor provides this
288-
// information directly in machineInfo. We should remove this once this
289-
// information is available from cadvisor.
290-
func GetNUMANodeInfo() (NUMANodeInfo, error) {
291-
// Get the possible NUMA nodes on this machine. If reading this file
292-
// is not possible, this is not an error. Instead, we just return a
293-
// nil NUMANodeInfo, indicating that no NUMA information is available
294-
// on this machine. This should implicitly be interpreted as having a
295-
// single NUMA node with id 0 for all CPUs.
296-
nodelist, err := ioutil.ReadFile("/sys/devices/system/node/online")
297-
if err != nil {
298-
return nil, nil
299-
}
300-
301-
// Parse the nodelist into a set of Node IDs
302-
nodes, err := cpuset.Parse(strings.TrimSpace(string(nodelist)))
303-
if err != nil {
304-
return nil, err
305-
}
306-
307-
info := make(NUMANodeInfo)
308-
309-
// For each node...
310-
for _, node := range nodes.ToSlice() {
311-
// Read the 'cpulist' of the NUMA node from sysfs.
312-
path := fmt.Sprintf("/sys/devices/system/node/node%d/cpulist", node)
313-
cpulist, err := ioutil.ReadFile(path)
314-
if err != nil {
315-
return nil, err
316-
}
317-
318-
// Convert the 'cpulist' into a set of CPUs.
319-
cpus, err := cpuset.Parse(strings.TrimSpace(string(cpulist)))
320-
if err != nil {
321-
return nil, err
322-
}
323-
324-
info[node] = cpus
325-
}
326-
327-
return info, nil
328-
}

‎pkg/kubelet/cm/cpumanager/topology/topology_test.go‎

Lines changed: 33 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -21,26 +21,23 @@ import (
2121
"testing"
2222

2323
cadvisorapi "github.com/google/cadvisor/info/v1"
24-
"k8s.io/kubernetes/pkg/kubelet/cm/cpuset"
2524
)
2625

2726
func Test_Discover(t *testing.T) {
2827

2928
tests := []struct {
30-
name string
31-
machineInfo cadvisorapi.MachineInfo
32-
numaNodeInfo NUMANodeInfo
33-
want *CPUTopology
34-
wantErr bool
29+
name string
30+
machineInfo cadvisorapi.MachineInfo
31+
want *CPUTopology
32+
wantErr bool
3533
}{
3634
{
3735
name: "FailNumCores",
3836
machineInfo: cadvisorapi.MachineInfo{
3937
NumCores: 0,
4038
},
41-
numaNodeInfo: NUMANodeInfo{},
42-
want: &CPUTopology{},
43-
wantErr: true,
39+
want: &CPUTopology{},
40+
wantErr: true,
4441
},
4542
{
4643
name: "OneSocketHT",
@@ -49,17 +46,14 @@ func Test_Discover(t *testing.T) {
4946
Topology: []cadvisorapi.Node{
5047
{Id: 0,
5148
Cores: []cadvisorapi.Core{
52-
{Id: 0, Threads: []int{0, 4}},
53-
{Id: 1, Threads: []int{1, 5}},
54-
{Id: 2, Threads: []int{2, 6}},
55-
{Id: 3, Threads: []int{3, 7}},
49+
{SocketID: 0, Id: 0, Threads: []int{0, 4}},
50+
{SocketID: 0, Id: 1, Threads: []int{1, 5}},
51+
{SocketID: 0, Id: 2, Threads: []int{2, 6}},
52+
{SocketID: 0, Id: 3, Threads: []int{3, 7}},
5653
},
5754
},
5855
},
5956
},
60-
numaNodeInfo: NUMANodeInfo{
61-
0: cpuset.NewCPUSet(0, 1, 2, 3, 4, 5, 6, 7),
62-
},
6357
want: &CPUTopology{
6458
NumCPUs: 8,
6559
NumSockets: 1,
@@ -84,22 +78,18 @@ func Test_Discover(t *testing.T) {
8478
Topology: []cadvisorapi.Node{
8579
{Id: 0,
8680
Cores: []cadvisorapi.Core{
87-
{Id: 0, Threads: []int{0}},
88-
{Id: 2, Threads: []int{2}},
81+
{SocketID: 0, Id: 0, Threads: []int{0}},
82+
{SocketID: 0, Id: 2, Threads: []int{2}},
8983
},
9084
},
9185
{Id: 1,
9286
Cores: []cadvisorapi.Core{
93-
{Id: 1, Threads: []int{1}},
94-
{Id: 3, Threads: []int{3}},
87+
{SocketID: 1, Id: 1, Threads: []int{1}},
88+
{SocketID: 1, Id: 3, Threads: []int{3}},
9589
},
9690
},
9791
},
9892
},
99-
numaNodeInfo: NUMANodeInfo{
100-
0: cpuset.NewCPUSet(0, 2),
101-
1: cpuset.NewCPUSet(1, 3),
102-
},
10393
want: &CPUTopology{
10494
NumCPUs: 4,
10595
NumSockets: 2,
@@ -120,24 +110,20 @@ func Test_Discover(t *testing.T) {
120110
Topology: []cadvisorapi.Node{
121111
{Id: 0,
122112
Cores: []cadvisorapi.Core{
123-
{Id: 0, Threads: []int{0, 6}},
124-
{Id: 1, Threads: []int{1, 7}},
125-
{Id: 2, Threads: []int{2, 8}},
113+
{SocketID: 0, Id: 0, Threads: []int{0, 6}},
114+
{SocketID: 0, Id: 1, Threads: []int{1, 7}},
115+
{SocketID: 0, Id: 2, Threads: []int{2, 8}},
126116
},
127117
},
128118
{Id: 1,
129119
Cores: []cadvisorapi.Core{
130-
{Id: 0, Threads: []int{3, 9}},
131-
{Id: 1, Threads: []int{4, 10}},
132-
{Id: 2, Threads: []int{5, 11}},
120+
{SocketID: 1, Id: 0, Threads: []int{3, 9}},
121+
{SocketID: 1, Id: 1, Threads: []int{4, 10}},
122+
{SocketID: 1, Id: 2, Threads: []int{5, 11}},
133123
},
134124
},
135125
},
136126
},
137-
numaNodeInfo: NUMANodeInfo{
138-
0: cpuset.NewCPUSet(0, 6, 1, 7, 2, 8),
139-
1: cpuset.NewCPUSet(3, 9, 4, 10, 5, 11),
140-
},
141127
want: &CPUTopology{
142128
NumCPUs: 12,
143129
NumSockets: 2,
@@ -166,17 +152,16 @@ func Test_Discover(t *testing.T) {
166152
Topology: []cadvisorapi.Node{
167153
{Id: 0,
168154
Cores: []cadvisorapi.Core{
169-
{Id: 0, Threads: []int{0, 4}},
170-
{Id: 1, Threads: []int{1, 5}},
171-
{Id: 2, Threads: []int{2, 2}}, // Wrong case - should fail here
172-
{Id: 3, Threads: []int{3, 7}},
155+
{SocketID: 0, Id: 0, Threads: []int{0, 4}},
156+
{SocketID: 0, Id: 1, Threads: []int{1, 5}},
157+
{SocketID: 0, Id: 2, Threads: []int{2, 2}}, // Wrong case - should fail here
158+
{SocketID: 0, Id: 3, Threads: []int{3, 7}},
173159
},
174160
},
175161
},
176162
},
177-
numaNodeInfo: NUMANodeInfo{},
178-
want: &CPUTopology{},
179-
wantErr: true,
163+
want: &CPUTopology{},
164+
wantErr: true,
180165
},
181166
{
182167
name: "OneSocketHT fail",
@@ -185,22 +170,21 @@ func Test_Discover(t *testing.T) {
185170
Topology: []cadvisorapi.Node{
186171
{Id: 0,
187172
Cores: []cadvisorapi.Core{
188-
{Id: 0, Threads: []int{0, 4}},
189-
{Id: 1, Threads: []int{1, 5}},
190-
{Id: 2, Threads: []int{2, 6}},
191-
{Id: 3, Threads: []int{}}, // Wrong case - should fail here
173+
{SocketID: 0, Id: 0, Threads: []int{0, 4}},
174+
{SocketID: 0, Id: 1, Threads: []int{1, 5}},
175+
{SocketID: 0, Id: 2, Threads: []int{2, 6}},
176+
{SocketID: 0, Id: 3, Threads: []int{}}, // Wrong case - should fail here
192177
},
193178
},
194179
},
195180
},
196-
numaNodeInfo: NUMANodeInfo{},
197-
want: &CPUTopology{},
198-
wantErr: true,
181+
want: &CPUTopology{},
182+
wantErr: true,
199183
},
200184
}
201185
for _, tt := range tests {
202186
t.Run(tt.name, func(t *testing.T) {
203-
got, err := Discover(&tt.machineInfo, tt.numaNodeInfo)
187+
got, err := Discover(&tt.machineInfo)
204188
if err != nil {
205189
if tt.wantErr {
206190
t.Logf("Discover() expected error = %v", err)

0 commit comments

Comments
 (0)