Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix incorrect error info when creating an azure file PVC failed #56550

Merged
merged 1 commit into from Dec 16, 2017
Merged

fix incorrect error info when creating an azure file PVC failed #56550

merged 1 commit into from Dec 16, 2017

Conversation

andyzhangx
Copy link
Member

@andyzhangx andyzhangx commented Nov 29, 2017 •
edited

What this PR does / why we need it:
when creating an azure file PVC failed, it will always return following error which is totally incorrect:

Failed to provision volume with StorageClass "azurefile-premium": failed to find a matching storage account

The incorrect error info would mislead customer a lot, I would suggest return error directly if create first file share failed.

By this PR, the error info would be like following, which would provide user detailed and correct info:

Events:
  Type     Reason              Age               From                         Message
  ----     ------              ----              ----                         -------
  Warning  ProvisioningFailed  13s                  persistentvolume-controller  Failed to provision volume with StorageClass "azurefile-premium": failed to create share andy-k8s182-dynamic-pvc-cd66f4bd-d4c4-11e7-9f09-000d3a019e90 in account 00mqk6lqaouexy6agnt0: failed to create file share, err: Put https://00mqk6lqaouexy6agnt0.file.core.windows.net/andy-k8s182-dynamic-pvc-cd66f4bd-d4c4-11e7-9f09-000d3a019e90?restype=share: dial tcp: lookup 00mqk6lqaouexy6agnt0.file.core.windows.net on 168.63.129.16:53: no such host

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #56548

Special notes for your reviewer:

Release note:

fix incorrect error info when creating an azure file PVC failed

/sig azure
/assign @rootfs

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. sig/azure size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 29, 2017
@andyzhangx
Copy link
Member Author

I just got lots of this incorrect error info when customer create azure file failed, mostly it's due to the azure file name length bug, while k8s just return failed to find a matching storage account which would mislead users.

rootfs
rootfs reviewed Nov 29, 2017
View reviewed changes
pkg/cloudprovider/providers/azure/azure_storage.go Outdated
@@ -42,14 +42,12 @@ func (az *Cloud) CreateFileShare(name, storageAccount, storageType, location str
// find the access key with this account
key, err := az.getStorageAccesskey(account.Name)
if err != nil {
glog.V(2).Infof("no key found for storage account %s", account.Name)
continue
return "", "", fmt.Errorf("no key found for storage account %s", account.Name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why return here when there are still other accounts to try?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds reasonable, I have used err var to save the error info, and return err in the end, that would be more error clear for user.

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 30, 2017
@andyzhangx
return error when create azure share failed
08f40ea 
remember error info in CreateFileShare

fix typo
@rootfs
Copy link
Contributor

rootfs commented Nov 30, 2017

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 30, 2017
@andyzhangx
Copy link
Member Author

/test pull-kubernetes-unit

@andyzhangx
Copy link
Member Author

/assign @brendandburns

@andyzhangx
Copy link
Member Author

/test pull-kubernetes-unit

@brendandburns
Copy link
Contributor

/retest
/approve

@kubernetes kubernetes deleted a comment from k8s-github-robot Dec 15, 2017
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx, brendandburns, rootfs

Associated issue: #56548

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 15, 2017
@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 56337, 56546, 56550, 56633, 56635). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 528f045 into kubernetes:master Dec 16, 2017
This was referenced Dec 18, 2017
Merged
Merged
Merged
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Dec 20, 2017
k8s-github-robot pushed a commit that referenced this pull request Dec 28, 2017
Merge pull request #57310 from andyzhangx/automated-cherry-pick-of-#5…
fc39724 
…6550-upstream-release-1.9

Automatic merge from submit-queue.

Automated cherry pick of #56550

Cherry pick of #56550 on release-1.9.

#56550: return error when create azure share failed
**Release note**:

```
fix incorrect error info when creating an azure file PVC failed
```
k8s-github-robot pushed a commit that referenced this pull request Jan 4, 2018
Merge pull request #57295 from andyzhangx/automated-cherry-pick-of-#5…
69bef84 
…6550-upstream-release-1.8

Automatic merge from submit-queue.

Automated cherry pick of #56550: return error when create azure share failed

Cherry pick of #56550 on release-1.8.

#56550: return error when create azure share failed

**Release note**:

```
fix incorrect error info when creating an azure file PVC failed
```
k8s-github-robot pushed a commit that referenced this pull request Jan 4, 2018
Merge pull request #57311 from andyzhangx/automated-cherry-pick-of-#5…
b5082e8 
…6550-upstream-release-1.7

Automatic merge from submit-queue.

Automated cherry pick of #56550

Cherry pick of #56550 on release-1.7.

#56550: return error when create azure share failed
**Release note**:

```
fix incorrect error info when creating an azure file PVC failed
```
@andyzhangx andyzhangx deleted the error-info-createshare branch May 8, 2018 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rootfs rootfs rootfs left review comments

Assignees

@brendandburns brendandburns

@rootfs rootfs

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

error info is incorrect when creating an azure file PVC failed
5 participants
@andyzhangx @rootfs @brendandburns @k8s-ci-robot @k8s-github-robot