feat: add the ability to clone to non-shared storage on different nodes (#178)

* feat: add workaround for cloning to non-shared storage

* fix: fix wrong API params used

* test: add new var to tests

* fix: lint issues

* docs: add new argument to docs

* docs: fix function documentation

* fix: better work with heterogeneous datastores

* docs: clarify clone behavior

* fix: go lint issues

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

docs/resources/virtual_environment_vm.md

@@ -325,3 +325,7 @@ output "ubuntu_vm_public_key" {
 
 When cloning an existing virtual machine, whether it's a template or not, the resource will only detect changes to the
 arguments which are not set to their default values.
+
+Furthermore, when cloning from one node to a different one, the behavior changes depening on the datastores of the source VM. If at least one non-shared datastore is used, the VM is first cloned to the source node before being migrated to the target node. This circumvents a limitation in the Proxmox clone API. 
+
+**Note:** Because the migration step after the clone tries to preserve the used datastores by their name, it may fail if a datastore used in the source VM is not available on the target node (e.g. `local-lvm` is used on the source node in the VM but no `local-lvm` datastore is availabel on the target node). In this case, it is recommended to set the `datastore_id` argument in the `clone` block to force the migration step to migrate all disks to a specific datastore on the target node. If you need certain disks to be on specific datastores, set the `datastore_id` argument of the disks in the `disks` block to move the disks to the correct datastore after the cloning and migrating succeeded.

proxmox/virtual_environment_datastores.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"io"
 	"mime/multipart"
 	"net/url"
@@ -16,6 +15,8 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/hashicorp/terraform-plugin-log/tflog"
+
 	"github.com/pkg/sftp"
 )
 
@@ -30,6 +31,22 @@ func (c *VirtualEnvironmentClient) DeleteDatastoreFile(ctx context.Context, node
 	return nil
 }
 
+// GetDatastoreStatus gets status information for a given datastore.
+func (c *VirtualEnvironmentClient) GetDatastoreStatus(ctx context.Context, nodeName, datastoreID string) (*VirtualEnvironmentDatastoreGetStatusResponseData, error) {
+	resBody := &VirtualEnvironmentDatastoreGetStatusResponseBody{}
+	err := c.DoRequest(ctx, hmGET, fmt.Sprintf("nodes/%s/storage/%s/status", url.PathEscape(nodeName), url.PathEscape(datastoreID)), nil, resBody)
+
+	if err != nil {
+		return nil, err
+	}
+
+	if resBody.Data == nil {
+		return nil, errors.New("the server did not include a data object in the response")
+	}
+
+	return resBody.Data, nil
+}
+
 // ListDatastoreFiles retrieves a list of the files in a datastore.
 func (c *VirtualEnvironmentClient) ListDatastoreFiles(ctx context.Context, nodeName, datastoreID string) ([]*VirtualEnvironmentDatastoreFileListResponseData, error) {
 	resBody := &VirtualEnvironmentDatastoreFileListResponseBody{}

proxmox/virtual_environment_datastores_types.go

@@ -24,6 +24,23 @@ type VirtualEnvironmentDatastoreFileListResponseData struct {
 	VolumeID       string  `json:"volid"`
 }
 
+// VirtualEnvironmentDatastoreGetStatusResponseBody contains the body from a datastore status get request.
+type VirtualEnvironmentDatastoreGetStatusResponseBody struct {
+	Data *VirtualEnvironmentDatastoreGetStatusResponseData `json:"data,omitempty"`
+}
+
+// VirtualEnvironmentDatastoreGetStatusResponseBody contains the data from a datastore status get request.
+type VirtualEnvironmentDatastoreGetStatusResponseData struct {
+	Active         *CustomBool               `json:"active,omitempty"`
+	AvailableBytes *int64                    `json:"avail,omitempty"`
+	Content        *CustomCommaSeparatedList `json:"content,omitempty" url:"content,omitempty,comma"`
+	Enabled        *CustomBool               `json:"enabled,omitempty"`
+	Shared         *CustomBool               `json:"shared,omitempty"`
+	TotalBytes     *int64                    `json:"total,omitempty"`
+	Type           *string                   `json:"type,omitempty"`
+	UsedBytes      *int64                    `json:"used,omitempty"`
+}
+
 // VirtualEnvironmentDatastoreListRequestBody contains the body for a datastore list request.
 type VirtualEnvironmentDatastoreListRequestBody struct {
 	ContentTypes CustomCommaSeparatedList `json:"content,omitempty" url:"content,omitempty,comma"`

proxmox/virtual_environment_vm.go

@@ -8,12 +8,13 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"net"
 	"net/url"
 	"strings"
 	"sync"
 	"time"
+
+	"github.com/hashicorp/terraform-plugin-log/tflog"
 )
 
 const (
@@ -163,6 +164,39 @@ func (c *VirtualEnvironmentClient) GetVMStatus(ctx context.Context, nodeName str
 	return resBody.Data, nil
 }
 
+// MigrateVM migrates a virtual machine.
+func (c *VirtualEnvironmentClient) MigrateVM(ctx context.Context, nodeName string, vmID int, d *VirtualEnvironmentVMMigrateRequestBody, timeout int) error {
+	taskID, err := c.MigrateVMAsync(ctx, nodeName, vmID, d)
+
+	if err != nil {
+		return err
+	}
+
+	err = c.WaitForNodeTask(ctx, nodeName, *taskID, timeout, 5)
+
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// MigrateVMAsync migrates a virtual machine asynchronously.
+func (c *VirtualEnvironmentClient) MigrateVMAsync(ctx context.Context, nodeName string, vmID int, d *VirtualEnvironmentVMMigrateRequestBody) (*string, error) {
+	resBody := &VirtualEnvironmentVMMigrateResponseBody{}
+	err := c.DoRequest(ctx, hmPOST, fmt.Sprintf("nodes/%s/qemu/%d/migrate", url.PathEscape(nodeName), vmID), d, resBody)
+
+	if err != nil {
+		return nil, err
+	}
+
+	if resBody.Data == nil {
+		return nil, errors.New("the server did not include a data object in the response")
+	}
+
+	return resBody.Data, nil
+}
+
 // MoveVMDisk moves a virtual machine disk.
 func (c *VirtualEnvironmentClient) MoveVMDisk(ctx context.Context, nodeName string, vmID int, d *VirtualEnvironmentVMMoveDiskRequestBody, timeout int) error {
 	taskID, err := c.MoveVMDiskAsync(ctx, nodeName, vmID, d)

proxmox/virtual_environment_vm_types.go

@@ -502,6 +502,19 @@ type VirtualEnvironmentVMListResponseData struct {
 	ACPI *CustomBool `json:"acpi,omitempty" url:"acpi,omitempty,int"`
 }
 
+// VirtualEnvironmentVMMigrateRequestBody contains the body for a VM migration request.
+type VirtualEnvironmentVMMigrateRequestBody struct {
+	OnlineMigration *CustomBool `json:"online,omitempty" url:"online,omitempty"`
+	TargetNode      string      `json:"target" url:"target"`
+	TargetStorage   *string     `json:"targetstorage,omitempty" url:"targetstorage,omitempty"`
+	WithLocalDisks  *CustomBool `json:"with-local-disks,omitempty" url:"with-local-disks,omitempty,int"`
+}
+
+// VirtualEnvironmentVMMigrateResponseBody contains the body from a VM migrate response.
+type VirtualEnvironmentVMMigrateResponseBody struct {
+	Data *string `json:"data,omitempty"`
+}
+
 // VirtualEnvironmentVMMoveDiskRequestBody contains the body for a VM move disk request.
 type VirtualEnvironmentVMMoveDiskRequestBody struct {
 	BandwidthLimit      *int        `json:"bwlimit,omitempty" url:"bwlimit,omitempty"`

proxmoxtf/resource_virtual_environment_vm.go

@@ -8,13 +8,14 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"sort"
 	"strconv"
 	"strings"
 	"time"
 
+	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
+
 	"github.com/bpg/terraform-provider-proxmox/proxmox"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
@@ -1164,9 +1165,70 @@ func resourceVirtualEnvironmentVMCreateClone(ctx context.Context, d *schema.Reso
 	cloneTimeout := d.Get(mkResourceVirtualEnvironmentVMTimeoutClone).(int)
 
 	if cloneNodeName != "" && cloneNodeName != nodeName {
-		cloneBody.TargetNodeName = &nodeName
+		// Check if any used datastores of the source VM are not shared
+		vmConfig, err := veClient.GetVM(ctx, cloneNodeName, cloneVMID)
+		if err != nil {
+			return diag.FromErr(err)
+		}
 
+		datastores := getDiskDatastores(vmConfig, d)
+
+		onlySharedDatastores := true
+		for _, datastore := range datastores {
+			datastoreStatus, err := veClient.GetDatastoreStatus(ctx, cloneNodeName, datastore)
+			if err != nil {
+				return diag.FromErr(err)
+			}
+
+			if datastoreStatus.Shared != nil && *datastoreStatus.Shared == proxmox.CustomBool(false) {
+				onlySharedDatastores = false
+				break
+			}
+		}
+
+		if onlySharedDatastores {
+			// If the source and the target node are not the same, only clone directly to the target node if
+			//  all used datastores in the source VM are shared. Directly cloning to non-shared storage
+			//  on a different node is currently not supported by proxmox.
+			cloneBody.TargetNodeName = &nodeName
 			err = veClient.CloneVM(ctx, cloneNodeName, cloneVMID, cloneRetries, cloneBody, cloneTimeout)
+			if err != nil {
+				return diag.FromErr(err)
+			}
+		} else {
+			// If the source and the target node are not the same and any used datastore in the source VM is
+			//  not shared, clone to the source node and then migrate to the target node. This is a workaround
+			//  for missing functionality in the proxmox api as recommended per
+			//  https://forum.proxmox.com/threads/500-cant-clone-to-non-shared-storage-local.49078/#post-229727
+
+			// Temporarily clone to local node
+			err = veClient.CloneVM(ctx, cloneNodeName, cloneVMID, cloneRetries, cloneBody, cloneTimeout)
+			if err != nil {
+				return diag.FromErr(err)
+			}
+
+			// Wait for the virtual machine to be created and its configuration lock to be released before migrating.
+			err = veClient.WaitForVMConfigUnlock(ctx, cloneNodeName, vmID, 600, 5, true)
+			if err != nil {
+				return diag.FromErr(err)
+			}
+
+			// Migrate to target node
+			withLocalDisks := proxmox.CustomBool(true)
+			migrateBody := &proxmox.VirtualEnvironmentVMMigrateRequestBody{
+				TargetNode:     nodeName,
+				WithLocalDisks: &withLocalDisks,
+			}
+
+			if cloneDatastoreID != "" {
+				migrateBody.TargetStorage = &cloneDatastoreID
+			}
+
+			err = veClient.MigrateVM(ctx, cloneNodeName, vmID, migrateBody, cloneTimeout)
+			if err != nil {
+				return diag.FromErr(err)
+			}
+		}
 	} else {
 		err = veClient.CloneVM(ctx, nodeName, cloneVMID, cloneRetries, cloneBody, cloneTimeout)
 	}

proxmoxtf/utils.go

@@ -7,8 +7,6 @@ package proxmoxtf
 import (
 	"errors"
 	"fmt"
-	"github.com/hashicorp/go-multierror"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"math"
 	"regexp"
 	"strconv"
@@ -17,6 +15,9 @@ import (
 	"time"
 	"unicode"
 
+	"github.com/hashicorp/go-multierror"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+
 	"github.com/bpg/terraform-provider-proxmox/proxmox"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
@@ -463,6 +464,28 @@ func getDiskInfo(vm *proxmox.VirtualEnvironmentVMGetResponseData, d *schema.Reso
 	return storageDevices
 }
 
+// getDiskDatastores returns a list of the used datastores in a VM
+func getDiskDatastores(vm *proxmox.VirtualEnvironmentVMGetResponseData, d *schema.ResourceData) []string {
+	storageDevices := getDiskInfo(vm, d)
+	datastoresSet := map[string]int{}
+
+	for _, diskInfo := range storageDevices {
+		// Ignore empty storage devices and storage devices (like ide) which may not have any media mounted
+		if diskInfo == nil || diskInfo.FileVolume == "none" {
+			continue
+		}
+		fileIDParts := strings.Split(diskInfo.FileVolume, ":")
+		datastoresSet[fileIDParts[0]] = 1
+	}
+
+	datastores := []string{}
+	for datastore := range datastoresSet {
+		datastores = append(datastores, datastore)
+	}
+
+	return datastores
+}
+
 func parseDiskSize(size *string) (int, error) {
 	var diskSize int
 	var err error