From 27ac3c758d234245215cb53d73ce81ded0521738 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Sun, 11 Aug 2019 13:19:40 +0000 Subject: [PATCH 1/2] fix: detach azure disk issue using dangling error fix: unit test failure --- pkg/volume/azure_dd/attacher.go | 2 +- .../azure/azure_controller_common.go | 30 ++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/pkg/volume/azure_dd/attacher.go b/pkg/volume/azure_dd/attacher.go index 4eec28f23b4f..77deb1ae773b 100644 --- a/pkg/volume/azure_dd/attacher.go +++ b/pkg/volume/azure_dd/attacher.go @@ -87,7 +87,7 @@ func (a *azureDiskAttacher) Attach(spec *volume.Spec, nodeName types.NodeName) ( klog.V(2).Infof("Attach operation successful: volume %q attached to node %q.", volumeSource.DataDiskURI, nodeName) } else { klog.V(2).Infof("Attach volume %q to instance %q failed with %v", volumeSource.DataDiskURI, nodeName, err) - return "", fmt.Errorf("Attach volume %q to instance %q failed with %v", volumeSource.DiskName, nodeName, err) + return "", err } } 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 7bad7d72db90..01844b7af594 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 @@ -19,14 +19,16 @@ package azure import ( "context" "fmt" + "path" "time" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-03-01/compute" - "k8s.io/klog" "k8s.io/apimachinery/pkg/types" kwait "k8s.io/apimachinery/pkg/util/wait" cloudprovider "k8s.io/cloud-provider" + volerr "k8s.io/cloud-provider/volume/errors" + "k8s.io/klog" "k8s.io/utils/keymutex" ) @@ -93,6 +95,32 @@ func (c *controllerCommon) getNodeVMSet(nodeName types.NodeName) (VMSet, error) // AttachDisk attaches a vhd to vm. The vhd must exist, can be identified by diskName, diskURI. // return (lun, error) func (c *controllerCommon) AttachDisk(isManagedDisk bool, diskName, diskURI string, nodeName types.NodeName, cachingMode compute.CachingTypes) (int32, error) { + if isManagedDisk { + diskName := path.Base(diskURI) + resourceGroup, err := getResourceGroupFromDiskURI(diskURI) + if err != nil { + return -1, err + } + + ctx, cancel := getContextWithCancel() + defer cancel() + + disk, err := c.cloud.DisksClient.Get(ctx, resourceGroup, diskName) + if err != nil { + return -1, err + } + + if disk.ManagedBy != nil { + attachErr := fmt.Sprintf( + "disk(%s) already attached to node(%s), could not be attached to node(%s)", + diskURI, *disk.ManagedBy, nodeName) + attachedNode := path.Base(*disk.ManagedBy) + klog.V(2).Infof("found dangling volume %s attached to node %s", diskURI, attachedNode) + danglingErr := volerr.NewDanglingError(attachErr, types.NodeName(attachedNode), "") + return -1, danglingErr + } + } + vmset, err := c.getNodeVMSet(nodeName) if err != nil { return -1, err From cbcae27318566852161fbdfc4fcfa19641087963 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Sun, 18 Aug 2019 03:34:25 +0000 Subject: [PATCH 2/2] fix: disk not found issue in detaching azure disk --- .../azure/azure_controller_standard.go | 6 +++--- .../legacy-cloud-providers/azure/azure_controller_vmss.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_standard.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_standard.go index 858c6e322c26..eaa646eb80ed 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_standard.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_standard.go @@ -118,9 +118,9 @@ func (as *availabilitySet) DetachDisk(diskName, diskURI string, nodeName types.N bFoundDisk := false for i, disk := range disks { - if disk.Lun != nil && (disk.Name != nil && diskName != "" && *disk.Name == diskName) || - (disk.Vhd != nil && disk.Vhd.URI != nil && diskURI != "" && *disk.Vhd.URI == diskURI) || - (disk.ManagedDisk != nil && diskURI != "" && *disk.ManagedDisk.ID == diskURI) { + if disk.Lun != nil && (disk.Name != nil && diskName != "" && strings.EqualFold(*disk.Name, diskName)) || + (disk.Vhd != nil && disk.Vhd.URI != nil && diskURI != "" && strings.EqualFold(*disk.Vhd.URI, diskURI)) || + (disk.ManagedDisk != nil && diskURI != "" && strings.EqualFold(*disk.ManagedDisk.ID, diskURI)) { // found the disk klog.V(2).Infof("azureDisk - detach disk: name %q uri %q", diskName, diskURI) disks = append(disks[:i], disks[i+1:]...) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_vmss.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_vmss.go index 5236cd0aa0b3..f07eb9d6d7ee 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_vmss.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_vmss.go @@ -123,9 +123,9 @@ func (ss *scaleSet) DetachDisk(diskName, diskURI string, nodeName types.NodeName } bFoundDisk := false for i, disk := range disks { - if disk.Lun != nil && (disk.Name != nil && diskName != "" && *disk.Name == diskName) || - (disk.Vhd != nil && disk.Vhd.URI != nil && diskURI != "" && *disk.Vhd.URI == diskURI) || - (disk.ManagedDisk != nil && diskURI != "" && *disk.ManagedDisk.ID == diskURI) { + if disk.Lun != nil && (disk.Name != nil && diskName != "" && strings.EqualFold(*disk.Name, diskName)) || + (disk.Vhd != nil && disk.Vhd.URI != nil && diskURI != "" && strings.EqualFold(*disk.Vhd.URI, diskURI)) || + (disk.ManagedDisk != nil && diskURI != "" && strings.EqualFold(*disk.ManagedDisk.ID, diskURI)) { // found the disk klog.V(2).Infof("azureDisk - detach disk: name %q uri %q", diskName, diskURI) disks = append(disks[:i], disks[i+1:]...)