Add documentation and unit tests

pkg/cloudprovider/providers/azure/BUILD

@@ -40,6 +40,7 @@ go_library(
         "//pkg/cloudprovider:go_default_library",
         "//pkg/cloudprovider/providers/azure/auth:go_default_library",
         "//pkg/controller:go_default_library",
+        "//pkg/kubelet/apis:go_default_library",
         "//pkg/version:go_default_library",
         "//pkg/volume:go_default_library",
         "//pkg/volume/util:go_default_library",
@@ -50,6 +51,7 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
+        "//staging/src/k8s.io/client-go/informers:go_default_library",
         "//staging/src/k8s.io/client-go/tools/cache:go_default_library",
         "//staging/src/k8s.io/client-go/util/flowcontrol:go_default_library",
         "//vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute:go_default_library",
@@ -82,6 +84,7 @@ go_test(
         "azure_vmss_cache_test.go",
         "azure_vmss_test.go",
         "azure_wrap_test.go",
+        "azure_zones_test.go",
     ],
     embed = [":go_default_library"],
     deps = [
@@ -92,6 +95,7 @@ go_test(
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//vendor/github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute:go_default_library",
         "//vendor/github.com/Azure/azure-sdk-for-go/services/network/mgmt/2017-09-01/network:go_default_library",
         "//vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage:go_default_library",

pkg/cloudprovider/providers/azure/azure_managedDiskController.go

@@ -43,16 +43,27 @@ type ManagedDiskController struct {
 
 // ManagedDiskOptions specifies the options of managed disks.
 type ManagedDiskOptions struct {
-	DiskName           string
-	SizeGB             int
-	PVCName            string
-	ResourceGroup      string
-	Zoned              bool
-	ZonePresent        bool
-	ZonesPresent       bool
-	AvailabilityZone   string
-	AvailabilityZones  string
-	Tags               map[string]string
+	// The name of the disk.
+	DiskName string
+	// The size in GB.
+	SizeGB int
+	// The name of PVC.
+	PVCName string
+	// The name of resource group.
+	ResourceGroup string
+	// Wether the disk is zoned.
+	Zoned bool
+	// Wether AvailabilityZone is set.
+	ZonePresent bool
+	// Wether AvailabilityZones is set.
+	ZonesPresent bool
+	// The AvailabilityZone to create the disk.
+	AvailabilityZone string
+	// List of AvailabilityZone to create the disk.
+	AvailabilityZones string
+	// The tags of the disk.
+	Tags map[string]string
+	// The SKU of storage account.
 	StorageAccountType storage.SkuName
 }
 
@@ -90,6 +101,18 @@ func (c *ManagedDiskController) CreateManagedDisk(options *ManagedDiskOptions) (
 		zones = make(sets.String)
 		zones.Insert(options.AvailabilityZone)
 	}
+	var createZones *[]string
+	if len(zones.List()) > 0 {
+		createAZ := util.ChooseZoneForVolume(zones, options.PVCName)
+		// Do not allow creation of disks in zones that are do not have nodes. Such disks
+		// are not currently usable.
+		if !activeZones.Has(createAZ) {
+			return "", fmt.Errorf("kubernetes does not have a node in zone %q", createAZ)
+		}
+
+		zoneList := []string{c.common.cloud.GetZoneID(createAZ)}
+		createZones = &zoneList
+	}
 
 	// insert original tags to newTags
 	newTags := make(map[string]*string)
@@ -108,6 +131,7 @@ func (c *ManagedDiskController) CreateManagedDisk(options *ManagedDiskOptions) (
 	model := compute.Disk{
 		Location: &c.common.location,
 		Tags:     newTags,
+		Zones:    createZones,
 		Sku: &compute.DiskSku{
 			Name: compute.StorageAccountTypes(options.StorageAccountType),
 		},
@@ -120,17 +144,6 @@ func (c *ManagedDiskController) CreateManagedDisk(options *ManagedDiskOptions) (
 	if options.ResourceGroup == "" {
 		options.ResourceGroup = c.common.resourceGroup
 	}
-	if len(zones.List()) > 0 {
-		createAZ := util.ChooseZoneForVolume(zones, options.PVCName)
-		// Do not allow creation of disks in zones that are do not have nodes. Such disks
-		// are not currently usable.
-		if !activeZones.Has(createAZ) {
-			return "", fmt.Errorf("kubernetes does not have a node in zone %q", createAZ)
-		}
-
-		createZones := []string{c.common.cloud.GetZoneID(createAZ)}
-		model.Zones = &createZones
-	}
 
 	ctx, cancel := getContextWithCancel()
 	defer cancel()
@@ -307,7 +320,7 @@ func (c *Cloud) GetAzureDiskLabels(diskURI string) (map[string]string, error) {
 	}
 
 	zone := c.makeZone(zoneID)
-	glog.V(4).Infof("Get zone %q for Azure disk %q", zone, diskName)
+	glog.V(4).Infof("Got zone %q for Azure disk %q", zone, diskName)
 	labels := map[string]string{
 		kubeletapis.LabelZoneRegion:        c.Location,
 		kubeletapis.LabelZoneFailureDomain: zone,

pkg/cloudprovider/providers/azure/azure_test.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/sets"
 	serviceapi "k8s.io/kubernetes/pkg/api/v1/service"
 	"k8s.io/kubernetes/pkg/cloudprovider/providers/azure/auth"
 	kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis"
@@ -953,6 +954,8 @@ func getTestCloud() (az *Cloud) {
 			MaximumLoadBalancerRuleCount: 250,
 			VMType: vmTypeStandard,
 		},
+		nodeZones:          map[string]sets.String{},
+		nodeInformerSynced: func() bool { return true },
 	}
 	az.DisksClient = newFakeDisksClient()
 	az.InterfacesClient = newFakeAzureInterfacesClient()

pkg/cloudprovider/providers/azure/azure_zones_test.go

+/*
+Copyright 2018 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"
+)
+
+func TestIsAvailabilityZone(t *testing.T) {
+	location := "eastus"
+	az := &Cloud{
+		Config: Config{
+			Location: location,
+		},
+	}
+	tests := []struct {
+		desc     string
+		zone     string
+		expected bool
+	}{
+		{"empty string should return false", "", false},
+		{"wrong farmat should return false", "123", false},
+		{"wrong location should return false", "chinanorth-1", false},
+		{"correct zone should return true", "eastus-1", true},
+	}
+
+	for _, test := range tests {
+		actual := az.isAvailabilityZone(test.zone)
+		if actual != test.expected {
+			t.Errorf("test [%q] get unexpected result: %v != %v", test.desc, actual, test.expected)
+		}
+	}
+}
+
+func TestGetZoneID(t *testing.T) {
+	location := "eastus"
+	az := &Cloud{
+		Config: Config{
+			Location: location,
+		},
+	}
+	tests := []struct {
+		desc     string
+		zone     string
+		expected string
+	}{
+		{"empty string should return empty string", "", ""},
+		{"wrong farmat should return empty string", "123", ""},
+		{"wrong location should return empty string", "chinanorth-1", ""},
+		{"correct zone should return zone ID", "eastus-1", "1"},
+	}
+
+	for _, test := range tests {
+		actual := az.GetZoneID(test.zone)
+		if actual != test.expected {
+			t.Errorf("test [%q] get unexpected result: %q != %q", test.desc, actual, test.expected)
+		}
+	}
+}

pkg/volume/azure_dd/BUILD

@@ -63,6 +63,7 @@ go_test(
         "azure_common_test.go",
         "azure_dd_block_test.go",
         "azure_dd_test.go",
+        "azure_provision_test.go",
     ],
     embed = [":go_default_library"],
     deps = [
@@ -73,5 +74,6 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
         "//staging/src/k8s.io/client-go/util/testing:go_default_library",
+        "//vendor/github.com/stretchr/testify/assert:go_default_library",
     ],
 )

pkg/volume/azure_dd/azure_provision.go

@@ -82,6 +82,10 @@ func parseZoned(zonedString string, kind v1.AzureDataDiskKind) (bool, error) {
 		return false, fmt.Errorf("failed to parse 'zoned': %v", err)
 	}
 
+	if zoned && kind != v1.AzureManagedDisk {
+		return false, fmt.Errorf("zoned is only supported by managed disks")
+	}
+
 	return zoned, nil
 }
 

pkg/volume/azure_dd/azure_provision_test.go

+/*
+Copyright 2018 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_dd
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"k8s.io/api/core/v1"
+)
+
+func TestParseZoned(t *testing.T) {
+	tests := []struct {
+		msg         string
+		zoneString  string
+		diskKind    v1.AzureDataDiskKind
+		expected    bool
+		expectError bool
+	}{
+		{
+			msg:      "managed disk should default to zoned",
+			diskKind: v1.AzureManagedDisk,
+			expected: true,
+		},
+		{
+			msg:      "shared blob disk should default to un-zoned",
+			diskKind: v1.AzureSharedBlobDisk,
+			expected: false,
+		},
+		{
+			msg:      "shared dedicated disk should default to un-zoned",
+			diskKind: v1.AzureDedicatedBlobDisk,
+			expected: false,
+		},
+		{
+			msg:        "managed disk should support zoned=true",
+			diskKind:   v1.AzureManagedDisk,
+			zoneString: "true",
+			expected:   true,
+		},
+		{
+			msg:        "managed disk should support zoned=false",
+			diskKind:   v1.AzureManagedDisk,
+			zoneString: "false",
+			expected:   false,
+		},
+		{
+			msg:        "shared blob disk should support zoned=false",
+			diskKind:   v1.AzureSharedBlobDisk,
+			zoneString: "false",
+			expected:   false,
+		},
+		{
+			msg:         "shared blob disk shouldn't support zoned=true",
+			diskKind:    v1.AzureSharedBlobDisk,
+			zoneString:  "true",
+			expectError: true,
+		},
+		{
+			msg:        "shared dedicated disk should support zoned=false",
+			diskKind:   v1.AzureDedicatedBlobDisk,
+			zoneString: "false",
+			expected:   false,
+		},
+		{
+			msg:         "dedicated blob disk shouldn't support zoned=true",
+			diskKind:    v1.AzureDedicatedBlobDisk,
+			zoneString:  "true",
+			expectError: true,
+		},
+	}
+
+	for i, test := range tests {
+		real, err := parseZoned(test.zoneString, test.diskKind)
+		if test.expectError {
+			assert.Error(t, err, fmt.Sprintf("TestCase[%d]: %s", i, test.msg))
+		} else {
+			assert.Equal(t, test.expected, real, fmt.Sprintf("TestCase[%d]: %s", i, test.msg))
+		}
+	}
+}

plugin/pkg/admission/storage/persistentvolume/label/BUILD

@@ -17,6 +17,7 @@ go_library(
         "//pkg/apis/core:go_default_library",
         "//pkg/cloudprovider:go_default_library",
         "//pkg/cloudprovider/providers/aws:go_default_library",
+        "//pkg/cloudprovider/providers/azure:go_default_library",
         "//pkg/cloudprovider/providers/gce:go_default_library",
         "//pkg/features:go_default_library",
         "//pkg/kubeapiserver/admission:go_default_library",

plugin/pkg/admission/storage/persistentvolume/label/admission.go

@@ -299,7 +299,7 @@ func (l *persistentVolumeLabel) getAzureCloudProvider() (*azure.Cloud, error) {
 		azureProvider, ok := cloudProvider.(*azure.Cloud)
 		if !ok {
 			// GetCloudProvider has gone very wrong
-			return nil, fmt.Errorf("error retrieving GCE cloud provider")
+			return nil, fmt.Errorf("error retrieving Azure cloud provider")
 		}
 		l.azureProvider = azureProvider
 	}