r/virtual_machine: Fix multiple change operations to a single device

vsphere/internal/virtualdevice/virtual_machine_device_subresource.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/hashicorp/terraform/helper/schema"
+	"github.com/mitchellh/copystructure"
 	"github.com/terraform-providers/terraform-provider-vsphere/vsphere/internal/helper/structure"
 	"github.com/vmware/govmomi"
 	"github.com/vmware/govmomi/object"
@@ -743,3 +744,21 @@ func scsiControllerListString(ctlrs []types.BaseVirtualSCSIController) string {
 	}
 	return DeviceListString(l)
 }
+
+// AppendDeviceChangeSpec appends unique copies of the supplied device change
+// operations and appends them to spec. The resulting list is returned.
+//
+// The object of this function is to provide deep copies of each virtual device
+// to the spec as they looked like when the append operation was called,
+// helping facilitate multiple update operations to the same device in a single
+// reconfigure call.
+func AppendDeviceChangeSpec(
+	spec []types.BaseVirtualDeviceConfigSpec,
+	ops ...types.BaseVirtualDeviceConfigSpec,
+) []types.BaseVirtualDeviceConfigSpec {
+	for _, op := range ops {
+		c := copystructure.Must(copystructure.Copy(op)).(types.BaseVirtualDeviceConfigSpec)
+		spec = append(spec, c)
+	}
+	return spec
+}

vsphere/resource_vsphere_virtual_machine.go

@@ -764,25 +764,25 @@ func resourceVSphereVirtualMachineCreateClone(d *schema.ResourceData, meta inter
 	if err != nil {
 		return nil, err
 	}
-	cfgSpec.DeviceChange = append(cfgSpec.DeviceChange, delta...)
+	cfgSpec.DeviceChange = virtualdevice.AppendDeviceChangeSpec(cfgSpec.DeviceChange, delta...)
 	// Disks
 	devices, delta, err = virtualdevice.DiskPostCloneOperation(d, client, devices)
 	if err != nil {
 		return nil, err
 	}
-	cfgSpec.DeviceChange = append(cfgSpec.DeviceChange, delta...)
+	cfgSpec.DeviceChange = virtualdevice.AppendDeviceChangeSpec(cfgSpec.DeviceChange, delta...)
 	// Network devices
 	devices, delta, err = virtualdevice.NetworkInterfacePostCloneOperation(d, client, devices)
 	if err != nil {
 		return nil, err
 	}
-	cfgSpec.DeviceChange = append(cfgSpec.DeviceChange, delta...)
+	cfgSpec.DeviceChange = virtualdevice.AppendDeviceChangeSpec(cfgSpec.DeviceChange, delta...)
 	// CDROM
 	devices, delta, err = virtualdevice.CdromPostCloneOperation(d, client, devices)
 	if err != nil {
 		return nil, err
 	}
-	cfgSpec.DeviceChange = append(cfgSpec.DeviceChange, delta...)
+	cfgSpec.DeviceChange = virtualdevice.AppendDeviceChangeSpec(cfgSpec.DeviceChange, delta...)
 	log.Printf("[DEBUG] %s: Final device list: %s", resourceVSphereVirtualMachineIDString(d), virtualdevice.DeviceListString(devices))
 	log.Printf("[DEBUG] %s: Final device change cfgSpec: %s", resourceVSphereVirtualMachineIDString(d), virtualdevice.DeviceChangeString(cfgSpec.DeviceChange))
 
@@ -945,25 +945,25 @@ func applyVirtualDevices(d *schema.ResourceData, c *govmomi.Client, l object.Vir
 		log.Printf("[DEBUG] %s: SCSI bus has changed and requires a VM restart", resourceVSphereVirtualMachineIDString(d))
 		d.Set("reboot_required", true)
 	}
-	spec = append(spec, delta...)
+	spec = virtualdevice.AppendDeviceChangeSpec(spec, delta...)
 	// Disks
 	l, delta, err = virtualdevice.DiskApplyOperation(d, c, l)
 	if err != nil {
 		return nil, err
 	}
-	spec = append(spec, delta...)
+	spec = virtualdevice.AppendDeviceChangeSpec(spec, delta...)
 	// Network devices
 	l, delta, err = virtualdevice.NetworkInterfaceApplyOperation(d, c, l)
 	if err != nil {
 		return nil, err
 	}
-	spec = append(spec, delta...)
+	spec = virtualdevice.AppendDeviceChangeSpec(spec, delta...)
 	// CDROM
 	l, delta, err = virtualdevice.CdromApplyOperation(d, c, l)
 	if err != nil {
 		return nil, err
 	}
-	spec = append(spec, delta...)
+	spec = virtualdevice.AppendDeviceChangeSpec(spec, delta...)
 	log.Printf("[DEBUG] %s: Final device list: %s", resourceVSphereVirtualMachineIDString(d), virtualdevice.DeviceListString(l))
 	log.Printf("[DEBUG] %s: Final device change spec: %s", resourceVSphereVirtualMachineIDString(d), virtualdevice.DeviceChangeString(spec))
 	return spec, nil

vsphere/resource_vsphere_virtual_machine_test.go

@@ -809,6 +809,41 @@ func TestAccResourceVSphereVirtualMachine(t *testing.T) {
 				},
 			},
 		},
