From 2d24858ff975b36eb4ae831b041f11df352b3e2c Mon Sep 17 00:00:00 2001 From: Enxebre Date: Mon, 4 May 2020 12:21:29 +0200 Subject: [PATCH] UPSTREAM: : Compare against minSize in deleteNodes() When calling deleteNodes() we should fail early if the operation could delete nodes below the nodeGroup minSize(). This is one in a series of PR to mitigate kubernetes#3104 Once we got all merge we'll put a PR upstream. --- .../clusterapi/clusterapi_nodegroup.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go index 8d0cf6d8c529..ff0a93641a00 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go @@ -91,6 +91,13 @@ func (ng *nodegroup) IncreaseSize(delta int) error { // group. This function should wait until node group size is updated. // Implementation required. func (ng *nodegroup) DeleteNodes(nodes []*corev1.Node) error { + replicas := ng.scalableResource.Replicas() + + // fail early if we are at minSize already. + if replicas <= int32(ng.MinSize()) { + return fmt.Errorf("unable to delete %d machines in %q, machine replicas are <= %d", len(nodes), ng.Id(), ng.MinSize()) + } + // Step 1: Verify all nodes belong to this node group. for _, node := range nodes { actualNodeGroup, err := ng.machineController.nodeGroupForNode(node) @@ -108,14 +115,10 @@ func (ng *nodegroup) DeleteNodes(nodes []*corev1.Node) error { } // Step 2: if deleting len(nodes) would make the replica count - // <= 0, then the request to delete that many nodes is bogus + // < MinSize, then the request to delete that many nodes is bogus // and we fail fast. - replicas := ng.scalableResource.Replicas() - - if replicas-int32(len(nodes)) < 0 { - return fmt.Errorf("unable to delete %d machines in %q, machine replicas are < 0 ", len(nodes), ng.Id()) - } else if replicas-int32(len(nodes)) == 0 && !ng.scalableResource.CanScaleFromZero() { - return fmt.Errorf("unable to delete %d machines in %q, machine replicas are 0", len(nodes), ng.Id()) + if replicas-int32(len(nodes)) < int32(ng.MinSize()) { + return fmt.Errorf("unable to delete %d machines in %q, machine replicas are %d, minSize is %d", len(nodes), ng.Id(), replicas, ng.MinSize()) } // Step 3: annotate the corresponding machine that it is a