Skip to content

Commit

Permalink
Don't scale down ASGs when placeholder nodes are discovered
Browse files Browse the repository at this point in the history
  • Loading branch information
theintz authored and Tobias Heintz committed Jun 9, 2023
1 parent 1d0a75c commit dfd4f23
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 12 deletions.
16 changes: 5 additions & 11 deletions cluster-autoscaler/cloudprovider/aws/auto_scaling_groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,10 +277,6 @@ func (m *asgCache) setAsgSizeNoLock(asg *asg, size int) error {
return nil
}

func (m *asgCache) decreaseAsgSizeByOneNoLock(asg *asg) error {
return m.setAsgSizeNoLock(asg, asg.curSize-1)
}

// DeleteInstances deletes the given instances. All instances must be controlled by the same ASG.
func (m *asgCache) DeleteInstances(instances []*AwsInstanceRef) error {
m.mutex.Lock()
Expand Down Expand Up @@ -308,11 +304,9 @@ func (m *asgCache) DeleteInstances(instances []*AwsInstanceRef) error {

for _, instance := range instances {
// check if the instance is a placeholder - a requested instance that was never created by the node group
// if it is, just decrease the size of the node group, as there's no specific instance we can remove
// if it is, simply remove it from the cache
if m.isPlaceholderInstance(instance) {
klog.V(4).Infof("instance %s is detected as a placeholder, decreasing ASG requested size instead "+
"of deleting instance", instance.Name)
m.decreaseAsgSizeByOneNoLock(commonAsg)
klog.V(4).Infof("instance %s is detected as a placeholder", instance.Name)
} else {
// check if the instance is already terminating - if it is, don't bother terminating again
// as doing so causes unnecessary API calls and can cause the curSize cached value to decrement
Expand Down Expand Up @@ -341,10 +335,10 @@ func (m *asgCache) DeleteInstances(instances []*AwsInstanceRef) error {
return err
}
klog.V(4).Infof(*resp.Activity.Description)

// Proactively decrement the size so autoscaler makes better decisions
commonAsg.curSize--
}

// Proactively decrement the size so autoscaler makes better decisions
commonAsg.curSize--
}
return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ func TestDeleteNodesWithPlaceholder(t *testing.T) {
}
err = asgs[0].DeleteNodes([]*apiv1.Node{node})
assert.NoError(t, err)
a.AssertNumberOfCalls(t, "SetDesiredCapacity", 1)
a.AssertNumberOfCalls(t, "SetDesiredCapacity", 0)
a.AssertNumberOfCalls(t, "DescribeAutoScalingGroupsPages", 1)

newSize, err := asgs[0].TargetSize()
Expand Down

0 comments on commit dfd4f23

Please sign in to comment.