remove time waiting after create storage account (save 25s) #56679

andyzhangx · 2017-12-01T07:52:15Z

What this PR does / why we need it:
I found azure cloud provider will always sleep 25 seconds after creating a new azure storage account:
https://github.com/kubernetes/kubernetes/blob/master/pkg/cloudprovider/providers/azure/azure_blobDiskController.go#L531
Actually it's not necessary now, since it's already using sync way to create a storage account:
https://github.com/kubernetes/kubernetes/blob/master/pkg/cloudprovider/providers/azure/azure_blobDiskController.go#L531
Above code will wait until the storage account is created in azure.

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 #56674

Special notes for your reviewer:
Below are logs without this PR:

I1201 06:41:22.486663       1 azure_blobDiskController.go:522] azureDisk - Creating storage account pvc3329812692002 type Standard_LRS
I1201 06:41:22.486810       1 azure_blobDiskController.go:531] azureDisk - Creating storage account pvc3329812692002 type Standard_LRS begin to wait
I1201 06:41:40.440005       1 azure_blobDiskController.go:533] azureDisk - Creating storage account pvc3329812692002 type Standard_LRS end wait
I1201 06:41:40.440030       1 azure_blobDiskController.go:551] azureDisk - storage account pvc3329812692002 was just created, allowing time before polling status
I1201 06:42:05.440176       1 azure_blobDiskController.go:553] azureDisk - storage account pvc3329812692002 was just created, allowing time before polling status, end wait

Below are logs with this PR, it could save 25s now:

I1201 07:36:07.755540       1 azure_blobDiskController.go:523] azureDisk - Creating storage account pvc33298126923895004820 type Standard_LRS
I1201 07:36:07.755652       1 azure_blobDiskController.go:532] azureDisk - Creating storage account pvc33298126923895004820 type Standard_LRS begin to wait
I1201 07:36:25.722540       1 azure_blobDiskController.go:534] azureDisk - Creating storage account pvc33298126923895004820 type Standard_LRS end wait
I1201 07:36:25.722557       1 azure_blobDiskController.go:552] azureDisk - storage account pvc33298126923895004820 was just created, allowing time before polling status
I1201 07:36:25.722562       1 azure_blobDiskController.go:554] azureDisk - storage account pvc33298126923895004820 was just created, allowing time before polling status, end wait
I1201 07:36:26.011157       1 azure_blobDiskController.go:436] azureDisk - storage account:pvc33298126923895004820 had no default container(3329812692) and it was created
I1201 07:36:26.011201       1 azure_blobDiskController.go:182] azureDisk - creating page blob andy-mgwin1710-dynamic-pvc-88c50c37-d668-11e7-94dc-000d3a041274.vhd in container 3329812692 account pvc33298126923895004820

Release note:

remove time waiting after create storage account

/sig azure
/assign @khenidak

k8s-ci-robot · 2017-12-01T07:52:16Z

@andyzhangx: GitHub didn't allow me to assign the following users: khenidak.

Note that only kubernetes members can be assigned.

In response to this:

What this PR does / why we need it:
I found azure cloud provider will always sleep 25 seconds after creating a new azure storage account:
https://github.com/kubernetes/kubernetes/blob/master/pkg/cloudprovider/providers/azure/azure_blobDiskController.go#L531
Actually it's not necessary now, since it's already using sync way to create a storage account:
https://github.com/kubernetes/kubernetes/blob/master/pkg/cloudprovider/providers/azure/azure_blobDiskController.go#L531

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 #56674

Special notes for your reviewer:
Below are logs without this PR:

