clone: Add support to clone VM by ID, check if VM IDs are in valid range.

builder/proxmox/clone/builder.go

@@ -69,20 +69,29 @@ func (*cloneVMCreator) Create(vmRef *proxmoxapi.VmRef, config proxmoxapi.ConfigQ
 	}
 	config.Ipconfig = IpconfigMap
 
-	sourceVmrs, err := client.GetVmRefsByName(c.CloneVM)
-	if err != nil {
-		return err
-	}
+	var sourceVmr *proxmoxapi.VmRef
+	if c.CloneVM != "" {
+		sourceVmrs, err := client.GetVmRefsByName(c.CloneVM)
+		if err != nil {
+			return err
+		}
 
-	// prefer source Vm located on same node
-	sourceVmr := sourceVmrs[0]
-	for _, candVmr := range sourceVmrs {
-		if candVmr.Node() == vmRef.Node() {
-			sourceVmr = candVmr
+		// prefer source Vm located on same node
+		sourceVmr = sourceVmrs[0]
+		for _, candVmr := range sourceVmrs {
+			if candVmr.Node() == vmRef.Node() {
+				sourceVmr = candVmr
+			}
+		}
+	} else if c.CloneVMID != 0 {
+		sourceVmr = proxmoxapi.NewVmRef(c.CloneVMID)
+		err := client.CheckVmRef(sourceVmr)
+		if err != nil {
+			return err
 		}
 	}
 
-	err = config.CloneVm(sourceVmr, vmRef, client)
+	err := config.CloneVm(sourceVmr, vmRef, client)
 	if err != nil {
 		return err
 	}

builder/proxmox/clone/config.go

@@ -18,6 +18,7 @@ type Config struct {
 	proxmoxcommon.Config `mapstructure:",squash"`
 
 	CloneVM   string         `mapstructure:"clone_vm" required:"true"`
+	CloneVMID int            `mapstructure:"clone_vm_id" required:"true"`
 	FullClone config.Trilean `mapstructure:"full_clone" required:"false"`
 
 	Nameserver   string              `mapstructure:"nameserver" required:"false"`
@@ -39,8 +40,17 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, []string, error) {
 		errs = packersdk.MultiErrorAppend(errs, merrs)
 	}
 
-	if c.CloneVM == "" {
-		errs = packersdk.MultiErrorAppend(errs, errors.New("clone_vm must be specified"))
+	if c.CloneVM == "" && c.CloneVMID == 0 {
+		errs = packersdk.MultiErrorAppend(errs, errors.New("one of clone_vm or clone_vm_id must be specified"))
+	}
+	if c.CloneVM != "" && c.CloneVMID != 0 {
+		errs = packersdk.MultiErrorAppend(errs, errors.New("clone_vm and clone_vm_id cannot both be specified"))
+	}
+	// Technically Proxmox VMIDs are unsigned 32bit integers, but are limited to
+	// the range 100-999999999. Source:
+	// https://pve-devel.pve.proxmox.narkive.com/Pa6mH1OP/avoiding-vmid-reuse#post8
+	if c.CloneVMID != 0 && (c.CloneVMID < 100 || c.CloneVMID > 999999999) {
+		errs = packersdk.MultiErrorAppend(errs, errors.New("clone_vm_id must be in range 100-999999999"))
 	}
 
 	// Check validity of given IP addresses

builder/proxmox/clone/config_test.go

@@ -30,7 +30,7 @@ func TestRequiredParameters(t *testing.T) {
 		t.Fatal("Expected errors to be packersdk.MultiError")
 	}
 
-	required := []string{"username", "token", "proxmox_url", "node", "ssh_username", "clone_vm"}
+	required := []string{"username", "token", "proxmox_url", "node", "ssh_username", "clone_vm", "clone_vm_id"}
 	for _, param := range required {
 		found := false
 		for _, err := range errs.Errors {
@@ -45,6 +45,63 @@ func TestRequiredParameters(t *testing.T) {
 	}
 }
 
+func TestVMNameOrID(t *testing.T) {
+	ipconfigTest := []struct {
+		name          string
+		cloneVM       string
+		cloneVMID     int
+		expectFailure bool
+	}{
+		{
+			name:          "clone_vm given, no error",
+			cloneVM:       "myVM",
+			cloneVMID:     0,
+			expectFailure: false,
+		},
+		{
+			name:          "valid clone_vm_id given, no error",
+			cloneVM:       "",
+			cloneVMID:     100,
+			expectFailure: false,
+		},
+		{
+			name:          "clone_vm_id out of range, error",
+			cloneVM:       "",
+			cloneVMID:     50,
+			expectFailure: true,
+		},
+		{
+			name:          "clone_vm and clone_vm_id given, error",
+			cloneVM:       "myVM",
+			cloneVMID:     100,
+			expectFailure: true,
+		},
+		{
+			name:          "neither clone_vm nor clone_vm_id given, error",
+			cloneVM:       "",
+			cloneVMID:     0,
+			expectFailure: true,
+		},
+	}
+
+	for _, tt := range ipconfigTest {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := mandatoryConfig(t)
+			cfg["clone_vm"] = tt.cloneVM
+			cfg["clone_vm_id"] = tt.cloneVMID
+
+			var c Config
+			_, _, err := c.Prepare(&c, cfg)
+			if err != nil && !tt.expectFailure {
+				t.Fatalf("unexpected failure: %s", err)
+			}
+			if err == nil && tt.expectFailure {
+				t.Errorf("expected failure, but prepare succeeded")
+			}
+		})
+	}
+}
+
 func TestNameserver(t *testing.T) {
 	ipconfigTest := []struct {
 		name          string

builder/proxmox/common/config.go

@@ -169,6 +169,12 @@ func (c *Config) Prepare(upper interface{}, raws ...interface{}) ([]string, []st
 		c.BootKeyInterval = 5 * time.Millisecond
 	}
 
+	// Technically Proxmox VMIDs are unsigned 32bit integers, but are limited to
+	// the range 100-999999999. Source:
+	// https://pve-devel.pve.proxmox.narkive.com/Pa6mH1OP/avoiding-vmid-reuse#post8
+	if c.VMID != 0 && (c.VMID < 100 || c.VMID > 999999999) {
+		errs = packersdk.MultiErrorAppend(errs, errors.New("vm_id must be in range 100-999999999"))
+	}
 	if c.VMName == "" {
 		// Default to packer-[time-ordered-uuid]
 		c.VMName = fmt.Sprintf("packer-%s", uuid.TimeOrderedUUID())

builder/proxmox/common/config_test.go

@@ -286,3 +286,52 @@ func TestSerials(t *testing.T) {
 		})
 	}
 }
+
+func TestVMID(t *testing.T) {
+	serialsTest := []struct {
+		name          string
+		VMID          int
+		expectFailure bool
+	}{
+		{
+			name:          "VMID zero, no error",
+			expectFailure: false,
+			VMID:          0,
+		},
+		{
+			name:          "VMID in range, no error",
+			expectFailure: false,
+			VMID:          1000,
+		},
+		{
+			name:          "VMID above range, fail",
+			expectFailure: true,
+			VMID:          1000000000,
+		},
+		{
+			name:          "VMID below range, fail",
+			expectFailure: true,
+			VMID:          50,
+		},
+	}
+
+	for _, tt := range serialsTest {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := mandatoryConfig(t)
+			cfg["vm_id"] = tt.VMID
+
+			var c Config
+			_, _, err := c.Prepare(&c, cfg)
+			if err != nil {
+				if !tt.expectFailure {
+					t.Fatalf("unexpected failure to prepare config: %s", err)
+				}
+				t.Logf("got expected failure: %s", err)
+			}
+
+			if err == nil && tt.expectFailure {
+				t.Errorf("expected failure, but prepare succeeded")
+			}
+		})
+	}
+}

