From 8342d39956f2e29c14749d502c8d5a68dd233bdf Mon Sep 17 00:00:00 2001 From: Tim Allclair Date: Tue, 12 Nov 2024 16:25:28 -0800 Subject: [PATCH 1/2] Equate CPU limits below the minimum effective limit (10m) --- pkg/kubelet/cm/helpers_linux.go | 4 + pkg/kubelet/cm/helpers_unsupported.go | 7 +- pkg/kubelet/kubelet_pods.go | 11 +- pkg/kubelet/kubelet_pods_test.go | 70 +++++- pkg/kubelet/kubelet_test.go | 231 +++++++++++------- .../kuberuntime/kuberuntime_manager.go | 20 +- .../kuberuntime/kuberuntime_manager_test.go | 3 + 7 files changed, 232 insertions(+), 114 deletions(-) diff --git a/pkg/kubelet/cm/helpers_linux.go b/pkg/kubelet/cm/helpers_linux.go index 9ea190aba71..6e1ee829d29 100644 --- a/pkg/kubelet/cm/helpers_linux.go +++ b/pkg/kubelet/cm/helpers_linux.go @@ -50,6 +50,10 @@ const ( // defined here: // https://github.com/torvalds/linux/blob/cac03ac368fabff0122853de2422d4e17a32de08/kernel/sched/core.c#L10546 MinQuotaPeriod = 1000 + + // From the inverse of the conversion in MilliCPUToQuota: + // MinQuotaPeriod * MilliCPUToCPU / QuotaPeriod + MinMilliCPULimit = 10 ) // MilliCPUToQuota converts milliCPU to CFS quota and period values. diff --git a/pkg/kubelet/cm/helpers_unsupported.go b/pkg/kubelet/cm/helpers_unsupported.go index e739e3db408..34d4ee59d12 100644 --- a/pkg/kubelet/cm/helpers_unsupported.go +++ b/pkg/kubelet/cm/helpers_unsupported.go @@ -20,7 +20,7 @@ limitations under the License. package cm import ( - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" ) @@ -31,8 +31,9 @@ const ( SharesPerCPU = 0 MilliCPUToCPU = 0 - QuotaPeriod = 0 - MinQuotaPeriod = 0 + QuotaPeriod = 0 + MinQuotaPeriod = 0 + MinMilliCPULimit = 0 ) // MilliCPUToQuota converts milliCPU and period to CFS quota values. diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 5dcc3c2085d..8bc0b56044c 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -1806,7 +1806,9 @@ func allocatedResourcesMatchStatus(allocatedPod *v1.Pod, podStatus *kubecontaine if !cpuLim.IsZero() { return false } - } else if !cpuLim.Equal(*cs.Resources.CPULimit) { + } else if !cpuLim.Equal(*cs.Resources.CPULimit) && + (cpuLim.MilliValue() > cm.MinMilliCPULimit || cs.Resources.CPULimit.MilliValue() > cm.MinMilliCPULimit) { + // If both allocated & status CPU limits are at or below the minimum limit, then they are considered equal. return false } } @@ -2152,7 +2154,12 @@ func (kl *Kubelet) convertToAPIContainerStatuses(pod *v1.Pod, podStatus *kubecon resources := alloc if resources.Limits != nil { if cStatus.Resources != nil && cStatus.Resources.CPULimit != nil { - resources.Limits[v1.ResourceCPU] = cStatus.Resources.CPULimit.DeepCopy() + // If both the allocated & actual resources are at or below the minimum effective limit, preserve the + // allocated value in the API to avoid confusion and simplify comparisons. + if cStatus.Resources.CPULimit.MilliValue() > cm.MinMilliCPULimit || + resources.Limits.Cpu().MilliValue() > cm.MinMilliCPULimit { + resources.Limits[v1.ResourceCPU] = cStatus.Resources.CPULimit.DeepCopy() + } } else { preserveOldResourcesValue(v1.ResourceCPU, oldStatus.Resources.Limits, resources.Limits) } diff --git a/pkg/kubelet/kubelet_pods_test.go b/pkg/kubelet/kubelet_pods_test.go index 22e1a6ea61a..5bab616350e 100644 --- a/pkg/kubelet/kubelet_pods_test.go +++ b/pkg/kubelet/kubelet_pods_test.go @@ -4797,22 +4797,33 @@ func TestConvertToAPIContainerStatusesForResources(t *testing.T) { }, }, "BurstableQoSPod with below min CPU": { - Resources: v1.ResourceRequirements{Requests: v1.ResourceList{ - v1.ResourceMemory: resource.MustParse("100M"), - v1.ResourceCPU: resource.MustParse("1m"), - }}, + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("100M"), + v1.ResourceCPU: resource.MustParse("1m"), + }, + Limits: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("5m"), + }, + }, ActualResources: &kubecontainer.ContainerResources{ CPURequest: resource.NewMilliQuantity(2, resource.DecimalSI), + CPULimit: resource.NewMilliQuantity(10, resource.DecimalSI), }, OldStatus: v1.ContainerStatus{ Name: testContainerName, Image: "img", ImageID: "img1234", State: v1.ContainerState{Running: &v1.ContainerStateRunning{}}, - Resources: &v1.ResourceRequirements{Requests: v1.ResourceList{ - v1.ResourceMemory: resource.MustParse("100M"), - v1.ResourceCPU: resource.MustParse("1m"), - }}, + Resources: &v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("100M"), + v1.ResourceCPU: resource.MustParse("1m"), + }, + Limits: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("5m"), + }, + }, }, Expected: v1.ContainerStatus{ Name: testContainerName, @@ -4824,10 +4835,15 @@ func TestConvertToAPIContainerStatusesForResources(t *testing.T) { v1.ResourceMemory: resource.MustParse("100M"), v1.ResourceCPU: resource.MustParse("1m"), }, - Resources: &v1.ResourceRequirements{Requests: v1.ResourceList{ - v1.ResourceMemory: resource.MustParse("100M"), - v1.ResourceCPU: resource.MustParse("1m"), - }}, + Resources: &v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("100M"), + v1.ResourceCPU: resource.MustParse("1m"), + }, + Limits: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("5m"), + }, + }, }, }, "GuaranteedQoSPod with CPU and memory CRI status, with ephemeral storage": { @@ -6790,6 +6806,36 @@ func TestAllocatedResourcesMatchStatus(t *testing.T) { CPURequest: resource.NewMilliQuantity(2, resource.DecimalSI), }, expectMatch: true, + }, { + name: "burstable: min cpu limit", + allocatedResources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("10m"), + }, + Limits: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("10m"), + }, + }, + statusResources: &kubecontainer.ContainerResources{ + CPURequest: resource.NewMilliQuantity(10, resource.DecimalSI), + CPULimit: resource.NewMilliQuantity(10, resource.DecimalSI), + }, + expectMatch: true, + }, { + name: "burstable: below min cpu limit", + allocatedResources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("5m"), + }, + Limits: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("5m"), + }, + }, + statusResources: &kubecontainer.ContainerResources{ + CPURequest: resource.NewMilliQuantity(5, resource.DecimalSI), + CPULimit: resource.NewMilliQuantity(10, resource.DecimalSI), + }, + expectMatch: true, }, { name: "best effort", allocatedResources: v1.ResourceRequirements{}, diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index f4c88d53f2c..6d67fdee7d6 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -2672,123 +2672,172 @@ func TestHandlePodResourcesResize(t *testing.T) { defer kubelet.podManager.RemovePod(testPod1) tests := []struct { - name string - originalRequests v1.ResourceList - newRequests v1.ResourceList - newRequestsAllocated bool // Whether the new requests have already been allocated (but not actuated) - expectedAllocations v1.ResourceList - expectedResize v1.PodResizeStatus - expectBackoffReset bool - goos string + name string + originalRequests v1.ResourceList + newRequests v1.ResourceList + originalLimits v1.ResourceList + newLimits v1.ResourceList + newResourcesAllocated bool // Whether the new requests have already been allocated (but not actuated) + expectedAllocatedReqs v1.ResourceList + expectedAllocatedLims v1.ResourceList + expectedResize v1.PodResizeStatus + expectBackoffReset bool + goos string }{ { - name: "Request CPU and memory decrease - expect InProgress", - originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - newRequests: v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}, - expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}, - expectedResize: v1.PodResizeStatusInProgress, - expectBackoffReset: true, + name: "Request CPU and memory decrease - expect InProgress", + originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + newRequests: v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}, + expectedResize: v1.PodResizeStatusInProgress, + expectBackoffReset: true, }, { - name: "Request CPU increase, memory decrease - expect InProgress", - originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - newRequests: v1.ResourceList{v1.ResourceCPU: cpu1500m, v1.ResourceMemory: mem500M}, - expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu1500m, v1.ResourceMemory: mem500M}, - expectedResize: v1.PodResizeStatusInProgress, - expectBackoffReset: true, + name: "Request CPU increase, memory decrease - expect InProgress", + originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + newRequests: v1.ResourceList{v1.ResourceCPU: cpu1500m, v1.ResourceMemory: mem500M}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: cpu1500m, v1.ResourceMemory: mem500M}, + expectedResize: v1.PodResizeStatusInProgress, + expectBackoffReset: true, }, { - name: "Request CPU decrease, memory increase - expect InProgress", - originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - newRequests: v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem1500M}, - expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem1500M}, - expectedResize: v1.PodResizeStatusInProgress, - expectBackoffReset: true, + name: "Request CPU decrease, memory increase - expect InProgress", + originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + newRequests: v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem1500M}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem1500M}, + expectedResize: v1.PodResizeStatusInProgress, + expectBackoffReset: true, }, { - name: "Request CPU and memory increase beyond current capacity - expect Deferred", - originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - newRequests: v1.ResourceList{v1.ResourceCPU: cpu2500m, v1.ResourceMemory: mem2500M}, - expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - expectedResize: v1.PodResizeStatusDeferred, + name: "Request CPU and memory increase beyond current capacity - expect Deferred", + originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + newRequests: v1.ResourceList{v1.ResourceCPU: cpu2500m, v1.ResourceMemory: mem2500M}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + expectedResize: v1.PodResizeStatusDeferred, }, { - name: "Request CPU decrease and memory increase beyond current capacity - expect Deferred", - originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - newRequests: v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem2500M}, - expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - expectedResize: v1.PodResizeStatusDeferred, + name: "Request CPU decrease and memory increase beyond current capacity - expect Deferred", + originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + newRequests: v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem2500M}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + expectedResize: v1.PodResizeStatusDeferred, }, { - name: "Request memory increase beyond node capacity - expect Infeasible", - originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - newRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem4500M}, - expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - expectedResize: v1.PodResizeStatusInfeasible, + name: "Request memory increase beyond node capacity - expect Infeasible", + originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + newRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem4500M}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + expectedResize: v1.PodResizeStatusInfeasible, }, { - name: "Request CPU increase beyond node capacity - expect Infeasible", - originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - newRequests: v1.ResourceList{v1.ResourceCPU: cpu5000m, v1.ResourceMemory: mem1000M}, - expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - expectedResize: v1.PodResizeStatusInfeasible, + name: "Request CPU increase beyond node capacity - expect Infeasible", + originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + newRequests: v1.ResourceList{v1.ResourceCPU: cpu5000m, v1.ResourceMemory: mem1000M}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + expectedResize: v1.PodResizeStatusInfeasible, }, { - name: "CPU increase in progress - expect InProgress", - originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - newRequests: v1.ResourceList{v1.ResourceCPU: cpu1500m, v1.ResourceMemory: mem1000M}, - newRequestsAllocated: true, - expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu1500m, v1.ResourceMemory: mem1000M}, - expectedResize: v1.PodResizeStatusInProgress, + name: "CPU increase in progress - expect InProgress", + originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + newRequests: v1.ResourceList{v1.ResourceCPU: cpu1500m, v1.ResourceMemory: mem1000M}, + newResourcesAllocated: true, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: cpu1500m, v1.ResourceMemory: mem1000M}, + expectedResize: v1.PodResizeStatusInProgress, }, { - name: "No resize", - originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - newRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - expectedResize: "", + name: "No resize", + originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + newRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + expectedResize: "", }, { - name: "windows node, expect Infeasible", - originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - newRequests: v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}, - expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, - expectedResize: v1.PodResizeStatusInfeasible, - goos: "windows", + name: "windows node, expect Infeasible", + originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + newRequests: v1.ResourceList{v1.ResourceCPU: cpu500m, v1.ResourceMemory: mem500M}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: cpu1000m, v1.ResourceMemory: mem1000M}, + expectedResize: v1.PodResizeStatusInfeasible, + goos: "windows", }, { - name: "Increase CPU from min shares", - originalRequests: v1.ResourceList{v1.ResourceCPU: cpu2m}, - newRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m}, - expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu1000m}, - expectedResize: v1.PodResizeStatusInProgress, - expectBackoffReset: true, + name: "Increase CPU from min shares", + originalRequests: v1.ResourceList{v1.ResourceCPU: cpu2m}, + newRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: cpu1000m}, + expectedResize: v1.PodResizeStatusInProgress, + expectBackoffReset: true, }, { - name: "Decrease CPU to min shares", - originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m}, - newRequests: v1.ResourceList{v1.ResourceCPU: cpu2m}, - expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu2m}, - expectedResize: v1.PodResizeStatusInProgress, - expectBackoffReset: true, + name: "Decrease CPU to min shares", + originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1000m}, + newRequests: v1.ResourceList{v1.ResourceCPU: cpu2m}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: cpu2m}, + expectedResize: v1.PodResizeStatusInProgress, + expectBackoffReset: true, }, { - name: "Equivalent min CPU shares", - originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1m}, - newRequests: v1.ResourceList{v1.ResourceCPU: cpu2m}, - expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu2m}, - expectedResize: "", + name: "Equivalent min CPU shares", + originalRequests: v1.ResourceList{v1.ResourceCPU: cpu1m}, + newRequests: v1.ResourceList{v1.ResourceCPU: cpu2m}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: cpu2m}, + expectedResize: "", // Even though the resize isn't being actuated, we still clear the container backoff // since the allocation is changing. expectBackoffReset: true, }, { - name: "Equivalent min CPU shares - already allocated", - originalRequests: v1.ResourceList{v1.ResourceCPU: cpu2m}, - newRequests: v1.ResourceList{v1.ResourceCPU: cpu1m}, - newRequestsAllocated: true, - expectedAllocations: v1.ResourceList{v1.ResourceCPU: cpu1m}, - expectedResize: "", + name: "Equivalent min CPU shares - already allocated", + originalRequests: v1.ResourceList{v1.ResourceCPU: cpu2m}, + newRequests: v1.ResourceList{v1.ResourceCPU: cpu1m}, + newResourcesAllocated: true, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: cpu1m}, + expectedResize: "", + }, + { + name: "Increase CPU from min limit", + originalRequests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("10m")}, + originalLimits: v1.ResourceList{v1.ResourceCPU: resource.MustParse("10m")}, + newRequests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("10m")}, // Unchanged + newLimits: v1.ResourceList{v1.ResourceCPU: resource.MustParse("20m")}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: resource.MustParse("10m")}, + expectedAllocatedLims: v1.ResourceList{v1.ResourceCPU: resource.MustParse("20m")}, + expectedResize: v1.PodResizeStatusInProgress, + expectBackoffReset: true, + }, + { + name: "Decrease CPU to min limit", + originalRequests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("10m")}, + originalLimits: v1.ResourceList{v1.ResourceCPU: resource.MustParse("20m")}, + newRequests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("10m")}, // Unchanged + newLimits: v1.ResourceList{v1.ResourceCPU: resource.MustParse("10m")}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: resource.MustParse("10m")}, + expectedAllocatedLims: v1.ResourceList{v1.ResourceCPU: resource.MustParse("10m")}, + expectedResize: v1.PodResizeStatusInProgress, + expectBackoffReset: true, + }, + { + name: "Equivalent min CPU limit", + originalRequests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("5m")}, + originalLimits: v1.ResourceList{v1.ResourceCPU: resource.MustParse("5m")}, + newRequests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("5m")}, // Unchanged + newLimits: v1.ResourceList{v1.ResourceCPU: resource.MustParse("10m")}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: resource.MustParse("5m")}, + expectedAllocatedLims: v1.ResourceList{v1.ResourceCPU: resource.MustParse("10m")}, + expectedResize: "", + // Even though the resize isn't being actuated, we still clear the container backoff + // since the allocation is changing. + expectBackoffReset: true, + }, + { + name: "Equivalent min CPU limit - already allocated", + originalRequests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("5m")}, + originalLimits: v1.ResourceList{v1.ResourceCPU: resource.MustParse("10m")}, + newRequests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("5m")}, // Unchanged + newLimits: v1.ResourceList{v1.ResourceCPU: resource.MustParse("5m")}, + expectedAllocatedReqs: v1.ResourceList{v1.ResourceCPU: resource.MustParse("5m")}, + expectedAllocatedLims: v1.ResourceList{v1.ResourceCPU: resource.MustParse("5m")}, + newResourcesAllocated: true, + expectedResize: "", }, } @@ -2803,12 +2852,14 @@ func TestHandlePodResourcesResize(t *testing.T) { originalPod := testPod1.DeepCopy() originalPod.Spec.Containers[0].Resources.Requests = tt.originalRequests + originalPod.Spec.Containers[0].Resources.Limits = tt.originalLimits kubelet.podManager.UpdatePod(originalPod) newPod := originalPod.DeepCopy() newPod.Spec.Containers[0].Resources.Requests = tt.newRequests + newPod.Spec.Containers[0].Resources.Limits = tt.newLimits - if !tt.newRequestsAllocated { + if !tt.newResourcesAllocated { require.NoError(t, kubelet.statusManager.SetPodAllocation(originalPod)) } else { require.NoError(t, kubelet.statusManager.SetPodAllocation(newPod)) @@ -2839,11 +2890,13 @@ func TestHandlePodResourcesResize(t *testing.T) { updatedPod, err := kubelet.handlePodResourcesResize(newPod, podStatus) require.NoError(t, err) - assert.Equal(t, tt.expectedAllocations, updatedPod.Spec.Containers[0].Resources.Requests, "updated pod spec resources") + assert.Equal(t, tt.expectedAllocatedReqs, updatedPod.Spec.Containers[0].Resources.Requests, "updated pod spec requests") + assert.Equal(t, tt.expectedAllocatedLims, updatedPod.Spec.Containers[0].Resources.Limits, "updated pod spec limits") alloc, found := kubelet.statusManager.GetContainerResourceAllocation(string(newPod.UID), newPod.Spec.Containers[0].Name) require.True(t, found, "container allocation") - assert.Equal(t, tt.expectedAllocations, alloc.Requests, "stored container allocation") + assert.Equal(t, tt.expectedAllocatedReqs, alloc.Requests, "stored container request allocation") + assert.Equal(t, tt.expectedAllocatedLims, alloc.Limits, "stored container limit allocation") resizeStatus := kubelet.statusManager.GetPodResizeStatus(newPod.UID) assert.Equal(t, tt.expectedResize, resizeStatus) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager.go b/pkg/kubelet/kuberuntime/kuberuntime_manager.go index 55b1fb04166..eba65767a3a 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager.go @@ -606,7 +606,12 @@ func (m *kubeGenericRuntimeManager) computePodResizeAction(pod *v1.Pod, containe // then consider these equal. desiredResources.cpuRequest = currentResources.cpuRequest } - + // Special case for minimum CPU limit + if desiredResources.cpuLimit <= cm.MinMilliCPULimit && currentResources.cpuLimit <= cm.MinMilliCPULimit { + // If both desired & current CPU limit are at or below the minimum effective limit, + // then consider these equal. + desiredResources.cpuLimit = currentResources.cpuLimit + } if currentResources == desiredResources { // No resize required. return true @@ -848,14 +853,13 @@ func (m *kubeGenericRuntimeManager) updatePodContainerResources(pod *v1.Pod, res actualRequest := nonNilQuantity(status.Resources.CPURequest) desiredLimit := container.Resources.Limits.Cpu() desiredRequest := container.Resources.Requests.Cpu() - if !actualLimit.Equal(*desiredLimit) { - return false // limits don't match - } else if actualRequest.Equal(*desiredRequest) { - return true // requests & limits both match - } + // Consider limits equal if both are at or below the effective minimum limit. + equalLimits := actualLimit.Equal(*desiredLimit) || (actualLimit.MilliValue() <= cm.MinMilliCPULimit && + desiredLimit.MilliValue() <= cm.MinMilliCPULimit) // Consider requests equal if both are at or below MinShares. - return actualRequest.MilliValue() <= cm.MinShares && - desiredRequest.MilliValue() <= cm.MinShares + equalRequests := actualRequest.Equal(*desiredRequest) || (actualRequest.MilliValue() <= cm.MinShares && + desiredRequest.MilliValue() <= cm.MinShares) + return equalLimits && equalRequests default: return true // Shouldn't happen. } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go index 264fd8505b2..97dbcaeaa0f 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go @@ -2215,6 +2215,7 @@ func TestComputePodActionsForPodResize(t *testing.T) { cpu1m := resource.MustParse("1m") cpu2m := resource.MustParse("2m") + cpu10m := resource.MustParse("10m") cpu100m := resource.MustParse("100m") cpu200m := resource.MustParse("200m") mem100M := resource.MustParse("100Mi") @@ -2406,10 +2407,12 @@ func TestComputePodActionsForPodResize(t *testing.T) { c := &pod.Spec.Containers[1] c.Resources = v1.ResourceRequirements{ Requests: v1.ResourceList{v1.ResourceCPU: cpu1m}, + Limits: v1.ResourceList{v1.ResourceCPU: cpu1m}, } if cStatus := status.FindContainerStatusByName(c.Name); cStatus != nil { cStatus.Resources = &kubecontainer.ContainerResources{ CPURequest: ptr.To(cpu2m.DeepCopy()), + CPULimit: ptr.To(cpu10m.DeepCopy()), } } }, From 18600f43e0115690826c7984347a4c2d18a9c8ff Mon Sep 17 00:00:00 2001 From: Tim Allclair Date: Tue, 12 Nov 2024 17:23:36 -0800 Subject: [PATCH 2/2] Min cpu limit resize e2e test --- test/e2e/common/node/pod_resize.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/e2e/common/node/pod_resize.go b/test/e2e/common/node/pod_resize.go index 3d185fe6674..ec8d8e923e7 100644 --- a/test/e2e/common/node/pod_resize.go +++ b/test/e2e/common/node/pod_resize.go @@ -584,20 +584,20 @@ func doPodResizeTests(f *framework.Framework) { }, }, { - name: "Burstable QoS pod, one container with cpu requests - resize with equivalent request", + name: "Burstable QoS pod, one container with cpu requests and limits - resize with equivalents", containers: []e2epod.ResizableContainerInfo{ { Name: "c1", - Resources: &e2epod.ContainerResources{CPUReq: "2m"}, + Resources: &e2epod.ContainerResources{CPUReq: "2m", CPULim: "10m"}, }, }, patchString: `{"spec":{"containers":[ - {"name":"c1", "resources":{"requests":{"cpu":"1m"}}} + {"name":"c1", "resources":{"requests":{"cpu":"1m"},"limits":{"cpu":"5m"}}} ]}}`, expected: []e2epod.ResizableContainerInfo{ { Name: "c1", - Resources: &e2epod.ContainerResources{CPUReq: "1m"}, + Resources: &e2epod.ContainerResources{CPUReq: "1m", CPULim: "5m"}, }, }, },