Skip to content

Commit

Permalink
provider/openstack: Fix Volume Attachment Detection in Instances
Browse files Browse the repository at this point in the history 
This commit makes the volume argument non-computed, which in turn
enables Terraform to now correctly know when a user has removed
a volume.

In addition, when volumes are read by the instance, volumes configured
in the Terraform configuration are the source of truth. Previously,
a call was being made to OpenStack to provide the list of attached
volumes.

It also adds a few new tests and fixes existing tests for various volume
attach-related scenarios.
  • Loading branch information
@jtopjian
jtopjian committed Aug 24, 2016
1 parent 51d669b commit f116fb0
Show file tree
Hide file tree
Showing 2 changed files with 238 additions and 76 deletions.
106 changes: 42 additions & 64 deletions builtin/providers/openstack/resource_openstack_compute_instance_v2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"github.com/rackspace/gophercloud/openstack/compute/v2/flavors"
"github.com/rackspace/gophercloud/openstack/compute/v2/images"
"github.com/rackspace/gophercloud/openstack/compute/v2/servers"
"github.com/rackspace/gophercloud/pagination"
)

func resourceComputeInstanceV2() *schema.Resource {
Expand Down Expand Up @@ -238,22 +237,19 @@ func resourceComputeInstanceV2() *schema.Resource {
"volume": &schema.Schema{
Type: schema.TypeSet,
Optional: true,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"id": &schema.Schema{
Type: schema.TypeString,
Computed: true,
Optional: true,
},
"volume_id": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Computed: true,
Required: true,
},
"device": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Computed: true,
},
},
},
Expand Down Expand Up @@ -351,12 +347,6 @@ func resourceComputeInstanceV2Create(d *schema.ResourceData, meta interface{}) e
return err
}

// determine if volume configuration is correct
// this includes ensuring volume_ids are set
if err := checkVolumeConfig(d); err != nil {
return err
}

// determine if block_device configuration is correct
// this includes valid combinations and required attributes
if err := checkBlockDeviceConfig(d); err != nil {
Expand Down Expand Up @@ -712,16 +702,12 @@ func resourceComputeInstanceV2Update(d *schema.ResourceData, meta interface{}) e
}

if d.HasChange("volume") {
// ensure the volume configuration is correct
if err := checkVolumeConfig(d); err != nil {
return err
}

// old attachments and new attachments
oldAttachments, newAttachments := d.GetChange("volume")
// for each old attachment, detach the volume
oldAttachmentSet := oldAttachments.(*schema.Set).List()

log.Printf("[DEBUG] Attempting to detach the following volumes: %#v", oldAttachmentSet)
if blockClient, err := config.blockStorageV1Client(d.Get("region").(string)); err != nil {
return err
} else {
Expand Down Expand Up @@ -814,6 +800,22 @@ func resourceComputeInstanceV2Delete(d *schema.ResourceData, meta interface{}) e
return fmt.Errorf("Error creating OpenStack compute client: %s", err)
}

// Make sure all volumes are detached before deleting
volumes := d.Get("volume")
if volumeSet, ok := volumes.(*schema.Set); ok {
volumeList := volumeSet.List()
if len(volumeList) > 0 {
log.Printf("[DEBUG] Attempting to detach the following volumes: %#v", volumeList)
if blockClient, err := config.blockStorageV1Client(d.Get("region").(string)); err != nil {
return err
} else {
if err := detachVolumesFromInstance(computeClient, blockClient, d.Id(), volumeList); err != nil {
return err
}
}
}
}

if d.Get("stop_before_destroy").(bool) {
err = startstop.Stop(computeClient, d.Id()).ExtractErr()
if err != nil {
Expand Down Expand Up @@ -1397,6 +1399,7 @@ func detachVolumesFromInstance(computeClient *gophercloud.ServiceClient, blockCl
va := v.(map[string]interface{})
aId := va["id"].(string)

log.Printf("[INFO] Attempting to detach volume %s", va["volume_id"])
if err := volumeattach.Delete(computeClient, serverId, aId).ExtractErr(); err != nil {
return err
}
Expand All @@ -1420,65 +1423,40 @@ func detachVolumesFromInstance(computeClient *gophercloud.ServiceClient, blockCl
}

func getVolumeAttachments(computeClient *gophercloud.ServiceClient, d *schema.ResourceData) error {
var attachments []volumeattach.VolumeAttachment

err := volumeattach.List(computeClient, d.Id()).EachPage(func(page pagination.Page) (bool, error) {
actual, err := volumeattach.ExtractVolumeAttachments(page)
if err != nil {
return false, err
}

attachments = actual
return true, nil
})
var vols []map[string]interface{}

allPages, err := volumeattach.List(computeClient, d.Id()).AllPages()
if err != nil {
return err
}

var vols []map[string]interface{}
for _, attachment := range attachments {
// ignore the volume if it is attached as a root device
rootDevFound := false
for _, rootDev := range []string{"/dev/vda", "/dev/xda", "/dev/sda", "/dev/xvda"} {
if attachment.Device == rootDev {
rootDevFound = true
}
}

if rootDevFound {
continue
}

vol := make(map[string]interface{})
vol["id"] = attachment.ID
vol["volume_id"] = attachment.VolumeID
vol["device"] = attachment.Device
vols = append(vols, vol)
allVolumeAttachments, err := volumeattach.ExtractVolumeAttachments(allPages)
if err != nil {
return err
}
log.Printf("[INFO] Volume attachments: %v", vols)
d.Set("volume", vols)

return nil
}

func checkVolumeConfig(d *schema.ResourceData) error {
// Although a volume_id is required to attach a volume, in order to be able to report
// the attached volumes of an instance, it must be "computed" and thus "optional".
// This accounts for situations such as "boot from volume" as well as volumes being
// attached to the instance outside of Terraform.
if v := d.Get("volume"); v != nil {
vols := v.(*schema.Set).List()
if len(vols) > 0 {
for _, v := range vols {
va := v.(map[string]interface{})
if va["volume_id"].(string) == "" {
return fmt.Errorf("A volume_id must be specified when attaching volumes.")
if v, ok := d.GetOk("volume"); ok {
volumes := v.(*schema.Set).List()
for _, volume := range volumes {
if volumeMap, ok := volume.(map[string]interface{}); ok {
if v, ok := volumeMap["volume_id"].(string); ok {
for _, volumeAttachment := range allVolumeAttachments {
if v == volumeAttachment.ID {
vol := make(map[string]interface{})
vol["id"] = volumeAttachment.ID
vol["volume_id"] = volumeAttachment.VolumeID
vol["device"] = volumeAttachment.Device
vols = append(vols, vol)
}
}
}
}
}
}

log.Printf("[INFO] Volume attachments: %v", vols)
d.Set("volume", vols)

return nil
}

Expand Down
Loading

0 comments on commit f116fb0

Please sign in to comment.