New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix CreateVolume func: use search mode instead #54687
Merged
k8s-github-robot
merged 2 commits into
kubernetes:master
from
andyzhangx:createvolume-fix
Nov 30, 2017
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,11 +59,12 @@ type BlobDiskController struct { | |
accounts map[string]*storageAccountState | ||
} | ||
|
||
var defaultContainerName = "" | ||
var storageAccountNamePrefix = "" | ||
var storageAccountNameMatch = "" | ||
|
||
var accountsLock = &sync.Mutex{} | ||
var ( | ||
defaultContainerName = "" | ||
storageAccountNamePrefix = "" | ||
storageAccountNameMatch = "" | ||
accountsLock = &sync.Mutex{} | ||
) | ||
|
||
func newBlobDiskController(common *controllerCommon) (*BlobDiskController, error) { | ||
c := BlobDiskController{common: common} | ||
|
@@ -80,52 +81,49 @@ func newBlobDiskController(common *controllerCommon) (*BlobDiskController, error | |
return &c, nil | ||
} | ||
|
||
// CreateVolume creates a VHD blob in a given storage account, will create the given storage account if it does not exist in current resource group | ||
func (c *BlobDiskController) CreateVolume(name, storageAccount string, storageAccountType storage.SkuName, location string, requestGB int) (string, string, int, error) { | ||
key, err := c.common.cloud.getStorageAccesskey(storageAccount) | ||
if err != nil { | ||
glog.V(2).Infof("azureDisk - no key found for storage account %s in resource group %s, begin to create a new storage account", storageAccount, c.common.resourceGroup) | ||
|
||
cp := storage.AccountCreateParameters{ | ||
Sku: &storage.Sku{Name: storageAccountType}, | ||
Tags: &map[string]*string{"created-by": to.StringPtr("azure-dd")}, | ||
Location: &location} | ||
cancel := make(chan struct{}) | ||
|
||
_, errchan := c.common.cloud.StorageAccountClient.Create(c.common.resourceGroup, storageAccount, cp, cancel) | ||
err = <-errchan | ||
if err != nil { | ||
return "", "", 0, fmt.Errorf(fmt.Sprintf("Create Storage Account %s, error: %s", storageAccount, err)) | ||
} | ||
|
||
key, err = c.common.cloud.getStorageAccesskey(storageAccount) | ||
// CreateVolume creates a VHD blob in a storage account that has storageType and location using the given storage account. | ||
// If no storage account is given, search all the storage accounts associated with the resource group and pick one that | ||
// fits storage type and location. | ||
func (c *BlobDiskController) CreateVolume(name, storageAccount, storageAccountType, location string, requestGB int) (string, string, int, error) { | ||
var err error | ||
accounts := []accountWithLocation{} | ||
if len(storageAccount) > 0 { | ||
accounts = append(accounts, accountWithLocation{Name: storageAccount}) | ||
} else { | ||
// find a storage account | ||
accounts, err = c.common.cloud.getStorageAccounts() | ||
if err != nil { | ||
return "", "", 0, fmt.Errorf("no key found for storage account %s even after creating a new storage account", storageAccount) | ||
// TODO: create a storage account and container | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can be fixed now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, I would like to fix it in a new PR. |
||
return "", "", 0, err | ||
} | ||
|
||
glog.Errorf("no key found for storage account %s in resource group %s", storageAccount, c.common.resourceGroup) | ||
return "", "", 0, err | ||
} | ||
for _, account := range accounts { | ||
glog.V(4).Infof("account %s type %s location %s", account.Name, account.StorageType, account.Location) | ||
if (storageAccountType == "" || account.StorageType == storageAccountType) && (location == "" || account.Location == location) || len(storageAccount) > 0 { | ||
// find the access key with this account | ||
key, err := c.common.cloud.getStorageAccesskey(account.Name) | ||
if err != nil { | ||
glog.V(2).Infof("no key found for storage account %s", account.Name) | ||
continue | ||
} | ||
|
||
client, err := azstorage.NewBasicClientOnSovereignCloud(storageAccount, key, c.common.cloud.Environment) | ||
if err != nil { | ||
return "", "", 0, err | ||
} | ||
blobClient := client.GetBlobService() | ||
client, err := azstorage.NewBasicClientOnSovereignCloud(account.Name, key, c.common.cloud.Environment) | ||
if err != nil { | ||
return "", "", 0, err | ||
} | ||
blobClient := client.GetBlobService() | ||
|
||
container := blobClient.GetContainerReference(vhdContainerName) | ||
_, err = container.CreateIfNotExists(&azstorage.CreateContainerOptions{Access: azstorage.ContainerAccessTypePrivate}) | ||
if err != nil { | ||
return "", "", 0, err | ||
} | ||
// create a page blob in this account's vhd container | ||
diskName, diskURI, err := c.createVHDBlobDisk(blobClient, account.Name, name, vhdContainerName, int64(requestGB)) | ||
if err != nil { | ||
return "", "", 0, err | ||
} | ||
|
||
diskName, diskURI, err := c.createVHDBlobDisk(blobClient, storageAccount, name, vhdContainerName, int64(requestGB)) | ||
if err != nil { | ||
return "", "", 0, err | ||
glog.V(4).Infof("azureDisk - created vhd blob uri: %s", diskURI) | ||
return diskName, diskURI, requestGB, err | ||
} | ||
} | ||
|
||
glog.V(4).Infof("azureDisk - created vhd blob uri: %s", diskURI) | ||
return diskName, diskURI, requestGB, err | ||
return "", "", 0, fmt.Errorf("failed to find a matching storage account") | ||
} | ||
|
||
// DeleteVolume deletes a VHD blob | ||
|
@@ -173,11 +171,6 @@ func (c *BlobDiskController) getBlobNameAndAccountFromURI(diskURI string) (strin | |
|
||
func (c *BlobDiskController) createVHDBlobDisk(blobClient azstorage.BlobStorageClient, accountName, vhdName, containerName string, sizeGB int64) (string, string, error) { | ||
container := blobClient.GetContainerReference(containerName) | ||
_, err := container.CreateIfNotExists(&azstorage.CreateContainerOptions{Access: azstorage.ContainerAccessTypePrivate}) | ||
if err != nil { | ||
return "", "", err | ||
} | ||
|
||
size := 1024 * 1024 * 1024 * sizeGB | ||
vhdSize := size + vhd.VHD_HEADER_SIZE /* header size */ | ||
// Blob name in URL must end with '.vhd' extension. | ||
|
@@ -190,7 +183,17 @@ func (c *BlobDiskController) createVHDBlobDisk(blobClient azstorage.BlobStorageC | |
blob := container.GetBlobReference(vhdName) | ||
blob.Properties.ContentLength = vhdSize | ||
blob.Metadata = tags | ||
err = blob.PutPageBlob(nil) | ||
err := blob.PutPageBlob(nil) | ||
if err != nil { | ||
// if container doesn't exist, create one and retry PutPageBlob | ||
detail := err.Error() | ||
if strings.Contains(detail, errContainerNotFound) { | ||
err = container.Create(&azstorage.CreateContainerOptions{Access: azstorage.ContainerAccessTypePrivate}) | ||
if err == nil { | ||
err = blob.PutPageBlob(nil) | ||
} | ||
} | ||
} | ||
if err != nil { | ||
return "", "", fmt.Errorf("failed to put page blob %s in container %s: %v", vhdName, containerName, err) | ||
} | ||
|
@@ -236,24 +239,12 @@ func (c *BlobDiskController) deleteVhdBlob(accountName, accountKey, blobName str | |
} | ||
|
||
//CreateBlobDisk : create a blob disk in a node | ||
func (c *BlobDiskController) CreateBlobDisk(dataDiskName string, storageAccountType storage.SkuName, sizeGB int, forceStandAlone bool) (string, error) { | ||
glog.V(4).Infof("azureDisk - creating blob data disk named:%s on StorageAccountType:%s StandAlone:%v", dataDiskName, storageAccountType, forceStandAlone) | ||
func (c *BlobDiskController) CreateBlobDisk(dataDiskName string, storageAccountType storage.SkuName, sizeGB int) (string, error) { | ||
glog.V(4).Infof("azureDisk - creating blob data disk named:%s on StorageAccountType:%s", dataDiskName, storageAccountType) | ||
|
||
var storageAccountName = "" | ||
var err error | ||
|
||
if forceStandAlone { | ||
// we have to wait until the storage account is is created | ||
storageAccountName = "p" + MakeCRC32(c.common.subscriptionID+c.common.resourceGroup+dataDiskName) | ||
err = c.createStorageAccount(storageAccountName, storageAccountType, c.common.location, false) | ||
if err != nil { | ||
return "", err | ||
} | ||
} else { | ||
storageAccountName, err = c.findSANameForDisk(storageAccountType) | ||
if err != nil { | ||
return "", err | ||
} | ||
storageAccountName, err := c.findSANameForDisk(storageAccountType) | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
blobClient, err := c.getBlobSvcClient(storageAccountName) | ||
|
@@ -266,15 +257,13 @@ func (c *BlobDiskController) CreateBlobDisk(dataDiskName string, storageAccountT | |
return "", err | ||
} | ||
|
||
if !forceStandAlone { | ||
atomic.AddInt32(&c.accounts[storageAccountName].diskCount, 1) | ||
} | ||
atomic.AddInt32(&c.accounts[storageAccountName].diskCount, 1) | ||
|
||
return diskURI, nil | ||
} | ||
|
||
//DeleteBlobDisk : delete a blob disk from a node | ||
func (c *BlobDiskController) DeleteBlobDisk(diskURI string, wasForced bool) error { | ||
func (c *BlobDiskController) DeleteBlobDisk(diskURI string) error { | ||
storageAccountName, vhdName, err := diskNameandSANameFromURI(diskURI) | ||
if err != nil { | ||
return err | ||
|
@@ -286,11 +275,6 @@ func (c *BlobDiskController) DeleteBlobDisk(diskURI string, wasForced bool) erro | |
glog.V(4).Infof("azureDisk - deleting volume %s", diskURI) | ||
return c.DeleteVolume(diskURI) | ||
} | ||
// if forced (as in one disk = one storage account) | ||
// delete the account completely | ||
if wasForced { | ||
return c.deleteStorageAccount(storageAccountName) | ||
} | ||
|
||
blobSvc, err := c.getBlobSvcClient(storageAccountName) | ||
if err != nil { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cannot comment on line 64, but it is a mess there. use a var block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed