From d07acb8da6e5cb9ad44f75dee092fa9c04625a15 Mon Sep 17 00:00:00 2001 From: nicky Date: Mon, 17 Jun 2019 09:06:19 +0000 Subject: [PATCH] Add unit tests for azure_controller_common.go and azure_controller_standard.go --- .../k8s.io/legacy-cloud-providers/azure/BUILD | 1 + .../azure/azure_controller_common_test.go | 237 +++++++++++++++--- .../azure/azure_controller_standard_test.go | 128 ++++++++++ .../azure/azure_instances_test.go | 49 ++-- .../azure/azure_test.go | 1 + 5 files changed, 368 insertions(+), 48 deletions(-) create mode 100644 staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_standard_test.go diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD b/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD index 71509f32feb..2cb54954378 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD @@ -84,6 +84,7 @@ go_test( "azure_cache_test.go", "azure_config_test.go", "azure_controller_common_test.go", + "azure_controller_standard_test.go", "azure_instances_test.go", "azure_loadbalancer_test.go", "azure_metrics_test.go", diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common_test.go index 4db263d5699..34b1037094c 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common_test.go @@ -21,46 +21,223 @@ import ( "testing" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute" + "github.com/stretchr/testify/assert" + + "k8s.io/apimachinery/pkg/types" ) -func TestAttachDisk(t *testing.T) { - c := getTestCloud() - - common := &controllerCommon{ - location: c.Location, - storageEndpointSuffix: c.Environment.StorageEndpointSuffix, - resourceGroup: c.ResourceGroup, - subscriptionID: c.SubscriptionID, - cloud: c, +func TestCommonAttachDisk(t *testing.T) { + testCases := []struct { + desc string + vmList map[string]string + nodeName types.NodeName + isDataDisksFull bool + expectedLun int32 + expectedErr bool + }{ + { + desc: "LUN -1 and error shall be returned if there's no such instance corresponding to given nodeName", + nodeName: "vm1", + expectedLun: -1, + expectedErr: true, + }, + { + desc: "LUN -1 and error shall be returned if there's no available LUN for instance", + vmList: map[string]string{"vm1": "PowerState/Running"}, + nodeName: "vm1", + isDataDisksFull: true, + 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", + expectedLun: 1, + expectedErr: false, + }, } - diskURI := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/disks/disk-name", c.SubscriptionID, c.ResourceGroup) + for i, test := range testCases { + testCloud := getTestCloud() + common := &controllerCommon{ + location: testCloud.Location, + storageEndpointSuffix: testCloud.Environment.StorageEndpointSuffix, + resourceGroup: testCloud.ResourceGroup, + subscriptionID: testCloud.SubscriptionID, + cloud: testCloud, + } + diskURI := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/disks/disk-name", + testCloud.SubscriptionID, testCloud.ResourceGroup) + setTestVirtualMachines(testCloud, test.vmList, test.isDataDisksFull) - _, err := common.AttachDisk(true, "", diskURI, "node1", compute.CachingTypesReadOnly) - if err != nil { - fmt.Printf("TestAttachDisk return expected error: %v", err) - } else { - t.Errorf("TestAttachDisk unexpected nil err") + lun, err := common.AttachDisk(true, "", diskURI, test.nodeName, compute.CachingTypesReadOnly) + assert.Equal(t, test.expectedLun, lun, "TestCase[%d]: %s", i, test.desc) + assert.Equal(t, test.expectedErr, err != nil, "TestCase[%d]: %s", i, test.desc) } } -func TestDetachDisk(t *testing.T) { - c := getTestCloud() - - common := &controllerCommon{ - location: c.Location, - storageEndpointSuffix: c.Environment.StorageEndpointSuffix, - resourceGroup: c.ResourceGroup, - subscriptionID: c.SubscriptionID, - cloud: c, +func TestCommonDetachDisk(t *testing.T) { + testCases := []struct { + desc string + vmList map[string]string + nodeName types.NodeName + diskName string + expectedErr bool + }{ + { + desc: "an error shall be returned if there's no such instance corresponding to given nodeName", + nodeName: "vm1", + expectedErr: true, + }, + { + desc: "no error shall be returned if there's no matching disk according to given diskName", + vmList: map[string]string{"vm1": "PowerState/Running"}, + nodeName: "vm1", + diskName: "disk2", + expectedErr: false, + }, + { + desc: "no error shall be returned if the disk exsists", + vmList: map[string]string{"vm1": "PowerState/Running"}, + nodeName: "vm1", + diskName: "disk1", + expectedErr: false, + }, } - diskURI := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/disks/disk-name", c.SubscriptionID, c.ResourceGroup) + for i, test := range testCases { + testCloud := getTestCloud() + common := &controllerCommon{ + location: testCloud.Location, + storageEndpointSuffix: testCloud.Environment.StorageEndpointSuffix, + resourceGroup: testCloud.ResourceGroup, + subscriptionID: testCloud.SubscriptionID, + cloud: testCloud, + } + diskURI := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/disks/disk-name", + testCloud.SubscriptionID, testCloud.ResourceGroup) + setTestVirtualMachines(testCloud, test.vmList, false) - err := common.DetachDisk("", diskURI, "node1") - if err != nil { - fmt.Printf("TestAttachDisk return expected error: %v", err) - } else { - t.Errorf("TestAttachDisk unexpected nil err") + err := common.DetachDisk(test.diskName, diskURI, test.nodeName) + assert.Equal(t, test.expectedErr, err != nil, "TestCase[%d]: %s", i, test.desc) + } +} + +func TestGetDiskLun(t *testing.T) { + testCases := []struct { + desc string + diskName string + diskURI string + expectedLun int32 + expectedErr bool + }{ + { + desc: "LUN -1 and error shall be returned if diskName != disk.Name or diskURI != disk.Vhd.URI", + diskName: "disk2", + expectedLun: -1, + expectedErr: true, + }, + { + desc: "correct LUN and no error shall be returned if diskName = disk.Name", + diskName: "disk1", + expectedLun: 0, + expectedErr: false, + }, + } + + for i, test := range testCases { + testCloud := getTestCloud() + common := &controllerCommon{ + location: testCloud.Location, + storageEndpointSuffix: testCloud.Environment.StorageEndpointSuffix, + resourceGroup: testCloud.ResourceGroup, + subscriptionID: testCloud.SubscriptionID, + cloud: testCloud, + } + setTestVirtualMachines(testCloud, map[string]string{"vm1": "PowerState/Running"}, false) + + lun, err := common.GetDiskLun(test.diskName, test.diskURI, "vm1") + assert.Equal(t, test.expectedLun, lun, "TestCase[%d]: %s", i, test.desc) + assert.Equal(t, test.expectedErr, err != nil, "TestCase[%d]: %s", i, test.desc) + } +} + +func TestGetNextDiskLun(t *testing.T) { + testCases := []struct { + desc string + isDataDisksFull bool + expectedLun int32 + expectedErr bool + }{ + { + desc: "the minimal LUN shall be returned if there's enough room for extra disks", + isDataDisksFull: false, + expectedLun: 1, + expectedErr: false, + }, + { + desc: "LUN -1 and and error shall be returned if there's no available LUN", + isDataDisksFull: true, + expectedLun: -1, + expectedErr: true, + }, + } + + for i, test := range testCases { + testCloud := getTestCloud() + common := &controllerCommon{ + location: testCloud.Location, + storageEndpointSuffix: testCloud.Environment.StorageEndpointSuffix, + resourceGroup: testCloud.ResourceGroup, + subscriptionID: testCloud.SubscriptionID, + cloud: testCloud, + } + setTestVirtualMachines(testCloud, map[string]string{"vm1": "PowerState/Running"}, test.isDataDisksFull) + + lun, err := common.GetNextDiskLun("vm1") + assert.Equal(t, test.expectedLun, lun, "TestCase[%d]: %s", i, test.desc) + assert.Equal(t, test.expectedErr, err != nil, "TestCase[%d]: %s", i, test.desc) + } +} + +func TestDisksAreAttached(t *testing.T) { + testCases := []struct { + desc string + diskNames []string + nodeName types.NodeName + expectedAttached map[string]bool + expectedErr bool + }{ + { + desc: "an error shall be returned if there's no such instance corresponding to given nodeName", + diskNames: []string{"disk1"}, + nodeName: "vm2", + expectedAttached: map[string]bool{"disk1": false}, + expectedErr: false, + }, + { + desc: "proper attach map shall be returned if everything is good", + diskNames: []string{"disk1", "disk2"}, + nodeName: "vm1", + expectedAttached: map[string]bool{"disk1": true, "disk2": false}, + expectedErr: false, + }, + } + + for i, test := range testCases { + testCloud := getTestCloud() + common := &controllerCommon{ + location: testCloud.Location, + storageEndpointSuffix: testCloud.Environment.StorageEndpointSuffix, + resourceGroup: testCloud.ResourceGroup, + subscriptionID: testCloud.SubscriptionID, + cloud: testCloud, + } + setTestVirtualMachines(testCloud, map[string]string{"vm1": "PowerState/Running"}, false) + + 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) } } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_standard_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_standard_test.go new file mode 100644 index 00000000000..d24b3fb4640 --- /dev/null +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_standard_test.go @@ -0,0 +1,128 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package azure + +import ( + "testing" + + "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute" + "github.com/Azure/go-autorest/autorest/to" + "github.com/stretchr/testify/assert" + + "k8s.io/apimachinery/pkg/types" +) + +func TestStandardAttachDisk(t *testing.T) { + testCases := []struct { + desc string + nodeName types.NodeName + expectedErr bool + }{ + { + desc: "an error shall be returned if there's no corresponding vms", + nodeName: "vm2", + expectedErr: true, + }, + { + desc: "no error shall be returned if everything's good", + nodeName: "vm1", + expectedErr: false, + }, + } + + for i, test := range testCases { + testCloud := getTestCloud() + vmSet := testCloud.vmSet + setTestVirtualMachines(testCloud, map[string]string{"vm1": "PowerState/Running"}, false) + + err := vmSet.AttachDisk(true, "", + "uri", test.nodeName, 0, compute.CachingTypesReadOnly) + assert.Equal(t, test.expectedErr, err != nil, "TestCase[%d]: %s", i, test.desc) + } +} + +func TestStandardDetachDisk(t *testing.T) { + testCases := []struct { + desc string + nodeName types.NodeName + diskName string + expectedError bool + }{ + { + desc: "no error shall be returned if there's no corresponding vm", + nodeName: "vm2", + expectedError: false, + }, + { + desc: "no error shall be returned if there's no corresponding disk", + nodeName: "vm1", + diskName: "disk2", + expectedError: false, + }, + { + desc: "no error shall be returned if there's a corresponding disk", + nodeName: "vm1", + diskName: "disk1", + expectedError: false, + }, + } + + for i, test := range testCases { + testCloud := getTestCloud() + vmSet := testCloud.vmSet + setTestVirtualMachines(testCloud, map[string]string{"vm1": "PowerState/Running"}, false) + + _, err := vmSet.DetachDisk(test.diskName, "", test.nodeName) + assert.Equal(t, test.expectedError, err != nil, "TestCase[%d]: %s", i, test.desc) + } +} + +func TestGetDataDisks(t *testing.T) { + var testCases = []struct { + desc string + nodeName types.NodeName + expectedDataDisks []compute.DataDisk + expectedError bool + }{ + { + desc: "an error shall be returned if there's no corresponding vm", + nodeName: "vm2", + expectedDataDisks: nil, + expectedError: true, + }, + { + desc: "correct list of data disks shall be returned if everything is good", + nodeName: "vm1", + expectedDataDisks: []compute.DataDisk{ + { + Lun: to.Int32Ptr(0), + Name: to.StringPtr("disk1"), + }, + }, + expectedError: false, + }, + } + for i, test := range testCases { + testCloud := getTestCloud() + vmSet := testCloud.vmSet + setTestVirtualMachines(testCloud, map[string]string{"vm1": "PowerState/Running"}, false) + + dataDisks, err := vmSet.GetDataDisks(test.nodeName) + assert.Equal(t, test.expectedDataDisks, dataDisks, "TestCase[%d]: %s", i, test.desc) + assert.Equal(t, test.expectedError, err != nil, "TestCase[%d]: %s", i, test.desc) + } +} diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances_test.go index 71f8114a3d4..6116e9b9ea3 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances_test.go @@ -26,12 +26,13 @@ import ( "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute" "github.com/Azure/go-autorest/autorest/to" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" ) // setTestVirtualMachines sets test virtual machine with powerstate. -func setTestVirtualMachines(c *Cloud, vmList map[string]string) { +func setTestVirtualMachines(c *Cloud, vmList map[string]string, isDataDisksFull bool) { virtualMachineClient := c.VirtualMachinesClient.(*fakeAzureVirtualMachinesClient) store := map[string]map[string]compute.VirtualMachine{ "rg": make(map[string]compute.VirtualMachine), @@ -44,22 +45,34 @@ func setTestVirtualMachines(c *Cloud, vmList map[string]string) { ID: &instanceID, Location: &c.Location, } - if powerState != "" { - status := []compute.InstanceViewStatus{ - { - Code: to.StringPtr(powerState), - }, - { - Code: to.StringPtr("ProvisioningState/succeeded"), - }, - } - vm.VirtualMachineProperties = &compute.VirtualMachineProperties{ - InstanceView: &compute.VirtualMachineInstanceView{ - Statuses: &status, - }, - } + status := []compute.InstanceViewStatus{ + { + Code: to.StringPtr(powerState), + }, + { + Code: to.StringPtr("ProvisioningState/succeeded"), + }, + } + vm.VirtualMachineProperties = &compute.VirtualMachineProperties{ + InstanceView: &compute.VirtualMachineInstanceView{ + Statuses: &status, + }, + StorageProfile: &compute.StorageProfile{ + DataDisks: &[]compute.DataDisk{}, + }, + } + if !isDataDisksFull { + vm.StorageProfile.DataDisks = &[]compute.DataDisk{{ + Lun: to.Int32Ptr(0), + Name: to.StringPtr("disk1"), + }} + } else { + dataDisks := make([]compute.DataDisk, maxLUN) + for i := 0; i < maxLUN; i++ { + dataDisks[i] = compute.DataDisk{Lun: to.Int32Ptr(int32(i))} + } + vm.StorageProfile.DataDisks = &dataDisks } - store["rg"][nodeName] = vm } @@ -122,7 +135,7 @@ func TestInstanceID(t *testing.T) { for _, vm := range test.vmList { vmListWithPowerState[vm] = "" } - setTestVirtualMachines(cloud, vmListWithPowerState) + setTestVirtualMachines(cloud, vmListWithPowerState, false) instanceID, err := cloud.InstanceID(context.Background(), types.NodeName(test.nodeName)) if test.expectError { if err == nil { @@ -200,7 +213,7 @@ func TestInstanceShutdownByProviderID(t *testing.T) { for _, test := range testcases { cloud := getTestCloud() - setTestVirtualMachines(cloud, test.vmList) + setTestVirtualMachines(cloud, test.vmList, false) providerID := "azure://" + cloud.getStandardMachineID("rg", test.nodeName) hasShutdown, err := cloud.InstanceShutdownByProviderID(context.Background(), providerID) if test.expectError { diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_test.go index 08a02c1585f..c540711b548 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_test.go @@ -996,6 +996,7 @@ func getTestCloud() (az *Cloud) { az.lbCache, _ = az.newLBCache() az.nsgCache, _ = az.newNSGCache() az.rtCache, _ = az.newRouteTableCache() + az.controllerCommon = &controllerCommon{cloud: az} return az }