From 0c2357710ab99a9e5491250f11c37c1d31d1879c Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Fri, 25 Oct 2024 08:39:16 -0700 Subject: [PATCH] respond to feedback Signed-off-by: James Sturtevant --- pkg/kubelet/cm/container_manager_windows.go | 8 +- pkg/kubelet/cm/cpumanager/cpu_manager.go | 35 ---- .../cm/cpumanager/cpu_manager_others.go | 43 +++++ pkg/kubelet/cm/cpumanager/cpu_manager_test.go | 36 ++++ .../cm/cpumanager/cpu_manager_windows.go | 49 +++++ .../internal_container_lifecycle_windows.go | 4 +- pkg/kubelet/kubelet.go | 2 +- pkg/kubelet/winstats/cpu_topology.go | 175 +++++++++--------- pkg/kubelet/winstats/cpu_topology_test.go | 127 +++++++++---- .../winstats/perfcounter_nodestats_windows.go | 3 +- .../test_data/versioned_feature_list.yaml | 6 + 11 files changed, 316 insertions(+), 172 deletions(-) create mode 100644 pkg/kubelet/cm/cpumanager/cpu_manager_others.go create mode 100644 pkg/kubelet/cm/cpumanager/cpu_manager_windows.go diff --git a/pkg/kubelet/cm/container_manager_windows.go b/pkg/kubelet/cm/container_manager_windows.go index 815533b2a40..8df3ff30cb9 100644 --- a/pkg/kubelet/cm/container_manager_windows.go +++ b/pkg/kubelet/cm/container_manager_windows.go @@ -130,6 +130,10 @@ func NewContainerManager(mountUtil mount.Interface, cadvisorInterface cadvisor.I nodeConfig.TopologyManagerPolicy, nodeConfig.TopologyManagerScope, nodeConfig.TopologyManagerPolicyOptions) + if err != nil { + klog.ErrorS(err, "Failed to initialize topology manager") + return nil, err + } klog.InfoS("Creating cpu manager") cm.cpuManager, err = cpumanager.NewManager( @@ -152,10 +156,6 @@ func NewContainerManager(mountUtil mount.Interface, cadvisorInterface cadvisor.I cm.cpuManager = cpumanager.NewFakeManager() } - if err != nil { - return nil, err - } - klog.InfoS("Creating device plugin manager") cm.deviceManager, err = devicemanager.NewManagerImpl(nil, cm.topologyManager) if err != nil { diff --git a/pkg/kubelet/cm/cpumanager/cpu_manager.go b/pkg/kubelet/cm/cpumanager/cpu_manager.go index 5922fe7ff89..12bd723e1ee 100644 --- a/pkg/kubelet/cm/cpumanager/cpu_manager.go +++ b/pkg/kubelet/cm/cpumanager/cpu_manager.go @@ -19,11 +19,7 @@ package cpumanager import ( "context" "fmt" - utilfeature "k8s.io/apiserver/pkg/util/feature" - kubefeatures "k8s.io/kubernetes/pkg/features" - "k8s.io/kubernetes/pkg/kubelet/winstats" "math" - "runtime" "sync" "time" @@ -532,37 +528,6 @@ func findContainerStatusByName(status *v1.PodStatus, name string) (*v1.Container return nil, fmt.Errorf("unable to find status for container with name %v in pod status (it may not be running)", name) } -func (m *manager) updateContainerCPUSet(ctx context.Context, containerID string, cpus cpuset.CPUSet) error { - // TODO: Consider adding a `ResourceConfigForContainer` helper in - // helpers_linux.go similar to what exists for pods. - // It would be better to pass the full container resources here instead of - // this patch-like partial resources. - - if runtime.GOOS == "windows" && utilfeature.DefaultFeatureGate.Enabled(kubefeatures.WindowsCPUAndMemoryAffinity) { - affinities := winstats.CpusToGroupAffinity(cpus.List()) - var cpuGroupAffinities []*runtimeapi.WindowsCpuGroupAffinity - for _, affinity := range affinities { - cpuGroupAffinities = append(cpuGroupAffinities, &runtimeapi.WindowsCpuGroupAffinity{ - CpuGroup: uint32(affinity.Group), - CpuMask: uint64(affinity.Mask), - }) - } - return m.containerRuntime.UpdateContainerResources(ctx, containerID, &runtimeapi.ContainerResources{ - Windows: &runtimeapi.WindowsContainerResources{ - AffinityCpus: cpuGroupAffinities, - }, - }) - } - return m.containerRuntime.UpdateContainerResources( - ctx, - containerID, - &runtimeapi.ContainerResources{ - Linux: &runtimeapi.LinuxContainerResources{ - CpusetCpus: cpus.String(), - }, - }) -} - func (m *manager) GetExclusiveCPUs(podUID, containerName string) cpuset.CPUSet { if result, ok := m.state.GetCPUSet(podUID, containerName); ok { return result diff --git a/pkg/kubelet/cm/cpumanager/cpu_manager_others.go b/pkg/kubelet/cm/cpumanager/cpu_manager_others.go new file mode 100644 index 00000000000..556583b1a7c --- /dev/null +++ b/pkg/kubelet/cm/cpumanager/cpu_manager_others.go @@ -0,0 +1,43 @@ +//go:build !windows +// +build !windows + +/* +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 cpumanager + +import ( + "context" + + runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1" + "k8s.io/utils/cpuset" +) + +func (m *manager) updateContainerCPUSet(ctx context.Context, containerID string, cpus cpuset.CPUSet) error { + // TODO: Consider adding a `ResourceConfigForContainer` helper in + // helpers_linux.go similar to what exists for pods. + // It would be better to pass the full container resources here instead of + // this patch-like partial resources. + + return m.containerRuntime.UpdateContainerResources( + ctx, + containerID, + &runtimeapi.ContainerResources{ + Linux: &runtimeapi.LinuxContainerResources{ + CpusetCpus: cpus.String(), + }, + }) +} diff --git a/pkg/kubelet/cm/cpumanager/cpu_manager_test.go b/pkg/kubelet/cm/cpumanager/cpu_manager_test.go index 0630032c511..30283db64c7 100644 --- a/pkg/kubelet/cm/cpumanager/cpu_manager_test.go +++ b/pkg/kubelet/cm/cpumanager/cpu_manager_test.go @@ -19,8 +19,12 @@ package cpumanager import ( "context" "fmt" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/kubernetes/pkg/features" "os" "reflect" + "runtime" "strconv" "strings" "testing" @@ -263,6 +267,10 @@ func makeMultiContainerPodWithOptions(initCPUs, appCPUs []*containerOptions) *v1 } func TestCPUManagerAdd(t *testing.T) { + if runtime.GOOS == "windows" { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WindowsCPUAndMemoryAffinity, true) + } + testPolicy, _ := NewStaticPolicy( &topology.CPUTopology{ NumCPUs: 4, @@ -347,6 +355,10 @@ func TestCPUManagerAdd(t *testing.T) { } func TestCPUManagerAddWithInitContainers(t *testing.T) { + if runtime.GOOS == "windows" { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WindowsCPUAndMemoryAffinity, true) + } + testCases := []struct { description string topo *topology.CPUTopology @@ -598,6 +610,10 @@ func TestCPUManagerAddWithInitContainers(t *testing.T) { } func TestCPUManagerGenerate(t *testing.T) { + if runtime.GOOS == "windows" { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WindowsCPUAndMemoryAffinity, true) + } + testCases := []struct { description string cpuPolicyName string @@ -703,6 +719,10 @@ func TestCPUManagerGenerate(t *testing.T) { } func TestCPUManagerRemove(t *testing.T) { + if runtime.GOOS == "windows" { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WindowsCPUAndMemoryAffinity, true) + } + containerID := "fakeID" containerMap := containermap.NewContainerMap() @@ -746,6 +766,10 @@ func TestCPUManagerRemove(t *testing.T) { } func TestReconcileState(t *testing.T) { + if runtime.GOOS == "windows" { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WindowsCPUAndMemoryAffinity, true) + } + testPolicy, _ := NewStaticPolicy( &topology.CPUTopology{ NumCPUs: 8, @@ -1269,6 +1293,10 @@ func TestReconcileState(t *testing.T) { // above test cases are without kubelet --reserved-cpus cmd option // the following tests are with --reserved-cpus configured func TestCPUManagerAddWithResvList(t *testing.T) { + if runtime.GOOS == "windows" { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WindowsCPUAndMemoryAffinity, true) + } + testPolicy, _ := NewStaticPolicy( &topology.CPUTopology{ NumCPUs: 4, @@ -1343,6 +1371,10 @@ func TestCPUManagerAddWithResvList(t *testing.T) { } func TestCPUManagerHandlePolicyOptions(t *testing.T) { + if runtime.GOOS == "windows" { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WindowsCPUAndMemoryAffinity, true) + } + testCases := []struct { description string cpuPolicyName string @@ -1409,6 +1441,10 @@ func TestCPUManagerHandlePolicyOptions(t *testing.T) { } func TestCPUManagerGetAllocatableCPUs(t *testing.T) { + if runtime.GOOS == "windows" { + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WindowsCPUAndMemoryAffinity, true) + } + nonePolicy, _ := NewNonePolicy(nil) staticPolicy, _ := NewStaticPolicy( &topology.CPUTopology{ diff --git a/pkg/kubelet/cm/cpumanager/cpu_manager_windows.go b/pkg/kubelet/cm/cpumanager/cpu_manager_windows.go new file mode 100644 index 00000000000..fedc61ac44b --- /dev/null +++ b/pkg/kubelet/cm/cpumanager/cpu_manager_windows.go @@ -0,0 +1,49 @@ +//go:build windows +// +build windows + +/* +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 cpumanager + +import ( + "context" + utilfeature "k8s.io/apiserver/pkg/util/feature" + runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1" + kubefeatures "k8s.io/kubernetes/pkg/features" + "k8s.io/kubernetes/pkg/kubelet/winstats" + "k8s.io/utils/cpuset" +) + +func (m *manager) updateContainerCPUSet(ctx context.Context, containerID string, cpus cpuset.CPUSet) error { + if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.WindowsCPUAndMemoryAffinity) { + affinities := winstats.CpusToGroupAffinity(cpus.List()) + var cpuGroupAffinities []*runtimeapi.WindowsCpuGroupAffinity + for _, affinity := range affinities { + cpuGroupAffinities = append(cpuGroupAffinities, &runtimeapi.WindowsCpuGroupAffinity{ + CpuGroup: uint32(affinity.Group), + CpuMask: uint64(affinity.Mask), + }) + } + return m.containerRuntime.UpdateContainerResources(ctx, containerID, &runtimeapi.ContainerResources{ + Windows: &runtimeapi.WindowsContainerResources{ + AffinityCpus: cpuGroupAffinities, + }, + }) + } + + return nil +} diff --git a/pkg/kubelet/cm/internal_container_lifecycle_windows.go b/pkg/kubelet/cm/internal_container_lifecycle_windows.go index 2261b47a2e8..96159e99a8e 100644 --- a/pkg/kubelet/cm/internal_container_lifecycle_windows.go +++ b/pkg/kubelet/cm/internal_container_lifecycle_windows.go @@ -30,14 +30,12 @@ import ( func (i *internalContainerLifecycleImpl) PreCreateContainer(pod *v1.Pod, container *v1.Container, containerConfig *runtimeapi.ContainerConfig) error { if i.cpuManager != nil && utilfeature.DefaultFeatureGate.Enabled(kubefeatures.WindowsCPUAndMemoryAffinity) { - klog.Info("PreCreateContainer for Windows") allocatedCPUs := i.cpuManager.GetCPUAffinity(string(pod.UID), container.Name) if !allocatedCPUs.IsEmpty() { - klog.Infof("Setting CPU affinity for container %q cpus %v", container.Name, allocatedCPUs.String()) var cpuGroupAffinities []*runtimeapi.WindowsCpuGroupAffinity affinities := winstats.CpusToGroupAffinity(allocatedCPUs.List()) for _, affinity := range affinities { - klog.Infof("Setting CPU affinity for container %q in group %v with mask %v (processors %v)", container.Name, affinity.Group, affinity.Mask, affinity.Processors()) + klog.V(4).InfoS("Setting CPU affinity", "container", container.Name, "pod", pod.Name, "group", affinity.Group, "mask", affinity.MaskString(), "processorIds", affinity.Processors()) cpuGroupAffinities = append(cpuGroupAffinities, &runtimeapi.WindowsCpuGroupAffinity{ CpuGroup: uint32(affinity.Group), CpuMask: uint64(affinity.Mask), diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 12561b737bb..c37d8e88ec7 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1421,7 +1421,7 @@ func (kl *Kubelet) setupDataDirs() error { if err := kl.hostutil.MakeRShared(kl.getRootDir()); err != nil { return fmt.Errorf("error configuring root directory: %v", err) } - if err := utilfs.MkdirAll(kl.getPodsDir(), 0750); err != nil { + if err := os.MkdirAll(kl.getPodsDir(), 0750); err != nil { return fmt.Errorf("error creating pods directory: %v", err) } if err := utilfs.MkdirAll(kl.getPluginsDir(), 0750); err != nil { diff --git a/pkg/kubelet/winstats/cpu_topology.go b/pkg/kubelet/winstats/cpu_topology.go index ce06895e401..0ea5af43ebe 100644 --- a/pkg/kubelet/winstats/cpu_topology.go +++ b/pkg/kubelet/winstats/cpu_topology.go @@ -1,3 +1,22 @@ +//go:build windows +// +build windows + +/* +Copyright 2024 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 winstats import ( @@ -13,41 +32,51 @@ var ( getNumaAvailableMemoryNodeEx = modkernel32.NewProc("GetNumaAvailableMemoryNodeEx") ) -type RelationType int +type relationType int const ( - RelationProcessorCore RelationType = iota - RelationNumaNode - RelationCache - RelationProcessorPackage - RelationGroup - RelationProcessorDie - RelationNumaNodeEx - RelationProcessorModule - RelationAll = 0xffff + relationProcessorCore relationType = iota + relationNumaNode + relationCache + relationProcessorPackage + relationGroup + relationProcessorDie + relationNumaNodeEx + relationProcessorModule + relationAll = 0xffff ) -type SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX struct { +type systemLogicalProcessorInformationEx struct { Relationship uint32 Size uint32 data interface{} } -type PROCESSOR_RELATIONSHIP struct { +type processorRelationship struct { Flags byte EfficiencyClass byte Reserved [20]byte GroupCount uint16 - GroupMasks interface{} //[]GROUP_AFFINITY // in c++ this is a union of either one or many GROUP_AFFINITY based on GroupCount + // groupMasks is an []GroupAffinity. In c++ this is a union of either one or many GroupAffinity based on GroupCount + GroupMasks interface{} } -type GROUP_AFFINITY struct { - Mask uintptr +// GroupAffinity represents the processor group affinity of cpus +// https://learn.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-group_affinity +type GroupAffinity struct { + Mask uint64 Group uint16 Reserved [3]uint16 } -func (a GROUP_AFFINITY) Processors() []int { +// MaskString returns the affinity mask as a string of 0s and 1s +func (a GroupAffinity) MaskString() string { + return fmt.Sprintf("%064b", a.Mask) +} + +// Processors returns a list of processors ids that are part of the affinity mask +// Windows doesn't track processors by ID but kubelet converts them to a number +func (a GroupAffinity) Processors() []int { processors := []int{} for i := 0; i < 64; i++ { if a.Mask&(1< len(buffer) { + return 0, 0, nil, fmt.Errorf("remaining buffer too small while reading windows processor relationship") + } + info := (*systemLogicalProcessorInformationEx)(unsafe.Pointer(&buffer[offset])) + // check one more time now that we know the size of the struct + if offset+int(info.Size) > len(buffer) { + return 0, 0, nil, fmt.Errorf("remaining buffer too small while reading windows processor relationship") + } + switch (relationType)(info.Relationship) { + case relationProcessorCore, relationProcessorPackage: + relationship := (*processorRelationship)(unsafe.Pointer(&info.data)) + groupMasks := make([]GroupAffinity, relationship.GroupCount) + for i := 0; i < int(relationship.GroupCount); i++ { + groupMasks[i] = *(*GroupAffinity)(unsafe.Pointer(uintptr(unsafe.Pointer(&relationship.GroupMasks)) + uintptr(i)*unsafe.Sizeof(GroupAffinity{}))) } - if RelationProcessorCore == (RelationType)(info.Relationship) { + if relationProcessorCore == (relationType)(info.Relationship) { numOfcores++ } - if RelationProcessorPackage == (RelationType)(info.Relationship) { + if relationProcessorPackage == (relationType)(info.Relationship) { numofSockets++ } @@ -184,20 +186,20 @@ func convertWinApiToCadvisorApi(buffer []byte) (int, int, []cadvisorapi.Node, er p = &processor{} logicalProcessors[processorId] = p } - if RelationProcessorCore == (RelationType)(info.Relationship) { + if relationProcessorCore == (relationType)(info.Relationship) { p.CoreID = numOfcores } - if RelationProcessorPackage == (RelationType)(info.Relationship) { + if relationProcessorPackage == (relationType)(info.Relationship) { p.SocketID = numofSockets } } } - case RelationNumaNode, RelationNumaNodeEx: - numaNodeRelationship := (*NUMA_NODE_RELATIONSHIP)(unsafe.Pointer(&info.data)) - groupMasks := make([]GROUP_AFFINITY, numaNodeRelationship.GroupCount) + case relationNumaNode, relationNumaNodeEx: + numaNodeRelationship := (*numaNodeRelationship)(unsafe.Pointer(&info.data)) + groupMasks := make([]GroupAffinity, numaNodeRelationship.GroupCount) for i := 0; i < int(numaNodeRelationship.GroupCount); i++ { - groupMasks[i] = *(*GROUP_AFFINITY)(unsafe.Pointer(uintptr(unsafe.Pointer(&numaNodeRelationship.GroupMasks)) + uintptr(i)*unsafe.Sizeof(GROUP_AFFINITY{}))) + groupMasks[i] = *(*GroupAffinity)(unsafe.Pointer(uintptr(unsafe.Pointer(&numaNodeRelationship.GroupMasks)) + uintptr(i)*unsafe.Sizeof(GroupAffinity{}))) } nodes = append(nodes, cadvisorapi.Node{Id: int(numaNodeRelationship.NodeNumber)}) @@ -213,12 +215,8 @@ func convertWinApiToCadvisorApi(buffer []byte) (int, int, []cadvisorapi.Node, er } } - case RelationCache: - //cacheRelationship := (*CACHE_RELATIONSHIP)(unsafe.Pointer(&info.data)) - // TODO Process cache relationship data - default: - klog.V(4).Infof("Not using relationship type: %d", info.Relationship) + klog.V(4).Infof("Not using Windows CPU relationship type: %d", info.Relationship) } // Move the offset to the next SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX struct @@ -226,21 +224,20 @@ func convertWinApiToCadvisorApi(buffer []byte) (int, int, []cadvisorapi.Node, er } for processId, p := range logicalProcessors { - klog.V(4).Infof("Processor (%d): %v", processId, p) node := nodes[p.NodeID] if node.Id != p.NodeID { - return 0, 0, nil, fmt.Errorf("Node ID mismatch: %d != %d", node.Id, p.NodeID) + return 0, 0, nil, fmt.Errorf("node ID mismatch: %d != %d", node.Id, p.NodeID) } availableBytes := uint64(0) r1, _, err := getNumaAvailableMemoryNodeEx.Call(uintptr(p.NodeID), uintptr(unsafe.Pointer(&availableBytes))) if r1 == 0 { - return 0, 0, nil, fmt.Errorf("Call to GetNumaAvailableMemoryNodeEx failed: %v", err) + return 0, 0, nil, fmt.Errorf("call to GetNumaAvailableMemoryNodeEx failed: %v", err) } node.Memory = availableBytes node.AddThread(processId, p.CoreID) ok, coreIdx := node.FindCore(p.CoreID) if !ok { - return 0, 0, nil, fmt.Errorf("Core not found: %d", p.CoreID) + return 0, 0, nil, fmt.Errorf("core not found: %d", p.CoreID) } node.Cores[coreIdx].SocketID = p.SocketID nodes[p.NodeID] = node diff --git a/pkg/kubelet/winstats/cpu_topology_test.go b/pkg/kubelet/winstats/cpu_topology_test.go index eb8df3f58dc..f22b4840522 100644 --- a/pkg/kubelet/winstats/cpu_topology_test.go +++ b/pkg/kubelet/winstats/cpu_topology_test.go @@ -1,7 +1,25 @@ +//go:build windows +// +build windows + +/* +Copyright 2024 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 winstats import ( - "fmt" cadvisorapi "github.com/google/cadvisor/info/v1" "github.com/stretchr/testify/assert" "testing" @@ -11,7 +29,7 @@ import ( func TestGROUP_AFFINITY_Processors(t *testing.T) { tests := []struct { name string - Mask uintptr + Mask uint64 Group uint16 want []int }{ @@ -78,7 +96,7 @@ func TestGROUP_AFFINITY_Processors(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - a := GROUP_AFFINITY{ + a := GroupAffinity{ Mask: tt.Mask, Group: tt.Group, } @@ -100,16 +118,16 @@ func TestCpusToGroupAffinity(t *testing.T) { tests := []struct { name string cpus []int - want map[int]*GROUP_AFFINITY + want map[int]*GroupAffinity }{ { name: "empty", - want: map[int]*GROUP_AFFINITY{}, + want: map[int]*GroupAffinity{}, }, { name: "single cpu group 0", cpus: []int{0}, - want: map[int]*GROUP_AFFINITY{ + want: map[int]*GroupAffinity{ 0: { Mask: 1, Group: 0, @@ -119,7 +137,7 @@ func TestCpusToGroupAffinity(t *testing.T) { { name: "single cpu group 0", cpus: []int{63}, - want: map[int]*GROUP_AFFINITY{ + want: map[int]*GroupAffinity{ 0: { Mask: 1 << 63, Group: 0, @@ -129,7 +147,7 @@ func TestCpusToGroupAffinity(t *testing.T) { { name: "single cpu group 1", cpus: []int{64}, - want: map[int]*GROUP_AFFINITY{ + want: map[int]*GroupAffinity{ 1: { Mask: 1, Group: 1, @@ -139,7 +157,7 @@ func TestCpusToGroupAffinity(t *testing.T) { { name: "multiple cpus same group", cpus: []int{0, 1, 2}, - want: map[int]*GROUP_AFFINITY{ + want: map[int]*GroupAffinity{ 0: { Mask: 1 | 2 | 4, // Binary OR to combine the masks Group: 0, @@ -149,7 +167,7 @@ func TestCpusToGroupAffinity(t *testing.T) { { name: "multiple cpus different groups", cpus: []int{0, 64}, - want: map[int]*GROUP_AFFINITY{ + want: map[int]*GroupAffinity{ 0: { Mask: 1, Group: 0, @@ -163,7 +181,7 @@ func TestCpusToGroupAffinity(t *testing.T) { { name: "multiple cpus different groups", cpus: []int{0, 1, 2, 64, 65, 66}, - want: map[int]*GROUP_AFFINITY{ + want: map[int]*GroupAffinity{ 0: { Mask: 1 | 2 | 4, Group: 0, @@ -177,7 +195,7 @@ func TestCpusToGroupAffinity(t *testing.T) { { name: "64 cpus group 0", cpus: makeRange(0, 63), - want: map[int]*GROUP_AFFINITY{ + want: map[int]*GroupAffinity{ 0: { Mask: 0xffffffffffffffff, // All 64 bits set Group: 0, @@ -187,7 +205,7 @@ func TestCpusToGroupAffinity(t *testing.T) { { name: "64 cpus group 1", cpus: makeRange(64, 127), - want: map[int]*GROUP_AFFINITY{ + want: map[int]*GroupAffinity{ 1: { Mask: 0xffffffffffffffff, // All 64 bits set Group: 1, @@ -209,7 +227,7 @@ func Test_convertWinApiToCadvisorApi(t *testing.T) { expectedNumOfCores int expectedNumOfSockets int expectedNodes []cadvisorapi.Node - wantErr assert.ErrorAssertionFunc + wantErr bool }{ { name: "empty", @@ -217,7 +235,7 @@ func Test_convertWinApiToCadvisorApi(t *testing.T) { expectedNumOfCores: 0, expectedNumOfSockets: 0, expectedNodes: []cadvisorapi.Node{}, - wantErr: assert.NoError, + wantErr: false, }, { name: "single core", @@ -235,7 +253,7 @@ func Test_convertWinApiToCadvisorApi(t *testing.T) { }, }, }, - wantErr: assert.NoError, + wantErr: false, }, { name: "single core, multiple cpus", @@ -253,7 +271,7 @@ func Test_convertWinApiToCadvisorApi(t *testing.T) { }, }, }, - wantErr: assert.NoError, + wantErr: false, }, { name: "single core, multiple groups", @@ -271,13 +289,32 @@ func Test_convertWinApiToCadvisorApi(t *testing.T) { }, }, }, - wantErr: assert.NoError, + wantErr: false, + }, + { + name: "buffer to small", + buffer: createProcessorRelationships([]int{0, 64})[:48], + expectedNumOfCores: 1, + expectedNumOfSockets: 1, + expectedNodes: []cadvisorapi.Node{ + { + Id: 0, + Cores: []cadvisorapi.Core{ + { + Id: 1, + Threads: []int{0, 64}, + }, + }, + }, + }, + wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { numOfCores, numOfSockets, nodes, err := convertWinApiToCadvisorApi(tt.buffer) - if !tt.wantErr(t, err, fmt.Sprintf("convertWinApiToCadvisorApi(%v)", tt.name)) { + if tt.wantErr { + assert.Error(t, err) return } assert.Equalf(t, tt.expectedNumOfCores, numOfCores, "num of cores") @@ -286,14 +323,26 @@ func Test_convertWinApiToCadvisorApi(t *testing.T) { assert.Equalf(t, tt.expectedNodes[node].Id, nodes[node].Id, "node id") for core := range nodes[node].Cores { assert.Equalf(t, tt.expectedNodes[node].Cores[core].Id, nodes[node].Cores[core].Id, "core id") - assert.Equalf(t, tt.expectedNodes[node].Cores[core].Threads, nodes[node].Cores[core].Threads, "threads") + assert.Equalf(t, len(tt.expectedNodes[node].Cores[core].Threads), len(nodes[node].Cores[core].Threads), "num of threads") + for _, thread := range nodes[node].Cores[core].Threads { + assert.Truef(t, containsThread(tt.expectedNodes[node].Cores[core].Threads, thread), "thread %d", thread) + } } } }) } } -func genbuffer(infos ...SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX) []byte { +func containsThread(threads []int, thread int) bool { + for _, t := range threads { + if t == thread { + return true + } + } + return false +} + +func genBuffer(infos ...systemLogicalProcessorInformationEx) []byte { var buffer []byte for _, info := range infos { buffer = append(buffer, structToBytes(info)...) @@ -304,32 +353,32 @@ func genbuffer(infos ...SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX) []byte { func createProcessorRelationships(cpus []int) []byte { groups := CpusToGroupAffinity(cpus) grouplen := len(groups) - groupAffinities := make([]GROUP_AFFINITY, 0, grouplen) + groupAffinities := make([]GroupAffinity, 0, grouplen) for _, group := range groups { groupAffinities = append(groupAffinities, *group) } - return genbuffer(SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX{ - Relationship: uint32(RelationProcessorCore), + return genBuffer(systemLogicalProcessorInformationEx{ + Relationship: uint32(relationProcessorCore), Size: uint32(SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX_SIZE + PROCESSOR_RELATIONSHIP_SIZE + (GROUP_AFFINITY_SIZE * grouplen)), - data: PROCESSOR_RELATIONSHIP{ + data: processorRelationship{ Flags: 0, EfficiencyClass: 0, Reserved: [20]byte{}, GroupCount: uint16(grouplen), GroupMasks: groupAffinities, }, - }, SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX{ - Relationship: uint32(RelationNumaNode), + }, systemLogicalProcessorInformationEx{ + Relationship: uint32(relationNumaNode), Size: uint32(SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX_SIZE + NUMA_NODE_RELATIONSHIP_SIZE + (GROUP_AFFINITY_SIZE * grouplen)), - data: NUMA_NODE_RELATIONSHIP{ + data: numaNodeRelationship{ NodeNumber: 0, Reserved: [18]byte{}, GroupCount: uint16(grouplen), GroupMasks: groupAffinities, - }}, SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX{ - Relationship: uint32(RelationProcessorPackage), + }}, systemLogicalProcessorInformationEx{ + Relationship: uint32(relationProcessorPackage), Size: uint32(SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX_SIZE + PROCESSOR_RELATIONSHIP_SIZE + (GROUP_AFFINITY_SIZE * grouplen)), - data: PROCESSOR_RELATIONSHIP{ + data: processorRelationship{ Flags: 0, EfficiencyClass: 0, Reserved: [20]byte{}, @@ -342,29 +391,29 @@ func createProcessorRelationships(cpus []int) []byte { const SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX_SIZE = 8 const PROCESSOR_RELATIONSHIP_SIZE = 24 const NUMA_NODE_RELATIONSHIP_SIZE = 24 -const GROUP_AFFINITY_SIZE = int(unsafe.Sizeof(GROUP_AFFINITY{})) // this one is known at compile time +const GROUP_AFFINITY_SIZE = int(unsafe.Sizeof(GroupAffinity{})) // this one is known at compile time -func structToBytes(info SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX) []byte { +func structToBytes(info systemLogicalProcessorInformationEx) []byte { var pri []byte = (*(*[SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX_SIZE]byte)(unsafe.Pointer(&info)))[:SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX_SIZE] switch info.data.(type) { - case PROCESSOR_RELATIONSHIP: - rel := info.data.(PROCESSOR_RELATIONSHIP) + case processorRelationship: + rel := info.data.(processorRelationship) var prBytes []byte = (*(*[PROCESSOR_RELATIONSHIP_SIZE]byte)(unsafe.Pointer(&rel)))[:PROCESSOR_RELATIONSHIP_SIZE] pri = append(pri, prBytes...) - groupAffinities := rel.GroupMasks.([]GROUP_AFFINITY) + groupAffinities := rel.GroupMasks.([]GroupAffinity) for _, groupAffinity := range groupAffinities { var groupByte []byte = (*(*[GROUP_AFFINITY_SIZE]byte)(unsafe.Pointer(&groupAffinity)))[:] pri = append(pri, groupByte...) } - case NUMA_NODE_RELATIONSHIP: - numa := info.data.(NUMA_NODE_RELATIONSHIP) + case numaNodeRelationship: + numa := info.data.(numaNodeRelationship) var nameBytes []byte = (*(*[NUMA_NODE_RELATIONSHIP_SIZE]byte)(unsafe.Pointer(&numa)))[:NUMA_NODE_RELATIONSHIP_SIZE] pri = append(pri, nameBytes...) - groupAffinities := numa.GroupMasks.([]GROUP_AFFINITY) + groupAffinities := numa.GroupMasks.([]GroupAffinity) for _, groupAffinity := range groupAffinities { var groupByte []byte = (*(*[GROUP_AFFINITY_SIZE]byte)(unsafe.Pointer(&groupAffinity)))[:] diff --git a/pkg/kubelet/winstats/perfcounter_nodestats_windows.go b/pkg/kubelet/winstats/perfcounter_nodestats_windows.go index 4024a14e135..7f77e9a355e 100644 --- a/pkg/kubelet/winstats/perfcounter_nodestats_windows.go +++ b/pkg/kubelet/winstats/perfcounter_nodestats_windows.go @@ -188,7 +188,7 @@ func (p *perfCounterNodeStatsClient) getMachineInfo() (*cadvisorapi.MachineInfo, } if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.WindowsCPUAndMemoryAffinity) { - numOfPysicalCores, numOfSockets, topology, err := processorInfo(RelationAll) + numOfPysicalCores, numOfSockets, topology, err := processorInfo(relationAll) if err != nil { return nil, err } @@ -201,6 +201,7 @@ func (p *perfCounterNodeStatsClient) getMachineInfo() (*cadvisorapi.MachineInfo, return mi, nil } +// ProcessorCount returns the number of logical processors on the system. // runtime.NumCPU() will only return the information for a single Processor Group. // Since a single group can only hold 64 logical processors, this // means when there are more they will be divided into multiple groups. diff --git a/test/featuregates_linter/test_data/versioned_feature_list.yaml b/test/featuregates_linter/test_data/versioned_feature_list.yaml index 817a665ab3f..7100ddb7b2f 100644 --- a/test/featuregates_linter/test_data/versioned_feature_list.yaml +++ b/test/featuregates_linter/test_data/versioned_feature_list.yaml @@ -1396,6 +1396,12 @@ lockToDefault: false preRelease: Beta version: "1.32" +- name: WindowsCPUAndMemoryAffinity + versionedSpecs: + - default: false + lockToDefault: false + preRelease: Alpha + version: "1.32" - name: WindowsHostNetwork versionedSpecs: - default: true