From 4c875893005ef5a3e2630e0c2c616fdbfd5b5fa9 Mon Sep 17 00:00:00 2001 From: waynepeking348 Date: Sun, 20 Mar 2022 21:53:07 +0800 Subject: [PATCH 1/3] fix bugs of container cpu shares when cpu request set to zero --- .../kuberuntime_container_linux.go | 10 +++++--- .../kuberuntime_container_linux_test.go | 23 +++++++++++++++++++ .../kuberuntime/kuberuntime_sandbox_linux.go | 7 +++++- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go b/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go index d12e1d4a681..1e1d59daade 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go @@ -63,7 +63,11 @@ func (m *kubeGenericRuntimeManager) generateLinuxContainerConfig(container *v1.C } // set linux container resources - lc.Resources = m.calculateLinuxResources(container.Resources.Requests.Cpu(), container.Resources.Limits.Cpu(), container.Resources.Limits.Memory()) + var cpuRequest *resource.Quantity + if _, cpuRequestExists := container.Resources.Requests[v1.ResourceCPU]; cpuRequestExists { + cpuRequest = container.Resources.Requests.Cpu() + } + lc.Resources = m.calculateLinuxResources(cpuRequest, container.Resources.Limits.Cpu(), container.Resources.Limits.Memory()) lc.Resources.OomScoreAdj = int64(qos.GetContainerOOMScoreAdjust(pod, container, int64(m.machineInfo.MemoryCapacity))) @@ -138,9 +142,9 @@ func (m *kubeGenericRuntimeManager) calculateLinuxResources(cpuRequest, cpuLimit // If request is not specified, but limit is, we want request to default to limit. // API server does this for new containers, but we repeat this logic in Kubelet // for containers running on existing Kubernetes clusters. - if cpuRequest.IsZero() && !cpuLimit.IsZero() { + if cpuRequest == nil && !cpuLimit.IsZero() { cpuShares = int64(cm.MilliCPUToShares(cpuLimit.MilliValue())) - } else { + } else if cpuRequest != nil { // if cpuRequest.Amount is nil, then MilliCPUToShares will return the minimal number // of CPU shares. cpuShares = int64(cm.MilliCPUToShares(cpuRequest.MilliValue())) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go index 6a494b20e50..68318b59515 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go @@ -260,6 +260,29 @@ func TestCalculateLinuxResources(t *testing.T) { MemoryLimitInBytes: 0, }, }, + { + name: "RequestNilCPU", + cpuLim: resource.MustParse("2"), + memLim: resource.MustParse("0"), + expected: &runtimeapi.LinuxContainerResources{ + CpuPeriod: 100000, + CpuQuota: 200000, + CpuShares: 2048, + MemoryLimitInBytes: 0, + }, + }, + { + name: "RequestZeroCPU", + cpuReq: resource.MustParse("0"), + cpuLim: resource.MustParse("2"), + memLim: resource.MustParse("0"), + expected: &runtimeapi.LinuxContainerResources{ + CpuPeriod: 100000, + CpuQuota: 200000, + CpuShares: 2, + MemoryLimitInBytes: 0, + }, + }, } for _, test := range tests { linuxContainerResources := m.calculateLinuxResources(&test.cpuReq, &test.cpuLim, &test.memLim) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_sandbox_linux.go b/pkg/kubelet/kuberuntime/kuberuntime_sandbox_linux.go index 646ee7f23cc..c2c6b1aaee3 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_sandbox_linux.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_sandbox_linux.go @@ -21,6 +21,7 @@ package kuberuntime import ( v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1" resourcehelper "k8s.io/kubernetes/pkg/api/v1/resource" ) @@ -41,7 +42,11 @@ func (m *kubeGenericRuntimeManager) convertOverheadToLinuxResources(pod *v1.Pod) func (m *kubeGenericRuntimeManager) calculateSandboxResources(pod *v1.Pod) *runtimeapi.LinuxContainerResources { req, lim := resourcehelper.PodRequestsAndLimitsWithoutOverhead(pod) - return m.calculateLinuxResources(req.Cpu(), lim.Cpu(), lim.Memory()) + var cpuRequest *resource.Quantity + if _, cpuRequestExists := req[v1.ResourceCPU]; cpuRequestExists { + cpuRequest = req.Cpu() + } + return m.calculateLinuxResources(cpuRequest, lim.Cpu(), lim.Memory()) } func (m *kubeGenericRuntimeManager) applySandboxResources(pod *v1.Pod, config *runtimeapi.PodSandboxConfig) error { From 0b6d27002fd3805da2885287bebab292991e09b7 Mon Sep 17 00:00:00 2001 From: waynepeking348 Date: Mon, 21 Mar 2022 21:45:49 +0800 Subject: [PATCH 2/3] fix test cases for cpu shares --- .../kuberuntime_container_linux.go | 4 +-- .../kuberuntime_container_linux_test.go | 35 +++++++++++-------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go b/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go index 1e1d59daade..ad6b378cccf 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go @@ -142,9 +142,9 @@ func (m *kubeGenericRuntimeManager) calculateLinuxResources(cpuRequest, cpuLimit // If request is not specified, but limit is, we want request to default to limit. // API server does this for new containers, but we repeat this logic in Kubelet // for containers running on existing Kubernetes clusters. - if cpuRequest == nil && !cpuLimit.IsZero() { + if cpuRequest == nil { cpuShares = int64(cm.MilliCPUToShares(cpuLimit.MilliValue())) - } else if cpuRequest != nil { + } else { // if cpuRequest.Amount is nil, then MilliCPUToShares will return the minimal number // of CPU shares. cpuShares = int64(cm.MilliCPUToShares(cpuRequest.MilliValue())) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go index 68318b59515..1b4b3b0e34c 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go @@ -229,18 +229,23 @@ func TestCalculateLinuxResources(t *testing.T) { assert.NoError(t, err) + generateResourceQuantity := func(str string) *resource.Quantity { + quantity := resource.MustParse(str) + return &quantity + } + tests := []struct { name string - cpuReq resource.Quantity - cpuLim resource.Quantity - memLim resource.Quantity + cpuReq *resource.Quantity + cpuLim *resource.Quantity + memLim *resource.Quantity expected *runtimeapi.LinuxContainerResources }{ { name: "Request128MBLimit256MB", - cpuReq: resource.MustParse("1"), - cpuLim: resource.MustParse("2"), - memLim: resource.MustParse("128Mi"), + cpuReq: generateResourceQuantity("1"), + cpuLim: generateResourceQuantity("2"), + memLim: generateResourceQuantity("128Mi"), expected: &runtimeapi.LinuxContainerResources{ CpuPeriod: 100000, CpuQuota: 200000, @@ -250,9 +255,9 @@ func TestCalculateLinuxResources(t *testing.T) { }, { name: "RequestNoMemory", - cpuReq: resource.MustParse("2"), - cpuLim: resource.MustParse("8"), - memLim: resource.MustParse("0"), + cpuReq: generateResourceQuantity("2"), + cpuLim: generateResourceQuantity("8"), + memLim: generateResourceQuantity("0"), expected: &runtimeapi.LinuxContainerResources{ CpuPeriod: 100000, CpuQuota: 800000, @@ -262,8 +267,8 @@ func TestCalculateLinuxResources(t *testing.T) { }, { name: "RequestNilCPU", - cpuLim: resource.MustParse("2"), - memLim: resource.MustParse("0"), + cpuLim: generateResourceQuantity("2"), + memLim: generateResourceQuantity("0"), expected: &runtimeapi.LinuxContainerResources{ CpuPeriod: 100000, CpuQuota: 200000, @@ -273,9 +278,9 @@ func TestCalculateLinuxResources(t *testing.T) { }, { name: "RequestZeroCPU", - cpuReq: resource.MustParse("0"), - cpuLim: resource.MustParse("2"), - memLim: resource.MustParse("0"), + cpuReq: generateResourceQuantity("0"), + cpuLim: generateResourceQuantity("2"), + memLim: generateResourceQuantity("0"), expected: &runtimeapi.LinuxContainerResources{ CpuPeriod: 100000, CpuQuota: 200000, @@ -285,7 +290,7 @@ func TestCalculateLinuxResources(t *testing.T) { }, } for _, test := range tests { - linuxContainerResources := m.calculateLinuxResources(&test.cpuReq, &test.cpuLim, &test.memLim) + linuxContainerResources := m.calculateLinuxResources(test.cpuReq, test.cpuLim, test.memLim) assert.Equal(t, test.expected, linuxContainerResources) } } From 51883193c537eb31d7a2f5e93e217a5061bd7415 Mon Sep 17 00:00:00 2001 From: waynepeking348 Date: Tue, 22 Mar 2022 10:08:59 +0800 Subject: [PATCH 3/3] keep original cpu limit logic to be in line with the comments --- pkg/kubelet/kuberuntime/kuberuntime_container_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go b/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go index ad6b378cccf..adeb7aa5b30 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go @@ -142,7 +142,7 @@ func (m *kubeGenericRuntimeManager) calculateLinuxResources(cpuRequest, cpuLimit // If request is not specified, but limit is, we want request to default to limit. // API server does this for new containers, but we repeat this logic in Kubelet // for containers running on existing Kubernetes clusters. - if cpuRequest == nil { + if cpuRequest == nil && cpuLimit != nil { cpuShares = int64(cm.MilliCPUToShares(cpuLimit.MilliValue())) } else { // if cpuRequest.Amount is nil, then MilliCPUToShares will return the minimal number