From b593d6903db8a95ef2db2b1fa1f074c13ba5dd15 Mon Sep 17 00:00:00 2001 From: Joe Topjian Date: Mon, 23 Jan 2017 15:19:50 -0700 Subject: [PATCH] provider/openstack: Volume Attachment Updates (#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. --- ...urce_openstack_compute_volume_attach_v2.go | 36 ++++++++++++-- ...openstack_compute_volume_attach_v2_test.go | 49 +++++++++++++++++++ .../r/compute_volume_attach_v2.html.markdown | 14 ++++-- 3 files changed, 92 insertions(+), 7 deletions(-) diff --git a/builtin/providers/openstack/resource_openstack_compute_volume_attach_v2.go b/builtin/providers/openstack/resource_openstack_compute_volume_attach_v2.go index ad2be47aea..34404ee853 100644 --- a/builtin/providers/openstack/resource_openstack_compute_volume_attach_v2.go +++ b/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 { diff --git a/builtin/providers/openstack/resource_openstack_compute_volume_attach_v2_test.go b/builtin/providers/openstack/resource_openstack_compute_volume_attach_v2_test.go index f839fa7a3a..a32e3ad3df 100644 --- a/builtin/providers/openstack/resource_openstack_compute_volume_attach_v2_test.go +++ b/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" +} +` diff --git a/website/source/docs/providers/openstack/r/compute_volume_attach_v2.html.markdown b/website/source/docs/providers/openstack/r/compute_volume_attach_v2.html.markdown index 389c5f356e..bab27b7358 100644 --- a/website/source/docs/providers/openstack/r/compute_volume_attach_v2.html.markdown +++ b/website/source/docs/providers/openstack/r/compute_volume_attach_v2.html.markdown @@ -43,6 +43,13 @@ 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: @@ -50,10 +57,9 @@ 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