Merge pull request #7612 from svanharmelen/b-cloudstack-provider

provider/cloudstack: fix and improve several use cases

Author: Sander van Harmelen

builtin/providers/cloudstack/resource_cloudstack_disk.go

@@ -28,8 +28,8 @@ func resourceCloudStackDisk() *schema.Resource {
 				Default:  false,
 			},
 
-			"device": &schema.Schema{
-				Type:     schema.TypeString,
+			"device_id": &schema.Schema{
+				Type:     schema.TypeInt,
 				Optional: true,
 				Computed: true,
 			},
@@ -59,6 +59,7 @@ func resourceCloudStackDisk() *schema.Resource {
 			"project": &schema.Schema{
 				Type:     schema.TypeString,
 				Optional: true,
+				Computed: true,
 				ForceNew: true,
 			},
 
@@ -116,7 +117,7 @@ func resourceCloudStackDiskCreate(d *schema.ResourceData, meta interface{}) erro
 	// Set the volume ID and partials
 	d.SetId(r.Id)
 	d.SetPartial("name")
-	d.SetPartial("device")
+	d.SetPartial("device_id")
 	d.SetPartial("disk_offering")
 	d.SetPartial("size")
 	d.SetPartial("virtual_machine_id")
@@ -163,28 +164,7 @@ func resourceCloudStackDiskRead(d *schema.ResourceData, meta interface{}) error
 	setValueOrID(d, "zone", v.Zonename, v.Zoneid)
 
 	if v.Attached != "" {
-		// Get the virtual machine details
-		vm, _, err := cs.VirtualMachine.GetVirtualMachineByID(
-			v.Virtualmachineid,
-			cloudstack.WithProject(d.Get("project").(string)),
-		)
-		if err != nil {
-			return err
-		}
-
-		// Get the guest OS type details
-		os, _, err := cs.GuestOS.GetOsTypeByID(vm.Guestosid)
-		if err != nil {
-			return err
-		}
-
-		// Get the guest OS category details
-		c, _, err := cs.GuestOS.GetOsCategoryByID(os.Oscategoryid)
-		if err != nil {
-			return err
-		}
-
-		d.Set("device", retrieveDeviceName(v.Deviceid, c.Name))
+		d.Set("device_id", int(v.Deviceid))
 		d.Set("virtual_machine_id", v.Virtualmachineid)
 	}
 
@@ -235,9 +215,9 @@ func resourceCloudStackDiskUpdate(d *schema.ResourceData, meta interface{}) erro
 		d.SetPartial("size")
 	}
 
-	// If the device changed, just detach here so we can re-attach the
+	// If the device ID changed, just detach here so we can re-attach the
 	// volume at the end of this function
-	if d.HasChange("device") || d.HasChange("virtual_machine") {
+	if d.HasChange("device_id") || d.HasChange("virtual_machine") {
 		// Detach the volume
 		if err := resourceCloudStackDiskDetach(d, meta); err != nil {
 			return fmt.Errorf("Error detaching disk %s from virtual machine: %s", name, err)
@@ -253,7 +233,7 @@ func resourceCloudStackDiskUpdate(d *schema.ResourceData, meta interface{}) erro
 
 		// Set the additional partials
 		d.SetPartial("attach")
-		d.SetPartial("device")
+		d.SetPartial("device_id")
 		d.SetPartial("virtual_machine_id")
 	} else {
 		// Detach the volume
@@ -304,21 +284,14 @@ func resourceCloudStackDiskAttach(d *schema.ResourceData, meta interface{}) erro
 		// Create a new parameter struct
 		p := cs.Volume.NewAttachVolumeParams(d.Id(), virtualmachineid.(string))
 
-		if device, ok := d.GetOk("device"); ok {
-			// Retrieve the device ID
-			deviceid := retrieveDeviceID(device.(string))
-			if deviceid == -1 {
-				return fmt.Errorf("Device %s is not a valid device", device.(string))
-			}
-
-			// Set the device ID
-			p.SetDeviceid(deviceid)
+		if deviceid, ok := d.GetOk("device_id"); ok {
+			p.SetDeviceid(int64(deviceid.(int)))
 		}
 
 		// Attach the new volume
-		r, err := Retry(4, retryableAttachVolumeFunc(cs, p))
+		r, err := Retry(10, retryableAttachVolumeFunc(cs, p))
 		if err != nil {
-			return err
+			return fmt.Errorf("Error attaching volume to VM: %s", err)
 		}
 
 		d.SetId(r.(*cloudstack.AttachVolumeResponse).Id)
@@ -397,115 +370,3 @@ func retryableAttachVolumeFunc(
 		return r, nil
 	}
 }
-
-func retrieveDeviceID(device string) int64 {
-	switch device {
-	case "/dev/xvdb", "D:":
-		return 1
-	case "/dev/xvdc", "E:":
-		return 2
-	case "/dev/xvde", "F:":
-		return 4
-	case "/dev/xvdf", "G:":
-		return 5
-	case "/dev/xvdg", "H:":
-		return 6
-	case "/dev/xvdh", "I:":
-		return 7
-	case "/dev/xvdi", "J:":
-		return 8
-	case "/dev/xvdj", "K:":
-		return 9
-	case "/dev/xvdk", "L:":
-		return 10
-	case "/dev/xvdl", "M:":
-		return 11
-	case "/dev/xvdm", "N:":
-		return 12
-	case "/dev/xvdn", "O:":
-		return 13
-	case "/dev/xvdo", "P:":
-		return 14
-	case "/dev/xvdp", "Q:":
-		return 15
-	default:
-		return -1
-	}
-}
-
-func retrieveDeviceName(device int64, os string) string {
-	switch device {
-	case 1:
-		if os == "Windows" {
-			return "D:"
-		}
-		return "/dev/xvdb"
-	case 2:
-		if os == "Windows" {
-			return "E:"
-		}
-		return "/dev/xvdc"
-	case 4:
-		if os == "Windows" {
-			return "F:"
-		}
-		return "/dev/xvde"
-	case 5:
-		if os == "Windows" {
-			return "G:"
-		}
-		return "/dev/xvdf"
-	case 6:
-		if os == "Windows" {
-			return "H:"
-		}
-		return "/dev/xvdg"
-	case 7:
-		if os == "Windows" {
-			return "I:"
-		}
-		return "/dev/xvdh"
-	case 8:
-		if os == "Windows" {
-			return "J:"
-		}
-		return "/dev/xvdi"
-	case 9:
-		if os == "Windows" {
-			return "K:"
-		}
-		return "/dev/xvdj"
-	case 10:
-		if os == "Windows" {
-			return "L:"
-		}
-		return "/dev/xvdk"
-	case 11:
-		if os == "Windows" {
-			return "M:"
-		}
-		return "/dev/xvdl"
-	case 12:
-		if os == "Windows" {
-			return "N:"
-		}
-		return "/dev/xvdm"
-	case 13:
-		if os == "Windows" {
-			return "O:"
-		}
-		return "/dev/xvdn"
-	case 14:
-		if os == "Windows" {
-			return "P:"
-		}
-		return "/dev/xvdo"
-	case 15:
-		if os == "Windows" {
-			return "Q:"
-		}
-		return "/dev/xvdp"
-	default:
-		return "unknown"
-	}
-}

builtin/providers/cloudstack/resource_cloudstack_disk_test.go

@@ -29,7 +29,7 @@ func TestAccCloudStackDisk_basic(t *testing.T) {
 	})
 }
 
-func TestAccCloudStackDisk_device(t *testing.T) {
+func TestAccCloudStackDisk_deviceID(t *testing.T) {
 	var disk cloudstack.Volume
 
 	resource.Test(t, resource.TestCase{
@@ -38,13 +38,13 @@ func TestAccCloudStackDisk_device(t *testing.T) {
 		CheckDestroy: testAccCheckCloudStackDiskDestroy,
 		Steps: []resource.TestStep{
 			resource.TestStep{
-				Config: testAccCloudStackDisk_device,
+				Config: testAccCloudStackDisk_deviceID,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckCloudStackDiskExists(
 						"cloudstack_disk.foo", &disk),
 					testAccCheckCloudStackDiskAttributes(&disk),
 					resource.TestCheckResourceAttr(
-						"cloudstack_disk.foo", "device", "/dev/xvde"),
+						"cloudstack_disk.foo", "device_id", "4"),
 				),
 			},
 		},
@@ -170,7 +170,7 @@ resource "cloudstack_disk" "foo" {
 	CLOUDSTACK_DISK_OFFERING_1,
 	CLOUDSTACK_ZONE)
 
-var testAccCloudStackDisk_device = fmt.Sprintf(`
+var testAccCloudStackDisk_deviceID = fmt.Sprintf(`
 resource "cloudstack_instance" "foobar" {
   name = "terraform-test"
   display_name = "terraform"
@@ -184,7 +184,7 @@ resource "cloudstack_instance" "foobar" {
 resource "cloudstack_disk" "foo" {
   name = "terraform-disk"
   attach = true
-  device = "/dev/xvde"
+  device_id = 4
   disk_offering = "%s"
   virtual_machine_id = "${cloudstack_instance.foobar.id}"
   zone = "${cloudstack_instance.foobar.zone}"

builtin/providers/cloudstack/resource_cloudstack_instance.go

@@ -105,6 +105,7 @@ func resourceCloudStackInstance() *schema.Resource {
 			"project": &schema.Schema{
 				Type:     schema.TypeString,
 				Optional: true,
+				Computed: true,
 				ForceNew: true,
 			},
 
@@ -251,9 +252,12 @@ func resourceCloudStackInstanceCreate(d *schema.ResourceData, meta interface{})
 		p.SetKeypair(keypair.(string))
 	}
 
-	if ud, err := getUserData(d, cs); err != nil {
-		return err
-	} else if len(ud) > 0 {
+	if userData, ok := d.GetOk("user_data"); ok {
+		ud, err := getUserData(userData.(string), cs.HTTPGETOnly)
+		if err != nil {
+			return err
+		}
+
 		p.SetUserdata(ud)
 	}
 
@@ -437,6 +441,7 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{})
 			d.SetPartial("service_offering")
 		}
 
+		// Check if the affinity group IDs have changed and if so, update the IDs
 		if d.HasChange("affinity_group_ids") {
 			p := cs.AffinityGroup.NewUpdateVMAffinityGroupParams(d.Id())
 			groups := []string{}
@@ -450,6 +455,7 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{})
 			p.SetAffinitygroupids(groups)
 		}
 
+		// Check if the affinity group names have changed and if so, update the names
 		if d.HasChange("affinity_group_names") {
 			p := cs.AffinityGroup.NewUpdateVMAffinityGroupParams(d.Id())
 			groups := []string{}
@@ -463,6 +469,7 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{})
 			p.SetAffinitygroupids(groups)
 		}
 
+		// Check if the keypair has changed and if so, update the keypair
 		if d.HasChange("keypair") {
 			log.Printf("[DEBUG] SSH keypair changed for %s, starting update", name)
 
@@ -477,10 +484,11 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{})
 			d.SetPartial("keypair")
 		}
 
+		// Check if the user data has changed and if so, update the user data
 		if d.HasChange("user_data") {
 			log.Printf("[DEBUG] user_data changed for %s, starting update", name)
 
-			ud, err := getUserData(d, cs)
+			ud, err := getUserData(d.Get("user_data").(string), cs.HTTPGETOnly)
 			if err != nil {
 				return err
 			}
@@ -533,28 +541,23 @@ func resourceCloudStackInstanceDelete(d *schema.ResourceData, meta interface{})
 	return nil
 }
 
-// getUserData returns user_data as a base64 encoded string. An empty
-// string is returned if unset.
-func getUserData(d *schema.ResourceData, cs *cloudstack.CloudStackClient) (string, error) {
-	if userData, ok := d.GetOk("user_data"); ok {
-		ud := base64.StdEncoding.EncodeToString([]byte(userData.(string)))
-
-		// deployVirtualMachine uses POST by default, so max userdata is 32K
-		maxUD := 32768
+// getUserData returns the user data as a base64 encoded string
+func getUserData(userData string, httpGetOnly bool) (string, error) {
+	ud := base64.StdEncoding.EncodeToString([]byte(userData))
 
-		if cs.HTTPGETOnly {
-			// deployVirtualMachine using GET instead, so max userdata is 2K
-			maxUD = 2048
-		}
+	// deployVirtualMachine uses POST by default, so max userdata is 32K
+	maxUD := 32768
 
-		if len(ud) > maxUD {
-			return "", fmt.Errorf(
-				"The supplied user_data contains %d bytes after encoding, "+
-					"this exeeds the limit of %d bytes", len(ud), maxUD)
-		}
+	if httpGetOnly {
+		// deployVirtualMachine using GET instead, so max userdata is 2K
+		maxUD = 2048
+	}
 
-		return ud, nil
+	if len(ud) > maxUD {
+		return "", fmt.Errorf(
+			"The supplied user_data contains %d bytes after encoding, "+
+				"this exeeds the limit of %d bytes", len(ud), maxUD)
 	}
 
-	return "", nil
+	return ud, nil
 }

builtin/providers/cloudstack/resource_cloudstack_ipaddress.go

@@ -31,6 +31,7 @@ func resourceCloudStackIPAddress() *schema.Resource {
 			"project": &schema.Schema{
 				Type:     schema.TypeString,
 				Optional: true,
+				Computed: true,
 				ForceNew: true,
 			},