fix(vm): fix timeout when resizing custom disk at create (#1260)

Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com>

proxmoxtf/resource/vm/disk/disk.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"regexp"
-	"strconv"
 	"strings"
 	"unicode"
 
@@ -18,7 +17,6 @@ import (
 	"github.com/bpg/terraform-provider-proxmox/proxmox/nodes/vms"
 	"github.com/bpg/terraform-provider-proxmox/proxmox/ssh"
 	"github.com/bpg/terraform-provider-proxmox/proxmox/types"
-	"github.com/bpg/terraform-provider-proxmox/proxmoxtf"
 	"github.com/bpg/terraform-provider-proxmox/proxmoxtf/structure"
 	"github.com/bpg/terraform-provider-proxmox/utils"
 )
@@ -272,6 +270,7 @@ func GetDiskDeviceObjects(
 			fileFormat = dvDiskFileFormat
 		}
 
+		// Explicitly disable the disk, so it won't be encoded in "Create" or "Update" operations.
 		if fileID != "" {
 			diskDevice.Enabled = false
 		}
@@ -372,204 +371,80 @@ func GetDiskDeviceObjects(
 // CreateCustomDisks creates custom disks for a VM.
 func CreateCustomDisks(
 	ctx context.Context,
+	client proxmox.Client,
 	nodeName string,
-	d *schema.ResourceData,
+	vmID int,
-	resource *schema.Resource,
+	storageDevices map[string]vms.CustomStorageDevices,
-	m interface{},
 ) diag.Diagnostics {
-	vmID, err := strconv.Atoi(d.Id())
+	// flatten the map of disk devices
-	if err != nil {
+	var disks []vms.CustomStorageDevice
-		return diag.FromErr(err)
-	}
 
-	//nolint:prealloc
+	for _, diskDevice := range storageDevices {
-	var commands []string
+		for _, disk := range diskDevice {
-
+			if disk != nil {
-	// Determine the ID of the next disk.
+				disks = append(disks, *disk)
-	disk := d.Get(MkDisk).([]interface{})
+			}
-	diskCount := 0
-
-	for _, d := range disk {
-		block := d.(map[string]interface{})
-		fileID, _ := block[mkDiskFileID].(string)
-
-		if fileID == "" {
-			diskCount++
 		}
 	}
 
-	// Retrieve some information about the disk schema.
+	commands := make([]string, 0, len(disks))
-	resourceSchema := resource.Schema
+	resizes := make([]*vms.ResizeDiskRequestBody, 0, len(disks))
-	diskSchemaElem := resourceSchema[MkDisk].Elem
-	diskSchemaResource := diskSchemaElem.(*schema.Resource)
-	diskSpeedResource := diskSchemaResource.Schema[mkDiskSpeed]
 
-	// Generate the commands required to import the specified disks.
+	for _, d := range disks {
-	importedDiskCount := 0
+		if d.FileID == nil || *d.FileID == "" {
-
-	for _, d := range disk {
-		block := d.(map[string]interface{})
-
-		fileID, _ := block[mkDiskFileID].(string)
-
-		if fileID == "" {
 			continue
 		}
 
-		aio, _ := block[mkDiskAIO].(string)
+		fileFormat := dvDiskFileFormat
-		backup := types.CustomBool(block[mkDiskBackup].(bool))
+		if d.Format != nil && *d.Format != "" {
-		cache, _ := block[mkDiskCache].(string)
+			fileFormat = *d.Format
-		datastoreID, _ := block[mkDiskDatastoreID].(string)
-		discard, _ := block[mkDiskDiscard].(string)
-		diskInterface, _ := block[mkDiskInterface].(string)
-		fileFormat, _ := block[mkDiskFileFormat].(string)
-		ioThread := types.CustomBool(block[mkDiskIOThread].(bool))
-		replicate := types.CustomBool(block[mkDiskReplicate].(bool))
-		size, _ := block[mkDiskSize].(int)
-		speed := block[mkDiskSpeed].([]interface{})
-		ssd := types.CustomBool(block[mkDiskSSD].(bool))
-
-		if fileFormat == "" {
-			fileFormat = dvDiskFileFormat
 		}
 
-		if len(speed) == 0 {
-			diskSpeedDefault, err := diskSpeedResource.DefaultValue()
-			if err != nil {
-				return diag.FromErr(err)
-			}
-
-			speed = diskSpeedDefault.([]interface{})
-		}
-
-		speedBlock := speed[0].(map[string]interface{})
-		iopsRead := speedBlock[mkDiskIopsRead].(int)
-		iopsReadBurstable := speedBlock[mkDiskIopsReadBurstable].(int)
-		iopsWrite := speedBlock[mkDiskIopsWrite].(int)
-		iopsWriteBurstable := speedBlock[mkDiskIopsWriteBurstable].(int)
-		speedLimitRead := speedBlock[mkDiskSpeedRead].(int)
-		speedLimitReadBurstable := speedBlock[mkDiskSpeedReadBurstable].(int)
-		speedLimitWrite := speedBlock[mkDiskSpeedWrite].(int)
-		speedLimitWriteBurstable := speedBlock[mkDiskSpeedWriteBurstable].(int)
-
-		diskOptions := ""
-
-		if aio != "" {
-			diskOptions += fmt.Sprintf(",aio=%s", aio)
-		}
-
-		if backup {
-			diskOptions += ",backup=1"
-		} else {
-			diskOptions += ",backup=0"
-		}
-
-		if ioThread {
-			diskOptions += ",iothread=1"
-		}
-
-		if replicate {
-			diskOptions += ",replicate=1"
-		} else {
-			diskOptions += ",replicate=0"
-		}
-
-		if ssd {
-			diskOptions += ",ssd=1"
-		}
-
-		if discard != "" {
-			diskOptions += fmt.Sprintf(",discard=%s", discard)
-		}
-
-		if cache != "" {
-			diskOptions += fmt.Sprintf(",cache=%s", cache)
-		}
-
-		if iopsRead > 0 {
-			diskOptions += fmt.Sprintf(",iops_rd=%d", iopsRead)
-		}
-
-		if iopsReadBurstable > 0 {
-			diskOptions += fmt.Sprintf(",iops_rd_max=%d", iopsReadBurstable)
-		}
-
-		if iopsWrite > 0 {
-			diskOptions += fmt.Sprintf(",iops_wr=%d", iopsWrite)
-		}
-
-		if iopsWriteBurstable > 0 {
-			diskOptions += fmt.Sprintf(",iops_wr_max=%d", iopsWriteBurstable)
-		}
-
-		if speedLimitRead > 0 {
-			diskOptions += fmt.Sprintf(",mbps_rd=%d", speedLimitRead)
-		}
-
-		if speedLimitReadBurstable > 0 {
-			diskOptions += fmt.Sprintf(",mbps_rd_max=%d", speedLimitReadBurstable)
-		}
-
-		if speedLimitWrite > 0 {
-			diskOptions += fmt.Sprintf(",mbps_wr=%d", speedLimitWrite)
-		}
-
-		if speedLimitWriteBurstable > 0 {
-			diskOptions += fmt.Sprintf(",mbps_wr_max=%d", speedLimitWriteBurstable)
-		}
-
-		filePathTmp := fmt.Sprintf(
-			"/tmp/vm-%d-disk-%d.%s",
-			vmID,
-			diskCount+importedDiskCount,
-			fileFormat,
-		)
-
 		//nolint:lll
 		commands = append(
 			commands,
 			`set -e`,
 			ssh.TrySudo,
-			fmt.Sprintf(`file_id="%s"`, fileID),
+			fmt.Sprintf(`file_id="%s"`, *d.FileID),
 			fmt.Sprintf(`file_format="%s"`, fileFormat),
-			fmt.Sprintf(`datastore_id_target="%s"`, datastoreID),
+			fmt.Sprintf(`datastore_id_target="%s"`, *d.DatastoreID),
-			fmt.Sprintf(`disk_options="%s"`, diskOptions),
-			fmt.Sprintf(`disk_size="%d"`, size),
-			fmt.Sprintf(`disk_interface="%s"`, diskInterface),
-			fmt.Sprintf(`file_path_tmp="%s"`, filePathTmp),
 			fmt.Sprintf(`vm_id="%d"`, vmID),
+			fmt.Sprintf(`disk_options="%s"`, d.EncodeOptions()),
+			fmt.Sprintf(`disk_interface="%s"`, *d.Interface),
 			`source_image=$(try_sudo "pvesm path $file_id")`,
 			`imported_disk="$(try_sudo "qm importdisk $vm_id $source_image $datastore_id_target -format $file_format" | grep "unused0" | cut -d ":" -f 3 | cut -d "'" -f 1)"`,
-			`disk_id="${datastore_id_target}:$imported_disk${disk_options}"`,
+			`disk_id="${datastore_id_target}:$imported_disk,${disk_options}"`,
 			`try_sudo "qm set $vm_id -${disk_interface} $disk_id"`,
-			`try_sudo "qm resize $vm_id ${disk_interface} ${disk_size}G"`,
 		)
 
-		importedDiskCount++
+		resizes = append(resizes, &vms.ResizeDiskRequestBody{
+			Disk: *d.Interface,
+			Size: *d.Size,
+		})
 	}
 
 	// Execute the commands on the node and wait for the result.
 	// This is a highly experimental approach to disk imports and is not recommended by Proxmox.
 	if len(commands) > 0 {
-		config := m.(proxmoxtf.ProviderConfiguration)
+		out, err := client.SSH().ExecuteNodeCommands(ctx, nodeName, commands)
-
-		api, err := config.GetClient()
-		if err != nil {
-			return diag.FromErr(err)
-		}
-
-		out, err := api.SSH().ExecuteNodeCommands(ctx, nodeName, commands)
 		if err != nil {
 			if matches, e := regexp.Match(`pvesm: .* not found`, out); e == nil && matches {
-				return diag.FromErr(ssh.NewErrUserHasNoPermission(api.SSH().Username()))
+				return diag.FromErr(ssh.NewErrUserHasNoPermission(client.SSH().Username()))
 			}
 
 			return diag.FromErr(err)
 		}
 
-		tflog.Debug(ctx, "vmCreateCustomDisks", map[string]interface{}{
+		tflog.Debug(ctx, "vmCreateCustomDisks: commands", map[string]interface{}{
 			"output": string(out),
 		})
+
+		for _, resize := range resizes {
+			err = client.Node(nodeName).VM(vmID).ResizeVMDisk(ctx, resize)
+			if err != nil {
+				return diag.FromErr(err)
+			}
+		}
 	}
 
 	return nil

proxmoxtf/resource/vm/vm.go

@@ -2456,16 +2456,6 @@ func vmCreateCustom(ctx context.Context, d *schema.ResourceData, m interface{})
 		}
 	}
 
-	diskDeviceObjects, err := disk.GetDiskDeviceObjects(d, resource, nil)
-	if err != nil {
-		return diag.FromErr(err)
-	}
-
-	virtioDeviceObjects := diskDeviceObjects["virtio"]
-	scsiDeviceObjects := diskDeviceObjects["scsi"]
-	ideDeviceObjects := diskDeviceObjects["ide"]
-	sataDeviceObjects := diskDeviceObjects["sata"]
-
 	initializationConfig := vmGetCloudInitConfig(d)
 
 	if initializationConfig != nil {
@@ -2566,7 +2556,15 @@ func vmCreateCustom(ctx context.Context, d *schema.ResourceData, m interface{})
 		}
 	}
 
-	var memorySharedObject *vms.CustomSharedMemory
+	diskDeviceObjects, err := disk.GetDiskDeviceObjects(d, resource, nil)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+
+	virtioDeviceObjects := diskDeviceObjects["virtio"]
+	scsiDeviceObjects := diskDeviceObjects["scsi"]
+	ideDeviceObjects := diskDeviceObjects["ide"]
+	sataDeviceObjects := diskDeviceObjects["sata"]
 
 	var bootOrderConverted []string
 	if cdromEnabled {
@@ -2623,6 +2621,8 @@ func vmCreateCustom(ctx context.Context, d *schema.ResourceData, m interface{})
 		}
 	}
 
+	var memorySharedObject *vms.CustomSharedMemory
+
 	if memoryShared > 0 {
 		memorySharedName := fmt.Sprintf("vm-%d-ivshmem", vmID)
 		memorySharedObject = &vms.CustomSharedMemory{
@@ -2757,7 +2757,7 @@ func vmCreateCustom(ctx context.Context, d *schema.ResourceData, m interface{})
 
 	d.SetId(strconv.Itoa(vmID))
 
-	diags := disk.CreateCustomDisks(ctx, nodeName, d, resource, m)
+	diags := disk.CreateCustomDisks(ctx, client, nodeName, vmID, diskDeviceObjects)
 	if diags.HasError() {
 		return diags
 	}