+		{
+			"clone, modify disk and SCSI type at same time",
+			resource.TestCase{
+				PreCheck: func() {
+					testAccPreCheck(tp)
+					testAccResourceVSphereVirtualMachinePreCheck(tp)
+				},
+				Providers:    testAccProviders,
+				CheckDestroy: testAccResourceVSphereVirtualMachineCheckExists(false),
+				Steps: []resource.TestStep{
+					{
+						Config: testAccResourceVSphereVirtualMachineConfigCloneChangeDiskAndSCSI(),
+						Check: resource.ComposeTestCheckFunc(
+							copyStatePtr(&state),
+							testAccResourceVSphereVirtualMachineCheckExists(true),
+							func(s *terraform.State) error {
+								oldSize, _ := strconv.Atoi(state.RootModule().Resources["data.vsphere_virtual_machine.template"].Primary.Attributes["disks.0.size"])
+								newSize := oldSize * 2
+								return resource.TestCheckResourceAttr("vsphere_virtual_machine.vm", "disk.0.size", strconv.Itoa(newSize))(s)
+							},
+							func(s *terraform.State) error {
+								oldBus := state.RootModule().Resources["data.vsphere_virtual_machine.template"].Primary.Attributes["scsi_type"]
+								var expected string
+								if oldBus == virtualdevice.SubresourceControllerTypeParaVirtual {
+									expected = virtualdevice.SubresourceControllerTypeLsiLogicSAS
+								} else {
+									expected = virtualdevice.SubresourceControllerTypeParaVirtual
+								}
+								return resource.TestCheckResourceAttr("vsphere_virtual_machine.vm", "scsi_type", expected)(s)
+							},
+						),
+					},
+				},
+			},
+		},
 		{
 			"clone, multi-nic (template should have one)",
 			resource.TestCase{
@@ -7936,3 +7971,119 @@ resource "vsphere_virtual_machine" "vm" {
 		testAccResourceVSphereVirtualMachineDiskNameExtraVmdk,
 	)
 }
+
+func testAccResourceVSphereVirtualMachineConfigCloneChangeDiskAndSCSI() string {
+	return fmt.Sprintf(`
+variable "datacenter" {
+  default = "%s"
+}
+
+variable "resource_pool" {
+  default = "%s"
+}
+
+variable "network_label" {
+  default = "%s"
+}
+
+variable "ipv4_address" {
+  default = "%s"
+}
+
+variable "ipv4_netmask" {
+  default = "%s"
+}
+
+variable "ipv4_gateway" {
+  default = "%s"
+}
+
+variable "dns_server" {
+  default = "%s"
+}
+
+variable "datastore" {
+  default = "%s"
+}
+
+variable "template" {
+  default = "%s"
+}
+
+data "vsphere_datacenter" "dc" {
+  name = "${var.datacenter}"
+}
+
+data "vsphere_datastore" "datastore" {
+  name          = "${var.datastore}"
+  datacenter_id = "${data.vsphere_datacenter.dc.id}"
+}
+
+data "vsphere_resource_pool" "pool" {
+  name          = "${var.resource_pool}"
+  datacenter_id = "${data.vsphere_datacenter.dc.id}"
+}
+
+data "vsphere_network" "network" {
+  name          = "${var.network_label}"
+  datacenter_id = "${data.vsphere_datacenter.dc.id}"
+}
+
+data "vsphere_virtual_machine" "template" {
+  name          = "${var.template}"
+  datacenter_id = "${data.vsphere_datacenter.dc.id}"
+}
+
+resource "vsphere_virtual_machine" "vm" {
+  name             = "terraform-test"
+  resource_pool_id = "${data.vsphere_resource_pool.pool.id}"
+  datastore_id     = "${data.vsphere_datastore.datastore.id}"
+
+  num_cpus = 2
+  memory   = 2048
+  guest_id = "${data.vsphere_virtual_machine.template.guest_id}"
+
+  scsi_type = "${data.vsphere_virtual_machine.template.scsi_type == "pvscsi" ? "lsilogic-sas" : "pvscsi"}"
+
+  network_interface {
+    network_id   = "${data.vsphere_network.network.id}"
+    adapter_type = "${data.vsphere_virtual_machine.template.network_interface_types[0]}"
+  }
+
+  disk {
+    label = "disk0"
+    size  = "${data.vsphere_virtual_machine.template.disks.0.size * 2}"
+  }
+
+  clone {
+    template_uuid = "${data.vsphere_virtual_machine.template.id}"
+
+    customize {
+      linux_options {
+        host_name = "terraform-test"
+        domain    = "test.internal"
+      }
+
+      network_interface {
+        ipv4_address = "${var.ipv4_address}"
+        ipv4_netmask = "${var.ipv4_netmask}"
+      }
+
+      ipv4_gateway    = "${var.ipv4_gateway}"
+      dns_server_list = ["${var.dns_server}"]
+      dns_suffix_list = ["test.internal"]
+    }
+  }
+}
+`,
+		os.Getenv("VSPHERE_DATACENTER"),
+		os.Getenv("VSPHERE_RESOURCE_POOL"),
+		os.Getenv("VSPHERE_NETWORK_LABEL"),
+		os.Getenv("VSPHERE_IPV4_ADDRESS"),
+		os.Getenv("VSPHERE_IPV4_PREFIX"),
+		os.Getenv("VSPHERE_IPV4_GATEWAY"),
+		os.Getenv("VSPHERE_DNS"),
+		os.Getenv("VSPHERE_DATASTORE"),
+		os.Getenv("VSPHERE_TEMPLATE"),
+	)
+}