Merge pull request #5715 from kisieland/revert-estimator-expansion

Revert "Add new method 'ReachedLimit' to EstimationLimiter"

cluster-autoscaler/estimator/estimator.go

@@ -72,9 +72,6 @@ type EstimationLimiter interface {
 	// There is no requirement for the Estimator to stop calculations, it's
 	// just not expected to add any more nodes.
 	PermissionToAddNode() bool
-	// ReachedLimit returns true if the limiter blocked addition of the new node.
-	// Otherwise returns false.
-	ReachedLimit() bool
 }
 
 // EstimationPodOrderer is an interface used to determine the order of the pods

cluster-autoscaler/estimator/threshold_based_limiter.go

@@ -25,17 +25,15 @@ import (
 )
 
 type thresholdBasedEstimationLimiter struct {
-	maxDuration  time.Duration
+	maxDuration time.Duration
-	maxNodes     int
+	maxNodes    int
-	nodes        int
+	nodes       int
-	start        time.Time
+	start       time.Time
-	reachedLimit bool
 }
 
 func (tbel *thresholdBasedEstimationLimiter) StartEstimation([]*apiv1.Pod, cloudprovider.NodeGroup) {
 	tbel.start = time.Now()
 	tbel.nodes = 0
-	tbel.reachedLimit = false
 }
 
 func (*thresholdBasedEstimationLimiter) EndEstimation() {}
@@ -43,23 +41,17 @@ func (*thresholdBasedEstimationLimiter) EndEstimation() {}
 func (tbel *thresholdBasedEstimationLimiter) PermissionToAddNode() bool {
 	if tbel.maxNodes > 0 && tbel.nodes >= tbel.maxNodes {
 		klog.V(4).Infof("Capping binpacking after exceeding threshold of %d nodes", tbel.maxNodes)
-		tbel.reachedLimit = true
 		return false
 	}
 	timeDefined := tbel.maxDuration > 0 && tbel.start != time.Time{}
 	if timeDefined && time.Now().After(tbel.start.Add(tbel.maxDuration)) {
 		klog.V(4).Infof("Capping binpacking after exceeding max duration of %v", tbel.maxDuration)
-		tbel.reachedLimit = true
 		return false
 	}
 	tbel.nodes++
 	return true
 }
 
-func (tbel *thresholdBasedEstimationLimiter) ReachedLimit() bool {
-	return tbel.reachedLimit
-}
-
 // NewThresholdBasedEstimationLimiter returns an EstimationLimiter that will prevent estimation
 // after either a node count- of time-based threshold is reached. This is meant to prevent cases
 // where binpacking of hundreds or thousands of nodes takes extremely long time rendering CA

cluster-autoscaler/estimator/threshold_based_limiter_test.go

@@ -42,13 +42,12 @@ func resetLimiter(t *testing.T, l EstimationLimiter) {
 
 func TestThresholdBasedLimiter(t *testing.T) {
 	testCases := []struct {
-		name                 string
+		name            string
-		maxNodes             int
+		maxNodes        int
-		maxDuration          time.Duration
+		maxDuration     time.Duration
-		startDelta           time.Duration
+		startDelta      time.Duration
-		operations           []limiterOperation
+		operations      []limiterOperation
-		expectNodeCount      int
+		expectNodeCount int
-		expectedReachedLimit bool
 	}{
 		{
 			name:     "no limiting happens",
@@ -69,8 +68,7 @@ func TestThresholdBasedLimiter(t *testing.T) {
 				expectDeny,
 				expectDeny,
 			},
-			expectNodeCount:      0,
+			expectNodeCount: 0,
-			expectedReachedLimit: true,
 		},
 		{
 			name:     "sequence of additions works until the threshold is hit",
@@ -81,8 +79,7 @@ func TestThresholdBasedLimiter(t *testing.T) {
 				expectAllow,
 				expectDeny,
 			},
-			expectNodeCount:      3,
+			expectNodeCount: 3,
-			expectedReachedLimit: true,
 		},
 		{
 			name:     "node counter is reset",
@@ -126,7 +123,6 @@ func TestThresholdBasedLimiter(t *testing.T) {
 				op(t, limiter)
 			}
 			assert.Equal(t, tc.expectNodeCount, limiter.nodes)
-			assert.Equal(t, tc.expectedReachedLimit, limiter.reachedLimit)
 			limiter.EndEstimation()
 		})
 	}