fix detach azure disk issue due to dirty cache

[andyzhangx]

What type of PR is this?
/kind bug

What this PR does / why we need it:
fix detch azure disk issue by cleaning up vm cache right after every update vm operation in disk attach/detach

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

Special notes for your reviewer:
When detaching lots of disks in one node, detach failure may happen, in the before, we use backoff mechanism to retries 6 times which is not good solution; now we switch to use k8s controller to detach disk volume, and it finally depends on DisksAreAttached func to decide whether one disk is attached or not:

kubernetes/pkg/volume/azure_dd/attacher.go

Line 135 in 5dfe48b

attachedResult, err := diskController.DisksAreAttached(volumeIDList, nodeName)

Unfortunately DisksAreAttached func is invoked right after DetachDiskByName failure which already make the vm cache dirty, this PR makes sure vm cache will be cleaned after every update vm operation in disk attach/detach operation. And this PR will also solve strange issue, e.g. detach disk failure due to the last attach disk failure.

The invocation chain for using vmCache in DisksAreAttached func:

DisksAreAttached --> getNodeDataDisks--> GetDataDisks --> getVirtualMachine --> vmCache.Get

I1128 05:04:58.206841       1 attacher.go:145] azureDisk - VolumesAreAttached: check volume "andy-vmss11010-dynamic-pvc-884809b5-f2ca-11e8-a757-000d3a01450e" (specName: "pvc-884809b5-f2ca-11e8-a757-000d3a01450e") is no longer attached
I1128 05:04:58.206945       1 operation_generator.go:193] VerifyVolumesAreAttached determined volume "kubernetes.io/azure-disk//subscriptions/.../resourceGroups/andy-vmss11010/providers/Microsoft.Compute/disks/andy-vmss11010-dynamic-pvc-049158b4-f2ba-11e8-ade9-000d3a01450e" (spec.Name: "pvc-049158b4-f2ba-11e8-ade9-000d3a01450e") is no longer attached to node "k8s-agentpool-39687990-vmss000000", therefore it was marked as detached.

Release note:

fix detach azure disk issue

/sig azure
/assign @feiskyer
cc @khenidak @brendandburns
@antoineco

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx

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

The pull request process is described here

Needs approval from an approver in each of these files:

• pkg/cloudprovider/providers/azure/OWNERS [andyzhangx]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[andyzhangx]

/priority important-soon

[feiskyer]

/milestone v1.13
/priority critical-urgent

[feiskyer]

Added to v1.13 since this is a critical bug fix.

[feiskyer]

/lgtm

[AishSundar]

@andyzhangx @feiskyer is this an issue introduced recently in 1.13 that we are trying to address with this fix? Are there CI tests to verify the goodness of the fix? We are very late in the 1.13 release cycle and I would like to avoid any code churn unless it is a critical urgent issue introduced in 1.13. Can this wait until 1.13.1?

/hold

[andyzhangx]

Hi @AishSundar, Yes, we are going to fix the issue right in v1.13.0, and the code change only remains in azure directory. It's important for this fix fit into v1.13.0, thanks.
Update:
About the test, I have done a few e2e tests on this, while since it's not a common scenario, we have not an automation test for this, it's manual now.

[andyzhangx]

/test pull-kubernetes-integration

[andyzhangx]

/hold cancel
@AishSundar just let me know if you have any concern of this PR, thanks.

[andyzhangx]

here is the PR for this dirty cache issue, originally whether succeed or not, this PR clean the cache anyway.
I can move this clean cache operation defer ss.vmssVMCache.Delete(key) to conditions when only error happens, would that solve your issue @khenidak ?I can build one hot fix image if you wan to try this fix.
cc@feiskyer