Skip to content

Commit

Permalink
Merge pull request kubernetes#90090 from nilo19/cleanup/use-mock-clie…
Browse files Browse the repository at this point in the history 
…nt-in-unit-tests

Switch from fake storage to mock clients in azure unit tests.
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Apr 13, 2020
2 parents 499fdfa + 68600b8 commit 3c7258d
Show file tree
Hide file tree
Showing 25 changed files with 1,512 additions and 1,198 deletions.
19 changes: 19 additions & 0 deletions staging/src/k8s.io/legacy-cloud-providers/azure/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,22 +64,31 @@ go_library(
"//staging/src/k8s.io/legacy-cloud-providers/azure/cache:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/diskclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/diskclient/mockdiskclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/interfaceclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/interfaceclient/mockinterfaceclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/loadbalancerclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/loadbalancerclient/mockloadbalancerclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/publicipclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/publicipclient/mockpublicipclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/routeclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/routeclient/mockrouteclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/routetableclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/routetableclient/mockroutetableclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/securitygroupclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/securitygroupclient/mocksecuritygroupclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/snapshotclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/storageaccountclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/subnetclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/subnetclient/mocksubnetclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/vmclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/vmclient/mockvmclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/vmsizeclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/vmssclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/vmssclient/mockvmssclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/vmssvmclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/vmssvmclient/mockvmssvmclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/retry:go_default_library",
"//vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-12-01/compute:go_default_library",
"//vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network:go_default_library",
Expand Down Expand Up @@ -130,8 +139,18 @@ go_test(
"//staging/src/k8s.io/legacy-cloud-providers/azure/auth:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/cache:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/diskclient/mockdiskclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/mockfileclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/interfaceclient/mockinterfaceclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/loadbalancerclient/mockloadbalancerclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/publicipclient/mockpublicipclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/routetableclient/mockroutetableclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/securitygroupclient/mocksecuritygroupclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/storageaccountclient/mockstorageaccountclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/subnetclient/mocksubnetclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/vmclient/mockvmclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/vmssclient/mockvmssclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/clients/vmssvmclient/mockvmssvmclient:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/mockvmsets:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure/retry:go_default_library",
"//vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-12-01/compute:go_default_library",
Expand Down
6 changes: 4 additions & 2 deletions staging/src/k8s.io/legacy-cloud-providers/azure/azure.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (
azcache "k8s.io/legacy-cloud-providers/azure/cache"
azclients "k8s.io/legacy-cloud-providers/azure/clients"
"k8s.io/legacy-cloud-providers/azure/clients/diskclient"
"k8s.io/legacy-cloud-providers/azure/clients/fileclient"
"k8s.io/legacy-cloud-providers/azure/clients/interfaceclient"
"k8s.io/legacy-cloud-providers/azure/clients/loadbalancerclient"
"k8s.io/legacy-cloud-providers/azure/clients/publicipclient"
Expand All @@ -61,6 +62,7 @@ import (
"k8s.io/legacy-cloud-providers/azure/clients/vmssclient"
"k8s.io/legacy-cloud-providers/azure/clients/vmssvmclient"
"k8s.io/legacy-cloud-providers/azure/retry"

"sigs.k8s.io/yaml"
)

Expand Down Expand Up @@ -234,7 +236,7 @@ type Cloud struct {
StorageAccountClient storageaccountclient.Interface
DisksClient diskclient.Interface
SnapshotsClient snapshotclient.Interface
FileClient FileClient
FileClient fileclient.Interface
VirtualMachineScaleSetsClient vmssclient.Interface
VirtualMachineScaleSetVMsClient vmssvmclient.Interface
VirtualMachineSizesClient vmsizeclient.Interface
Expand Down Expand Up @@ -590,7 +592,7 @@ func (az *Cloud) configAzureClients(
az.SecurityGroupsClient = securitygroupclient.New(securityGroupClientConfig)
az.PublicIPAddressesClient = publicipclient.New(publicIPClientConfig)
// fileClient is not based on armclient, but it's still backoff retried.
az.FileClient = newAzureFileClient(&az.Environment, azClientConfig.Backoff)
az.FileClient = fileclient.NewAzureFileClient(&az.Environment, azClientConfig.Backoff)
}

func (az *Cloud) getAzureClientConfig(servicePrincipalToken *adal.ServicePrincipalToken) *azclients.ClientConfig {
Expand Down
83 changes: 67 additions & 16 deletions staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,20 @@ package azure

import (
"fmt"
"net/http"
"reflect"
"testing"

"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-12-01/compute"
"github.com/Azure/go-autorest/autorest/to"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"

"k8s.io/apimachinery/pkg/types"
cloudprovider "k8s.io/cloud-provider"
"k8s.io/legacy-cloud-providers/azure/clients/diskclient/mockdiskclient"
"k8s.io/legacy-cloud-providers/azure/clients/vmclient/mockvmclient"
"k8s.io/legacy-cloud-providers/azure/retry"
"k8s.io/utils/pointer"
)

Expand All @@ -40,12 +46,14 @@ func TestCommonAttachDisk(t *testing.T) {
vmList map[string]string
nodeName types.NodeName
isDataDisksFull bool
existedDisk compute.Disk
expectedLun int32
expectedErr bool
}{
{
desc: "LUN -1 and error shall be returned if there's no such instance corresponding to given nodeName",
nodeName: "vm1",
existedDisk: compute.Disk{Name: to.StringPtr("disk-name")},
expectedLun: -1,
expectedErr: true,
},
Expand All @@ -54,13 +62,23 @@ func TestCommonAttachDisk(t *testing.T) {
vmList: map[string]string{"vm1": "PowerState/Running"},
nodeName: "vm1",
isDataDisksFull: true,
existedDisk: compute.Disk{Name: to.StringPtr("disk-name")},
expectedLun: -1,
expectedErr: true,
},
{
desc: "correct LUN and no error shall be returned if everything is good",
vmList: map[string]string{"vm1": "PowerState/Running"},
nodeName: "vm1",
existedDisk: compute.Disk{Name: to.StringPtr("disk-name")},
expectedLun: 1,
expectedErr: false,
},
{
desc: "an error shall be returned if there's no matching disk",
vmList: map[string]string{"vm1": "PowerState/Running"},
nodeName: "vm1",
existedDisk: compute.Disk{Name: to.StringPtr("disk-name-1")},
expectedLun: -1,
expectedErr: true,
},
Expand All @@ -76,9 +94,21 @@ func TestCommonAttachDisk(t *testing.T) {
cloud: testCloud,
vmLockMap: newLockMap(),
}
diskURI := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/disks/disk-name",
testCloud.SubscriptionID, testCloud.ResourceGroup)
setTestVirtualMachines(testCloud, test.vmList, test.isDataDisksFull)
diskURI := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/disks/%s",
testCloud.SubscriptionID, testCloud.ResourceGroup, *test.existedDisk.Name)
expectedVMs := setTestVirtualMachines(testCloud, test.vmList, test.isDataDisksFull)
mockVMsClient := testCloud.VirtualMachinesClient.(*mockvmclient.MockInterface)
for _, vm := range expectedVMs {
mockVMsClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, *vm.Name, gomock.Any()).Return(vm, nil).AnyTimes()
}
if len(expectedVMs) == 0 {
mockVMsClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, gomock.Any(), gomock.Any()).Return(compute.VirtualMachine{}, &retry.Error{HTTPStatusCode: http.StatusNotFound, RawError: cloudprovider.InstanceNotFound}).AnyTimes()
}
mockVMsClient.EXPECT().Update(gomock.Any(), testCloud.ResourceGroup, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()

mockDisksClient := testCloud.DisksClient.(*mockdiskclient.MockInterface)
mockDisksClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, "disk-name").Return(test.existedDisk, nil).AnyTimes()
mockDisksClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, gomock.Not("disk-name")).Return(compute.Disk{}, &retry.Error{HTTPStatusCode: http.StatusNotFound, RawError: cloudprovider.InstanceNotFound}).AnyTimes()

lun, err := common.AttachDisk(true, "", diskURI, test.nodeName, compute.CachingTypesReadOnly)
assert.Equal(t, test.expectedLun, lun, "TestCase[%d]: %s", i, test.desc)
Expand Down Expand Up @@ -130,7 +160,15 @@ func TestCommonDetachDisk(t *testing.T) {
}
diskURI := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/disks/disk-name",
testCloud.SubscriptionID, testCloud.ResourceGroup)
setTestVirtualMachines(testCloud, test.vmList, false)
expectedVMs := setTestVirtualMachines(testCloud, test.vmList, false)
mockVMsClient := testCloud.VirtualMachinesClient.(*mockvmclient.MockInterface)
for _, vm := range expectedVMs {
mockVMsClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, *vm.Name, gomock.Any()).Return(vm, nil).AnyTimes()
}
if len(expectedVMs) == 0 {
mockVMsClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, gomock.Any(), gomock.Any()).Return(compute.VirtualMachine{}, &retry.Error{HTTPStatusCode: http.StatusNotFound, RawError: cloudprovider.InstanceNotFound}).AnyTimes()
}
mockVMsClient.EXPECT().Update(gomock.Any(), testCloud.ResourceGroup, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()

err := common.DetachDisk(test.diskName, diskURI, test.nodeName)
assert.Equal(t, test.expectedErr, err != nil, "TestCase[%d]: %s, err: %v", i, test.desc, err)
Expand Down Expand Up @@ -172,7 +210,11 @@ func TestGetDiskLun(t *testing.T) {
cloud: testCloud,
vmLockMap: newLockMap(),
}
setTestVirtualMachines(testCloud, map[string]string{"vm1": "PowerState/Running"}, false)
expectedVMs := setTestVirtualMachines(testCloud, map[string]string{"vm1": "PowerState/Running"}, false)
mockVMsClient := testCloud.VirtualMachinesClient.(*mockvmclient.MockInterface)
for _, vm := range expectedVMs {
mockVMsClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, *vm.Name, gomock.Any()).Return(vm, nil).AnyTimes()
}

