From 8c53db09415652ab0def7bdd378f8c416ebc36d3 Mon Sep 17 00:00:00 2001
From: andyzhangx <xiazhang@microsoft.com>
Date: Fri, 22 Feb 2019 05:13:16 +0000
Subject: [PATCH] add retry for detach azure disk

add more logging info in detach disk
---
 .../providers/azure/azure_backoff.go          | 12 ++++++++++++
 .../azure/azure_controller_standard.go        | 19 ++++++++++++++-----
 .../providers/azure/azure_controller_vmss.go  | 16 ++++++++++++----
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/pkg/cloudprovider/providers/azure/azure_backoff.go b/pkg/cloudprovider/providers/azure/azure_backoff.go
index 2771bef26b1..6545ec1c44b 100644
--- a/pkg/cloudprovider/providers/azure/azure_backoff.go
+++ b/pkg/cloudprovider/providers/azure/azure_backoff.go
@@ -511,6 +511,18 @@ func (az *Cloud) CreateOrUpdateVMWithRetry(resourceGroup, vmName string, newVM c
 	})
 }
 
+// UpdateVmssVMWithRetry invokes az.VirtualMachineScaleSetVMsClient.Update with exponential backoff retry
+func (az *Cloud) UpdateVmssVMWithRetry(resourceGroupName string, VMScaleSetName string, instanceID string, parameters compute.VirtualMachineScaleSetVM) error {
+	return wait.ExponentialBackoff(az.requestBackoff(), func() (bool, error) {
+		ctx, cancel := getContextWithCancel()
+		defer cancel()
+
+		resp, err := az.VirtualMachineScaleSetVMsClient.Update(ctx, resourceGroupName, VMScaleSetName, instanceID, parameters)
+		klog.V(10).Infof("VirtualMachinesClient.CreateOrUpdate(%s,%s): end", VMScaleSetName, instanceID)
+		return az.processHTTPRetryResponse(nil, "", resp, err)
+	})
+}
+
 // isSuccessHTTPResponse determines if the response from an HTTP request suggests success
 func isSuccessHTTPResponse(resp *http.Response) bool {
 	if resp == nil {
diff --git a/pkg/cloudprovider/providers/azure/azure_controller_standard.go b/pkg/cloudprovider/providers/azure/azure_controller_standard.go
index f600209e284..610289e67ba 100644
--- a/pkg/cloudprovider/providers/azure/azure_controller_standard.go
+++ b/pkg/cloudprovider/providers/azure/azure_controller_standard.go
@@ -146,12 +146,21 @@ func (as *availabilitySet) DetachDiskByName(diskName, diskURI string, nodeName t
 	// Invalidate the cache right after updating
 	defer as.cloud.vmCache.Delete(vmName)
 
-	_, err = as.VirtualMachinesClient.CreateOrUpdate(ctx, nodeResourceGroup, vmName, newVM)
-	if err != nil {
-		klog.Errorf("azureDisk - detach disk(%s) failed, err: %v", diskName, err)
-	} else {
-		klog.V(2).Infof("azureDisk - detach disk(%s) succeeded", diskName)
+	resp, err := as.VirtualMachinesClient.CreateOrUpdate(ctx, nodeResourceGroup, vmName, newVM)
+	if as.CloudProviderBackoff && shouldRetryHTTPRequest(resp, err) {
+		klog.V(2).Infof("azureDisk - update(%s) backing off: vm(%s) detach disk(%s, %s), err: %v", nodeResourceGroup, vmName, diskName, diskURI, err)
+		retryErr := as.CreateOrUpdateVMWithRetry(nodeResourceGroup, vmName, newVM)
+		if retryErr != nil {
+			err = retryErr
+			klog.V(2).Infof("azureDisk - update(%s) abort backoff: vm(%s) detach disk(%s, %s), err: %v", nodeResourceGroup, vmName, diskName, diskURI, err)
+		}
 	}
+	if err != nil {
+		klog.Errorf("azureDisk - detach disk(%s, %s)) failed, err: %v", diskName, diskURI, err)
+	} else {
+		klog.V(2).Infof("azureDisk - detach disk(%s, %s) succeeded", diskName, diskURI)
+	}
+
 	return err
 }
 
diff --git a/pkg/cloudprovider/providers/azure/azure_controller_vmss.go b/pkg/cloudprovider/providers/azure/azure_controller_vmss.go
index 80a27bf0385..43f49aaeec3 100644
--- a/pkg/cloudprovider/providers/azure/azure_controller_vmss.go
+++ b/pkg/cloudprovider/providers/azure/azure_controller_vmss.go
@@ -155,12 +155,20 @@ func (ss *scaleSet) DetachDiskByName(diskName, diskURI string, nodeName types.No
 	key := buildVmssCacheKey(nodeResourceGroup, ss.makeVmssVMName(ssName, instanceID))
 	defer ss.vmssVMCache.Delete(key)
 
-	klog.V(2).Infof("azureDisk - update(%s): vm(%s) - detach disk(%s)", nodeResourceGroup, nodeName, diskName)
-	_, err = ss.VirtualMachineScaleSetVMsClient.Update(ctx, nodeResourceGroup, ssName, instanceID, newVM)
+	klog.V(2).Infof("azureDisk - update(%s): vm(%s) - detach disk(%s, %s)", nodeResourceGroup, nodeName, diskName, diskURI)
+	resp, err := ss.VirtualMachineScaleSetVMsClient.Update(ctx, nodeResourceGroup, ssName, instanceID, newVM)
+	if ss.CloudProviderBackoff && shouldRetryHTTPRequest(resp, err) {
+		klog.V(2).Infof("azureDisk - update(%s) backing off: vm(%s) detach disk(%s, %s), err: %v", nodeResourceGroup, nodeName, diskName, diskURI, err)
+		retryErr := ss.UpdateVmssVMWithRetry(nodeResourceGroup, ssName, instanceID, newVM)
+		if retryErr != nil {
+			err = retryErr
+			klog.V(2).Infof("azureDisk - update(%s) abort backoff: vm(%s) detach disk(%s, %s), err: %v", nodeResourceGroup, nodeName, diskName, diskURI, err)
+		}
+	}
 	if err != nil {
-		klog.Errorf("azureDisk - detach disk(%s) from %s failed, err: %v", diskName, nodeName, err)
+		klog.Errorf("azureDisk - detach disk(%s, %s) from %s failed, err: %v", diskName, diskURI, nodeName, err)
 	} else {
-		klog.V(2).Infof("azureDisk - detach disk(%s) succeeded", diskName)
+		klog.V(2).Infof("azureDisk - detach disk(%s, %s) succeeded", diskName, diskURI)
 	}
 
 	return err