diff --git a/cluster-autoscaler/estimator/binpacking_estimator.go b/cluster-autoscaler/estimator/binpacking_estimator.go index 2f7760cbe0a0..86a26de22550 100644 --- a/cluster-autoscaler/estimator/binpacking_estimator.go +++ b/cluster-autoscaler/estimator/binpacking_estimator.go @@ -106,12 +106,6 @@ func (e *BinpackingNodeEstimator) Estimate( } if !found { - // Stop binpacking if we reach the limit of nodes we can add. - // We return the result of the binpacking that we already performed. - if !e.limiter.PermissionToAddNode() { - break - } - // If the last node we've added is empty and the pod couldn't schedule on it, it wouldn't be able to schedule // on a new node either. There is no point adding more nodes to snapshot in such case, especially because of // performance cost each extra node adds to future FitsAnyNodeMatching calls. @@ -119,6 +113,16 @@ func (e *BinpackingNodeEstimator) Estimate( continue } + // Stop binpacking if we reach the limit of nodes we can add. + // We return the result of the binpacking that we already performed. + // + // The thresholdBasedEstimationLimiter implementation assumes that for + // each call that returns true, one node gets added. Therefore this + // must be the last check right before really adding a node. + if !e.limiter.PermissionToAddNode() { + break + } + // Add new node newNodeName, err := e.addNewNodeToSnapshot(nodeTemplate, newNodeNameIndex) if err != nil {