From 8d58fb8b3d72e174888028ebab8d8bf528577cba Mon Sep 17 00:00:00 2001 From: Enxebre Date: Tue, 5 May 2020 14:09:37 +0200 Subject: [PATCH] Compare against minSize in deleteNodes() in cluster-autoscaler CAPI provider 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 --- .../clusterapi/clusterapi_nodegroup.go | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go index c73fd0e96b5..c1906f418d6 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go @@ -95,6 +95,16 @@ func (ng *nodegroup) DeleteNodes(nodes []*corev1.Node) error { ng.machineController.accessLock.Lock() defer ng.machineController.accessLock.Unlock() + replicas, err := ng.scalableResource.Replicas() + if err != nil { + return err + } + + // if we are at minSize already we wail early. + if int(replicas) <= ng.MinSize() { + return fmt.Errorf("min size reached, nodes will not be deleted") + } + // Step 1: Verify all nodes belong to this node group. for _, node := range nodes { actualNodeGroup, err := ng.machineController.nodeGroupForNode(node) @@ -112,15 +122,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, err := ng.scalableResource.Replicas() - if err != nil { - return err - } - - if replicas-int32(len(nodes)) <= 0 { - 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 %q, minSize is %q ", len(nodes), ng.Id(), replicas, ng.MinSize()) } // Step 3: annotate the corresponding machine that it is a