diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common.go index 1d2cc30e7d8..dd1e6e4d866 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common.go @@ -49,7 +49,9 @@ const ( errLeaseFailed = "AcquireDiskLeaseFailed" errLeaseIDMissing = "LeaseIdMissing" errContainerNotFound = "ContainerNotFound" - errDiskBlobNotFound = "DiskBlobNotFound" + errStatusCode400 = "statuscode=400" + errInvalidParameter = `code="invalidparameter"` + errTargetInstanceIds = `target="instanceids"` sourceSnapshot = "snapshot" sourceVolume = "volume" @@ -214,24 +216,32 @@ func (c *controllerCommon) DetachDisk(diskName, diskURI string, nodeName types.N c.diskAttachDetachMap.Delete(strings.ToLower(diskURI)) c.vmLockMap.UnlockEntry(strings.ToLower(string(nodeName))) - if err != nil && retry.IsErrorRetriable(err) && c.cloud.CloudProviderBackoff { - klog.V(2).Infof("azureDisk - update backing off: detach disk(%s, %s), err: %v", diskName, diskURI, err) - retryErr := kwait.ExponentialBackoff(c.cloud.RequestBackoff(), func() (bool, error) { - c.vmLockMap.LockEntry(strings.ToLower(string(nodeName))) - c.diskAttachDetachMap.Store(strings.ToLower(diskURI), "detaching") - err := vmset.DetachDisk(diskName, diskURI, nodeName) - c.diskAttachDetachMap.Delete(strings.ToLower(diskURI)) - c.vmLockMap.UnlockEntry(strings.ToLower(string(nodeName))) + if err != nil { + if isInstanceNotFoundError(err) { + // if host doesn't exist, no need to detach + klog.Warningf("azureDisk - got InstanceNotFoundError(%v), DetachDisk(%s) will assume disk is already detached", + err, diskURI) + return nil + } + if retry.IsErrorRetriable(err) && c.cloud.CloudProviderBackoff { + klog.Warningf("azureDisk - update backing off: detach disk(%s, %s), err: %v", diskName, diskURI, err) + retryErr := kwait.ExponentialBackoff(c.cloud.RequestBackoff(), func() (bool, error) { + c.vmLockMap.LockEntry(strings.ToLower(string(nodeName))) + c.diskAttachDetachMap.Store(strings.ToLower(diskURI), "detaching") + err := vmset.DetachDisk(diskName, diskURI, nodeName) + c.diskAttachDetachMap.Delete(strings.ToLower(diskURI)) + c.vmLockMap.UnlockEntry(strings.ToLower(string(nodeName))) - retriable := false - if err != nil && retry.IsErrorRetriable(err) { - retriable = true + retriable := false + if err != nil && retry.IsErrorRetriable(err) { + retriable = true + } + return !retriable, err + }) + if retryErr != nil { + err = retryErr + klog.V(2).Infof("azureDisk - update abort backoff: detach disk(%s, %s), err: %v", diskName, diskURI, err) } - return !retriable, err - }) - if retryErr != nil { - err = retryErr - klog.V(2).Infof("azureDisk - update abort backoff: detach disk(%s, %s), err: %v", diskName, diskURI, err) } } if err != nil { @@ -426,3 +436,8 @@ func getValidCreationData(subscriptionID, resourceGroup, sourceResourceID, sourc SourceResourceID: &sourceResourceID, }, nil } + +func isInstanceNotFoundError(err error) bool { + errMsg := strings.ToLower(err.Error()) + return strings.Contains(errMsg, errStatusCode400) && strings.Contains(errMsg, errInvalidParameter) && strings.Contains(errMsg, errTargetInstanceIds) +} diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common_test.go index 5423204d31e..f2e40ad2a6e 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_common_test.go @@ -788,3 +788,32 @@ func TestFilterNonExistingDisksWithSpecialHTTPStatusCode(t *testing.T) { assert.Equal(t, 1, len(filteredDisks)) assert.Equal(t, newDiskName, *filteredDisks[0].Name) } + +func TestIsInstanceNotFoundError(t *testing.T) { + testCases := []struct { + errMsg string + expectedResult bool + }{ + { + errMsg: "", + expectedResult: false, + }, + { + errMsg: "other error", + expectedResult: false, + }, + { + errMsg: "not an active Virtual Machine scale set vm", + expectedResult: false, + }, + { + errMsg: `compute.VirtualMachineScaleSetVMsClient#Update: Failure sending request: StatusCode=400 -- Original Error: Code="InvalidParameter" Message="The provided instanceId 1181 is not an active Virtual Machine Scale Set VM instanceId." Target="instanceIds"`, + expectedResult: true, + }, + } + + for i, test := range testCases { + result := isInstanceNotFoundError(fmt.Errorf(test.errMsg)) + assert.Equal(t, test.expectedResult, result, "TestCase[%d]", i, result) + } +}