From 156602408ade3f4e20c43a1390cf0c1b6f61f6ef Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Mon, 18 Feb 2019 05:20:35 +0000 Subject: [PATCH 1/2] remove get azure accounts in the init process set timeout for get azure account operation use const for timeout value remove get azure accounts in the init process add lock for account init --- pkg/cloudprovider/providers/azure/azure.go | 18 ++------------- .../azure/azure_blobDiskController.go | 23 +++++++++++-------- .../azure/azure_managedDiskController.go | 4 ---- 3 files changed, 15 insertions(+), 30 deletions(-) diff --git a/pkg/cloudprovider/providers/azure/azure.go b/pkg/cloudprovider/providers/azure/azure.go index 25650e022b7..62da8dc27e3 100644 --- a/pkg/cloudprovider/providers/azure/azure.go +++ b/pkg/cloudprovider/providers/azure/azure.go @@ -490,22 +490,8 @@ func initDiskControllers(az *Cloud) error { cloud: az, } - // BlobDiskController: contains the function needed to - // create/attach/detach/delete blob based (unmanaged disks) - blobController, err := newBlobDiskController(common) - if err != nil { - return fmt.Errorf("AzureDisk - failed to init Blob Disk Controller with error (%s)", err.Error()) - } - - // ManagedDiskController: contains the functions needed to - // create/attach/detach/delete managed disks - managedController, err := newManagedDiskController(common) - if err != nil { - return fmt.Errorf("AzureDisk - failed to init Managed Disk Controller with error (%s)", err.Error()) - } - - az.BlobDiskController = blobController - az.ManagedDiskController = managedController + az.BlobDiskController = &BlobDiskController{common: common} + az.ManagedDiskController = &ManagedDiskController{common: common} az.controllerCommon = common return nil diff --git a/pkg/cloudprovider/providers/azure/azure_blobDiskController.go b/pkg/cloudprovider/providers/azure/azure_blobDiskController.go index 1ff0a892cd4..2323a860f2a 100644 --- a/pkg/cloudprovider/providers/azure/azure_blobDiskController.go +++ b/pkg/cloudprovider/providers/azure/azure_blobDiskController.go @@ -62,18 +62,19 @@ var ( accountsLock = &sync.Mutex{} ) -func newBlobDiskController(common *controllerCommon) (*BlobDiskController, error) { - c := BlobDiskController{common: common} +func (c *BlobDiskController) initStorageAccounts() { + accountsLock.Lock() + defer accountsLock.Unlock() - // get accounts - accounts, err := c.getAllStorageAccounts() - if err != nil { - klog.Errorf("azureDisk - getAllStorageAccounts error: %v", err) - c.accounts = make(map[string]*storageAccountState) - return &c, nil + if c.accounts == nil { + // get accounts + accounts, err := c.getAllStorageAccounts() + if err != nil { + klog.Errorf("azureDisk - getAllStorageAccounts error: %v", err) + c.accounts = make(map[string]*storageAccountState) + } + c.accounts = accounts } - c.accounts = accounts - return &c, nil } // CreateVolume creates a VHD blob in a storage account that has storageType and location using the given storage account. @@ -217,6 +218,8 @@ func (c *BlobDiskController) deleteVhdBlob(accountName, accountKey, blobName str func (c *BlobDiskController) CreateBlobDisk(dataDiskName string, storageAccountType storage.SkuName, sizeGB int) (string, error) { klog.V(4).Infof("azureDisk - creating blob data disk named:%s on StorageAccountType:%s", dataDiskName, storageAccountType) + c.initStorageAccounts() + storageAccountName, err := c.findSANameForDisk(storageAccountType) if err != nil { return "", err diff --git a/pkg/cloudprovider/providers/azure/azure_managedDiskController.go b/pkg/cloudprovider/providers/azure/azure_managedDiskController.go index efdbb67275e..ba0d1b81bd8 100644 --- a/pkg/cloudprovider/providers/azure/azure_managedDiskController.go +++ b/pkg/cloudprovider/providers/azure/azure_managedDiskController.go @@ -67,10 +67,6 @@ type ManagedDiskOptions struct { DiskMBpsReadWrite string } -func newManagedDiskController(common *controllerCommon) (*ManagedDiskController, error) { - return &ManagedDiskController{common: common}, nil -} - //CreateManagedDisk : create managed disk func (c *ManagedDiskController) CreateManagedDisk(options *ManagedDiskOptions) (string, error) { var err error From 8cd09bb143669a3dc21acac9964647d78f283c4e Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Fri, 22 Feb 2019 06:50:48 +0000 Subject: [PATCH 2/2] add timeout in GetVolumeLimits operation add timeout for getAllStorageAccounts --- .../providers/azure/azure_blobDiskController.go | 3 ++- pkg/volume/azure_dd/azure_dd.go | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/cloudprovider/providers/azure/azure_blobDiskController.go b/pkg/cloudprovider/providers/azure/azure_blobDiskController.go index 2323a860f2a..2f1eb20c537 100644 --- a/pkg/cloudprovider/providers/azure/azure_blobDiskController.go +++ b/pkg/cloudprovider/providers/azure/azure_blobDiskController.go @@ -18,6 +18,7 @@ package azure import ( "bytes" + "context" "encoding/binary" "fmt" "net/url" @@ -439,7 +440,7 @@ func (c *BlobDiskController) getDiskCount(SAName string) (int, error) { } func (c *BlobDiskController) getAllStorageAccounts() (map[string]*storageAccountState, error) { - ctx, cancel := getContextWithCancel() + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() accountListResult, err := c.common.cloud.StorageAccountClient.ListByResourceGroup(ctx, c.common.resourceGroup) if err != nil { diff --git a/pkg/volume/azure_dd/azure_dd.go b/pkg/volume/azure_dd/azure_dd.go index b130c3ca355..e9a850f9016 100644 --- a/pkg/volume/azure_dd/azure_dd.go +++ b/pkg/volume/azure_dd/azure_dd.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "strings" + "time" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-10-01/compute" "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2018-07-01/storage" @@ -164,7 +165,9 @@ func (plugin *azureDataDiskPlugin) GetVolumeLimits() (map[string]int64, error) { } if vmSizeList == nil { - result, err := az.VirtualMachineSizesClient.List(context.TODO(), az.Location) + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + result, err := az.VirtualMachineSizesClient.List(ctx, az.Location) if err != nil || result.Value == nil { klog.Errorf("failed to list vm sizes in GetVolumeLimits, plugin.host: %s, location: %s", plugin.host.GetHostName(), az.Location) return volumeLimits, nil