Skip to content

Commit

Permalink
switch scalesets to delete asynchronously without waiting on future
Browse files Browse the repository at this point in the history
  • Loading branch information
marwanad committed Jun 17, 2020
1 parent d4c5604 commit 694e089
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 10 deletions.
38 changes: 32 additions & 6 deletions cluster-autoscaler/cloudprovider/azure/azure_scale_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,29 @@ func (scaleSet *ScaleSet) GetScaleSetSize() (int64, error) {
return scaleSet.getCurSize()
}

func (scaleSet *ScaleSet) waitForDeleteInstances(future *azure.Future, requiredIds *compute.VirtualMachineScaleSetVMInstanceRequiredIDs) {
var err error

defer func() {
if err != nil {
klog.Errorf("Failed to delete instances %v. Invalidating the cache to get the real scale set size", requiredIds.InstanceIds)
scaleSet.invalidateStatusCacheWithLock()
}
}()

ctx, cancel := getContextWithCancel()
defer cancel()

httpResponse, err := scaleSet.manager.azClient.virtualMachineScaleSetsClient.WaitForAsyncOperationResult(ctx, future)
isSuccess, err := isSuccessHTTPResponse(httpResponse, err)
if isSuccess {
klog.V(3).Infof("virtualMachineScaleSetsClient.WaitForAsyncOperationResult - DeleteInstances(%v) success", requiredIds.InstanceIds)
scaleSet.invalidateInstanceCache()
return
}
klog.Errorf("virtualMachineScaleSetsClient.WaitForAsyncOperationResult - DeleteInstances for instances %v failed with error: %v", requiredIds.InstanceIds, err)
}

// updateVMSSCapacity invokes virtualMachineScaleSetsClient to update the capacity for VMSS.
func (scaleSet *ScaleSet) updateVMSSCapacity(future *azure.Future) {
var err error
Expand Down Expand Up @@ -428,19 +451,22 @@ func (scaleSet *ScaleSet) DeleteInstances(instances []*azureRef) error {
ctx, cancel := getContextWithCancel()
defer cancel()
resourceGroup := scaleSet.manager.config.ResourceGroup
klog.V(3).Infof("Calling virtualMachineScaleSetsClient.DeleteInstancesAsync(%v)", requiredIds.InstanceIds)

future, rerr := scaleSet.manager.azClient.virtualMachineScaleSetsClient.DeleteInstancesAsync(ctx, resourceGroup, commonAsg.Id(), *requiredIds)
if rerr != nil {
klog.Errorf("virtualMachineScaleSetsClient.DeleteInstancesAsync for instances %v failed: %v", requiredIds.InstanceIds, err)
return rerr.Error()
}

// Proactively decrement scale set size so that we don't
// go below minimum node count if cache data is stale
scaleSet.sizeMutex.Lock()
scaleSet.curSize--
scaleSet.sizeMutex.Unlock()

rerr := scaleSet.manager.azClient.virtualMachineScaleSetsClient.DeleteInstances(ctx, resourceGroup, commonAsg.Id(), *requiredIds)
if rerr != nil {
klog.Errorf("Failed to delete instances %v. Invalidating the cache to get the real scale set size", requiredIds)
scaleSet.invalidateStatusCacheWithLock()
}
return rerr.Error()
go scaleSet.waitForDeleteInstances(future, requiredIds)
return nil
}

// DeleteNodes deletes the nodes from the group.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,6 @@ func TestTargetSize(t *testing.T) {
assert.True(t, registered)
assert.Equal(t, len(provider.NodeGroups()), 1)

ng := provider.NodeGroups()[0]
size, err := ng.TargetSize()
println(size)
targetSize, err := provider.NodeGroups()[0].TargetSize()
assert.NoError(t, err)
assert.Equal(t, 3, targetSize)
Expand Down Expand Up @@ -259,7 +256,8 @@ func TestDeleteNodes(t *testing.T) {

mockVMSSClient := mockvmssclient.NewMockInterface(ctrl)
mockVMSSClient.EXPECT().List(gomock.Any(), manager.config.ResourceGroup).Return(expectedScaleSets, nil).AnyTimes()
mockVMSSClient.EXPECT().DeleteInstances(gomock.Any(), manager.config.ResourceGroup, gomock.Any(), gomock.Any()).Return(nil)
mockVMSSClient.EXPECT().DeleteInstancesAsync(gomock.Any(), manager.config.ResourceGroup, gomock.Any(), gomock.Any()).Return(nil, nil)
mockVMSSClient.EXPECT().WaitForAsyncOperationResult(gomock.Any(), gomock.Any()).Return(&http.Response{StatusCode: http.StatusOK}, nil).AnyTimes()
manager.azClient.virtualMachineScaleSetsClient = mockVMSSClient
mockVMSSVMClient := mockvmssvmclient.NewMockInterface(ctrl)
mockVMSSVMClient.EXPECT().List(gomock.Any(), manager.config.ResourceGroup, "test-asg", gomock.Any()).Return(expectedVMSSVMs, nil).AnyTimes()
Expand Down

0 comments on commit 694e089

Please sign in to comment.