fixup

Signed-off-by: Anatolii Bazko <[email protected]>

Author: tolusha

pkg/library/resources/helper.go

@@ -273,19 +273,26 @@ func ApplyCaps(resources, caps *corev1.ResourceRequirements) *corev1.ResourceReq
 		capMemRequest := caps.Requests[corev1.ResourceMemory]
 		switch {
 		case !capMemLimit.IsZero() && capMemRequest.IsZero():
-			// Cap limit caused the issue, adjust request down to match limit
-			result.Requests[corev1.ResourceMemory] = capMemLimit
-		case capMemLimit.IsZero() && !capMemRequest.IsZero():
-			// Cap request as maximum shouldn't cause limit < request, but adjust request down to match limit
-			result.Requests[corev1.ResourceMemory] = memLimit
-		default:
-			// Both caps or neither caps - use caps limit for both to ensure validity
-			if !capMemLimit.IsZero() {
-				result.Limits[corev1.ResourceMemory] = capMemLimit
+			// Only a memory limit cap was set, and it caused the limit to be lower than the request.
+			// Adjust the request down to match the capped limit.
+			if memLimit.Equal(capMemLimit) {
 				result.Requests[corev1.ResourceMemory] = capMemLimit
 			} else {
-				result.Requests[corev1.ResourceMemory] = memLimit
+				// The invalid state (limit < request) existed in the original resources before caps were applied.
 			}
+		case capMemLimit.IsZero() && !capMemRequest.IsZero():
+			// Only a memory request cap was set, and it's higher than the existing limit.
+			// Adjust the limit up to match the capped request.
+			if memRequest.Equal(capMemRequest) {
+				result.Limits[corev1.ResourceMemory] = capMemRequest
+			} else {
+				// The invalid state (limit < request) existed in the original resources before caps were applied.
+				break
+			}
+		default:
+			// Both limit and request caps were set (or neither was set), so the invalid state was present
+			// in the original resources.
+			break
 		}
 	}
 
@@ -296,19 +303,27 @@ func ApplyCaps(resources, caps *corev1.ResourceRequirements) *corev1.ResourceReq
 		capCPURequest := caps.Requests[corev1.ResourceCPU]
 		switch {
 		case !capCPULimit.IsZero() && capCPURequest.IsZero():
-			// Cap limit caused the issue, adjust request down to match limit
-			result.Requests[corev1.ResourceCPU] = capCPULimit
-		case capCPULimit.IsZero() && !capCPURequest.IsZero():
-			// Cap request as maximum shouldn't cause limit < request, but adjust request down to match limit
-			result.Requests[corev1.ResourceCPU] = cpuLimit
-		default:
-			// Both caps or neither caps - use caps limit for both to ensure validity
-			if !capCPULimit.IsZero() {
-				result.Limits[corev1.ResourceCPU] = capCPULimit
+			// Only a CPU limit cap was set, and it caused the limit to be lower than the request.
+			// Adjust the request down to match the capped limit.
+			if cpuLimit.Equal(capCPULimit) {
 				result.Requests[corev1.ResourceCPU] = capCPULimit
 			} else {
-				result.Requests[corev1.ResourceCPU] = cpuLimit
+				// The invalid state (limit < request) existed in the original resources before caps were applied.
+				break
 			}
+		case capCPULimit.IsZero() && !capCPURequest.IsZero():
+			// Only a CPU request cap was set, and it's higher than the existing limit.
+			// Adjust the limit up to match the capped request.
+			if cpuRequest.Equal(capCPURequest) {
+				result.Limits[corev1.ResourceCPU] = capCPURequest
+			} else {
+				// The invalid state (limit < request) existed in the original resources before caps were applied.
+				break
+			}
+		default:
+			// Both limit and request caps were set (or neither was set), so the invalid state was present
+			// in the original resources.
+			break
 		}
 	}
 

pkg/library/resources/helper_test.go

