From ff0b0f5dbac2bdd6e0de4a20523fdc86e9decdd8 Mon Sep 17 00:00:00 2001 From: Alex Kulikovskikh Date: Wed, 16 Dec 2020 21:41:29 -0500 Subject: [PATCH] Fix multiple bugs in VM Resource - Fix #41 - Fix #42 - Fix issues when `vm` resource is created without `initialization` block. Related to #42 VM resource created without `initialization` block does not correctly set `initialization` block state attributes ## To Reproduce 1. Create a VM ```hcl resource "proxmox_virtual_environment_vm" "testvm" { node_name = "" pool_id = "" # required due to #41 } ``` 2. `terraform apply` 3. `terraform plan` ```hcl # proxmox_virtual_environment_vm.testvm must be replaced -/+ resource "proxmox_virtual_environment_vm" "testvm" { acpi = true bios = "seabios" ~ id = "103" -> (known after apply) ~ ipv4_addresses = [] -> (known after apply) ~ ipv6_addresses = [] -> (known after apply) keyboard_layout = "en-us" ~ mac_addresses = [] -> (known after apply) ~ network_interface_names = [] -> (known after apply) node_name = "pve" pool_id = "pve" started = true tablet_device = true template = false vm_id = -1 - initialization { } } Plan: 1 to add, 0 to change, 1 to destroy. ``` --- .../resource_virtual_environment_vm_test.go | 95 ++++++++++++++++++ proxmoxtf/acceptancetests/testutils/vm.go | 97 +++++++++++++++++++ proxmoxtf/resource_virtual_environment_vm.go | 34 +++++-- 3 files changed, 217 insertions(+), 9 deletions(-) create mode 100644 proxmoxtf/acceptancetests/resource_virtual_environment_vm_test.go create mode 100644 proxmoxtf/acceptancetests/testutils/vm.go diff --git a/proxmoxtf/acceptancetests/resource_virtual_environment_vm_test.go b/proxmoxtf/acceptancetests/resource_virtual_environment_vm_test.go new file mode 100644 index 0000000..eea144a --- /dev/null +++ b/proxmoxtf/acceptancetests/resource_virtual_environment_vm_test.go @@ -0,0 +1,95 @@ +package acceptancetests + +import ( + "fmt" + "github.com/danitso/terraform-provider-proxmox/proxmoxtf" + "github.com/danitso/terraform-provider-proxmox/proxmoxtf/acceptancetests/testutils" + "github.com/hashicorp/terraform/helper/resource" + "github.com/hashicorp/terraform/terraform" + "os" + "testing" +) + +// Verifies that VM can be created and updated +func TestAccResourceVirtualEnvironmentVM_CreateAndUpdate(t *testing.T) { + // Set environmental variable that will signal Terraform to + // Stop the VM rather than wait for shutdown + os.Setenv("TF_ACC_VM_FORCE_STOP", "true") + + tfNode := "proxmox_virtual_environment_vm.vm" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testutils.PreCheck(t, nil) }, + Providers: testutils.GetProviders(), + CheckDestroy: CheckVMDestroyed, + Steps: []resource.TestStep{ + // Create empty VM + { + Config: testutils.HclVMResource(map[string]string{}), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(tfNode, "name", ""), + ), + }, + // Update VM + // - Add name + { + Config: testutils.HclVMResource(map[string]string{ + "Name": "AccTestVM", + }), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(tfNode, "name", "AccTestVM"), + ), + }, + // Update VM + // - Remove name + { + Config: testutils.HclVMResource(map[string]string{}), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(tfNode, "name", ""), + ), + }, + // Update VM + // - Add name + // - Add to Pool + { + Config: testutils.HclVMResource(map[string]string{ + "Name": "AccTestVM", + "PoolID": "test-pool", + }), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(tfNode, "name", "AccTestVM"), + ), + }, + }, + }) +} + +// CheckVMDestroyed verifies that vm referenced in the state +// was destroyed. This will be invoked *after* terraform destroys +// the resource but *before* the state is wiped clean +func CheckVMDestroyed(s *terraform.State) error { + config := testutils.GetProvider().Meta().(proxmoxtf.ProviderConfiguration) + vmState, err := testutils.GetVMFromState(s, "vm") + + if err != nil { + return err + } + + if vmState.NodeName == "" || vmState.VMID == 0 { + return fmt.Errorf("Unable to find `proxmox_virtual_environment_vm` resource") + } + + conn, err := config.GetVEClient() + + if err != nil { + return err + } + + _, err = conn.GetVM(vmState.NodeName, vmState.VMID) + + if err == nil { + return fmt.Errorf("VM with id `%d` should not exist", vmState.VMID) + } + + return nil +} diff --git a/proxmoxtf/acceptancetests/testutils/vm.go b/proxmoxtf/acceptancetests/testutils/vm.go new file mode 100644 index 0000000..2497ed5 --- /dev/null +++ b/proxmoxtf/acceptancetests/testutils/vm.go @@ -0,0 +1,97 @@ +package testutils + +import ( + "bytes" + "fmt" + "github.com/danitso/terraform-provider-proxmox/proxmox" + "github.com/danitso/terraform-provider-proxmox/proxmoxtf" + "github.com/hashicorp/terraform/terraform" + "html/template" + "strconv" +) + +type VMStateAttributes struct { + NodeName string + VMID int +} + +// readVM is helper function that reads VM +func ReadVM(clients proxmoxtf.ProviderConfiguration, nodeName string, vmID int) (*proxmox.VirtualEnvironmentVMGetResponseData, error) { + conn, err := clients.GetVEClient() + + if err != nil { + return nil, err + } + + res, err := conn.GetVM(nodeName, vmID) + + if err != nil { + return nil, err + } + + return res, nil +} + +// GetVMFromState helper function that retrieves VM Attributes from TF state +func GetVMFromState(s *terraform.State, resourceName string) (VMStateAttributes, error) { + + vm, ok := s.RootModule().Resources[fmt.Sprintf("proxmox_virtual_environment_vm.%s", resourceName)] + + if !ok { + return VMStateAttributes{}, fmt.Errorf("Did not find a VM with name %s in TF state", resourceName) + } + + nodeName := vm.Primary.Attributes["node_name"] + vmID, err := strconv.Atoi(vm.Primary.Attributes["vm_id"]) + + if err != nil { + return VMStateAttributes{}, fmt.Errorf("Unable to convert `vm_id` attribute to integer") + } + + vmAttributes := VMStateAttributes{ + NodeName: nodeName, + VMID: vmID, + } + + return vmAttributes, nil +} + +// HclVMResource HCL describing of a PVE VM resource +func HclVMResource(vm map[string]string) string { + var b bytes.Buffer + + tmpl, err := template.New("").Parse(` +{{ if .PoolID }} +resource "proxmox_virtual_environment_pool" "test_pool" { + comment = "Managed by Terraform" + pool_id = "{{ .PoolID }}" +} +{{ end }} + +data "proxmox_virtual_environment_nodes" "pve_nodes" {} + +resource "proxmox_virtual_environment_vm" "vm" { + {{ if .NodeName }} + node_name = "{{.NodeName}}" + {{ else }} + node_name = data.proxmox_virtual_environment_nodes.pve_nodes.names[0] + {{ end }} + + {{ if .Name }} + name = "{{.Name}}" + {{ end }} + + {{ if .PoolID }} + pool_id = proxmox_virtual_environment_pool.test_pool.pool_id + {{ end }} +} +`) + + t := template.Must(tmpl, err) + if err := t.Execute(&b, vm); err != nil { + _ = fmt.Errorf("Unable to parse template %p", err) + return "" + } + + return b.String() +} \ No newline at end of file diff --git a/proxmoxtf/resource_virtual_environment_vm.go b/proxmoxtf/resource_virtual_environment_vm.go index 97eefc5..502a8cd 100644 --- a/proxmoxtf/resource_virtual_environment_vm.go +++ b/proxmoxtf/resource_virtual_environment_vm.go @@ -7,6 +7,7 @@ package proxmoxtf import ( "fmt" "math" + "os" "strconv" "strings" "time" @@ -1503,7 +1504,6 @@ func resourceVirtualEnvironmentVMCreateCustom(d *schema.ResourceData, m interfac KeyboardLayout: &keyboardLayout, NetworkDevices: networkDeviceObjects, OSType: &operatingSystemType, - PoolID: &poolID, SCSIDevices: diskDeviceObjects, SCSIHardware: &scsiHardware, SerialDevices: serialDevices, @@ -1532,6 +1532,10 @@ func resourceVirtualEnvironmentVMCreateCustom(d *schema.ResourceData, m interfac createBody.Name = &name } + if poolID != "" { + createBody.PoolID = &poolID + } + err = veClient.CreateVM(nodeName, createBody) if err != nil { @@ -2519,7 +2523,9 @@ func resourceVirtualEnvironmentVMReadCustom(d *schema.ResourceData, m interface{ ipConfigList[ipConfigIndex] = ipConfigItem } - initialization[mkResourceVirtualEnvironmentVMInitializationIPConfig] = ipConfigList[:ipConfigLast+1] + if ipConfigLast != -1 { + initialization[mkResourceVirtualEnvironmentVMInitializationIPConfig] = ipConfigList[:ipConfigLast+1] + } if vmConfig.CloudInitPassword != nil || vmConfig.CloudInitSSHKeys != nil || vmConfig.CloudInitUsername != nil { initializationUserAccount := map[string]interface{}{} @@ -2551,8 +2557,6 @@ func resourceVirtualEnvironmentVMReadCustom(d *schema.ResourceData, m interface{ } else { initialization[mkResourceVirtualEnvironmentVMInitializationUserDataFileID] = "" } - } else { - initialization[mkResourceVirtualEnvironmentVMInitializationUserDataFileID] = "" } currentInitialization := d.Get(mkResourceVirtualEnvironmentVMInitialization).([]interface{}) @@ -3011,7 +3015,12 @@ func resourceVirtualEnvironmentVMUpdate(d *schema.ResourceData, m interface{}) e } name := d.Get(mkResourceVirtualEnvironmentVMName).(string) - updateBody.Name = &name + + if name == "" { + delete = append(delete, "name") + } else { + updateBody.Name = &name + } if d.HasChange(mkResourceVirtualEnvironmentVMTabletDevice) { tabletDevice := proxmox.CustomBool(d.Get(mkResourceVirtualEnvironmentVMTabletDevice).(bool)) @@ -3404,10 +3413,17 @@ func resourceVirtualEnvironmentVMDelete(d *schema.ResourceData, m interface{}) e forceStop := proxmox.CustomBool(true) shutdownTimeout := 300 - err = veClient.ShutdownVM(nodeName, vmID, &proxmox.VirtualEnvironmentVMShutdownRequestBody{ - ForceStop: &forceStop, - Timeout: &shutdownTimeout, - }) + // For acceptance tests we force VM to stop rather than wait for shutdown + // Proxmox unable to shutdown VMs due to the lack of operating system + // and qemu guest agent + if _, ok := os.LookupEnv("TF_ACC_VM_FORCE_STOP"); ok { + err = veClient.StopVM(nodeName, vmID) + } else { + err = veClient.ShutdownVM(nodeName, vmID, &proxmox.VirtualEnvironmentVMShutdownRequestBody{ + ForceStop: &forceStop, + Timeout: &shutdownTimeout, + }) + } if err != nil { return err