Block Storage Acceptance Test Cleanup (#43)

diff --git a/acceptance/openstack/blockstorage/v2/blockstorage.go b/acceptance/openstack/blockstorage/v2/blockstorage.go
new file mode 100644
index 0000000..e007c2a
--- /dev/null
+++ b/acceptance/openstack/blockstorage/v2/blockstorage.go
@@ -0,0 +1,74 @@
+// Package v2 contains common functions for creating block storage based
+// resources for use in acceptance tests. See the `*_test.go` files for
+// example usages.
+package v2
+
+import (
+	"testing"
+
+	"github.com/gophercloud/gophercloud"
+	"github.com/gophercloud/gophercloud/acceptance/tools"
+	"github.com/gophercloud/gophercloud/openstack/blockstorage/v2/volumes"
+)
+
+// CreateVolume will create a volume with a random name and size of 1GB. An
+// error will be returned if the volume was unable to be created.
+func CreateVolume(t *testing.T, client *gophercloud.ServiceClient) (*volumes.Volume, error) {
+	if testing.Short() {
+		t.Skip("Skipping test that requires volume creation in short mode.")
+	}
+
+	volumeName := tools.RandomString("ACPTTEST", 16)
+	t.Logf("Attempting to create volume: %s", volumeName)
+
+	createOpts := volumes.CreateOpts{
+		Size: 1,
+		Name: volumeName,
+	}
+
+	volume, err := volumes.Create(client, createOpts).Extract()
+	if err != nil {
+		return volume, err
+	}
+
+	err = volumes.WaitForStatus(client, volume.ID, "available", 60)
+	if err != nil {
+		return volume, err
+	}
+
+	return volume, nil
+}
+
+// DeleteVolume will delete a volume. A fatal error will occur if the volume
+// failed to be deleted. This works best when used as a deferred function.
+func DeleteVolume(t *testing.T, client *gophercloud.ServiceClient, volume *volumes.Volume) {
+	err := volumes.Delete(client, volume.ID).ExtractErr()
+	if err != nil {
+		t.Fatalf("Unable to delete volume %s: %v", volume.ID, err)
+	}
+
+	t.Logf("Deleted volume: %s", volume.ID)
+}
+
+// PrintVolume will print a volume and all of its attributes.
+func PrintVolume(t *testing.T, volume *volumes.Volume) {
+	t.Logf("ID: %s", volume.ID)
+	t.Logf("Status: %s", volume.Status)
+	t.Logf("Size: %d", volume.Size)
+	t.Logf("AvailabilityZone: %s", volume.AvailabilityZone)
+	t.Logf("CreatedAt: %v", volume.CreatedAt)
+	t.Logf("UpdatedAt: %v", volume.CreatedAt)
+	t.Logf("Attachments: %#v", volume.Attachments)
+	t.Logf("Name: %s", volume.Name)
+	t.Logf("Description: %s", volume.Description)
+	t.Logf("VolumeType: %s", volume.VolumeType)
+	t.Logf("SnapshotID: %s", volume.SnapshotID)
+	t.Logf("SourceVolID: %s", volume.SourceVolID)
+	t.Logf("Metadata: %#v", volume.Metadata)
+	t.Logf("UserID: %s", volume.UserID)
+	t.Logf("Bootable: %s", volume.Bootable)
+	t.Logf("Encrypted: %s", volume.Encrypted)
+	t.Logf("ReplicationStatus: %s", volume.ReplicationStatus)
+	t.Logf("ConsistencyGroupID: %s", volume.ConsistencyGroupID)
+	t.Logf("Multiattach: %t", volume.Multiattach)
+}
diff --git a/acceptance/openstack/blockstorage/v2/extensions/extensions.go b/acceptance/openstack/blockstorage/v2/extensions/extensions.go
new file mode 100644
index 0000000..685f769
--- /dev/null
+++ b/acceptance/openstack/blockstorage/v2/extensions/extensions.go
@@ -0,0 +1,97 @@
+// Package extensions contains common functions for creating block storage
+// resources that are extensions of the block storage API. See the `*_test.go`
+// files for example usages.
+package extensions
+
+import (
+	"testing"
+
+	"github.com/gophercloud/gophercloud"
+	"github.com/gophercloud/gophercloud/openstack/blockstorage/v2/extensions/volumeactions"
+	"github.com/gophercloud/gophercloud/openstack/blockstorage/v2/volumes"
+	"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
+)
+
+// CreateVolumeAttach will attach a volume to an instance. An error will be
+// returned if the attachment failed.
+func CreateVolumeAttach(t *testing.T, client *gophercloud.ServiceClient, volume *volumes.Volume, server *servers.Server) error {
+	if testing.Short() {
+		t.Skip("Skipping test that requires volume attachment in short mode.")
+	}
+
+	attachOpts := volumeactions.AttachOpts{
+		MountPoint:   "/mnt",
+		Mode:         "rw",
+		InstanceUUID: server.ID,
+	}
+
+	t.Logf("Attempting to attach volume %s to server %s", volume.ID, server.ID)
+
+	if err := volumeactions.Attach(client, volume.ID, attachOpts).ExtractErr(); err != nil {
+		return err
+	}
+
+	if err := volumes.WaitForStatus(client, volume.ID, "in-use", 60); err != nil {
+		return err
+	}
+
+	t.Logf("Attached volume %s to server %s", volume.ID, server.ID)
+
+	return nil
+}
+
+// CreateVolumeReserve creates a volume reservation. An error will be returned
+// if the reservation failed.
+func CreateVolumeReserve(t *testing.T, client *gophercloud.ServiceClient, volume *volumes.Volume) error {
+	if testing.Short() {
+		t.Skip("Skipping test that requires volume reservation in short mode.")
+	}
+
+	t.Logf("Attempting to reserve volume %s", volume.ID)
+
+	if err := volumeactions.Reserve(client, volume.ID).ExtractErr(); err != nil {
+		return err
+	}
+
+	t.Logf("Reserved volume %s", volume.ID)
+
+	return nil
+}
+
+// DeleteVolumeAttach will detach a volume from an instance. A fatal error will
+// occur if the snapshot failed to be deleted. This works best when used as a
+// deferred function.
+func DeleteVolumeAttach(t *testing.T, client *gophercloud.ServiceClient, volume *volumes.Volume) {
+	t.Logf("Attepting to detach volume volume: %s", volume.ID)
+
+	detachOpts := volumeactions.DetachOpts{
+		AttachmentID: volume.Attachments[0].AttachmentID,
+	}
+
+	if err := volumeactions.Detach(client, volume.ID, detachOpts).ExtractErr(); err != nil {
+		t.Fatalf("Unable to detach volume %s: %v", volume.ID, err)
+	}
+
+	if err := volumes.WaitForStatus(client, volume.ID, "available", 60); err != nil {
+		t.Fatalf("Volume %s failed to become unavailable in 60 seconds: %v", volume.ID, err)
+	}
+
+	t.Logf("Detached volume: %s", volume.ID)
+}
+
+// DeleteVolumeReserve deletes a volume reservation. A fatal error will occur
+// if the deletion request failed. This works best when used as a deferred
+// function.
+func DeleteVolumeReserve(t *testing.T, client *gophercloud.ServiceClient, volume *volumes.Volume) {
+	if testing.Short() {
+		t.Skip("Skipping test that requires volume reservation in short mode.")
+	}
+
+	t.Logf("Attempting to unreserve volume %s", volume.ID)
+
+	if err := volumeactions.Unreserve(client, volume.ID).ExtractErr(); err != nil {
+		t.Fatalf("Unable to unreserve volume %s: %v", volume.ID, err)
+	}
+
+	t.Logf("Unreserved volume %s", volume.ID)
+}
diff --git a/acceptance/openstack/blockstorage/v2/extensions/volumeactions_test.go b/acceptance/openstack/blockstorage/v2/extensions/volumeactions_test.go
index 20a4597..83d1571 100644
--- a/acceptance/openstack/blockstorage/v2/extensions/volumeactions_test.go
+++ b/acceptance/openstack/blockstorage/v2/extensions/volumeactions_test.go
@@ -3,147 +3,116 @@
 package extensions
 
 import (
-	"os"
 	"testing"
 
-	"github.com/gophercloud/gophercloud"
-	"github.com/gophercloud/gophercloud/openstack"
-	"github.com/gophercloud/gophercloud/openstack/blockstorage/v2/extensions/volumeactions"
+	"github.com/gophercloud/gophercloud/acceptance/clients"
 	"github.com/gophercloud/gophercloud/openstack/blockstorage/v2/volumes"
-	th "github.com/gophercloud/gophercloud/testhelper"
+
+	blockstorage "github.com/gophercloud/gophercloud/acceptance/openstack/blockstorage/v2"
+	compute "github.com/gophercloud/gophercloud/acceptance/openstack/compute/v2"
 )
 
-func newClient(t *testing.T) (*gophercloud.ServiceClient, error) {
-	ao, err := openstack.AuthOptionsFromEnv()
-	th.AssertNoErr(t, err)
-
-	client, err := openstack.AuthenticatedClient(ao)
-	th.AssertNoErr(t, err)
-
-	return openstack.NewBlockStorageV2(client, gophercloud.EndpointOpts{
-		Region: os.Getenv("OS_REGION_NAME"),
-	})
-}
-
-func TestVolumeAttach(t *testing.T) {
-	client, err := newClient(t)
-	th.AssertNoErr(t, err)
-
-	t.Logf("Creating volume")
-	cv, err := volumes.Create(client, &volumes.CreateOpts{
-		Size: 1,
-		Name: "blockv2-volume",
-	}).Extract()
-	th.AssertNoErr(t, err)
-
-	defer func() {
-		err = volumes.WaitForStatus(client, cv.ID, "available", 60)
-		th.AssertNoErr(t, err)
-
-		t.Logf("Deleting volume")
-		err = volumes.Delete(client, cv.ID).ExtractErr()
-		th.AssertNoErr(t, err)
-	}()
-
-	err = volumes.WaitForStatus(client, cv.ID, "available", 60)
-	th.AssertNoErr(t, err)
-
-	instanceID := os.Getenv("OS_INSTANCE_ID")
-	if instanceID == "" {
-		t.Fatal("Environment variable OS_INSTANCE_ID is required")
+func TestVolumeActionsAttachCreateDestroy(t *testing.T) {
+	blockClient, err := clients.NewBlockStorageV2Client()
+	if err != nil {
+		t.Fatalf("Unable to create a blockstorage client: %v", err)
 	}
 
-	t.Logf("Attaching volume")
-	err = volumeactions.Attach(client, cv.ID, &volumeactions.AttachOpts{
-		MountPoint:   "/mnt",
-		Mode:         "rw",
-		InstanceUUID: instanceID,
-	}).ExtractErr()
-	th.AssertNoErr(t, err)
+	computeClient, err := clients.NewComputeV2Client()
+	if err != nil {
+		t.Fatalf("Unable to create a compute client: %v", err)
+	}
 
-	err = volumes.WaitForStatus(client, cv.ID, "in-use", 60)
-	th.AssertNoErr(t, err)
+	choices, err := clients.AcceptanceTestChoicesFromEnv()
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	t.Logf("Detaching volume")
-	err = volumeactions.Detach(client, cv.ID).ExtractErr()
-	th.AssertNoErr(t, err)
+	server, err := compute.CreateServer(t, computeClient, choices)
+	if err != nil {
+		t.Fatalf("Unable to create server: %v", err)
+	}
+	defer compute.DeleteServer(t, computeClient, server)
+
+	volume, err := blockstorage.CreateVolume(t, blockClient)
+	if err != nil {
+		t.Fatalf("Unable to create volume: %v", err)
+	}
+	defer blockstorage.DeleteVolume(t, blockClient, volume)
+
+	err = CreateVolumeAttach(t, blockClient, volume, server)
+	if err != nil {
+		t.Fatalf("Unable to attach volume: %v", err)
+	}
+
+	newVolume, err := volumes.Get(blockClient, volume.ID).Extract()
+	if err != nil {
+		t.Fatal("Unable to get updated volume information: %v", err)
+	}
+
+	DeleteVolumeAttach(t, blockClient, newVolume)
 }
 
-func TestVolumeReserve(t *testing.T) {
-	client, err := newClient(t)
-	th.AssertNoErr(t, err)
+func TestVolumeActionsReserveUnreserve(t *testing.T) {
+	client, err := clients.NewBlockStorageV2Client()
+	if err != nil {
+		t.Fatalf("Unable to create blockstorage client: %v", err)
+	}
 
-	t.Logf("Creating volume")
-	cv, err := volumes.Create(client, &volumes.CreateOpts{
-		Size: 1,
-		Name: "blockv2-volume",
-	}).Extract()
-	th.AssertNoErr(t, err)
+	volume, err := blockstorage.CreateVolume(t, client)
+	if err != nil {
+		t.Fatalf("Unable to create volume: %v", err)
+	}
+	defer blockstorage.DeleteVolume(t, client, volume)
 
-	defer func() {
-		err = volumes.WaitForStatus(client, cv.ID, "available", 60)
-		th.AssertNoErr(t, err)
-
-		t.Logf("Deleting volume")
-		err = volumes.Delete(client, cv.ID).ExtractErr()
-		th.AssertNoErr(t, err)
-	}()
-
-	err = volumes.WaitForStatus(client, cv.ID, "available", 60)
-	th.AssertNoErr(t, err)
-
-	t.Logf("Reserving volume")
-	err = volumeactions.Reserve(client, cv.ID).ExtractErr()
-	th.AssertNoErr(t, err)
-
-	err = volumes.WaitForStatus(client, cv.ID, "attaching", 60)
-	th.AssertNoErr(t, err)
-
-	t.Logf("Unreserving volume")
-	err = volumeactions.Unreserve(client, cv.ID).ExtractErr()
-	th.AssertNoErr(t, err)
-
-	err = volumes.WaitForStatus(client, cv.ID, "available", 60)
-	th.AssertNoErr(t, err)
+	err = CreateVolumeReserve(t, client, volume)
+	if err != nil {
+		t.Fatalf("Unable to create volume reserve: %v", err)
+	}
+	defer DeleteVolumeReserve(t, client, volume)
 }
 
+// Note(jtopjian): I plan to work on this at some point, but it requires
+// setting up a server with iscsi utils.
+/*
 func TestVolumeConns(t *testing.T) {
-	client, err := newClient(t)
-	th.AssertNoErr(t, err)
+    client, err := newClient()
+    th.AssertNoErr(t, err)
 
-	t.Logf("Creating volume")
-	cv, err := volumes.Create(client, &volumes.CreateOpts{
-		Size: 1,
-		Name: "blockv2-volume",
-	}).Extract()
-	th.AssertNoErr(t, err)
+    t.Logf("Creating volume")
+    cv, err := volumes.Create(client, &volumes.CreateOpts{
+        Size: 1,
+        Name: "blockv2-volume",
+    }).Extract()
+    th.AssertNoErr(t, err)
 
-	defer func() {
-		err = volumes.WaitForStatus(client, cv.ID, "available", 60)
-		th.AssertNoErr(t, err)
+    defer func() {
+        err = volumes.WaitForStatus(client, cv.ID, "available", 60)
+        th.AssertNoErr(t, err)
 
-		t.Logf("Deleting volume")
-		err = volumes.Delete(client, cv.ID).ExtractErr()
-		th.AssertNoErr(t, err)
-	}()
+        t.Logf("Deleting volume")
+        err = volumes.Delete(client, cv.ID).ExtractErr()
+        th.AssertNoErr(t, err)
+    }()
 
-	err = volumes.WaitForStatus(client, cv.ID, "available", 60)
-	th.AssertNoErr(t, err)
+    err = volumes.WaitForStatus(client, cv.ID, "available", 60)
+    th.AssertNoErr(t, err)
 
-	connOpts := &volumeactions.ConnectorOpts{
-		IP:        "127.0.0.1",
-		Host:      "stack",
-		Initiator: "iqn.1994-05.com.redhat:17cf566367d2",
-		Multipath: false,
-		Platform:  "x86_64",
-		OSType:    "linux2",
-	}
+    connOpts := &volumeactions.ConnectorOpts{
+        IP:        "127.0.0.1",
+        Host:      "stack",
+        Initiator: "iqn.1994-05.com.redhat:17cf566367d2",
+        Multipath: false,
+        Platform:  "x86_64",
+        OSType:    "linux2",
+    }
 
-	t.Logf("Initializing connection")
-	_, err = volumeactions.InitializeConnection(client, cv.ID, connOpts).Extract()
-	th.AssertNoErr(t, err)
+    t.Logf("Initializing connection")
+    _, err = volumeactions.InitializeConnection(client, cv.ID, connOpts).Extract()
+    th.AssertNoErr(t, err)
 
-	t.Logf("Terminating connection")
-	err = volumeactions.TerminateConnection(client, cv.ID, connOpts).ExtractErr()
-	th.AssertNoErr(t, err)
+    t.Logf("Terminating connection")
+    err = volumeactions.TerminateConnection(client, cv.ID, connOpts).ExtractErr()
+    th.AssertNoErr(t, err)
 }
+*/
diff --git a/acceptance/openstack/blockstorage/v2/volumes_test.go b/acceptance/openstack/blockstorage/v2/volumes_test.go
index 9edf31a..3508bc5 100644
--- a/acceptance/openstack/blockstorage/v2/volumes_test.go
+++ b/acceptance/openstack/blockstorage/v2/volumes_test.go
@@ -3,61 +3,49 @@
 package v2
 
 import (
-	"os"
 	"testing"
 
-	"github.com/gophercloud/gophercloud"
-	"github.com/gophercloud/gophercloud/openstack"
+	"github.com/gophercloud/gophercloud/acceptance/clients"
 	"github.com/gophercloud/gophercloud/openstack/blockstorage/v2/volumes"
-	"github.com/gophercloud/gophercloud/pagination"
-	th "github.com/gophercloud/gophercloud/testhelper"
 )
 
-func newClient(t *testing.T) (*gophercloud.ServiceClient, error) {
-	ao, err := openstack.AuthOptionsFromEnv()
-	th.AssertNoErr(t, err)
-
-	client, err := openstack.AuthenticatedClient(ao)
-	th.AssertNoErr(t, err)
-
-	return openstack.NewBlockStorageV2(client, gophercloud.EndpointOpts{
-		Region: os.Getenv("OS_REGION_NAME"),
-	})
-}
-
-func TestVolumes(t *testing.T) {
-	client, err := newClient(t)
-	th.AssertNoErr(t, err)
-
-	cv, err := volumes.Create(client, &volumes.CreateOpts{
-		Size: 1,
-		Name: "blockv2-volume",
-	}).Extract()
-	th.AssertNoErr(t, err)
-	defer func() {
-		err = volumes.WaitForStatus(client, cv.ID, "available", 60)
-		th.AssertNoErr(t, err)
-		err = volumes.Delete(client, cv.ID).ExtractErr()
-		th.AssertNoErr(t, err)
-	}()
-
-	_, err = volumes.Update(client, cv.ID, &volumes.UpdateOpts{
-		Name: "blockv2-updated-volume",
-	}).Extract()
-	th.AssertNoErr(t, err)
-
-	v, err := volumes.Get(client, cv.ID).Extract()
-	th.AssertNoErr(t, err)
-	t.Logf("Got volume: %+v\n", v)
-
-	if v.Name != "blockv2-updated-volume" {
-		t.Errorf("Unable to update volume: Expected name: blockv2-updated-volume\nActual name: %s", v.Name)
+func TestVolumesList(t *testing.T) {
+	client, err := clients.NewBlockStorageV2Client()
+	if err != nil {
+		t.Fatalf("Unable to create a blockstorage client: %v", err)
 	}
 
-	err = volumes.List(client, &volumes.ListOpts{Name: "blockv2-updated-volume"}).EachPage(func(page pagination.Page) (bool, error) {
-		vols, err := volumes.ExtractVolumes(page)
-		th.CheckEquals(t, 1, len(vols))
-		return true, err
-	})
-	th.AssertNoErr(t, err)
+	allPages, err := volumes.List(client, volumes.ListOpts{}).AllPages()
+	if err != nil {
+		t.Fatalf("Unable to retrieve volumes: %v", err)
+	}
+
+	allVolumes, err := volumes.ExtractVolumes(allPages)
+	if err != nil {
+		t.Fatalf("Unable to extract volumes: %v", err)
+	}
+
+	for _, volume := range allVolumes {
+		PrintVolume(t, &volume)
+	}
+}
+
+func TestVolumesCreateDestroy(t *testing.T) {
+	client, err := clients.NewBlockStorageV2Client()
+	if err != nil {
+		t.Fatalf("Unable to create blockstorage client: %v", err)
+	}
+
+	volume, err := CreateVolume(t, client)
+	if err != nil {
+		t.Fatalf("Unable to create volume: %v", err)
+	}
+	defer DeleteVolume(t, client, volume)
+
+	newVolume, err := volumes.Get(client, volume.ID).Extract()
+	if err != nil {
+		t.Errorf("Unable to retrieve volume: %v", err)
+	}
+
+	PrintVolume(t, newVolume)
 }