From e7f6e8f8f4b763143296a912795038dc7a101d74 Mon Sep 17 00:00:00 2001
From: Wolfgang Felbermeier <wolfgang@ventx.de>
Date: Fri, 10 May 2019 15:27:02 +0200
Subject: [PATCH] Add option to stop an instance before trying to remove an
 attached volume By stopping the instance, the volume is unmounted in the
 instance and the detaching of the volume doesn't run into a timeout

fixes #6673
fixes #2084
fixes #2957
fixes #4770
fixes #288
fixes #1017
---
 aws/resource_aws_volume_attachment.go         | 64 +++++++++++++++++++
 .../docs/r/volume_attachment.html.markdown    |  2 +
 2 files changed, 66 insertions(+)

diff --git a/aws/resource_aws_volume_attachment.go b/aws/resource_aws_volume_attachment.go
index 8d0514c9e4ac..9b1cf75d8a23 100644
--- a/aws/resource_aws_volume_attachment.go
+++ b/aws/resource_aws_volume_attachment.go
@@ -65,6 +65,10 @@ func resourceAwsVolumeAttachment() *schema.Resource {
 				Type:     schema.TypeBool,
 				Optional: true,
 			},
+			"stop_instance_before_detaching": {
+				Type:     schema.TypeBool,
+				Optional: true,
+			},
 		},
 	}
 }
@@ -186,6 +190,43 @@ func volumeAttachmentStateRefreshFunc(conn *ec2.EC2, name, volumeID, instanceID
 	}
 }
 
+// InstanceStateRefreshFunc returns a resource.StateRefreshFunc that is used to watch
+// an EC2 instance.
+func instanceStateRefreshFunc(conn *ec2.EC2, instanceID string, failStates []string) resource.StateRefreshFunc {
+	return func() (interface{}, string, error) {
+		resp, err := conn.DescribeInstances(&ec2.DescribeInstancesInput{
+			InstanceIds: []*string{aws.String(instanceID)},
+		})
+		if err != nil {
+			if ec2err, ok := err.(awserr.Error); ok && ec2err.Code() == "InvalidInstanceID.NotFound" {
+				// Set this to nil as if we didn't find anything.
+				resp = nil
+			} else {
+				log.Printf("Error on InstanceStateRefresh: %s", err)
+				return nil, "", err
+			}
+		}
+
+		if resp == nil || len(resp.Reservations) == 0 || len(resp.Reservations[0].Instances) == 0 {
+			// Sometimes AWS just has consistency issues and doesn't see
+			// our instance yet. Return an empty state.
+			return nil, "", nil
+		}
+
+		i := resp.Reservations[0].Instances[0]
+		state := *i.State.Name
+
+		for _, failState := range failStates {
+			if state == failState {
+				return i, state, fmt.Errorf("Failed to reach target state. Reason: %s",
+					stringifyStateReason(i.StateReason))
+			}
+		}
+
+		return i, state, nil
+	}
+}
+
 func resourceAwsVolumeAttachmentRead(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).ec2conn
 
@@ -236,6 +277,29 @@ func resourceAwsVolumeAttachmentDelete(d *schema.ResourceData, meta interface{})
 	vID := d.Get("volume_id").(string)
 	iID := d.Get("instance_id").(string)
 
+	if _, ok := d.GetOk("stop_instance_before_detaching"); ok {
+		opts := &ec2.StopInstancesInput{
+			InstanceIds: []*string{aws.String(iID)},
+		}
+		stateConf := &resource.StateChangeConf{
+			Pending:    []string{"pending", "running", "shutting-down", "stopping"},
+			Target:     []string{"stopped"},
+			Refresh:    instanceStateRefreshFunc(conn, iID, []string{}),
+			Timeout:    5 * time.Minute,
+			Delay:      10 * time.Second,
+			MinTimeout: 3 * time.Second,
+		}
+		log.Printf("[Debug] Stopping target instance (%s) before detaching Volume (%s)", iID, vID)
+		_, err := conn.StopInstances(opts)
+		if err != nil {
+			return fmt.Errorf("Error stopping Instance (%s)", iID)
+		}
+		_, err = stateConf.WaitForState()
+		if err != nil {
+			return fmt.Errorf("Error waiting Instance (%s) to stop", iID)
+		}
+	}
+
 	opts := &ec2.DetachVolumeInput{
 		Device:     aws.String(name),
 		InstanceId: aws.String(iID),
diff --git a/website/docs/r/volume_attachment.html.markdown b/website/docs/r/volume_attachment.html.markdown
index 193e60707233..464e923558c6 100644
--- a/website/docs/r/volume_attachment.html.markdown
+++ b/website/docs/r/volume_attachment.html.markdown
@@ -55,6 +55,8 @@ to detach the volume from the instance to which it is attached at destroy
 time, and instead just remove the attachment from Terraform state. This is
 useful when destroying an instance which has volumes created by some other
 means attached.
+* `stop_instance_before_detaching` - (Optional, Boolean) Set this to true to ensure that the target instance is stopped
+before trying to detach the volume. Stops the instance, if it is not already stopped.
 
 ## Attributes Reference