Skip to content

Commit

Permalink
Fix multiple bugs in VM Resource
Browse files Browse the repository at this point in the history
- Fix danitso#41
- Fix danitso#42
- Fix issues when `vm` resource is created without `initialization` block. Related to danitso#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   = "<your_node_name>"
  pool_id     = "<existing_pool>" # required due to danitso#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.
```
  • Loading branch information
blz-ea committed Dec 17, 2020
1 parent 34a7445 commit ff0b0f5
Show file tree
Hide file tree
Showing 3 changed files with 217 additions and 9 deletions.
95 changes: 95 additions & 0 deletions proxmoxtf/acceptancetests/resource_virtual_environment_vm_test.go
Original file line number Diff line number Diff line change
@@ -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
}
97 changes: 97 additions & 0 deletions proxmoxtf/acceptancetests/testutils/vm.go
Original file line number Diff line number Diff line change
@@ -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()
}
34 changes: 25 additions & 9 deletions proxmoxtf/resource_virtual_environment_vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package proxmoxtf
import (
"fmt"
"math"
"os"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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{}{}
Expand Down Expand Up @@ -2551,8 +2557,6 @@ func resourceVirtualEnvironmentVMReadCustom(d *schema.ResourceData, m interface{
} else {
initialization[mkResourceVirtualEnvironmentVMInitializationUserDataFileID] = ""
}
} else {
initialization[mkResourceVirtualEnvironmentVMInitializationUserDataFileID] = ""
}

currentInitialization := d.Get(mkResourceVirtualEnvironmentVMInitialization).([]interface{})
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit ff0b0f5

Please sign in to comment.