diff --git a/pkg/volume/vsphere_volume/attacher.go b/pkg/volume/vsphere_volume/attacher.go index a07ddce489e..edfbdbe50c2 100644 --- a/pkg/volume/vsphere_volume/attacher.go +++ b/pkg/volume/vsphere_volume/attacher.go @@ -277,7 +277,7 @@ func (plugin *vsphereVolumePlugin) NewDeviceUnmounter() (volume.DeviceUnmounter, func (detacher *vsphereVMDKDetacher) Detach(volumeName string, nodeName types.NodeName) error { volPath := getVolPathfromVolumeName(volumeName) - attached, err := detacher.vsphereVolumes.DiskIsAttached(volPath, nodeName) + attached, newVolumePath, err := detacher.vsphereVolumes.DiskIsAttached(volPath, nodeName) if err != nil { // Log error and continue with detach klog.Errorf( @@ -293,7 +293,7 @@ func (detacher *vsphereVMDKDetacher) Detach(volumeName string, nodeName types.No attachdetachMutex.LockKey(string(nodeName)) defer attachdetachMutex.UnlockKey(string(nodeName)) - if err := detacher.vsphereVolumes.DetachDisk(volPath, nodeName); err != nil { + if err := detacher.vsphereVolumes.DetachDisk(newVolumePath, nodeName); err != nil { klog.Errorf("Error detaching volume %q: %v", volPath, err) return err } diff --git a/pkg/volume/vsphere_volume/attacher_test.go b/pkg/volume/vsphere_volume/attacher_test.go index 42af1c80b1b..bb83f3a865c 100644 --- a/pkg/volume/vsphere_volume/attacher_test.go +++ b/pkg/volume/vsphere_volume/attacher_test.go @@ -22,7 +22,7 @@ import ( "errors" "testing" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/volume" volumetest "k8s.io/kubernetes/pkg/volume/testing" @@ -285,29 +285,29 @@ func (testcase *testcase) DetachDisk(diskName string, nodeName types.NodeName) e return expected.ret } -func (testcase *testcase) DiskIsAttached(diskName string, nodeName types.NodeName) (bool, error) { +func (testcase *testcase) DiskIsAttached(diskName string, nodeName types.NodeName) (bool, string, error) { expected := &testcase.diskIsAttached if expected.diskName == "" && expected.nodeName == "" { // testcase.diskIsAttached looks uninitialized, test did not expect to // call DiskIsAttached testcase.t.Errorf("Unexpected DiskIsAttached call!") - return false, errors.New("Unexpected DiskIsAttached call!") + return false, diskName, errors.New("Unexpected DiskIsAttached call!") } if expected.diskName != diskName { testcase.t.Errorf("Unexpected DiskIsAttached call: expected diskName %s, got %s", expected.diskName, diskName) - return false, errors.New("Unexpected DiskIsAttached call: wrong diskName") + return false, diskName, errors.New("Unexpected DiskIsAttached call: wrong diskName") } if expected.nodeName != nodeName { testcase.t.Errorf("Unexpected DiskIsAttached call: expected nodeName %s, got %s", expected.nodeName, nodeName) - return false, errors.New("Unexpected DiskIsAttached call: wrong nodeName") + return false, diskName, errors.New("Unexpected DiskIsAttached call: wrong nodeName") } klog.V(4).Infof("DiskIsAttached call: %s, %s, returning %v, %v", diskName, nodeName, expected.isAttached, expected.ret) - return expected.isAttached, expected.ret + return expected.isAttached, diskName, expected.ret } func (testcase *testcase) DisksAreAttached(nodeVolumes map[types.NodeName][]string) (map[types.NodeName]map[string]bool, error) { diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/virtualmachine.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/virtualmachine.go index 4837b7db373..8ba7d74ef2e 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/virtualmachine.go +++ b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vclib/virtualmachine.go @@ -168,7 +168,6 @@ func (vm *VirtualMachine) AttachDisk(ctx context.Context, vmDiskPath string, vol // DetachDisk detaches the disk specified by vmDiskPath func (vm *VirtualMachine) DetachDisk(ctx context.Context, vmDiskPath string) error { - vmDiskPath = RemoveStorageClusterORFolderNameFromVDiskPath(vmDiskPath) device, err := vm.getVirtualDeviceByPath(ctx, vmDiskPath) if err != nil { klog.Errorf("Disk ID not found for VM: %q with diskPath: %q", vm.InventoryPath, vmDiskPath) diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go index aa09c141975..aa53e3435d9 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go +++ b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go @@ -221,7 +221,7 @@ type Volumes interface { // DiskIsAttached checks if a disk is attached to the given node. // Assumption: If node doesn't exist, disk is not attached to the node. - DiskIsAttached(volPath string, nodeName k8stypes.NodeName) (bool, error) + DiskIsAttached(volPath string, nodeName k8stypes.NodeName) (bool, string, error) // DisksAreAttached checks if a list disks are attached to the given node. // Assumption: If node doesn't exist, disks are not attached to the node. @@ -924,6 +924,13 @@ func (vs *VSphere) AttachDisk(vmDiskPath string, storagePolicyName string, nodeN return "", err } + // try and get canonical path for disk and if we can't throw error + vmDiskPath, err = getcanonicalVolumePath(ctx, vm.Datacenter, vmDiskPath) + if err != nil { + klog.Errorf("failed to get canonical path for %s on node %s: %v", vmDiskPath, convertToString(nodeName), err) + return "", err + } + diskUUID, err = vm.AttachDisk(ctx, vmDiskPath, &vclib.VolumeOptions{SCSIControllerType: vclib.PVSCSIControllerType, StoragePolicyName: storagePolicyName}) if err != nil { klog.Errorf("Failed to attach disk: %s for node: %s. err: +%v", vmDiskPath, convertToString(nodeName), err) @@ -1004,8 +1011,8 @@ func (vs *VSphere) DetachDisk(volPath string, nodeName k8stypes.NodeName) error } // DiskIsAttached returns if disk is attached to the VM using controllers supported by the plugin. -func (vs *VSphere) DiskIsAttached(volPath string, nodeName k8stypes.NodeName) (bool, error) { - diskIsAttachedInternal := func(volPath string, nodeName k8stypes.NodeName) (bool, error) { +func (vs *VSphere) DiskIsAttached(volPath string, nodeName k8stypes.NodeName) (bool, string, error) { + diskIsAttachedInternal := func(volPath string, nodeName k8stypes.NodeName) (bool, string, error) { var vSphereInstance string if nodeName == "" { vSphereInstance = vs.hostName @@ -1018,25 +1025,30 @@ func (vs *VSphere) DiskIsAttached(volPath string, nodeName k8stypes.NodeName) (b defer cancel() vsi, err := vs.getVSphereInstance(nodeName) if err != nil { - return false, err + return false, volPath, err } // Ensure client is logged in and session is valid err = vs.nodeManager.vcConnect(ctx, vsi) if err != nil { - return false, err + return false, volPath, err } vm, err := vs.getVMFromNodeName(ctx, nodeName) if err != nil { if err == vclib.ErrNoVMFound { klog.Warningf("Node %q does not exist, vsphere CP will assume disk %v is not attached to it.", nodeName, volPath) // make the disk as detached and return false without error. - return false, nil + return false, volPath, nil } klog.Errorf("Failed to get VM object for node: %q. err: +%v", vSphereInstance, err) - return false, err + return false, volPath, err } volPath = vclib.RemoveStorageClusterORFolderNameFromVDiskPath(volPath) + canonicalPath, pathFetchErr := getcanonicalVolumePath(ctx, vm.Datacenter, volPath) + // if canonicalPath is not empty string and pathFetchErr is nil then we can use canonical path to perform detach + if canonicalPath != "" && pathFetchErr == nil { + volPath = canonicalPath + } attached, err := vm.IsDiskAttached(ctx, volPath) if err != nil { klog.Errorf("DiskIsAttached failed to determine whether disk %q is still attached on node %q", @@ -1044,22 +1056,22 @@ func (vs *VSphere) DiskIsAttached(volPath string, nodeName k8stypes.NodeName) (b vSphereInstance) } klog.V(4).Infof("DiskIsAttached result: %v and error: %q, for volume: %q", attached, err, volPath) - return attached, err + return attached, volPath, err } requestTime := time.Now() - isAttached, err := diskIsAttachedInternal(volPath, nodeName) + isAttached, newVolumePath, err := diskIsAttachedInternal(volPath, nodeName) if err != nil { if vclib.IsManagedObjectNotFoundError(err) { err = vs.nodeManager.RediscoverNode(nodeName) if err == vclib.ErrNoVMFound { isAttached, err = false, nil } else if err == nil { - isAttached, err = diskIsAttachedInternal(volPath, nodeName) + isAttached, newVolumePath, err = diskIsAttachedInternal(volPath, nodeName) } } } vclib.RecordvSphereMetric(vclib.OperationDiskIsAttached, requestTime, err) - return isAttached, err + return isAttached, newVolumePath, err } // DisksAreAttached returns if disks are attached to the VM using controllers supported by the plugin.