Merge pull request #33392 from sjenning/min-reclaim-percent

Automatic merge from submit-queue

kubelet: eviction: allow minimum reclaim as percentage

Fixes #33354 

xref #32537

**Release note**:
```release-note
The kubelet --eviction-minimum-reclaim option can now take precentages as well as absolute values for resources quantities
```
@derekwaynecarr @vishh @mtaufen

pkg/kubelet/eviction/eviction_manager_test.go

@@ -613,7 +613,9 @@ func TestMinReclaim(t *testing.T) {
 				Value: ThresholdValue{
 					Quantity: quantityMustParse("1Gi"),
 				},
-				MinReclaim: quantityMustParse("500Mi"),
+				MinReclaim: &ThresholdValue{
+					Quantity: quantityMustParse("500Mi"),
+				},
 			},
 		},
 	}
@@ -788,7 +790,9 @@ func TestNodeReclaimFuncs(t *testing.T) {
 				Value: ThresholdValue{
 					Quantity: quantityMustParse("1Gi"),
 				},
-				MinReclaim: quantityMustParse("500Mi"),
+				MinReclaim: &ThresholdValue{
+					Quantity: quantityMustParse("500Mi"),
+				},
 			},
 		},
 	}

pkg/kubelet/eviction/helpers.go

@@ -256,21 +256,40 @@ func parseGracePeriods(expr string) (map[Signal]time.Duration, error) {
 }
 
 // parseMinimumReclaims parses the minimum reclaim statements
