From e872c3d8baf01bd24283b7c74b46df05ce7ae1c3 Mon Sep 17 00:00:00 2001
From: Joe Topjian <joe@topjian.net>
Date: Sat, 27 Feb 2016 05:56:12 +0000
Subject: [PATCH] provider/openstack: Instance Block Device cleanup

This commit fixes and cleans up instance block_device configuration.

Reverts #5354 in that `volume_size` is only required in certain
block_device configuration combinations. Therefore, the actual
attribute must be set to Optional and later checks done.

Doc upates, too.
---
 .../resource_openstack_compute_instance_v2.go | 28 +++++++++++++++++--
 ...urce_openstack_compute_instance_v2_test.go |  3 --
 .../r/compute_instance_v2.html.markdown       |  6 ++--
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/builtin/providers/openstack/resource_openstack_compute_instance_v2.go b/builtin/providers/openstack/resource_openstack_compute_instance_v2.go
index d41024432..862b428f9 100644
--- a/builtin/providers/openstack/resource_openstack_compute_instance_v2.go
+++ b/builtin/providers/openstack/resource_openstack_compute_instance_v2.go
@@ -201,7 +201,7 @@ func resourceComputeInstanceV2() *schema.Resource {
 						},
 						"volume_size": &schema.Schema{
 							Type:     schema.TypeInt,
-							Required: true,
+							Optional: true,
 						},
 						"destination_type": &schema.Schema{
 							Type:     schema.TypeString,
@@ -334,13 +334,18 @@ func resourceComputeInstanceV2Create(d *schema.ResourceData, meta interface{}) e
 		return err
 	}
 
-	// determine if volume/block_device configuration is correct
+	// determine if volume configuration is correct
 	// this includes ensuring volume_ids are set
-	// and if only one block_device was specified.
 	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 {
+		return err
+	}
+
 	// check if floating IP configuration is correct
 	if err := checkInstanceFloatingIPs(d); err != nil {
 		return err
@@ -1416,12 +1421,29 @@ func checkVolumeConfig(d *schema.ResourceData) error {
 		}
 	}
 
+	return nil
+}
+
+func checkBlockDeviceConfig(d *schema.ResourceData) error {
 	if vL, ok := d.GetOk("block_device"); ok {
 		for _, v := range vL.([]interface{}) {
 			vM := v.(map[string]interface{})
+
 			if vM["source_type"] != "blank" && vM["uuid"] == "" {
 				return fmt.Errorf("You must specify a uuid for %s block device types", vM["source_type"])
 			}
+
+			if vM["source_type"] == "image" && vM["destination_type"] == "volume" {
+				if vM["volume_size"] == 0 {
+					return fmt.Errorf("You must specify a volume_size when creating a volume from an image")
+				}
+			}
+
+			if vM["source_type"] == "blank" && vM["destination_type"] == "local" {
+				if vM["volume_size"] == 0 {
+					return fmt.Errorf("You must specify a volume_size when creating a blank block device")
+				}
+			}
 		}
 	}
 
diff --git a/builtin/providers/openstack/resource_openstack_compute_instance_v2_test.go b/builtin/providers/openstack/resource_openstack_compute_instance_v2_test.go
index 09ac11345..1627693ba 100644
--- a/builtin/providers/openstack/resource_openstack_compute_instance_v2_test.go
+++ b/builtin/providers/openstack/resource_openstack_compute_instance_v2_test.go
@@ -403,7 +403,6 @@ func TestAccComputeV2Instance_bootFromVolumeVolume(t *testing.T) {
 			block_device {
 				uuid = "${openstack_blockstorage_volume_v1.foo.id}"
 				source_type = "volume"
-				volume_size = 5
 				boot_index = 0
 				destination_type = "volume"
 				delete_on_termination = true
@@ -476,7 +475,6 @@ func TestAccComputeV2Instance_multiEphemeral(t *testing.T) {
 				boot_index = -1
 				delete_on_termination = true
 				destination_type = "local"
-				guest_format = "ext4"
 				source_type = "blank"
 				volume_size = 1
 			}
@@ -484,7 +482,6 @@ func TestAccComputeV2Instance_multiEphemeral(t *testing.T) {
 				boot_index = -1
 				delete_on_termination = true
 				destination_type = "local"
-				guest_format = "ext4"
 				source_type = "blank"
 				volume_size = 1
 			}
diff --git a/website/source/docs/providers/openstack/r/compute_instance_v2.html.markdown b/website/source/docs/providers/openstack/r/compute_instance_v2.html.markdown
index eb1b47e56..ac855aa30 100644
--- a/website/source/docs/providers/openstack/r/compute_instance_v2.html.markdown
+++ b/website/source/docs/providers/openstack/r/compute_instance_v2.html.markdown
@@ -123,7 +123,9 @@ The `block_device` block supports:
 * `source_type` - (Required) The source type of the device. Must be one of
     "image", "volume", or "snapshot".
 
-* `volume_size` - (Required) The size of the volume to create (in gigabytes).
+* `volume_size` - The size of the volume to create (in gigabytes). Required
+    in the following combinations: source=image and destination=volume,
+    source=blank and destination=local.
 
 * `boot_index` - (Optional) The boot index of the volume. It defaults to 0.
 
@@ -231,7 +233,6 @@ resource "openstack_compute_instance_v2" "foo" {
     boot_index = -1
     delete_on_termination = true
     destination_type = "local"
-    guest_format = "ext4"
     source_type = "blank"
     volume_size = 1
   }
@@ -240,7 +241,6 @@ resource "openstack_compute_instance_v2" "foo" {
     boot_index = -1
     delete_on_termination = true
     destination_type = "local"
-    guest_format = "ext4"
     source_type = "blank"
     volume_size = 1
   }