provider/openstack: Volume Attachment Updates (hashicorp#11285)

This commit adds a StateRefresh func for volume attachments. Mostly
this is to add a buffer of time between the request and the return
of the attachment to give time for the volume to become attached,
however, in some cases the refresh function could work as specified.

Docs have also been updated to reflect that a device could be specified,
but to use with caution.

Author: jtopjian

builtin/providers/openstack/resource_openstack_compute_volume_attach_v2.go

@@ -78,6 +78,19 @@ func resourceComputeVolumeAttachV2Create(d *schema.ResourceData, meta interface{
 		return err
 	}
 
+	stateConf := &resource.StateChangeConf{
+		Pending:    []string{"ATTACHING"},
+		Target:     []string{"ATTACHED"},
+		Refresh:    resourceComputeVolumeAttachV2AttachFunc(computeClient, instanceId, attachment.ID),
+		Timeout:    10 * time.Minute,
+		Delay:      30 * time.Second,
+		MinTimeout: 15 * time.Second,
+	}
+
+	if _, err = stateConf.WaitForState(); err != nil {
+		return fmt.Errorf("Error attaching OpenStack volume: %s", err)
+	}
+
 	log.Printf("[DEBUG] Created volume attachment: %#v", attachment)
 
 	// Use the instance ID and attachment ID as the resource ID.
@@ -131,7 +144,7 @@ func resourceComputeVolumeAttachV2Delete(d *schema.ResourceData, meta interface{
 	stateConf := &resource.StateChangeConf{
 		Pending:    []string{""},
 		Target:     []string{"DETACHED"},
-		Refresh:    volumeDetachRefreshFunc(computeClient, instanceId, attachmentId),
+		Refresh:    resourceComputeVolumeAttachV2DetachFunc(computeClient, instanceId, attachmentId),
 		Timeout:    10 * time.Minute,
 		Delay:      15 * time.Second,
 		MinTimeout: 15 * time.Second,
@@ -144,9 +157,26 @@ func resourceComputeVolumeAttachV2Delete(d *schema.ResourceData, meta interface{
 	return nil
 }
 
-func volumeDetachRefreshFunc(computeClient *gophercloud.ServiceClient, instanceId, attachmentId string) resource.StateRefreshFunc {
+func resourceComputeVolumeAttachV2AttachFunc(
+	computeClient *gophercloud.ServiceClient, instanceId, attachmentId string) resource.StateRefreshFunc {
+	return func() (interface{}, string, error) {
+		va, err := volumeattach.Get(computeClient, instanceId, attachmentId).Extract()
+		if err != nil {
+			if _, ok := err.(gophercloud.ErrDefault404); ok {
+				return va, "ATTACHING", nil
+			}
+			return va, "", err
+		}
+
+		return va, "ATTACHED", nil
+	}
+}
+
+func resourceComputeVolumeAttachV2DetachFunc(
+	computeClient *gophercloud.ServiceClient, instanceId, attachmentId string) resource.StateRefreshFunc {
 	return func() (interface{}, string, error) {
-		log.Printf("[DEBUG] Attempting to detach OpenStack volume %s from instance %s", attachmentId, instanceId)
+		log.Printf("[DEBUG] Attempting to detach OpenStack volume %s from instance %s",
+			attachmentId, instanceId)
 
 		va, err := volumeattach.Get(computeClient, instanceId, attachmentId).Extract()
 		if err != nil {

builtin/providers/openstack/resource_openstack_compute_volume_attach_v2_test.go

@@ -28,6 +28,25 @@ func TestAccComputeV2VolumeAttach_basic(t *testing.T) {
 	})
 }
 
+func TestAccComputeV2VolumeAttach_device(t *testing.T) {
+	var va volumeattach.VolumeAttachment
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckComputeV2VolumeAttachDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccComputeV2VolumeAttach_device,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeV2VolumeAttachExists("openstack_compute_volume_attach_v2.va_1", &va),
+					testAccCheckComputeV2VolumeAttachDevice(&va, "/dev/vdc"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckComputeV2VolumeAttachDestroy(s *terraform.State) error {
 	config := testAccProvider.Meta().(*Config)
 	computeClient, err := config.computeV2Client(OS_REGION_NAME)
@@ -91,6 +110,18 @@ func testAccCheckComputeV2VolumeAttachExists(n string, va *volumeattach.VolumeAt
 	}
 }
 
+func testAccCheckComputeV2VolumeAttachDevice(
+	va *volumeattach.VolumeAttachment, device string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		if va.Device != device {
+			return fmt.Errorf("Requested device of volume attachment (%s) does not match: %s",
+				device, va.Device)
+		}
+
+		return nil
+	}
+}
+
 const testAccComputeV2VolumeAttach_basic = `
 resource "openstack_blockstorage_volume_v2" "volume_1" {
   name = "volume_1"
@@ -107,3 +138,21 @@ resource "openstack_compute_volume_attach_v2" "va_1" {
   volume_id = "${openstack_blockstorage_volume_v2.volume_1.id}"
 }
 `
+
+const testAccComputeV2VolumeAttach_device = `
+resource "openstack_blockstorage_volume_v2" "volume_1" {
+  name = "volume_1"
+  size = 1
+}
+
+resource "openstack_compute_instance_v2" "instance_1" {
+  name = "instance_1"
+  security_groups = ["default"]
+}
+
+resource "openstack_compute_volume_attach_v2" "va_1" {
+  instance_id = "${openstack_compute_instance_v2.instance_1.id}"
+  volume_id = "${openstack_blockstorage_volume_v2.volume_1.id}"
+  device = "/dev/vdc"
+}
+`

website/source/docs/providers/openstack/r/compute_volume_attach_v2.html.markdown

@@ -43,17 +43,23 @@ The following arguments are supported:
 
 * `volume_id` - (Required) The ID of the Volume to attach to an Instance.
 
+* `device` - (Optional) The device of the volume attachment (ex: `/dev/vdc`).
+  _NOTE_: Being able to specify a device is dependent upon the hypervisor in
+  use. There is a chance that the device specified in Terraform will not be
+  the same device the hypervisor chose. If this happens, Terraform will wish
+  to update the device upon subsequent applying which will cause the volume
+  to be detached and reattached indefinitely. Please use with caution.
+
 ## Attributes Reference
 
 The following attributes are exported:
 
 * `region` - See Argument Reference above.
 * `instance_id` - See Argument Reference above.
 * `volume_id` - See Argument Reference above.
-* `device` - The device of the volume attachment (ex: `/dev/vdc`).
-  _NOTE_: This is the device reported by the Compute API and the real device
-  might actually differ depending on the hypervisor being used. This should
-  not be used as an authoritative piece of information.
+* `device` - See Argument Reference above. _NOTE_: The correctness of this
+  information is dependent upon the hypervisor in use. In some cases, this
+  should not be used as an authoritative piece of information.
 
 ## Import