From c00b136c745de45024420092d55b3421e079a782 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Thu, 2 Nov 2017 12:33:43 -0400 Subject: [PATCH] Check for available volume before attach/delete in EBS We should check for available volume before performing attach or delete of EBS volume. This will make sure that we do not blow up API quota of mutable operations in AWS and stay a good citizen. --- pkg/cloudprovider/providers/aws/aws.go | 45 ++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 94f4323a1b2..db63f968216 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -1716,6 +1716,12 @@ func (c *Cloud) AttachDisk(diskName KubernetesVolumeID, nodeName types.NodeName, ec2Device := "/dev/xvd" + string(mountDevice) if !alreadyAttached { + available, err := c.checkIfAvailable(disk, "attaching", awsInstance.awsID) + + if !available { + attachEnded = true + return "", err + } request := &ec2.AttachVolumeInput{ Device: aws.String(ec2Device), InstanceId: aws.String(awsInstance.awsID), @@ -1948,9 +1954,48 @@ func (c *Cloud) DeleteDisk(volumeName KubernetesVolumeID) (bool, error) { if err != nil { return false, err } + available, err := c.checkIfAvailable(awsDisk, "deleting", "") + + if !available { + return false, err + } + return awsDisk.deleteVolume() } +func (c *Cloud) checkIfAvailable(disk *awsDisk, opName string, instance string) (bool, error) { + info, err := disk.describeVolume() + + if err != nil { + glog.Errorf("Error describing volume %q: %q", disk.awsID, err) + // if for some reason we can not describe volume we will return error + return false, err + } + + volumeState := aws.StringValue(info.State) + opError := fmt.Sprintf("Error %s EBS volume %q", opName, disk.awsID) + if len(instance) != 0 { + opError = fmt.Sprintf("%q to instance %q", opError, instance) + } + + // Only available volumes can be attached or deleted + if volumeState != "available" { + // Volume is attached somewhere else and we can not attach it here + if len(info.Attachments) > 0 { + attachment := info.Attachments[0] + attachErr := fmt.Errorf("%s since volume is currently attached to %q", opError, aws.StringValue(attachment.InstanceId)) + glog.Error(attachErr) + return false, attachErr + } + + attachErr := fmt.Errorf("%s since volume is in %q state", opError, volumeState) + glog.Error(attachErr) + return false, attachErr + } + + return true, nil +} + func (c *Cloud) GetLabelsForVolume(pv *v1.PersistentVolume) (map[string]string, error) { // Ignore any volumes that are being provisioned if pv.Spec.AWSElasticBlockStore.VolumeID == volume.ProvisionedVolumeName {