add unit test for in progress deletion cases

Author: marwanad

cluster-autoscaler/cloudprovider/azure/azure_cloud_provider_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-12-01/compute"
 	"github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2017-05-10/resources"
 	"github.com/Azure/go-autorest/autorest/azure"
+	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/stretchr/testify/assert"
 
 	apiv1 "k8s.io/api/core/v1"
@@ -67,7 +68,19 @@ func newTestAzureManager(t *testing.T) *AzureManager {
 					},
 				},
 			},
-			virtualMachineScaleSetVMsClient: &VirtualMachineScaleSetVMsClientMock{},
+			virtualMachineScaleSetVMsClient: &VirtualMachineScaleSetVMsClientMock{
+				FakeStore: map[string]map[string]compute.VirtualMachineScaleSetVM{
+					"test": {
+						"0": {
+							ID:         to.StringPtr(fakeVirtualMachineScaleSetVMID),
+							InstanceID: to.StringPtr("0"),
+							VirtualMachineScaleSetVMProperties: &compute.VirtualMachineScaleSetVMProperties{
+								VMID: to.StringPtr("123E4567-E89B-12D3-A456-426655440000"),
+							},
+						},
+					},
+				},
+			},
 		},
 	}
 

cluster-autoscaler/cloudprovider/azure/azure_fakes.go

@@ -110,6 +110,8 @@ func (client *VirtualMachineScaleSetsClientMock) List(ctx context.Context, resou
 // VirtualMachineScaleSetVMsClientMock mocks for VirtualMachineScaleSetVMsClient.
 type VirtualMachineScaleSetVMsClientMock struct {
 	mock.Mock
+	mutex     sync.Mutex
+	FakeStore map[string]map[string]compute.VirtualMachineScaleSetVM
 }
 
 // Get gets a VirtualMachineScaleSetVM by VMScaleSetName and instanceID.
@@ -128,18 +130,14 @@ func (m *VirtualMachineScaleSetVMsClientMock) Get(ctx context.Context, resourceG
 
 // List gets a list of VirtualMachineScaleSetVMs.
 func (m *VirtualMachineScaleSetVMsClientMock) List(ctx context.Context, resourceGroupName string, virtualMachineScaleSetName string, expand string) (result []compute.VirtualMachineScaleSetVM, rerr *retry.Error) {
-	ID := fakeVirtualMachineScaleSetVMID
-	instanceID := "0"
-	vmID := "123E4567-E89B-12D3-A456-426655440000"
-	properties := compute.VirtualMachineScaleSetVMProperties{
-		VMID: &vmID,
-	}
-	result = append(result, compute.VirtualMachineScaleSetVM{
-		ID:                                 &ID,
-		InstanceID:                         &instanceID,
-		VirtualMachineScaleSetVMProperties: &properties,
-	})
+	m.mutex.Lock()
+	defer m.mutex.Unlock()
 
+	if _, ok := m.FakeStore[resourceGroupName]; ok {
+		for _, v := range m.FakeStore[resourceGroupName] {
+			result = append(result, v)
+		}
+	}
 	return result, nil
 }
 

cluster-autoscaler/cloudprovider/azure/azure_scale_set_test.go

@@ -226,6 +226,72 @@ func TestDeleteNodes(t *testing.T) {
 	scaleSetClient.AssertNumberOfCalls(t, "DeleteInstances", 1)
 }
 
+func TestDeleteNoConflictRequest(t *testing.T) {
+	vmssName := "test-asg"
+	var vmssCapacity int64 = 3
+
+	manager := newTestAzureManager(t)
+	vmsClient := &VirtualMachineScaleSetVMsClientMock{
+		FakeStore: map[string]map[string]compute.VirtualMachineScaleSetVM{
+			"test": {
+				"0": {
+					ID:         to.StringPtr(fakeVirtualMachineScaleSetVMID),
+					InstanceID: to.StringPtr("0"),
+					VirtualMachineScaleSetVMProperties: &compute.VirtualMachineScaleSetVMProperties{
+						VMID:              to.StringPtr("123E4567-E89B-12D3-A456-426655440000"),
+						ProvisioningState: to.StringPtr("Deleting"),
+					},
+				},
+			},
+		},
+	}
+
+	scaleSetClient := &VirtualMachineScaleSetsClientMock{
+		FakeStore: map[string]map[string]compute.VirtualMachineScaleSet{
+			"test": {
+				"test-asg": {
+					Name: &vmssName,
+					Sku: &compute.Sku{
+						Capacity: &vmssCapacity,
+					},
+				},
+			},
+		},
+	}
+
+	response := autorest.Response{
+		Response: &http.Response{
+			Status: "OK",
+		},
+	}
+
+	scaleSetClient.On("DeleteInstances", mock.Anything, "test-asg", mock.Anything, mock.Anything).Return(response, nil)
+	manager.azClient.virtualMachineScaleSetsClient = scaleSetClient
+	manager.azClient.virtualMachineScaleSetVMsClient = vmsClient
+
+	resourceLimiter := cloudprovider.NewResourceLimiter(
+		map[string]int64{cloudprovider.ResourceNameCores: 1, cloudprovider.ResourceNameMemory: 10000000},
+		map[string]int64{cloudprovider.ResourceNameCores: 10, cloudprovider.ResourceNameMemory: 100000000})
+	provider, err := BuildAzureCloudProvider(manager, resourceLimiter)
+	assert.NoError(t, err)
+
+	registered := manager.RegisterAsg(newTestScaleSet(manager, "test-asg"))
+	assert.True(t, registered)
+
+	node := &apiv1.Node{
+		Spec: apiv1.NodeSpec{
+			ProviderID: "azure://" + fakeVirtualMachineScaleSetVMID,
+		},
+	}
+
+	scaleSet, ok := provider.NodeGroups()[0].(*ScaleSet)
+	assert.True(t, ok)
+
+	err = scaleSet.DeleteNodes([]*apiv1.Node{node})
+	// ensure that DeleteInstances isn't called
+	scaleSetClient.AssertNumberOfCalls(t, "DeleteInstances", 0)
+}
+
 func TestId(t *testing.T) {
 	provider := newTestProvider(t)
 	registered := provider.azureManager.RegisterAsg(