From d11cac9ada1e987e7997489e5050cd6794875955 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Thu, 12 Sep 2019 14:05:50 +0000 Subject: [PATCH] fix: azure disk detach failure if node not exists fix comments --- .../azure/azure_controller_common.go | 18 ++++++++++++------ .../azure/azure_controller_common_test.go | 4 ++-- 2 files changed, 14 insertions(+), 8 deletions(-) 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 10518b05e2a..39e7073fffa 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 @@ -150,17 +150,23 @@ func (c *controllerCommon) AttachDisk(isManagedDisk bool, diskName, diskURI stri // DetachDisk detaches a disk from host. The vhd can be identified by diskName or diskURI. func (c *controllerCommon) DetachDisk(diskName, diskURI string, nodeName types.NodeName) error { + instanceid, err := c.cloud.InstanceID(context.TODO(), nodeName) + if err != nil { + if err == cloudprovider.InstanceNotFound { + // if host doesn't exist, no need to detach + klog.Warningf("azureDisk - failed to get azure instance id(%q), DetachDisk(%s) will assume disk is already detached", + nodeName, diskURI) + return nil + } + klog.Warningf("failed to get azure instance id (%v)", err) + return fmt.Errorf("failed to get azure instance id for node %q (%v)", nodeName, err) + } + vmset, err := c.getNodeVMSet(nodeName) if err != nil { return err } - instanceid, err := c.cloud.InstanceID(context.TODO(), nodeName) - if err != nil { - klog.Warningf("failed to get azure instance id (%v)", err) - return fmt.Errorf("failed to get azure instance id for node %q (%v)", nodeName, err) - } - klog.V(2).Infof("detach %v from node %q", diskURI, nodeName) // make the lock here as small as possible 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 32e6da6bd2c..477c35b1cf5 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 @@ -88,9 +88,9 @@ func TestCommonDetachDisk(t *testing.T) { expectedErr bool }{ { - desc: "an error shall be returned if there's no such instance corresponding to given nodeName", + desc: "error should not be returned if there's no such instance corresponding to given nodeName", nodeName: "vm1", - expectedErr: true, + expectedErr: false, }, { desc: "no error shall be returned if there's no matching disk according to given diskName",