@@ -318,10 +318,10 @@ func TestApplyCaps(t *testing.T) {
 			expected: getResourceRequirements("2000Mi", "200Mi", "2000m", "200m"),
 		},
 		{
-			name:     "Caps maximum limits and maximum requests",
-			base:     getResourceRequirements("3000Mi", "50Mi", "3000m", "50m"),
+			name:     "Applies all caps values",
+			base:     getResourceRequirements("3000Mi", "500Mi", "3000m", "500m"),
 			caps:     getResourceRequirements("2000Mi", "200Mi", "2000m", "200m"),
-			expected: getResourceRequirements("2000Mi", "50Mi", "2000m", "50m"),
+			expected: getResourceRequirements("2000Mi", "200Mi", "2000m", "200m"),
 		},
 		{
 			name:     "Keeps existing values when within caps bounds",
@@ -332,7 +332,7 @@ func TestApplyCaps(t *testing.T) {
 		{
 			name:     "Does not apply empty caps fields",
 			base:     getResourceRequirements("1000Mi", "100Mi", "1000m", "100m"),
-			caps:     getResourceRequirements("2000Mi", "", "2000m", ""),
+			caps:     getResourceRequirements("", "", "", ""),
 			expected: getResourceRequirements("1000Mi", "100Mi", "1000m", "100m"),
 		},
 		{
@@ -348,46 +348,16 @@ func TestApplyCaps(t *testing.T) {
 			expected: getResourceRequirements("1000Mi", "100Mi", "1000m", "100m"),
 		},
 		{
-			name:     "Caps maximum limit (existing higher than caps)",
-			base:     getResourceRequirements("5000Mi", "100Mi", "5000m", "100m"),
-			caps:     getResourceRequirements("2000Mi", "", "2000m", ""),
-			expected: getResourceRequirements("2000Mi", "100Mi", "2000m", "100m"),
-		},
-		{
-			name:     "Caps maximum request (existing higher than caps)",
-			base:     getResourceRequirements("5000Mi", "500Mi", "5000m", "500m"),
-			caps:     getResourceRequirements("", "200Mi", "", "200m"),
-			expected: getResourceRequirements("5000Mi", "200Mi", "5000m", "200m"),
-		},
-		{
-			name:     "Adjusts memory request when caps limit creates conflict",
-			base:     getResourceRequirements("", "1000Mi", "", ""),
-			caps:     getResourceRequirements("500Mi", "", "", ""),
-			expected: getResourceRequirements("500Mi", "500Mi", "", ""),
-		},
-		{
-			name:     "Adjusts memory request when caps request creates conflict",
-			base:     getResourceRequirements("100Mi", "200Mi", "", ""),
-			caps:     getResourceRequirements("", "500Mi", "", ""),
-			expected: getResourceRequirements("100Mi", "100Mi", "", ""),
-		},
-		{
-			name:     "Adjusts CPU request when caps limit creates conflict",
-			base:     getResourceRequirements("", "", "", "1000m"),
-			caps:     getResourceRequirements("", "", "500m", ""),
-			expected: getResourceRequirements("", "", "500m", "500m"),
-		},
-		{
-			name:     "Adjusts CPU request when caps request creates conflict",
-			base:     getResourceRequirements("", "", "100m", "200m"),
-			caps:     getResourceRequirements("", "", "", "500m"),
-			expected: getResourceRequirements("", "", "100m", "100m"),
+			name:     "Adjusts request when caps limit creates conflict",
+			base:     getResourceRequirements("2000Mi", "1000Mi", "", "1000m"),
+			caps:     getResourceRequirements("500Mi", "", "500m", ""),
+			expected: getResourceRequirements("500Mi", "500Mi", "500m", "500m"),
 		},
 		{
-			name:     "Handles conflicting caps values (both limit and request capped, request > limit)",
-			base:     getResourceRequirements("100Mi", "1000Mi", "100m", "1000m"),
-			caps:     getResourceRequirements("500Mi", "1000Mi", "500m", "1000m"),
-			expected: getResourceRequirements("500Mi", "500Mi", "500m", "500m"),
+			name:     "Adjusts limit when caps request creates conflict",
+			base:     getResourceRequirements("500Mi", "", "500m", ""),
+			caps:     getResourceRequirements("", "1000Mi", "", "1000m"),
+			expected: getResourceRequirements("1000Mi", "1000Mi", "1000m", "1000m"),
 		},
 	}
 	for _, tt := range tests {