Skip to content

Commit

Permalink
fix(vm): Make vm_id computed (#367)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
otopetrik and bpg committed Jun 7, 2023
1 parent 926382c commit 2a5abb1
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 13 deletions.
2 changes: 1 addition & 1 deletion docs/resources/virtual_environment_container.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
Expand Down
3 changes: 2 additions & 1 deletion example/resource_virtual_environment_container.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion example/resource_virtual_environment_vm.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down
55 changes: 45 additions & 10 deletions proxmoxtf/resource/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ const (
dvResourceVirtualEnvironmentVMVGAEnabled = true
dvResourceVirtualEnvironmentVMVGAMemory = 16
dvResourceVirtualEnvironmentVMVGAType = "std"
dvResourceVirtualEnvironmentVMVMID = -1
dvResourceVirtualEnvironmentVMSCSIHardware = "virtio-scsi-pci"

maxResourceVirtualEnvironmentVMAudioDevices = 1
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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()
},
),
),
}
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{})

Expand Down

0 comments on commit 2a5abb1

Please sign in to comment.