Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch scalesets to delete asynchronously without waiting on future #3221

Merged
merged 1 commit into from
Jun 18, 2020
Merged

switch scalesets to delete asynchronously without waiting on future #3221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 32 additions & 6 deletions cluster-autoscaler/cloudprovider/azure/azure_scale_set.go
View file
Open in desktop
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
6 changes: 2 additions & 4 deletions cluster-autoscaler/cloudprovider/azure/azure_scale_set_test.go
View file
Open in desktop
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