Skip to content

Commit

Permalink
Add documentation and unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
feiskyer committed Jul 30, 2018
1 parent 811e831 commit 6bfd2be
Show file tree
Hide file tree
Showing 9 changed files with 219 additions and 23 deletions.
4 changes: 4 additions & 0 deletions pkg/cloudprovider/providers/azure/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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 = [
Expand All @@ -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",
Expand Down
57 changes: 35 additions & 22 deletions pkg/cloudprovider/providers/azure/azure_managedDiskController.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand All @@ -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),
},
Expand All @@ -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()
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions pkg/cloudprovider/providers/azure/azure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()
Expand Down
73 changes: 73 additions & 0 deletions pkg/cloudprovider/providers/azure/azure_zones_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
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)
}
}
}
2 changes: 2 additions & 0 deletions pkg/volume/azure_dd/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand All @@ -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",
],
)
4 changes: 4 additions & 0 deletions pkg/volume/azure_dd/azure_provision.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
96 changes: 96 additions & 0 deletions pkg/volume/azure_dd/azure_provision_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
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))
}
}
}
1 change: 1 addition & 0 deletions plugin/pkg/admission/storage/persistentvolume/label/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 6bfd2be

Please sign in to comment.