docs/builders/clone.mdx

@@ -68,6 +68,11 @@ in the image's Cloud-Init settings for provisioning.
   machine on during creation.
 
 - `clone_vm` (string) - The name of the VM Packer should clone and build from.
+  Either `clone_vm` or `clone_vm_id` must be specifed.
+
+- `clone_vm_id` (int) - The ID of the VM Packer should clone and build from.
+  Proxmox VMIDs are limited to the range 100-999999999.
+  Either `clone_vm` or `clone_vm_id` must be specifed.
 
 ### Optional:
 
@@ -82,8 +87,9 @@ in the image's Cloud-Init settings for provisioning.
   given, a random uuid will be used.
 
 - `vm_id` (int) - The ID used to reference the virtual machine. This will
-  also be the ID of the final template. If not given, the next free ID on
-  the node will be used.
+  also be the ID of the final template. Proxmox VMIDs are unique cluster-wide
+  and are limited to the range 100-999999999.
+  If not given, the next free ID on the cluster will be used.
 
 - `memory` (int) - How much memory, in megabytes, to give the virtual
   machine. Defaults to `512`.

docs/builders/iso.mdx

@@ -98,8 +98,9 @@ in the image's Cloud-Init settings for provisioning.
   given, a random uuid will be used.
 
 - `vm_id` (int) - The ID used to reference the virtual machine. This will
-  also be the ID of the final template. If not given, the next free ID on
-  the node will be used.
+  also be the ID of the final template. Proxmox VMIDs are unique cluster-wide
+  and are limited to the range 100-999999999.
+  If not given, the next free ID on the cluster will be used.
 
 - `memory` (int) - How much memory, in megabytes, to give the virtual
   machine. Defaults to `512`.