From 3346aec8d01141053b692dd0773ea48643073148 Mon Sep 17 00:00:00 2001 From: Marwan Ahmed Date: Wed, 20 May 2020 01:15:46 -0700 Subject: [PATCH] add unit test for in progress deletion cases --- .../azure/azure_cloud_provider_test.go | 31 ++++++++- .../cloudprovider/azure/azure_fakes.go | 23 ++++--- .../azure/azure_scale_set_test.go | 66 +++++++++++++++++++ 3 files changed, 106 insertions(+), 14 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider_test.go b/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider_test.go index 7205d14a35dc..d38d68b84066 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider_test.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider_test.go @@ -22,6 +22,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-10-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" "github.com/stretchr/testify/mock" @@ -79,8 +80,34 @@ func newTestAzureManager(t *testing.T) *AzureManager { virtualMachinesClient: &VirtualMachinesClientMock{ FakeStore: make(map[string]map[string]compute.VirtualMachine), }, - virtualMachineScaleSetsClient: scaleSetsClient, - virtualMachineScaleSetVMsClient: &VirtualMachineScaleSetVMsClientMock{}, + virtualMachineScaleSetsClient: &VirtualMachineScaleSetsClientMock{ + FakeStore: map[string]map[string]compute.VirtualMachineScaleSet{ + "test": { + "test-asg": { + Name: &vmssName, + Sku: &compute.Sku{ + Capacity: &vmssCapacity, + Name: &skuName, + }, + VirtualMachineScaleSetProperties: &compute.VirtualMachineScaleSetProperties{}, + Location: &location, + }, + }, + }, + }, + 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"), + }, + }, + }, + }, + }, }, } diff --git a/cluster-autoscaler/cloudprovider/azure/azure_fakes.go b/cluster-autoscaler/cloudprovider/azure/azure_fakes.go index 0efa00a95df3..58de48ef7207 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_fakes.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_fakes.go @@ -26,6 +26,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2017-05-10/resources" "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2018-07-01/storage" "github.com/Azure/go-autorest/autorest" + "github.com/avast/retry-go" "github.com/stretchr/testify/mock" ) @@ -111,6 +112,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,19 +131,15 @@ 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, filter string, selectParameter string, expand string) (result []compute.VirtualMachineScaleSetVM, err 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, - }) +func (m *VirtualMachineScaleSetVMsClientMock) List(ctx context.Context, resourceGroupName string, virtualMachineScaleSetName string, expand string) (result []compute.VirtualMachineScaleSetVM, rerr *retry.Error) { + 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 } diff --git a/cluster-autoscaler/cloudprovider/azure/azure_scale_set_test.go b/cluster-autoscaler/cloudprovider/azure/azure_scale_set_test.go index 96396db7b21f..d6adb96b70f6 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_scale_set_test.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_scale_set_test.go @@ -224,6 +224,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(