Equate CPU limits below the minimum effective limit (10m)

liggitt · Nov 13, 2024 · 8342d39 · 8342d39
1 parent 252e9cb
commit 8342d39
Show file tree

Hide file tree

Showing 7 changed files with 232 additions and 114 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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{},