Add unit tests for azure_controller_common.go and azure_controller_standard.go

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",

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)
 	}
 }

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)
+	}
+}

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 {

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
 }