Merge pull request #21144 from hashicorp/f-stop-instance-volume-attach

Add option to stop an instance before trying to remove an attached volume

.changelog/21144.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement
+resource/aws_volume_attachment: Add `stop_instance_before_detaching` argument
+```

aws/internal/service/ec2/waiter/waiter.go

@@ -19,6 +19,8 @@ const (
 	// Maximum amount of time to wait for EC2 Instance attribute modifications to propagate
 	InstanceAttributePropagationTimeout = 2 * time.Minute
 
+	InstanceStopTimeout = 10 * time.Minute
+
 	// General timeout for EC2 resource creations to propagate
 	PropagationTimeout = 2 * time.Minute
 )

aws/resource_aws_volume_attachment.go

@@ -13,6 +13,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter"
 )
 
 func resourceAwsVolumeAttachment() *schema.Resource {
@@ -65,6 +66,10 @@ func resourceAwsVolumeAttachment() *schema.Resource {
 				Type:     schema.TypeBool,
 				Optional: true,
 			},
+			"stop_instance_before_detaching": {
+				Type:     schema.TypeBool,
+				Optional: true,
+			},
 		},
 	}
 }
@@ -149,43 +154,6 @@ func resourceAwsVolumeAttachmentCreate(d *schema.ResourceData, meta interface{})
 	return resourceAwsVolumeAttachmentRead(d, meta)
 }
 
-func volumeAttachmentStateRefreshFunc(conn *ec2.EC2, name, volumeID, instanceID string) resource.StateRefreshFunc {
-	return func() (interface{}, string, error) {
-		request := &ec2.DescribeVolumesInput{
-			VolumeIds: []*string{aws.String(volumeID)},
-			Filters: []*ec2.Filter{
-				{
-					Name:   aws.String("attachment.device"),
-					Values: []*string{aws.String(name)},
-				},
-				{
-					Name:   aws.String("attachment.instance-id"),
-					Values: []*string{aws.String(instanceID)},
-				},
-			},
-		}
-
-		resp, err := conn.DescribeVolumes(request)
-		if err != nil {
-			if awsErr, ok := err.(awserr.Error); ok {
-				return nil, "failed", fmt.Errorf("code: %s, message: %s", awsErr.Code(), awsErr.Message())
-			}
-			return nil, "failed", err
-		}
-
-		if len(resp.Volumes) > 0 {
-			v := resp.Volumes[0]
-			for _, a := range v.Attachments {
-				if aws.StringValue(a.InstanceId) == instanceID {
-					return a, aws.StringValue(a.State), nil
-				}
-			}
-		}
-		// assume detached if volume count is 0
-		return 42, ec2.VolumeAttachmentStateDetached, nil
-	}
-}
-
 func resourceAwsVolumeAttachmentRead(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).ec2conn
 
@@ -236,6 +204,19 @@ 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 {
+		_, err := conn.StopInstances(&ec2.StopInstancesInput{
+			InstanceIds: []*string{aws.String(iID)},
+		})
+		if err != nil {
+			return fmt.Errorf("error stopping instance (%s): %s", iID, err)
+		}
+
+		if err := waitForInstanceStopping(conn, iID, waiter.InstanceStopTimeout); err != nil {
+			return err
+		}
+	}
+
 	opts := &ec2.DetachVolumeInput{
 		Device:     aws.String(name),
 		InstanceId: aws.String(iID),
@@ -268,6 +249,43 @@ func resourceAwsVolumeAttachmentDelete(d *schema.ResourceData, meta interface{})
 	return nil
 }
 
+func volumeAttachmentStateRefreshFunc(conn *ec2.EC2, name, volumeID, instanceID string) resource.StateRefreshFunc {
+	return func() (interface{}, string, error) {
+		request := &ec2.DescribeVolumesInput{
+			VolumeIds: []*string{aws.String(volumeID)},
+			Filters: []*ec2.Filter{
+				{
+					Name:   aws.String("attachment.device"),
+					Values: []*string{aws.String(name)},
+				},
+				{
+					Name:   aws.String("attachment.instance-id"),
+					Values: []*string{aws.String(instanceID)},
+				},
+			},
+		}
+
+		resp, err := conn.DescribeVolumes(request)
+		if err != nil {
+			if awsErr, ok := err.(awserr.Error); ok {
+				return nil, "failed", fmt.Errorf("code: %s, message: %s", awsErr.Code(), awsErr.Message())
+			}
+			return nil, "failed", err
+		}
+
+		if len(resp.Volumes) > 0 {
+			v := resp.Volumes[0]
+			for _, a := range v.Attachments {
+				if aws.StringValue(a.InstanceId) == instanceID {
+					return a, aws.StringValue(a.State), nil
+				}
+			}
+		}
+		// assume detached if volume count is 0
+		return 42, ec2.VolumeAttachmentStateDetached, nil
+	}
+}
+
 func volumeAttachmentID(name, volumeID, instanceID string) string {
 	var buf bytes.Buffer
 	buf.WriteString(fmt.Sprintf("%s-", name))

aws/resource_aws_volume_attachment_test.go

@@ -214,6 +214,40 @@ func TestAccAWSVolumeAttachment_disappears(t *testing.T) {
 	})
 }
 
+func TestAccAWSVolumeAttachment_stopInstance(t *testing.T) {
+	var i ec2.Instance
+	var v ec2.Volume
+	resourceName := "aws_volume_attachment.test"
+	rName := acctest.RandomWithPrefix("tf-acc-test")
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		ErrorCheck:   testAccErrorCheck(t, ec2.EndpointsID),
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckVolumeAttachmentDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccVolumeAttachmentStopInstanceConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "device_name", "/dev/sdh"),
+					testAccCheckInstanceExists("aws_instance.test", &i),
+					testAccCheckVolumeExists("aws_ebs_volume.test", &v),
+					testAccCheckVolumeAttachmentExists(resourceName, &i, &v),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateIdFunc: testAccAWSVolumeAttachmentImportStateIDFunc(resourceName),
+				ImportStateVerify: true,
+				ImportStateVerifyIgnore: []string{
+					"stop_instance_before_detaching",
+				},
+			},
+		},
+	})
+}
+
 func testAccCheckVolumeAttachmentExists(n string, i *ec2.Instance, v *ec2.Volume) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]
@@ -322,6 +356,27 @@ resource "aws_volume_attachment" "test" {
 `
 }
 
+func testAccVolumeAttachmentStopInstanceConfig(rName string) string {
+	return composeConfig(testAccVolumeAttachmentInstanceOnlyConfigBase(rName),
+		fmt.Sprintf(`
+resource "aws_ebs_volume" "test" {
+  availability_zone = data.aws_availability_zones.available.names[0]
+  size              = 1000
+
+  tags = {
+    Name = %[1]q
+  }
+}
+
+resource "aws_volume_attachment" "test" {
+  device_name                    = "/dev/sdh"
+  volume_id                      = aws_ebs_volume.test.id
+  instance_id                    = aws_instance.test.id
+  stop_instance_before_detaching = "true"
+}
+`, rName))
+}
+
 func testAccVolumeAttachmentConfigSkipDestroy(rName string) string {
 	return testAccVolumeAttachmentConfigBase(rName) + fmt.Sprintf(`
 data "aws_ebs_volume" "test" {

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