lun, err := common.GetDiskLun(test.diskName, test.diskURI, "vm1")
assert.Equal(t, test.expectedLun, lun, "TestCase[%d]: %s", i, test.desc)
Expand Down Expand Up @@ -214,7 +256,11 @@ func TestGetNextDiskLun(t *testing.T) {
cloud: testCloud,
vmLockMap: newLockMap(),
}
setTestVirtualMachines(testCloud, map[string]string{"vm1": "PowerState/Running"}, test.isDataDisksFull)
expectedVMs := setTestVirtualMachines(testCloud, map[string]string{"vm1": "PowerState/Running"}, test.isDataDisksFull)
mockVMsClient := testCloud.VirtualMachinesClient.(*mockvmclient.MockInterface)
for _, vm := range expectedVMs {
mockVMsClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, *vm.Name, gomock.Any()).Return(vm, nil).AnyTimes()
}

lun, err := common.GetNextDiskLun("vm1")
assert.Equal(t, test.expectedLun, lun, "TestCase[%d]: %s", i, test.desc)
Expand Down Expand Up @@ -259,15 +305,20 @@ func TestDisksAreAttached(t *testing.T) {
cloud: testCloud,
vmLockMap: newLockMap(),
}
setTestVirtualMachines(testCloud, map[string]string{"vm1": "PowerState/Running"}, false)
expectedVMs := setTestVirtualMachines(testCloud, map[string]string{"vm1": "PowerState/Running"}, false)
mockVMsClient := testCloud.VirtualMachinesClient.(*mockvmclient.MockInterface)
for _, vm := range expectedVMs {
mockVMsClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, *vm.Name, gomock.Any()).Return(vm, nil).AnyTimes()
}
mockVMsClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, "vm2", gomock.Any()).Return(compute.VirtualMachine{}, &retry.Error{HTTPStatusCode: http.StatusNotFound, RawError: cloudprovider.InstanceNotFound}).AnyTimes()