I1201 06:41:22.486663       1 azure_blobDiskController.go:522] azureDisk - Creating storage account pvc3329812692002 type Standard_LRS
I1201 06:41:22.486810       1 azure_blobDiskController.go:531] azureDisk - Creating storage account pvc3329812692002 type Standard_LRS begin to wait
I1201 06:41:40.440005       1 azure_blobDiskController.go:533] azureDisk - Creating storage account pvc3329812692002 type Standard_LRS end wait
I1201 06:41:40.440030       1 azure_blobDiskController.go:551] azureDisk - storage account pvc3329812692002 was just created, allowing time before polling status
I1201 06:42:05.440176       1 azure_blobDiskController.go:553] azureDisk - storage account pvc3329812692002 was just created, allowing time before polling status, end wait

Below are logs with this PR, it could save 25s now:

I1201 07:36:07.755540       1 azure_blobDiskController.go:523] azureDisk - Creating storage account pvc33298126923895004820 type Standard_LRS
I1201 07:36:07.755652       1 azure_blobDiskController.go:532] azureDisk - Creating storage account pvc33298126923895004820 type Standard_LRS begin to wait
I1201 07:36:25.722540       1 azure_blobDiskController.go:534] azureDisk - Creating storage account pvc33298126923895004820 type Standard_LRS end wait
I1201 07:36:25.722557       1 azure_blobDiskController.go:552] azureDisk - storage account pvc33298126923895004820 was just created, allowing time before polling status
I1201 07:36:25.722562       1 azure_blobDiskController.go:554] azureDisk - storage account pvc33298126923895004820 was just created, allowing time before polling status, end wait
I1201 07:36:26.011157       1 azure_blobDiskController.go:436] azureDisk - storage account:pvc33298126923895004820 had no default container(3329812692) and it was created
I1201 07:36:26.011201       1 azure_blobDiskController.go:182] azureDisk - creating page blob andy-mgwin1710-dynamic-pvc-88c50c37-d668-11e7-94dc-000d3a041274.vhd in container 3329812692 account pvc33298126923895004820

Release note:

none

/sig azure
/assign @khenidak

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

andyzhangx · 2017-12-01T07:52:49Z

/assign @brendandburns

dims · 2017-12-01T11:42:34Z

/unassign @dims

andyzhangx · 2017-12-04T03:06:27Z

/unassign @piosz

andyzhangx · 2017-12-04T03:07:06Z

@khenidak pls take a review, thanks.

brendandburns · 2017-12-05T21:11:51Z

/lgtm
/approve no-issue

k8s-github-robot · 2017-12-05T21:12:27Z

[APPROVALNOTIFIER] This PR is APPROVED

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

Associated issue: 56674

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

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

~~pkg/cloudprovider/providers/azure/OWNERS~~ [brendandburns]

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

brendandburns · 2017-12-09T01:02:00Z

I'd like to get this into 1.9, the change is small and the impact is large.

andyzhangx · 2017-12-09T02:29:02Z

/kind bug

jberkus · 2017-12-10T03:49:14Z

@brendanburns can you approve this for the milestone, please?

colemickens · 2017-12-12T00:16:34Z

pkg/cloudprovider/providers/azure/azure_blobDiskController.go

-		// so if this account was just created allow it sometime
-		// before polling
-		glog.V(2).Infof("azureDisk - storage account %s was just created, allowing time before polling status", storageAccountName)
-		time.Sleep(25 * time.Second) // as observed 25 is the average time for SA to be provisioned


It's surprising we didn't get bugs on this previously. There are definitely cases where this can take longer than 25 seconds, but I guess downstream code probably retries too.

point is we already use sync way to wait for storage account creating successfully, it's not necessary to wait any more:
err := <-errChan

I understand, just surprised this sleep lasted this long without being hardened but I guess it was good enough.

k8s-github-robot · 2017-12-12T00:25:45Z

[MILESTONENOTIFIER] Milestone Pull Request Current

@andyzhangx @brendandburns

Note: This pull request is marked as priority/critical-urgent, and must be updated every 1 day during code freeze.

Example update:

ACK.  In progress
ETA: DD/MM/YYYY
Risks: Complicated fix required

Pull Request Labels

sig/azure: Pull Request will be escalated to these SIGs if needed.
priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
kind/bug: Fixes a bug discovered during the current release.

