From dd63d407a403a87f99b60de1dd5a138db1f1049c Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Tue, 24 Jul 2018 02:00:57 +0000 Subject: [PATCH] Enable dynamic azure disk volume limits use API to get max disk num use continue when got exception add logging add cache and unit test fix comments --- pkg/cloudprovider/providers/azure/azure.go | 4 ++ .../providers/azure/azure_client.go | 43 ++++++++++++ pkg/volume/azure_dd/BUILD | 2 + pkg/volume/azure_dd/azure_dd.go | 68 ++++++++++++++++--- pkg/volume/azure_dd/azure_dd_test.go | 38 +++++++++++ 5 files changed, 144 insertions(+), 11 deletions(-) diff --git a/pkg/cloudprovider/providers/azure/azure.go b/pkg/cloudprovider/providers/azure/azure.go index 12a5cee0ad..f9fa926853 100644 --- a/pkg/cloudprovider/providers/azure/azure.go +++ b/pkg/cloudprovider/providers/azure/azure.go @@ -167,6 +167,9 @@ type Cloud struct { VirtualMachineScaleSetsClient VirtualMachineScaleSetsClient VirtualMachineScaleSetVMsClient VirtualMachineScaleSetVMsClient + // client for vm sizes list + VirtualMachineSizesClient VirtualMachineSizesClient + vmCache *timedCache lbCache *timedCache nsgCache *timedCache @@ -268,6 +271,7 @@ func NewCloud(configReader io.Reader) (cloudprovider.Interface, error) { StorageAccountClient: newAzStorageAccountClient(azClientConfig), VirtualMachinesClient: newAzVirtualMachinesClient(azClientConfig), PublicIPAddressesClient: newAzPublicIPAddressesClient(azClientConfig), + VirtualMachineSizesClient: newAzVirtualMachineSizesClient(azClientConfig), VirtualMachineScaleSetsClient: newAzVirtualMachineScaleSetsClient(azClientConfig), VirtualMachineScaleSetVMsClient: newAzVirtualMachineScaleSetVMsClient(azClientConfig), FileClient: &azureFileClient{env: *env}, diff --git a/pkg/cloudprovider/providers/azure/azure_client.go b/pkg/cloudprovider/providers/azure/azure_client.go index d5393e1d45..19d5e7a158 100644 --- a/pkg/cloudprovider/providers/azure/azure_client.go +++ b/pkg/cloudprovider/providers/azure/azure_client.go @@ -131,6 +131,11 @@ type DisksClient interface { Get(ctx context.Context, resourceGroupName string, diskName string) (result compute.Disk, err error) } +// VirtualMachineSizesClient defines needed functions for azure compute.VirtualMachineSizesClient +type VirtualMachineSizesClient interface { + List(ctx context.Context, location string) (result compute.VirtualMachineSizeListResult, err error) +} + // azClientConfig contains all essential information to create an Azure client. type azClientConfig struct { subscriptionID string @@ -1327,3 +1332,41 @@ func (az *azDisksClient) Get(ctx context.Context, resourceGroupName string, disk mc.Observe(err) return } + +// azVirtualMachineSizesClient implements VirtualMachineSizesClient. +type azVirtualMachineSizesClient struct { + client compute.VirtualMachineSizesClient + rateLimiterReader flowcontrol.RateLimiter + rateLimiterWriter flowcontrol.RateLimiter +} + +func newAzVirtualMachineSizesClient(config *azClientConfig) *azVirtualMachineSizesClient { + VirtualMachineSizesClient := compute.NewVirtualMachineSizesClient(config.subscriptionID) + VirtualMachineSizesClient.BaseURI = config.resourceManagerEndpoint + VirtualMachineSizesClient.Authorizer = autorest.NewBearerAuthorizer(config.servicePrincipalToken) + VirtualMachineSizesClient.PollingDelay = 5 * time.Second + configureUserAgent(&VirtualMachineSizesClient.Client) + + return &azVirtualMachineSizesClient{ + rateLimiterReader: config.rateLimiterReader, + rateLimiterWriter: config.rateLimiterWriter, + client: VirtualMachineSizesClient, + } +} + +func (az *azVirtualMachineSizesClient) List(ctx context.Context, location string) (result compute.VirtualMachineSizeListResult, err error) { + if !az.rateLimiterReader.TryAccept() { + err = createRateLimitErr(false, "VMSizesList") + return + } + + glog.V(10).Infof("azVirtualMachineSizesClient.List(%q): start", location) + defer func() { + glog.V(10).Infof("azVirtualMachineSizesClient.List(%q): end", location) + }() + + mc := newMetricContext("vmsizes", "list", "", az.client.SubscriptionID) + result, err = az.client.List(ctx, location) + mc.Observe(err) + return +} diff --git a/pkg/volume/azure_dd/BUILD b/pkg/volume/azure_dd/BUILD index c41804fc5d..647bb91980 100644 --- a/pkg/volume/azure_dd/BUILD +++ b/pkg/volume/azure_dd/BUILD @@ -75,7 +75,9 @@ 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/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute:go_default_library", "//vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage:go_default_library", + "//vendor/github.com/Azure/go-autorest/autorest/to:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", ], ) diff --git a/pkg/volume/azure_dd/azure_dd.go b/pkg/volume/azure_dd/azure_dd.go index 4adfc24eb3..5e237f5e55 100644 --- a/pkg/volume/azure_dd/azure_dd.go +++ b/pkg/volume/azure_dd/azure_dd.go @@ -17,7 +17,9 @@ limitations under the License. package azure_dd import ( + "context" "fmt" + "strings" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute" "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2017-10-01/storage" @@ -84,8 +86,12 @@ var _ volume.VolumePluginWithAttachLimits = &azureDataDiskPlugin{} var _ volume.ExpandableVolumePlugin = &azureDataDiskPlugin{} var _ volume.DeviceMountableVolumePlugin = &azureDataDiskPlugin{} +// store vm size list in current region +var vmSizeList *[]compute.VirtualMachineSize + const ( azureDataDiskPluginName = "kubernetes.io/azure-disk" + defaultAzureVolumeLimit = 16 ) func ProbeVolumePlugins() []volume.VolumePlugin { @@ -129,26 +135,66 @@ func (plugin *azureDataDiskPlugin) SupportsBulkVolumeVerification() bool { func (plugin *azureDataDiskPlugin) GetVolumeLimits() (map[string]int64, error) { volumeLimits := map[string]int64{ - util.AzureVolumeLimitKey: 16, + util.AzureVolumeLimitKey: defaultAzureVolumeLimit, } - cloud := plugin.host.GetCloudProvider() - - // if we can't fetch cloudprovider we return an error - // hoping external CCM or admin can set it. Returning - // default values from here will mean, no one can - // override them. - if cloud == nil { - return nil, fmt.Errorf("No cloudprovider present") + az, err := getCloud(plugin.host) + if err != nil { + // if we can't fetch cloudprovider we return an error + // hoping external CCM or admin can set it. Returning + // default values from here will mean, no one can + // override them. + glog.Errorf("failed to get azure cloud in GetVolumeLimits, plugin.host: %s", plugin.host.GetHostName()) + return volumeLimits, nil } - if cloud.ProviderName() != azure.CloudProviderName { - return nil, fmt.Errorf("Expected Azure cloudprovider, got %s", cloud.ProviderName()) + instances, ok := az.Instances() + if !ok { + glog.Warningf("Failed to get instances from cloud provider") + return volumeLimits, nil + } + + instanceType, err := instances.InstanceType(context.TODO(), plugin.host.GetNodeName()) + if err != nil { + glog.Errorf("Failed to get instance type from Azure cloud provider, nodeName: %s", plugin.host.GetNodeName()) + return volumeLimits, nil + } + + if vmSizeList == nil { + result, err := az.VirtualMachineSizesClient.List(context.TODO(), az.Location) + if err != nil || result.Value == nil { + glog.Errorf("failed to list vm sizes in GetVolumeLimits, plugin.host: %s, location: %s", plugin.host.GetHostName(), az.Location) + return volumeLimits, nil + } + vmSizeList = result.Value + } + + volumeLimits = map[string]int64{ + util.AzureVolumeLimitKey: getMaxDataDiskCount(instanceType, vmSizeList), } return volumeLimits, nil } +func getMaxDataDiskCount(instanceType string, sizeList *[]compute.VirtualMachineSize) int64 { + if sizeList == nil { + return defaultAzureVolumeLimit + } + + vmsize := strings.ToUpper(instanceType) + for _, size := range *sizeList { + if size.Name == nil || size.MaxDataDiskCount == nil { + glog.Errorf("failed to get vm size in getMaxDataDiskCount") + continue + } + if strings.ToUpper(*size.Name) == vmsize { + glog.V(2).Infof("got a matching size in getMaxDataDiskCount, Name: %s, MaxDataDiskCount: %s", *size.Name, *size.MaxDataDiskCount) + return int64(*size.MaxDataDiskCount) + } + } + return defaultAzureVolumeLimit +} + func (plugin *azureDataDiskPlugin) VolumeLimitKey(spec *volume.Spec) string { return util.AzureVolumeLimitKey } diff --git a/pkg/volume/azure_dd/azure_dd_test.go b/pkg/volume/azure_dd/azure_dd_test.go index e1d24c9141..fa97c4951e 100644 --- a/pkg/volume/azure_dd/azure_dd_test.go +++ b/pkg/volume/azure_dd/azure_dd_test.go @@ -20,6 +20,10 @@ import ( "os" "testing" + "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute" + "github.com/Azure/go-autorest/autorest/to" + "github.com/stretchr/testify/assert" + "k8s.io/api/core/v1" utiltesting "k8s.io/client-go/util/testing" "k8s.io/kubernetes/pkg/volume" @@ -53,3 +57,37 @@ func TestCanSupport(t *testing.T) { // fakeAzureProvider type was removed because all functions were not used // Testing mounting will require path calculation which depends on the cloud provider, which is faked in the above test. + +func TestGetMaxDataDiskCount(t *testing.T) { + tests := []struct { + instanceType string + sizeList *[]compute.VirtualMachineSize + expectResult int64 + }{ + { + instanceType: "standard_d2_v2", + sizeList: &[]compute.VirtualMachineSize{ + {Name: to.StringPtr("Standard_D2_V2"), MaxDataDiskCount: to.Int32Ptr(8)}, + {Name: to.StringPtr("Standard_D3_V2"), MaxDataDiskCount: to.Int32Ptr(16)}, + }, + expectResult: 8, + }, + { + instanceType: "NOT_EXISTING", + sizeList: &[]compute.VirtualMachineSize{ + {Name: to.StringPtr("Standard_D2_V2"), MaxDataDiskCount: to.Int32Ptr(8)}, + }, + expectResult: defaultAzureVolumeLimit, + }, + { + instanceType: "", + sizeList: &[]compute.VirtualMachineSize{}, + expectResult: defaultAzureVolumeLimit, + }, + } + + for _, test := range tests { + result := getMaxDataDiskCount(test.instanceType, test.sizeList) + assert.Equal(t, test.expectResult, result) + } +}