From 2a5abb10fc43603d2c786ad806cba056886c7f29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oto=20Pet=C5=99=C3=ADk?= Date: Thu, 8 Jun 2023 01:40:39 +0200 Subject: [PATCH] fix(vm): Make `vm_id` computed (#367) * fix(vm): Make vm_id computed, fix #364 Defaulting vm_id to -1 prevents resources depending on vm_id value. Make vm_id computed, also update existing vm_id = -1 with correct vm_id. * update examples to use auto-generated `vm_id`s --------- Co-authored-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> --- .../virtual_environment_container.md | 2 +- .../resource_virtual_environment_container.tf | 3 +- example/resource_virtual_environment_vm.tf | 3 +- proxmoxtf/resource/vm.go | 55 +++++++++++++++---- 4 files changed, 50 insertions(+), 13 deletions(-) diff --git a/docs/resources/virtual_environment_container.md b/docs/resources/virtual_environment_container.md index 0d9637fd..3af3245d 100644 --- a/docs/resources/virtual_environment_container.md +++ b/docs/resources/virtual_environment_container.md @@ -173,7 +173,7 @@ output "ubuntu_container_public_key" { - `template` - (Optional) Whether to create a template (defaults to `false`). - `unprivileged` - (Optional) Whether the container runs as unprivileged on the host (defaults to `false`). -- `vm_id` - (Optional) The virtual machine identifier +- `vm_id` - (Optional) The container identifier - `features` - (Optional) The container features - `nesting` - (Optional) Whether the container is nested (defaults to `false`) diff --git a/example/resource_virtual_environment_container.tf b/example/resource_virtual_environment_container.tf index 46c502e3..0e8dde14 100644 --- a/example/resource_virtual_environment_container.tf +++ b/example/resource_virtual_environment_container.tf @@ -39,7 +39,8 @@ resource "proxmox_virtual_environment_container" "example_template" { pool_id = proxmox_virtual_environment_pool.example.id template = true - vm_id = 2042 + + // use auto-generated vm_id tags = [ "container", diff --git a/example/resource_virtual_environment_vm.tf b/example/resource_virtual_environment_vm.tf index 5a92ebf5..1b47720a 100644 --- a/example/resource_virtual_environment_vm.tf +++ b/example/resource_virtual_environment_vm.tf @@ -72,7 +72,8 @@ resource "proxmox_virtual_environment_vm" "example_template" { serial_device {} template = true - vm_id = 2040 + + // use auto-generated vm_id } resource "proxmox_virtual_environment_vm" "example" { diff --git a/proxmoxtf/resource/vm.go b/proxmoxtf/resource/vm.go index 5f64d6b5..3a5f473f 100644 --- a/proxmoxtf/resource/vm.go +++ b/proxmoxtf/resource/vm.go @@ -103,7 +103,6 @@ const ( dvResourceVirtualEnvironmentVMVGAEnabled = true dvResourceVirtualEnvironmentVMVGAMemory = 16 dvResourceVirtualEnvironmentVMVGAType = "std" - dvResourceVirtualEnvironmentVMVMID = -1 dvResourceVirtualEnvironmentVMSCSIHardware = "virtio-scsi-pci" maxResourceVirtualEnvironmentVMAudioDevices = 1 @@ -1208,11 +1207,12 @@ func VM() *schema.Resource { MinItems: 0, }, mkResourceVirtualEnvironmentVMVMID: { - Type: schema.TypeInt, - Description: "The VM identifier", - Optional: true, - ForceNew: true, - Default: dvResourceVirtualEnvironmentVMVMID, + Type: schema.TypeInt, + Description: "The VM identifier", + Optional: true, + Computed: true, + // "ForceNew: true" handled in CustomizeDiff, making sure VMs with legacy configs with vm_id = -1 + // do not require re-creation. ValidateDiagFunc: getVMIDValidator(), }, mkResourceVirtualEnvironmentVMSCSIHardware: { @@ -1249,6 +1249,16 @@ func VM() *schema.Resource { d.HasChange(mkResourceVirtualEnvironmentVMNetworkDevice) }, ), + customdiff.ForceNewIf( + mkResourceVirtualEnvironmentVMVMID, + func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) bool { + newValue := d.Get(mkResourceVirtualEnvironmentVMVMID) + + // 'vm_id' is ForceNew, except when changing 'vm_id' to existing correct id + // (automatic fix from -1 to actual vm_id must not re-create VM) + return strconv.Itoa(newValue.(int)) != d.Id() + }, + ), ), } } @@ -1284,14 +1294,21 @@ func vmCreateClone(ctx context.Context, d *schema.ResourceData, m interface{}) d tags := d.Get(mkResourceVirtualEnvironmentVMTags).([]interface{}) nodeName := d.Get(mkResourceVirtualEnvironmentVMNodeName).(string) poolID := d.Get(mkResourceVirtualEnvironmentVMPoolID).(string) - vmID := d.Get(mkResourceVirtualEnvironmentVMVMID).(int) + vmIDUntyped, hasVMID := d.GetOk(mkResourceVirtualEnvironmentVMVMID) + vmID := vmIDUntyped.(int) - if vmID == -1 { + if !hasVMID { vmIDNew, err := api.Cluster().GetVMID(ctx) if err != nil { return diag.FromErr(err) } + vmID = *vmIDNew + err = d.Set(mkResourceVirtualEnvironmentVMVMID, vmID) + + if err != nil { + return diag.FromErr(err) + } } fullCopy := types.CustomBool(cloneFull) @@ -1928,15 +1945,21 @@ func vmCreateCustom(ctx context.Context, d *schema.ResourceData, m interface{}) return diag.FromErr(err) } - vmID := d.Get(mkResourceVirtualEnvironmentVMVMID).(int) + vmIDUntyped, hasVMID := d.GetOk(mkResourceVirtualEnvironmentVMVMID) + vmID := vmIDUntyped.(int) - if vmID == -1 { + if !hasVMID { vmIDNew, e := api.Cluster().GetVMID(ctx) if e != nil { return diag.FromErr(e) } vmID = *vmIDNew + e = d.Set(mkResourceVirtualEnvironmentVMVMID, vmID) + + if e != nil { + return diag.FromErr(e) + } } var memorySharedObject *vms.CustomSharedMemory @@ -2812,6 +2835,18 @@ func vmReadCustom( return diags } + // Fix terraform.tfstate, by replacing '-1' (the old default value) with actual vm_id value + if storedVMID := d.Get(mkResourceVirtualEnvironmentVMVMID).(int); storedVMID == -1 { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Warning, + Summary: fmt.Sprintf("VM %s has stored legacy vm_id %d, setting vm_id to its correct value %d.", + d.Id(), storedVMID, vmID), + }) + + err = d.Set(mkResourceVirtualEnvironmentVMVMID, vmID) + diags = append(diags, diag.FromErr(err)...) + } + nodeName := d.Get(mkResourceVirtualEnvironmentVMNodeName).(string) clone := d.Get(mkResourceVirtualEnvironmentVMClone).([]interface{})