Help

k8s-github-robot · 2017-12-12T00:25:47Z

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

…6679-upstream-release-1.8 Automatic merge from submit-queue. Automated cherry pick of #56679 Cherry pick of #56679 on release-1.8. #56679: remove time waiting after create storage account

…6679-upstream-release-1.7 Automatic merge from submit-queue. Automated cherry pick of #56679 Cherry pick of #56679 on release-1.7. #56679: remove time waiting after create storage account **Release note**: ``` Remove time waiting after create storage account in Azure ```

remove time waiting after create storage account

2afba1e

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. labels Dec 1, 2017

k8s-github-robot assigned piosz and dims Dec 1, 2017

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 1, 2017

k8s-ci-robot assigned brendandburns Dec 1, 2017

k8s-ci-robot unassigned dims Dec 1, 2017

k8s-ci-robot unassigned piosz Dec 4, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 5, 2017

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

brendandburns added this to the v1.9 milestone Dec 9, 2017

brendandburns added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Dec 9, 2017

k8s-github-robot added the milestone/incomplete-labels label Dec 9, 2017

k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Dec 9, 2017

k8s-github-robot added milestone/needs-approval and removed milestone/incomplete-labels labels Dec 9, 2017

jberkus mentioned this pull request Dec 10, 2017

[1.9] Issue Burndown kubernetes/sig-release#38

Closed

jdumars added the status/approved-for-milestone label Dec 11, 2017

jdumars added the status/in-progress label Dec 11, 2017

k8s-github-robot added milestone/needs-attention and removed milestone/needs-approval labels Dec 11, 2017

colemickens reviewed Dec 12, 2017

View reviewed changes

k8s-github-robot removed the milestone/needs-attention label Dec 12, 2017

k8s-github-robot merged commit 08c9828 into kubernetes:master Dec 12, 2017

This was referenced Dec 12, 2017

Automated cherry pick of #56679 #57071

Merged

Automated cherry pick of #56679 #57072

Merged

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

andyzhangx deleted the fix-time-waiting-issue branch May 8, 2018 07:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove time waiting after create storage account (save 25s) #56679

remove time waiting after create storage account (save 25s) #56679

andyzhangx commented Dec 1, 2017 •

edited

Loading

k8s-ci-robot commented Dec 1, 2017

andyzhangx commented Dec 1, 2017

dims commented Dec 1, 2017

andyzhangx commented Dec 4, 2017

andyzhangx commented Dec 4, 2017

brendandburns commented Dec 5, 2017

k8s-github-robot commented Dec 5, 2017

brendandburns commented Dec 9, 2017

andyzhangx commented Dec 9, 2017

jberkus commented Dec 10, 2017

colemickens Dec 12, 2017

andyzhangx Dec 12, 2017

colemickens Dec 12, 2017

k8s-github-robot commented Dec 12, 2017

k8s-github-robot commented Dec 12, 2017

remove time waiting after create storage account (save 25s) #56679

remove time waiting after create storage account (save 25s) #56679

Conversation

andyzhangx commented Dec 1, 2017 • edited Loading

k8s-ci-robot commented Dec 1, 2017

andyzhangx commented Dec 1, 2017

dims commented Dec 1, 2017

andyzhangx commented Dec 4, 2017

andyzhangx commented Dec 4, 2017

brendandburns commented Dec 5, 2017

k8s-github-robot commented Dec 5, 2017

brendandburns commented Dec 9, 2017

andyzhangx commented Dec 9, 2017

jberkus commented Dec 10, 2017

colemickens Dec 12, 2017

Choose a reason for hiding this comment

andyzhangx Dec 12, 2017

Choose a reason for hiding this comment

colemickens Dec 12, 2017

Choose a reason for hiding this comment

k8s-github-robot commented Dec 12, 2017

k8s-github-robot commented Dec 12, 2017

andyzhangx commented Dec 1, 2017 •

edited

Loading