From bb42103fcafdd989e8776abbc8374c2c592fc356 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Thu, 9 Nov 2017 09:33:58 +0000 Subject: [PATCH 1/2] move InitStorageAccount into azure disk provision func --- .../providers/azure/azure_blobDiskController.go | 17 ++--------------- pkg/volume/azure_dd/azure_dd.go | 2 ++ pkg/volume/azure_dd/azure_provision.go | 3 +++ 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/pkg/cloudprovider/providers/azure/azure_blobDiskController.go b/pkg/cloudprovider/providers/azure/azure_blobDiskController.go index d738599eabc..a3e9248378e 100644 --- a/pkg/cloudprovider/providers/azure/azure_blobDiskController.go +++ b/pkg/cloudprovider/providers/azure/azure_blobDiskController.go @@ -22,7 +22,6 @@ import ( "fmt" "math" "net/url" - "os" "regexp" "sync" @@ -70,12 +69,6 @@ var accountsLock = &sync.Mutex{} func newBlobDiskController(common *controllerCommon) (*BlobDiskController, error) { c := BlobDiskController{common: common} - err := c.init() - - if err != nil { - return nil, err - } - return &c, nil } @@ -316,7 +309,7 @@ func (c *BlobDiskController) DeleteBlobDisk(diskURI string, wasForced bool) erro // Init tries best effort to ensure that 2 accounts standard/premium were created // to be used by shared blob disks. This to increase the speed pvc provisioning (in most of cases) -func (c *BlobDiskController) init() error { +func (c *BlobDiskController) InitStorageAccount() error { if !c.shouldInit() { return nil } @@ -543,13 +536,7 @@ func (c *BlobDiskController) getDiskCount(SAName string) (int, error) { // and we only do that in the controller func (c *BlobDiskController) shouldInit() bool { - if os.Args[0] == "kube-controller-manager" || (os.Args[0] == "/hyperkube" && os.Args[1] == "controller-manager") { - swapped := atomic.CompareAndSwapInt64(&initFlag, 0, 1) - if swapped { - return true - } - } - return false + return atomic.CompareAndSwapInt64(&initFlag, 0, 1) } func (c *BlobDiskController) getAllStorageAccounts() (map[string]*storageAccountState, error) { diff --git a/pkg/volume/azure_dd/azure_dd.go b/pkg/volume/azure_dd/azure_dd.go index bb45bf3b4d9..2b40c41dead 100644 --- a/pkg/volume/azure_dd/azure_dd.go +++ b/pkg/volume/azure_dd/azure_dd.go @@ -28,6 +28,8 @@ import ( // interface exposed by the cloud provider implementing Disk functionlity type DiskController interface { + InitStorageAccount() error + CreateBlobDisk(dataDiskName string, storageAccountType storage.SkuName, sizeGB int, forceStandAlone bool) (string, error) DeleteBlobDisk(diskUri string, wasForced bool) error diff --git a/pkg/volume/azure_dd/azure_provision.go b/pkg/volume/azure_dd/azure_provision.go index d037f636d80..230be9211dc 100644 --- a/pkg/volume/azure_dd/azure_provision.go +++ b/pkg/volume/azure_dd/azure_provision.go @@ -168,6 +168,9 @@ func (p *azureDiskProvisioner) Provision() (*v1.PersistentVolume, error) { } } } else { + if err = diskController.InitStorageAccount(); err != nil { + return nil, err + } diskURI, err = diskController.CreateBlobDisk(name, skuName, requestGB, forceStandAlone) if err != nil { return nil, err From 3d60d180025e796811a26c60133f45eb0a63d2b4 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Fri, 17 Nov 2017 07:27:01 +0000 Subject: [PATCH 2/2] remove initialize storage account pool process remove init func allow getStorageAccounts failed --- .../azure/azure_blobDiskController.go | 73 +++---------------- pkg/volume/azure_dd/azure_dd.go | 2 - pkg/volume/azure_dd/azure_provision.go | 3 - 3 files changed, 10 insertions(+), 68 deletions(-) diff --git a/pkg/cloudprovider/providers/azure/azure_blobDiskController.go b/pkg/cloudprovider/providers/azure/azure_blobDiskController.go index a3e9248378e..32c009483b4 100644 --- a/pkg/cloudprovider/providers/azure/azure_blobDiskController.go +++ b/pkg/cloudprovider/providers/azure/azure_blobDiskController.go @@ -20,7 +20,6 @@ import ( "bytes" "encoding/binary" "fmt" - "math" "net/url" "regexp" "sync" @@ -63,12 +62,21 @@ type BlobDiskController struct { var defaultContainerName = "" var storageAccountNamePrefix = "" var storageAccountNameMatch = "" -var initFlag int64 var accountsLock = &sync.Mutex{} func newBlobDiskController(common *controllerCommon) (*BlobDiskController, error) { c := BlobDiskController{common: common} + c.setUniqueStrings() + + // get accounts + accounts, err := c.getAllStorageAccounts() + if err != nil { + glog.Errorf("azureDisk - getAllStorageAccounts error: %v", err) + c.accounts = make(map[string]*storageAccountState) + return &c, nil + } + c.accounts = accounts return &c, nil } @@ -307,60 +315,6 @@ func (c *BlobDiskController) DeleteBlobDisk(diskURI string, wasForced bool) erro return err } -// Init tries best effort to ensure that 2 accounts standard/premium were created -// to be used by shared blob disks. This to increase the speed pvc provisioning (in most of cases) -func (c *BlobDiskController) InitStorageAccount() error { - if !c.shouldInit() { - return nil - } - - c.setUniqueStrings() - - // get accounts - accounts, err := c.getAllStorageAccounts() - if err != nil { - return err - } - c.accounts = accounts - - if len(c.accounts) == 0 { - counter := 1 - for counter <= storageAccountsCountInit { - - accountType := storage.PremiumLRS - if n := math.Mod(float64(counter), 2); n == 0 { - accountType = storage.StandardLRS - } - - // We don't really care if these calls failed - // at this stage, we are trying to ensure 2 accounts (Standard/Premium) - // are there ready for PVC creation - - // if we failed here, the accounts will be created in the process - // of creating PVC - - // nor do we care if they were partially created, as the entire - // account creation process is idempotent - go func(thisNext int) { - newAccountName := getAccountNameForNum(thisNext) - - glog.Infof("azureDisk - BlobDiskController init process will create new storageAccount:%s type:%s", newAccountName, accountType) - err := c.createStorageAccount(newAccountName, accountType, c.common.location, true) - // TODO return created and error from - if err != nil { - glog.Infof("azureDisk - BlobDiskController init: create account %s with error:%s", newAccountName, err.Error()) - - } else { - glog.Infof("azureDisk - BlobDiskController init: created account %s", newAccountName) - } - }(counter) - counter = counter + 1 - } - } - - return nil -} - //Sets unique strings to be used as accountnames && || blob containers names func (c *BlobDiskController) setUniqueStrings() { uniqueString := c.common.resourceGroup + c.common.location + c.common.subscriptionID @@ -532,13 +486,6 @@ func (c *BlobDiskController) getDiskCount(SAName string) (int, error) { return int(c.accounts[SAName].diskCount), nil } -// shouldInit ensures that we only init the plugin once -// and we only do that in the controller - -func (c *BlobDiskController) shouldInit() bool { - return atomic.CompareAndSwapInt64(&initFlag, 0, 1) -} - func (c *BlobDiskController) getAllStorageAccounts() (map[string]*storageAccountState, error) { accountListResult, err := c.common.cloud.StorageAccountClient.List() if err != nil { diff --git a/pkg/volume/azure_dd/azure_dd.go b/pkg/volume/azure_dd/azure_dd.go index 2b40c41dead..bb45bf3b4d9 100644 --- a/pkg/volume/azure_dd/azure_dd.go +++ b/pkg/volume/azure_dd/azure_dd.go @@ -28,8 +28,6 @@ import ( // interface exposed by the cloud provider implementing Disk functionlity type DiskController interface { - InitStorageAccount() error - CreateBlobDisk(dataDiskName string, storageAccountType storage.SkuName, sizeGB int, forceStandAlone bool) (string, error) DeleteBlobDisk(diskUri string, wasForced bool) error diff --git a/pkg/volume/azure_dd/azure_provision.go b/pkg/volume/azure_dd/azure_provision.go index 230be9211dc..d037f636d80 100644 --- a/pkg/volume/azure_dd/azure_provision.go +++ b/pkg/volume/azure_dd/azure_provision.go @@ -168,9 +168,6 @@ func (p *azureDiskProvisioner) Provision() (*v1.PersistentVolume, error) { } } } else { - if err = diskController.InitStorageAccount(); err != nil { - return nil, err - } diskURI, err = diskController.CreateBlobDisk(name, skuName, requestGB, forceStandAlone) if err != nil { return nil, err