Merge pull request #55008 from gnufied/fix-mutable-api-calls-aws

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Check for available volume before attach/delete in EBS We should check for available volume before performing attach or delete of EBS volume. Mutable API calls such as Attach Volume or Delete Volume has much lower quota than immutable API calls such as DescribeVolume or DescribeInstance. Checking for available state before attach or delete ensures that we are not making useless API calls. Fixes https://github.com/kubernetes/kubernetes/issues/55014 ```release-note Check for available volume before attach/delete operation in EBS ```

Merge pull request #55008 from gnufied/fix-mutable-api-calls-aws
65b8f5bd · Kubernetes Submit Queue · GitHub · d837a6a2 · c00b136c · 65b8f5bd
Unverified Commit 65b8f5bd authored Nov 03, 2017 by Kubernetes Submit Queue Committed by GitHub Nov 03, 2017
Hide whitespace changes
Inline Side-by-side

Showing with 45 additions and 0 deletions

aws.go pkg/cloudprovider/providers/aws/aws.go +45 -0

No files found.
--- 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 {