fix(vm)!: revert cpu.units default to use PVE server default (#2402)

* fix(vm)!: revert `cpu.units` default to use PVE server default

Fixed a regression introduced in v0.85.0 where `cpu.units` default was changed from 1024 to 100, causing unexpected plan diffs for existing VMs.
The `cpu.units` attribute is now computed from the Proxmox server instead of using a hardcoded provider default. When not explicitly set, PVE will use its own default (1024 for cgroups v1 or 100 for cgroups v2), and the provider will read and store the actual value from the server.

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

* acceptance tests

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

* cleanup

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

---------

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

Author: bpg

docs/resources/virtual_environment_vm.md

@@ -266,7 +266,7 @@ output "ubuntu_vm_public_key" {
             See <https://en.wikipedia.org/wiki/X86-64#Microarchitecture_levels>
         - `custom-<model>` - Custom CPU model. All `custom-<model>` values
             should be defined in `/etc/pve/virtual-guest/cpu-models.conf` file.
-    - `units` - (Optional) The CPU units (defaults to `1024`).
+    - `units` - (Optional) The CPU units. PVE default is `1024` for cgroups v1 and `100` for cgroups v2.
     - `affinity` - (Optional) The CPU cores that are used to run the VM’s vCPU. The
         value is a list of CPU IDs, separated by commas. The CPU IDs are zero-based.
         For example, `0,1,2,3` (which also can be shortened to `0-3`) means that the VM’s vCPUs are run on the first four

fwprovider/nodes/vm/cpu/resource_test.go

@@ -206,6 +206,19 @@ func TestAccResourceVM2CPU(t *testing.T) {
 				}),
 			),
 		}}},
+		// regression test for https://github.com/bpg/terraform-provider-proxmox/issues/2353
+		{"create VM without cpu.units and verify no drift", []resource.TestStep{
+			{
+				Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_vm2" "test_vm" {
+					node_name = "{{.NodeName}}"
+					name = "test-cpu-units-default"
+				}`),
+			},
+			{
+				RefreshState: true,
+			},
+		}},
 	}
 
 	for _, tt := range tests {

fwprovider/test/resource_vm_test.go

@@ -150,6 +150,22 @@ func TestAccResourceVM(t *testing.T) {
 				),
 			},
 		}},
+		// regression test for https://github.com/bpg/terraform-provider-proxmox/issues/2353
+		{"create VM without cpu.units and verify no drift", []resource.TestStep{
+			{
+				Config: te.RenderConfig(`
+				resource "proxmox_virtual_environment_vm" "test_cpu_units" {
+					node_name = "{{.NodeName}}"
+					started   = false
+					cpu {
+						cores = 2
+					}
+				}`),
+			},
+			{
+				RefreshState: true,
+			},
+		}},
 		{"set cpu.architecture as non root is not supported", []resource.TestStep{
 			{
 				Config: te.RenderConfig(`

proxmoxtf/resource/vm/vm.go

@@ -73,7 +73,6 @@ const (
 	dvCPUNUMA             = false
 	dvCPUSockets          = 1
 	dvCPUType             = "qemu64"
-	dvCPUUnits            = 100
 	dvCPUAffinity         = ""
 	dvDescription         = ""
 
@@ -634,7 +633,7 @@ func VM() *schema.Resource {
 						mkCPUNUMA:         dvCPUNUMA,
 						mkCPUSockets:      dvCPUSockets,
 						mkCPUType:         dvCPUType,
-						mkCPUUnits:        dvCPUUnits,
+						mkCPUUnits:        0,
 						mkCPUAffinity:     dvCPUAffinity,
 					},
 				}, nil
@@ -704,7 +703,7 @@ func VM() *schema.Resource {
 						Type:        schema.TypeInt,
 						Description: "The CPU units",
 						Optional:    true,
-						Default:     dvCPUUnits,
+						Computed:    true,
 						ValidateDiagFunc: validation.ToDiagFunc(
 							validation.IntBetween(1, 262144),
 						),
@@ -2186,7 +2185,6 @@ func vmCreateClone(ctx context.Context, d *schema.ResourceData, m any) diag.Diag
 		}
 		updateBody.NUMAEnabled = &cpuNUMA
 		updateBody.CPUSockets = ptr.Ptr(int64(cpuSockets))
-		updateBody.CPUUnits = ptr.Ptr(int64(cpuUnits))
 
 		if cpuAffinity != "" {
 			updateBody.CPUAffinity = &cpuAffinity
@@ -2199,6 +2197,10 @@ func vmCreateClone(ctx context.Context, d *schema.ResourceData, m any) diag.Diag
 		if cpuLimit > 0 {
 			updateBody.CPULimit = ptr.Ptr(int64(cpuLimit))
 		}
+
+		if cpuUnits > 0 {
+			updateBody.CPUUnits = ptr.Ptr(int64(cpuUnits))
+		}
 	}
 
 	vmConfig, err := vmAPI.GetVM(ctx)
@@ -2890,7 +2892,6 @@ func vmCreateCustom(ctx context.Context, d *schema.ResourceData, m any) diag.Dia
 			Type:  cpuType,
 		},
 		CPUSockets:           ptr.Ptr(int64(cpuSockets)),
-		CPUUnits:             ptr.Ptr(int64(cpuUnits)),
 		DedicatedMemory:      &memoryDedicated,
 		DeletionProtection:   &protection,
 		EFIDisk:              efiDisk,
@@ -2931,6 +2932,10 @@ func vmCreateCustom(ctx context.Context, d *schema.ResourceData, m any) diag.Dia
 		createBody.CPULimit = ptr.Ptr(int64(cpuLimit))
 	}
 
+	if cpuUnits > 0 {
+		createBody.CPUUnits = ptr.Ptr(int64(cpuUnits))
+	}
+
 	if cpuAffinity != "" {
 		createBody.CPUAffinity = &cpuAffinity
 	}
@@ -4118,8 +4123,7 @@ func vmReadCustom(
 	if vmConfig.CPUUnits != nil {
 		cpu[mkCPUUnits] = int(*vmConfig.CPUUnits)
 	} else {
-		// Default value of "cpuunits" is "1024" according to the API documentation.
-		cpu[mkCPUUnits] = 1024
+		cpu[mkCPUUnits] = 0
 	}
 
 	if vmConfig.CPUAffinity != nil {
@@ -4143,7 +4147,7 @@ func vmReadCustom(
 		cpu[mkCPULimit] != dvCPULimit ||
 		cpu[mkCPUSockets] != dvCPUSockets ||
 		cpu[mkCPUType] != dvCPUType ||
-		cpu[mkCPUUnits] != dvCPUUnits {
+		cpu[mkCPUUnits] != 0 {
 		err := d.Set(mkCPU, []any{cpu})
 		diags = append(diags, diag.FromErr(err)...)
 	}
@@ -5541,9 +5545,12 @@ func vmUpdate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnosti
 
 		updateBody.CPUCores = ptr.Ptr(int64(cpuCores))
 		updateBody.CPUSockets = ptr.Ptr(int64(cpuSockets))
-		updateBody.CPUUnits = ptr.Ptr(int64(cpuUnits))
 		updateBody.NUMAEnabled = &cpuNUMA
 
+		if cpuUnits > 0 {
+			updateBody.CPUUnits = ptr.Ptr(int64(cpuUnits))
+		}
+
 		// CPU affinity is a special case, only root can change it.
 		// we can't even have it in the delete list, as PVE will return an error for non-root.
 		// Hence, checking explicitly if it has changed.