attached, err := common.DisksAreAttached(test.diskNames, test.nodeName)
assert.Equal(t, test.expectedAttached, attached, "TestCase[%d]: %s", i, test.desc)
assert.Equal(t, test.expectedErr, err != nil, "TestCase[%d]: %s", i, test.desc)
}
}

func TestFilteredDetatchingDisks(t *testing.T) {
func TestFilteredDetachingDisks(t *testing.T) {

disks := []compute.DataDisk{
{
Expand Down Expand Up @@ -448,10 +499,10 @@ func TestCheckDiskExists(t *testing.T) {
newDiskName := "newdisk"
newDiskURI := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/disks/%s",
testCloud.SubscriptionID, testCloud.ResourceGroup, newDiskName)
fDC := newFakeDisksClient()
rerr := fDC.CreateOrUpdate(ctx, testCloud.ResourceGroup, newDiskName, compute.Disk{})
assert.Equal(t, rerr == nil, true, "return error: %v", rerr)
testCloud.DisksClient = fDC

mockDisksClient := testCloud.DisksClient.(*mockdiskclient.MockInterface)
mockDisksClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, newDiskName).Return(compute.Disk{}, nil).AnyTimes()
mockDisksClient.EXPECT().Get(gomock.Any(), gomock.Not(testCloud.ResourceGroup), gomock.Any()).Return(compute.Disk{}, &retry.Error{HTTPStatusCode: http.StatusNotFound, RawError: cloudprovider.InstanceNotFound}).AnyTimes()

testCases := []struct {
diskURI string
Expand Down Expand Up @@ -503,10 +554,10 @@ func TestFilterNonExistingDisks(t *testing.T) {
testCloud.SubscriptionID, testCloud.ResourceGroup)
newDiskName := "newdisk"
newDiskURI := diskURIPrefix + newDiskName
fDC := newFakeDisksClient()
rerr := fDC.CreateOrUpdate(ctx, testCloud.ResourceGroup, newDiskName, compute.Disk{})
assert.Equal(t, rerr == nil, true, "return error: %v", rerr)
testCloud.DisksClient = fDC

mockDisksClient := testCloud.DisksClient.(*mockdiskclient.MockInterface)
mockDisksClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, newDiskName).Return(compute.Disk{}, nil).AnyTimes()
mockDisksClient.EXPECT().Get(gomock.Any(), testCloud.ResourceGroup, gomock.Not(newDiskName)).Return(compute.Disk{}, &retry.Error{HTTPStatusCode: http.StatusNotFound, RawError: cloudprovider.InstanceNotFound}).AnyTimes()

disks := []compute.DataDisk{
{
Expand Down
Loading

0 comments on commit 3c7258d

Please sign in to comment.