Skip to content

Commit 9a1f3fe

Browse files
authored
Merge pull request kubernetes#3221 from marwanad/parallel-deletes
switch scalesets to delete asynchronously without waiting on future
2 parents d4c5604 + 694e089 commit 9a1f3fe

File tree

2 files changed

+34
-10
lines changed

2 files changed

+34
-10
lines changed

cluster-autoscaler/cloudprovider/azure/azure_scale_set.go

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,29 @@ func (scaleSet *ScaleSet) GetScaleSetSize() (int64, error) {
227227
return scaleSet.getCurSize()
228228
}
229229

230+
func (scaleSet *ScaleSet) waitForDeleteInstances(future *azure.Future, requiredIds *compute.VirtualMachineScaleSetVMInstanceRequiredIDs) {
231+
var err error
232+
233+
defer func() {
234+
if err != nil {
235+
klog.Errorf("Failed to delete instances %v. Invalidating the cache to get the real scale set size", requiredIds.InstanceIds)
236+
scaleSet.invalidateStatusCacheWithLock()
237+
}
238+
}()
239+
240+
ctx, cancel := getContextWithCancel()
241+
defer cancel()
242+
243+
httpResponse, err := scaleSet.manager.azClient.virtualMachineScaleSetsClient.WaitForAsyncOperationResult(ctx, future)
244+
isSuccess, err := isSuccessHTTPResponse(httpResponse, err)
245+
if isSuccess {
246+
klog.V(3).Infof("virtualMachineScaleSetsClient.WaitForAsyncOperationResult - DeleteInstances(%v) success", requiredIds.InstanceIds)
247+
scaleSet.invalidateInstanceCache()
248+
return
249+
}
250+
klog.Errorf("virtualMachineScaleSetsClient.WaitForAsyncOperationResult - DeleteInstances for instances %v failed with error: %v", requiredIds.InstanceIds, err)
251+
}
252+
230253
// updateVMSSCapacity invokes virtualMachineScaleSetsClient to update the capacity for VMSS.
231254
func (scaleSet *ScaleSet) updateVMSSCapacity(future *azure.Future) {
232255
var err error
@@ -428,19 +451,22 @@ func (scaleSet *ScaleSet) DeleteInstances(instances []*azureRef) error {
428451
ctx, cancel := getContextWithCancel()
429452
defer cancel()
430453
resourceGroup := scaleSet.manager.config.ResourceGroup
454+
klog.V(3).Infof("Calling virtualMachineScaleSetsClient.DeleteInstancesAsync(%v)", requiredIds.InstanceIds)
455+
456+
future, rerr := scaleSet.manager.azClient.virtualMachineScaleSetsClient.DeleteInstancesAsync(ctx, resourceGroup, commonAsg.Id(), *requiredIds)
457+
if rerr != nil {
458+
klog.Errorf("virtualMachineScaleSetsClient.DeleteInstancesAsync for instances %v failed: %v", requiredIds.InstanceIds, err)
459+
return rerr.Error()
460+
}
431461

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

438-
rerr := scaleSet.manager.azClient.virtualMachineScaleSetsClient.DeleteInstances(ctx, resourceGroup, commonAsg.Id(), *requiredIds)
439-
if rerr != nil {
440-
klog.Errorf("Failed to delete instances %v. Invalidating the cache to get the real scale set size", requiredIds)
441-
scaleSet.invalidateStatusCacheWithLock()
442-
}
443-
return rerr.Error()
468+
go scaleSet.waitForDeleteInstances(future, requiredIds)
469+
return nil
444470
}
445471

446472
// DeleteNodes deletes the nodes from the group.

cluster-autoscaler/cloudprovider/azure/azure_scale_set_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,6 @@ func TestTargetSize(t *testing.T) {
108108
assert.True(t, registered)
109109
assert.Equal(t, len(provider.NodeGroups()), 1)
110110

111-
ng := provider.NodeGroups()[0]
112-
size, err := ng.TargetSize()
113-
println(size)
114111
targetSize, err := provider.NodeGroups()[0].TargetSize()
115112
assert.NoError(t, err)
116113
assert.Equal(t, 3, targetSize)
@@ -259,7 +256,8 @@ func TestDeleteNodes(t *testing.T) {
259256

260257
mockVMSSClient := mockvmssclient.NewMockInterface(ctrl)
261258
mockVMSSClient.EXPECT().List(gomock.Any(), manager.config.ResourceGroup).Return(expectedScaleSets, nil).AnyTimes()
262-
mockVMSSClient.EXPECT().DeleteInstances(gomock.Any(), manager.config.ResourceGroup, gomock.Any(), gomock.Any()).Return(nil)
259+
mockVMSSClient.EXPECT().DeleteInstancesAsync(gomock.Any(), manager.config.ResourceGroup, gomock.Any(), gomock.Any()).Return(nil, nil)
260+
mockVMSSClient.EXPECT().WaitForAsyncOperationResult(gomock.Any(), gomock.Any()).Return(&http.Response{StatusCode: http.StatusOK}, nil).AnyTimes()
263261
manager.azClient.virtualMachineScaleSetsClient = mockVMSSClient
264262
mockVMSSVMClient := mockvmssvmclient.NewMockInterface(ctrl)
265263
mockVMSSVMClient.EXPECT().List(gomock.Any(), manager.config.ResourceGroup, "test-asg", gomock.Any()).Return(expectedVMSSVMs, nil).AnyTimes()

0 commit comments

Comments
 (0)