diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_windows.go b/pkg/kubelet/kuberuntime/kuberuntime_container_windows.go index 66430cf163e9..4ae6ffa09d72 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_windows.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_windows.go @@ -20,12 +20,12 @@ limitations under the License. package kuberuntime import ( - "runtime" - v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1" "k8s.io/klog/v2" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" + "k8s.io/kubernetes/pkg/kubelet/winstats" "k8s.io/kubernetes/pkg/securitycontext" ) @@ -50,11 +50,6 @@ func (m *kubeGenericRuntimeManager) generateWindowsContainerConfig(container *v1 cpuLimit := container.Resources.Limits.Cpu() if !cpuLimit.IsZero() { - // Note that sysinfo.NumCPU() is limited to 64 CPUs on Windows due to Processor Groups, - // as only 64 processors are available for execution by a given process. This causes - // some oddities on systems with more than 64 processors. - // Refer https://msdn.microsoft.com/en-us/library/windows/desktop/dd405503(v=vs.85).aspx. - // Since Kubernetes doesn't have any notion of weight in the Pod/Container API, only limits/reserves, then applying CpuMaximum only // will better follow the intent of the user. At one point CpuWeights were set, but this prevented limits from having any effect. @@ -78,17 +73,7 @@ func (m *kubeGenericRuntimeManager) generateWindowsContainerConfig(container *v1 // https://github.com/kubernetes/kubernetes/blob/56d1c3b96d0a544130a82caad33dd57629b8a7f8/staging/src/k8s.io/cri-api/pkg/apis/runtime/v1/api.proto#L681-L682 // https://github.com/opencontainers/runtime-spec/blob/ad53dcdc39f1f7f7472b10aa0a45648fe4865496/config-windows.md#cpu // If both CpuWeight and CpuMaximum are set - ContainerD catches this invalid case and returns an error instead. - - cpuMaximum := 10000 * cpuLimit.MilliValue() / int64(runtime.NumCPU()) / 1000 - - // ensure cpuMaximum is in range [1, 10000]. - if cpuMaximum < 1 { - cpuMaximum = 1 - } else if cpuMaximum > 10000 { - cpuMaximum = 10000 - } - - wc.Resources.CpuMaximum = cpuMaximum + wc.Resources.CpuMaximum = calculateCPUMaximum(cpuLimit, int64(winstats.ProcessorCount())) } // The processor resource controls are mutually exclusive on @@ -128,3 +113,16 @@ func (m *kubeGenericRuntimeManager) generateWindowsContainerConfig(container *v1 return wc, nil } + +// calculateCPUMaximum calculates the maximum CPU given a limit and a number of cpus while ensuring it's in range [1,10000]. +func calculateCPUMaximum(cpuLimit *resource.Quantity, cpuCount int64) int64 { + cpuMaximum := 10 * cpuLimit.MilliValue() / cpuCount + + // ensure cpuMaximum is in range [1, 10000]. + if cpuMaximum < 1 { + cpuMaximum = 1 + } else if cpuMaximum > 10000 { + cpuMaximum = 10000 + } + return cpuMaximum +} diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_windows_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_windows_test.go index 5b8be205c132..7a303e061522 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_windows_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_windows_test.go @@ -20,7 +20,6 @@ limitations under the License. package kuberuntime import ( - "runtime" "testing" "github.com/stretchr/testify/assert" @@ -29,10 +28,8 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1" - "k8s.io/kubernetes/pkg/features" + "k8s.io/kubernetes/pkg/kubelet/winstats" ) func TestApplyPlatformSpecificContainerConfig(t *testing.T) { @@ -87,7 +84,8 @@ func TestApplyPlatformSpecificContainerConfig(t *testing.T) { err = fakeRuntimeSvc.applyPlatformSpecificContainerConfig(containerConfig, &pod.Spec.Containers[0], pod, new(int64), "foo", nil) require.NoError(t, err) - expectedCpuMax := ((10000 * 3000) / int64(runtime.NumCPU()) / 1000) + limit := int64(3000) + expectedCpuMax := 10 * limit / int64(winstats.ProcessorCount()) expectedWindowsConfig := &runtimeapi.WindowsContainerConfig{ Resources: &runtimeapi.WindowsContainerResources{ CpuMaximum: expectedCpuMax, @@ -101,3 +99,48 @@ func TestApplyPlatformSpecificContainerConfig(t *testing.T) { } assert.Equal(t, expectedWindowsConfig, containerConfig.Windows) } + +func TestCalculateCPUMaximum(t *testing.T) { + tests := []struct { + name string + cpuLimit resource.Quantity + cpuCount int64 + want int64 + }{ + { + name: "max range when same amount", + cpuLimit: resource.MustParse("1"), + cpuCount: 1, + want: 10000, + }, + { + name: "percentage calculation is working as intended", + cpuLimit: resource.MustParse("94"), + cpuCount: 96, + want: 9791, + }, + { + name: "half range when half amount", + cpuLimit: resource.MustParse("1"), + cpuCount: 2, + want: 5000, + }, + { + name: "max range when more requested than available", + cpuLimit: resource.MustParse("2"), + cpuCount: 1, + want: 10000, + }, + { + name: "min range when less than minimum", + cpuLimit: resource.MustParse("1m"), + cpuCount: 100, + want: 1, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, calculateCPUMaximum(&tt.cpuLimit, tt.cpuCount)) + }) + } +} diff --git a/pkg/kubelet/winstats/perfcounter_nodestats.go b/pkg/kubelet/winstats/perfcounter_nodestats.go index 9eee9772f65c..9bd4e13796ad 100644 --- a/pkg/kubelet/winstats/perfcounter_nodestats.go +++ b/pkg/kubelet/winstats/perfcounter_nodestats.go @@ -157,7 +157,7 @@ func (p *perfCounterNodeStatsClient) getMachineInfo() (*cadvisorapi.MachineInfo, } return &cadvisorapi.MachineInfo{ - NumCores: processorCount(), + NumCores: ProcessorCount(), MemoryCapacity: p.nodeInfo.memoryPhysicalCapacityBytes, MachineID: hostname, SystemUUID: systemUUID, @@ -173,7 +173,7 @@ func (p *perfCounterNodeStatsClient) getMachineInfo() (*cadvisorapi.MachineInfo, // more notes for this issue: // same issue in moby: https://github.com/moby/moby/issues/38935#issuecomment-744638345 // solution in hcsshim: https://github.com/microsoft/hcsshim/blob/master/internal/processorinfo/processor_count.go -func processorCount() int { +func ProcessorCount() int { if amount := getActiveProcessorCount(allProcessorGroups); amount != 0 { return int(amount) }