Merge pull request kubernetes#31978 from jsafrane/detach-before-delete

Automatic merge from submit-queue

Do not report error when deleting an attached volume

Persistent volume controller should not send warning events to a PV and mark the PV as failed when the volume is still attached.

This happens when a user quickly deletes a pod and associated PVC - PV is slowly detaching, while the PVC is already deleted and the PV enters Failed phase.

`Deleter.Deleter` can now return `tryAgainError`, which is sent as INFO to the PV to let the user know we did not forget to delete the PV, however the PV stays in Released state. The controller tries again in the next sync (15 seconds by default).

Fixes kubernetes#31511

pkg/cloudprovider/providers/openstack/openstack_volumes.go

@@ -23,6 +23,8 @@ import (
 	"path"
 	"strings"
 
+	"k8s.io/kubernetes/pkg/volume"
+
 	"github.com/rackspace/gophercloud"
 	"github.com/rackspace/gophercloud/openstack"
 	"github.com/rackspace/gophercloud/openstack/blockstorage/v1/volumes"
@@ -182,6 +184,15 @@ func (os *OpenStack) GetDevicePath(diskId string) string {
 }
 
 func (os *OpenStack) DeleteVolume(volumeName string) error {
+	used, err := os.diskIsUsed(volumeName)
+	if err != nil {
+		return err
+	}
+	if used {
+		msg := fmt.Sprintf("Cannot delete the volume %q, it's still attached to a node", volumeName)
+		return volume.NewDeletedVolumeInUseError(msg)
+	}
+
 	sClient, err := openstack.NewBlockStorageV1(os.provider, gophercloud.EndpointOpts{
 		Region: os.region,
 	})
@@ -228,3 +239,15 @@ func (os *OpenStack) DiskIsAttached(diskName, instanceID string) (bool, error) {
 	}
 	return false, nil
 }
+
+// diskIsUsed returns true a disk is attached to any node.
+func (os *OpenStack) diskIsUsed(diskName string) (bool, error) {
+	disk, err := os.getVolume(diskName)
+	if err != nil {
+		return false, err
+	}
+	if len(disk.Attachments) > 0 {
+		return true, nil
+	}
+	return false, nil
+}

pkg/volume/cinder/cinder_util.go

@@ -124,6 +124,8 @@ func (util *CinderDiskUtil) DeleteVolume(cd *cinderVolumeDeleter) error {
 	}
 
 	if err = cloud.DeleteVolume(cd.pdName); err != nil {
+		// OpenStack cloud provider returns volume.tryAgainError when necessary,
+		// no handling needed here.
 		glog.V(2).Infof("Error deleting cinder volume %s: %v", cd.pdName, err)
 		return err
 	}