-func parseMinimumReclaims(expr string) (map[Signal]resource.Quantity, error) {
+func parseMinimumReclaims(expr string) (map[Signal]ThresholdValue, error) {
 	if len(expr) == 0 {
 		return nil, nil
 	}
-	results := map[Signal]resource.Quantity{}
+	results := map[Signal]ThresholdValue{}
 	statements := strings.Split(expr, ",")
 	for _, statement := range statements {
 		parts := strings.Split(statement, "=")
 		if len(parts) != 2 {
-			return nil, fmt.Errorf("invalid eviction minimum reclaim syntax: %v, expected <signal>=<quantity>", statement)
+			return nil, fmt.Errorf("invalid eviction minimum reclaim syntax: %v, expected <signal>=<value>", statement)
 		}
 		signal := Signal(parts[0])
 		if !validSignal(signal) {
 			return nil, fmt.Errorf(unsupportedEvictionSignal, signal)
 		}
+
+		quantityValue := parts[1]
+		if strings.HasSuffix(quantityValue, "%") {
+			percentage, err := parsePercentage(quantityValue)
+			if err != nil {
+				return nil, err
+			}
+			if percentage <= 0 {
+				return nil, fmt.Errorf("eviction percentage minimum reclaim %v must be positive: %s", signal, quantityValue)
+			}
+			// check against duplicate statements
+			if _, found := results[signal]; found {
+				return nil, fmt.Errorf("duplicate eviction minimum reclaim specified for %v", signal)
+			}
+			results[signal] = ThresholdValue{
+				Percentage: percentage,
+			}
+			continue
+		}
 		// check against duplicate statements
 		if _, found := results[signal]; found {
 			return nil, fmt.Errorf("duplicate eviction minimum reclaim specified for %v", signal)
@@ -282,7 +301,9 @@ func parseMinimumReclaims(expr string) (map[Signal]resource.Quantity, error) {
 		if err != nil {
 			return nil, err
 		}
-		results[signal] = quantity
+		results[signal] = ThresholdValue{
+			Quantity: &quantity,
+		}
 	}
 	return results, nil
 }
@@ -650,7 +671,7 @@ func thresholdsMet(thresholds []Threshold, observations signalObservations, enfo
 		quantity := getThresholdQuantity(threshold.Value, observed.capacity)
 		// if enforceMinReclaim is specified, we compare relative to value - minreclaim
 		if enforceMinReclaim && threshold.MinReclaim != nil {
-			quantity.Add(*threshold.MinReclaim)
+			quantity.Add(*getThresholdQuantity(*threshold.MinReclaim, observed.capacity))
 		}
 		thresholdResult := quantity.Cmp(*observed.available)
 		switch threshold.Operator {

pkg/kubelet/eviction/helpers_test.go

@@ -66,7 +66,9 @@ func TestParseThresholdConfig(t *testing.T) {
 					Value: ThresholdValue{
 						Quantity: quantityMustParse("150Mi"),
 					},
-					MinReclaim: quantityMustParse("0"),
+					MinReclaim: &ThresholdValue{
+						Quantity: quantityMustParse("0"),
+					},
 				},
 				{
 					Signal:   SignalMemoryAvailable,
@@ -75,7 +77,9 @@ func TestParseThresholdConfig(t *testing.T) {
 						Quantity: quantityMustParse("300Mi"),
 					},
 					GracePeriod: gracePeriod,
-					MinReclaim:  quantityMustParse("0"),
+					MinReclaim: &ThresholdValue{
+						Quantity: quantityMustParse("0"),
+					},
 				},
 			},
 		},
@@ -83,7 +87,7 @@ func TestParseThresholdConfig(t *testing.T) {
 			evictionHard:            "memory.available<10%",
 			evictionSoft:            "memory.available<30%",
 			evictionSoftGracePeriod: "memory.available=30s",
-			evictionMinReclaim:      "memory.available=0",
+			evictionMinReclaim:      "memory.available=5%",
 			expectErr:               false,
 			expectThresholds: []Threshold{
 				{
@@ -92,7 +96,9 @@ func TestParseThresholdConfig(t *testing.T) {
 					Value: ThresholdValue{
 						Percentage: 0.1,
 					},
-					MinReclaim: quantityMustParse("0"),
+					MinReclaim: &ThresholdValue{
+						Percentage: 0.05,
+					},
 				},
 				{
 					Signal:   SignalMemoryAvailable,
@@ -101,7 +107,9 @@ func TestParseThresholdConfig(t *testing.T) {
 						Percentage: 0.3,
 					},
 					GracePeriod: gracePeriod,
-					MinReclaim:  quantityMustParse("0"),
+					MinReclaim: &ThresholdValue{
+						Percentage: 0.05,
+					},
 				},
 			},
 		},
@@ -118,7 +126,9 @@ func TestParseThresholdConfig(t *testing.T) {
 					Value: ThresholdValue{
 						Quantity: quantityMustParse("150Mi"),
 					},
-					MinReclaim: quantityMustParse("2Gi"),
+					MinReclaim: &ThresholdValue{
+						Quantity: quantityMustParse("2Gi"),
+					},
 				},
 				{
 					Signal:   SignalNodeFsAvailable,
@@ -126,7 +136,9 @@ func TestParseThresholdConfig(t *testing.T) {
 					Value: ThresholdValue{
 						Quantity: quantityMustParse("100Mi"),
 					},
-					MinReclaim: quantityMustParse("1Gi"),
+					MinReclaim: &ThresholdValue{
+						Quantity: quantityMustParse("1Gi"),
+					},
 				},
 				{
 					Signal:   SignalImageFsAvailable,
@@ -135,7 +147,9 @@ func TestParseThresholdConfig(t *testing.T) {
 						Quantity: quantityMustParse("300Mi"),
 					},
 					GracePeriod: gracePeriod,
-					MinReclaim:  quantityMustParse("2Gi"),
+					MinReclaim: &ThresholdValue{
+						Quantity: quantityMustParse("2Gi"),
+					},
 				},
 				{
 					Signal:   SignalNodeFsAvailable,
@@ -144,7 +158,9 @@ func TestParseThresholdConfig(t *testing.T) {
 						Quantity: quantityMustParse("200Mi"),
 					},
 					GracePeriod: gracePeriod,
-					MinReclaim:  quantityMustParse("1Gi"),
+					MinReclaim: &ThresholdValue{
+						Quantity: quantityMustParse("1Gi"),
+					},
 				},
 			},
 		},
@@ -152,7 +168,7 @@ func TestParseThresholdConfig(t *testing.T) {
 			evictionHard:            "imagefs.available<15%,nodefs.available<10.5%",
 			evictionSoft:            "imagefs.available<30%,nodefs.available<20.5%",
 			evictionSoftGracePeriod: "imagefs.available=30s,nodefs.available=30s",
-			evictionMinReclaim:      "imagefs.available=2Gi,nodefs.available=1Gi",
+			evictionMinReclaim:      "imagefs.available=10%,nodefs.available=5%",
 			expectErr:               false,
 			expectThresholds: []Threshold{
 				{
@@ -161,7 +177,9 @@ func TestParseThresholdConfig(t *testing.T) {
 					Value: ThresholdValue{
 						Percentage: 0.15,
 					},
-					MinReclaim: quantityMustParse("2Gi"),
+					MinReclaim: &ThresholdValue{
+						Percentage: 0.1,
+					},
 				},
 				{
 					Signal:   SignalNodeFsAvailable,
@@ -169,7 +187,9 @@ func TestParseThresholdConfig(t *testing.T) {
 					Value: ThresholdValue{
 						Percentage: 0.105,
 					},
-					MinReclaim: quantityMustParse("1Gi"),
+					MinReclaim: &ThresholdValue{
+						Percentage: 0.05,
+					},
 				},
 				{
 					Signal:   SignalImageFsAvailable,
@@ -178,7 +198,9 @@ func TestParseThresholdConfig(t *testing.T) {
 						Percentage: 0.3,
 					},
 					GracePeriod: gracePeriod,
-					MinReclaim:  quantityMustParse("2Gi"),
+					MinReclaim: &ThresholdValue{
+						Percentage: 0.1,
+					},
 				},
 				{
 					Signal:   SignalNodeFsAvailable,
@@ -187,7 +209,9 @@ func TestParseThresholdConfig(t *testing.T) {
 						Percentage: 0.205,
 					},
 					GracePeriod: gracePeriod,
-					MinReclaim:  quantityMustParse("1Gi"),
+					MinReclaim: &ThresholdValue{
+						Percentage: 0.05,
+					},
 				},
 			},
 		},
@@ -204,7 +228,9 @@ func TestParseThresholdConfig(t *testing.T) {
 					Value: ThresholdValue{
 						Quantity: quantityMustParse("150Mi"),
 					},
-					MinReclaim: quantityMustParse("2Gi"),
+					MinReclaim: &ThresholdValue{
+						Quantity: quantityMustParse("2Gi"),
+					},
 				},
 				{
 					Signal:   SignalNodeFsInodesFree,
@@ -212,7 +238,9 @@ func TestParseThresholdConfig(t *testing.T) {
 					Value: ThresholdValue{
 						Quantity: quantityMustParse("100Mi"),
 					},
-					MinReclaim: quantityMustParse("1Gi"),
+					MinReclaim: &ThresholdValue{
+						Quantity: quantityMustParse("1Gi"),
+					},
 				},
 				{
 					Signal:   SignalImageFsInodesFree,
@@ -221,7 +249,9 @@ func TestParseThresholdConfig(t *testing.T) {
 						Quantity: quantityMustParse("300Mi"),
 					},
 					GracePeriod: gracePeriod,
-					MinReclaim:  quantityMustParse("2Gi"),
+					MinReclaim: &ThresholdValue{
+						Quantity: quantityMustParse("2Gi"),
+					},
 				},
 				{
 					Signal:   SignalNodeFsInodesFree,
@@ -230,7 +260,9 @@ func TestParseThresholdConfig(t *testing.T) {
 						Quantity: quantityMustParse("200Mi"),
 					},
 					GracePeriod: gracePeriod,
-					MinReclaim:  quantityMustParse("1Gi"),
+					MinReclaim: &ThresholdValue{
+						Quantity: quantityMustParse("1Gi"),
+					},
 				},
 			},
 		},
@@ -359,7 +391,7 @@ func thresholdEqual(a Threshold, b Threshold) bool {
 	return a.GracePeriod == b.GracePeriod &&
 		a.Operator == b.Operator &&
 		a.Signal == b.Signal &&
-		a.MinReclaim.Cmp(*b.MinReclaim) == 0 &&
+		compareThresholdValue(*a.MinReclaim, *b.MinReclaim) &&
 		compareThresholdValue(a.Value, b.Value)
 }
 
@@ -766,7 +798,9 @@ func TestThresholdsMet(t *testing.T) {
 		Value: ThresholdValue{
 			Quantity: quantityMustParse("1Gi"),
 		},
-		MinReclaim: quantityMustParse("500Mi"),
+		MinReclaim: &ThresholdValue{
+			Quantity: quantityMustParse("500Mi"),
+		},
 	}
 	testCases := map[string]struct {
 		enforceMinReclaim bool
@@ -830,13 +864,16 @@ func TestThresholdsMet(t *testing.T) {
 }
 
 func TestPercentageThresholdsMet(t *testing.T) {
-	specifiecThresholds := []Threshold{
+	specificThresholds := []Threshold{
 		{
 			Signal:   SignalMemoryAvailable,
 			Operator: OpLessThan,
 			Value: ThresholdValue{
 				Percentage: 0.2,
 			},
+			MinReclaim: &ThresholdValue{
+				Percentage: 0.05,
+			},
 		},
 		{
 			Signal:   SignalNodeFsAvailable,
@@ -848,12 +885,14 @@ func TestPercentageThresholdsMet(t *testing.T) {
 	}
 
 	testCases := map[string]struct {
-		thresholds   []Threshold
-		observations signalObservations
-		result       []Threshold
+		enforceMinRelaim bool
+		thresholds       []Threshold
+		observations     signalObservations
+		result           []Threshold
 	}{
 		"BothMet": {
-			thresholds: specifiecThresholds,
+			enforceMinRelaim: false,
+			thresholds:       specificThresholds,
 			observations: signalObservations{
 				SignalMemoryAvailable: signalObservation{
 					available: quantityMustParse("100Mi"),
@@ -864,10 +903,11 @@ func TestPercentageThresholdsMet(t *testing.T) {
 					capacity:  quantityMustParse("1000Gi"),
 				},
 			},
-			result: specifiecThresholds,
+			result: specificThresholds,
 		},
 		"NoneMet": {
-			thresholds: specifiecThresholds,
+			enforceMinRelaim: false,
+			thresholds:       specificThresholds,
 			observations: signalObservations{
 				SignalMemoryAvailable: signalObservation{
 					available: quantityMustParse("300Mi"),
@@ -881,7 +921,8 @@ func TestPercentageThresholdsMet(t *testing.T) {
 			result: []Threshold{},
 		},
 		"DiskMet": {
-			thresholds: specifiecThresholds,
+			enforceMinRelaim: false,
+			thresholds:       specificThresholds,
 			observations: signalObservations{
 				SignalMemoryAvailable: signalObservation{
 					available: quantityMustParse("300Mi"),
@@ -892,10 +933,11 @@ func TestPercentageThresholdsMet(t *testing.T) {
 					capacity:  quantityMustParse("1000Gi"),
 				},
 			},
-			result: []Threshold{specifiecThresholds[1]},
+			result: []Threshold{specificThresholds[1]},
 		},
 		"MemoryMet": {
-			thresholds: specifiecThresholds,
+			enforceMinRelaim: false,
+			thresholds:       specificThresholds,
 			observations: signalObservations{
 				SignalMemoryAvailable: signalObservation{
 					available: quantityMustParse("100Mi"),
@@ -906,11 +948,33 @@ func TestPercentageThresholdsMet(t *testing.T) {
 					capacity:  quantityMustParse("1000Gi"),
 				},
 			},
-			result: []Threshold{specifiecThresholds[0]},
+			result: []Threshold{specificThresholds[0]},
+		},
+		"MemoryMetWithMinReclaim": {
+			enforceMinRelaim: true,
+			thresholds:       specificThresholds,
+			observations: signalObservations{
+				SignalMemoryAvailable: signalObservation{
+					available: quantityMustParse("225Mi"),
+					capacity:  quantityMustParse("1000Mi"),
+				},
+			},
+			result: []Threshold{specificThresholds[0]},
+		},
+		"MemoryNotMetWithMinReclaim": {
+			enforceMinRelaim: true,
+			thresholds:       specificThresholds,
+			observations: signalObservations{
+				SignalMemoryAvailable: signalObservation{
+					available: quantityMustParse("300Mi"),
+					capacity:  quantityMustParse("1000Mi"),
+				},
+			},
+			result: []Threshold{},
 		},
 	}
 	for testName, testCase := range testCases {
-		actual := thresholdsMet(testCase.thresholds, testCase.observations, false)
+		actual := thresholdsMet(testCase.thresholds, testCase.observations, testCase.enforceMinRelaim)
 		if !thresholdList(actual).Equal(thresholdList(testCase.result)) {
 			t.Errorf("Test case: %s, expected: %v, actual: %v", testName, testCase.result, actual)
 		}

pkg/kubelet/eviction/types.go

@@ -91,7 +91,7 @@ type Threshold struct {
 	// GracePeriod represents the amount of time that a threshold must be met before eviction is triggered.
 	GracePeriod time.Duration
 	// MinReclaim represents the minimum amount of resource to reclaim if the threshold is met.
-	MinReclaim *resource.Quantity
+	MinReclaim *ThresholdValue
 }
 
 // Manager evaluates when an eviction threshold for node stability has been met on the node.