Skip to content

Commit f0bb501

Browse files
hsnprsdbpggemini-code-assist[bot]
authored
fix(vm): prevent state changes after VM import (#2294)
* fix(vm): prevent state changes after VM import Signed-off-by: Ehsan Poursaeed <hsnprsd@gmail.com> * add new attributes missed from vmReadCustom Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> * fix reading booleans defaults in vmReadCustom Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> * add boot order to test Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> * Update proxmoxtf/resource/vm/vm.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> 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: Ehsan Poursaeed <hsnprsd@gmail.com> Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Co-authored-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 6e2a5aa commit f0bb501

File tree

5 files changed

+166
-15
lines changed

5 files changed

+166
-15
lines changed

‎docs/resources/virtual_environment_vm.md‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,7 @@ output "ubuntu_vm_public_key" {
159159
- `bios` - (Optional) The BIOS implementation (defaults to `seabios`).
160160
- `ovmf` - OVMF (UEFI).
161161
- `seabios` - SeaBIOS.
162-
- `boot_order` - (Optional) Specify a list of devices to boot from in the order
163-
they appear in the list (defaults to `[]`).
162+
- `boot_order` - (Optional) Specify a list of devices to boot from in the order they appear in the list.
164163
- `cdrom` - (Optional) The CD-ROM configuration.
165164
- `enabled` - (Optional) Whether to enable the CD-ROM drive (defaults
166165
to `false`). *Deprecated*. The attribute will be removed in the next version of the provider.

‎fwprovider/provider_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ func TestIDGenerator_Random(t *testing.T) {
138138
t.Cleanup(func() {
139139
for _, id := range ids {
140140
if id > 100 {
141-
_ = te.NodeClient().VM(id).DeleteVM(ctx, true, true) //nolint:errcheck
141+
_ = te.NodeClient().VM(id).DeleteVM(ctx, true, true) //nolint:errcheck
142142
}
143143
}
144144
})

‎fwprovider/test/resource_vm_test.go‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ package test
1010

1111
import (
1212
"fmt"
13+
"math/rand"
1314
"regexp"
1415
"testing"
1516

@@ -526,6 +527,81 @@ func TestAccResourceVM(t *testing.T) {
526527
}
527528
}
528529

530+
func TestAccResourceVMImport(t *testing.T) {
531+
te := InitEnvironment(t)
532+
533+
// Generate dynamic VM ID to avoid conflicts
534+
testVMID := 100000 + rand.Intn(99999)
535+
536+
te.AddTemplateVars(map[string]interface{}{
537+
"TestVMID": testVMID,
538+
})
539+
540+
tests := []struct {
541+
name string
542+
step []resource.TestStep
543+
}{
544+
{"vm import", []resource.TestStep{
545+
{
546+
Config: te.RenderConfig(`
547+
resource "proxmox_virtual_environment_vm" "vm_import" {
548+
node_name = "{{.NodeName}}"
549+
550+
vm_id = {{.TestVMID}}
551+
552+
started = false
553+
agent {
554+
enabled = true
555+
}
556+
boot_order = ["virtio0", "net0"]
557+
cpu {
558+
cores = 2
559+
}
560+
memory {
561+
dedicated = 2048
562+
}
563+
564+
disk {
565+
datastore_id = "local-lvm"
566+
interface = "virtio0"
567+
iothread = true
568+
discard = "on"
569+
size = 20
570+
}
571+
572+
initialization {
573+
interface = "scsi1"
574+
575+
ip_config {
576+
ipv4 {
577+
address = "dhcp"
578+
}
579+
}
580+
}
581+
network_device {
582+
bridge = "vmbr0"
583+
}
584+
}`),
585+
},
586+
{
587+
ResourceName: "proxmox_virtual_environment_vm.vm_import",
588+
ImportState: true,
589+
ImportStateVerify: true,
590+
ImportStateId: fmt.Sprintf("%s/%d", te.NodeName, testVMID),
591+
},
592+
}},
593+
}
594+
595+
for _, tt := range tests {
596+
t.Run(tt.name, func(t *testing.T) {
597+
resource.Test(t, resource.TestCase{
598+
ProtoV6ProviderFactories: te.AccProviders,
599+
Steps: tt.step,
600+
})
601+
})
602+
}
603+
}
604+
529605
func TestAccResourceVMInitialization(t *testing.T) {
530606
te := InitEnvironment(t)
531607

@@ -1012,6 +1088,30 @@ func TestAccResourceVMClone(t *testing.T) {
10121088
name string
10131089
step []resource.TestStep
10141090
}{
1091+
{"clone with network device", []resource.TestStep{{
1092+
Config: te.RenderConfig(`
1093+
resource "proxmox_virtual_environment_vm" "template" {
1094+
node_name = "{{.NodeName}}"
1095+
started = false
1096+
template = true
1097+
network_device {
1098+
bridge = "vmbr0"
1099+
}
1100+
}
1101+
resource "proxmox_virtual_environment_vm" "clone" {
1102+
node_name = "{{.NodeName}}"
1103+
started = false
1104+
clone {
1105+
vm_id = proxmox_virtual_environment_vm.template.vm_id
1106+
}
1107+
}`),
1108+
Check: resource.ComposeTestCheckFunc(
1109+
ResourceAttributes("proxmox_virtual_environment_vm.clone", map[string]string{
1110+
"network_device.#": "1",
1111+
"network_device.0.bridge": "vmbr0",
1112+
}),
1113+
),
1114+
}}},
10151115
{"clone cpu.architecture as root", []resource.TestStep{{
10161116
Config: te.RenderConfig(`
10171117
resource "proxmox_virtual_environment_vm" "template" {

‎proxmox/nodes/vms/vms_types.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,7 @@ type GetResponseData struct {
191191
BackupFile *string `json:"archive,omitempty"`
192192
BandwidthLimit *int `json:"bwlimit,omitempty"`
193193
BIOS *string `json:"bios,omitempty"`
194-
BootDisk *string `json:"bootdisk,omitempty"`
195-
BootOrder *string `json:"boot,omitempty"`
194+
BootOrder *CustomBoot `json:"boot,omitempty"`
196195
CDROM *string `json:"cdrom,omitempty"`
197196
CloudInitDNSDomain *string `json:"searchdomain,omitempty"`
198197
CloudInitDNSServer *string `json:"nameserver,omitempty"`

‎proxmoxtf/resource/vm/vm.go‎

Lines changed: 63 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ const (
126126
dvTimeoutShutdownVM = 1800
127127
dvTimeoutStartVM = 1800
128128
dvTimeoutStopVM = 300
129+
dvTimeoutMoveDisk = 1800
129130
dvVGAClipboard = ""
130131
dvVGAMemory = 16
131132
dvVGAType = "std"
@@ -287,6 +288,7 @@ const (
287288
mkTimeoutShutdownVM = "timeout_shutdown_vm"
288289
mkTimeoutStartVM = "timeout_start_vm"
289290
mkTimeoutStopVM = "timeout_stop_vm"
291+
mkTimeoutMoveDisk = "timeout_move_disk"
290292
mkHostUSB = "usb"
291293
mkHostUSBDevice = "host"
292294
mkHostUSBDeviceMapping = "mapping"
@@ -346,10 +348,8 @@ func VM() *schema.Resource {
346348
Type: schema.TypeList,
347349
Description: "The guest will attempt to boot from devices in the order they appear here",
348350
Optional: true,
351+
Computed: true,
349352
Elem: &schema.Schema{Type: schema.TypeString},
350-
DefaultFunc: func() (interface{}, error) {
351-
return []interface{}{}, nil
352-
},
353353
},
354354
mkACPI: {
355355
Type: schema.TypeBool,
@@ -1462,11 +1462,11 @@ func VM() *schema.Resource {
14621462
Optional: true,
14631463
Default: dvTimeoutCreate,
14641464
},
1465-
"timeout_move_disk": {
1465+
mkTimeoutMoveDisk: {
14661466
Type: schema.TypeInt,
1467-
Description: "MoveDisk timeout",
1467+
Description: "Disk move timeout",
14681468
Optional: true,
1469-
Default: 1800,
1469+
Default: dvTimeoutMoveDisk,
14701470
Deprecated: "This field is deprecated and will be removed in a future release. " +
14711471
"An overall operation timeout (timeout_create / timeout_clone / timeout_migrate) is used instead.",
14721472
},
@@ -2725,13 +2725,13 @@ func vmCreateCustom(ctx context.Context, d *schema.ResourceData, m interface{})
27252725
}
27262726

27272727
var bootOrderConverted []string
2728-
if cdromInterface != "" {
2729-
bootOrderConverted = []string{cdromInterface}
2730-
}
2731-
27322728
bootOrder := d.Get(mkBootOrder).([]interface{})
27332729

27342730
if len(bootOrder) == 0 {
2731+
if cdromInterface != "" {
2732+
bootOrderConverted = []string{cdromInterface}
2733+
}
2734+
27352735
if _, ok := diskDeviceObjects["ide0"]; ok {
27362736
bootOrderConverted = append(bootOrderConverted, "ide0")
27372737
}
@@ -3678,6 +3678,27 @@ func vmRead(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Dia
36783678
return vmReadCustom(ctx, d, m, vmID, vmConfig, vmStatus)
36793679
}
36803680

3681+
func setDefaultIfNotSet(d *schema.ResourceData, diags diag.Diagnostics, key string, value interface{}) diag.Diagnostics {
3682+
if _, ok := d.GetOk(key); !ok {
3683+
if err := d.Set(key, value); err != nil {
3684+
return append(diags, diag.FromErr(err)...)
3685+
}
3686+
}
3687+
3688+
return diags
3689+
}
3690+
3691+
//nolint:staticcheck
3692+
func setDefaultIfNotExists(d *schema.ResourceData, diags diag.Diagnostics, key string, value interface{}) diag.Diagnostics {
3693+
if _, ok := d.GetOkExists(key); !ok {
3694+
if err := d.Set(key, value); err != nil {
3695+
return append(diags, diag.FromErr(err)...)
3696+
}
3697+
}
3698+
3699+
return diags
3700+
}
3701+
36813702
func vmReadCustom(
36823703
ctx context.Context,
36833704
d *schema.ResourceData,
@@ -4914,6 +4935,21 @@ func vmReadCustom(
49144935
e = d.Set(mkNodeName, nodeName)
49154936
diags = append(diags, diag.FromErr(e)...)
49164937

4938+
diags = setDefaultIfNotExists(d, diags, mkMigrate, dvMigrate)
4939+
diags = setDefaultIfNotSet(d, diags, mkTimeoutClone, dvTimeoutClone)
4940+
diags = setDefaultIfNotSet(d, diags, mkTimeoutCreate, dvTimeoutCreate)
4941+
diags = setDefaultIfNotSet(d, diags, mkTimeoutMigrate, dvTimeoutMigrate)
4942+
diags = setDefaultIfNotSet(d, diags, mkTimeoutReboot, dvTimeoutReboot)
4943+
diags = setDefaultIfNotSet(d, diags, mkTimeoutShutdownVM, dvTimeoutShutdownVM)
4944+
diags = setDefaultIfNotSet(d, diags, mkTimeoutStartVM, dvTimeoutStartVM)
4945+
diags = setDefaultIfNotSet(d, diags, mkTimeoutStopVM, dvTimeoutStopVM)
4946+
diags = setDefaultIfNotSet(d, diags, mkTimeoutMoveDisk, dvTimeoutMoveDisk)
4947+
diags = setDefaultIfNotExists(d, diags, mkStopOnDestroy, dvStopOnDestroy)
4948+
diags = setDefaultIfNotExists(d, diags, mkPurgeOnDestroy, dvPurgeOnDestroy)
4949+
diags = setDefaultIfNotExists(d, diags, mkDeleteUnreferencedDisksOnDestroy, dvDeleteUnreferencedDisksOnDestroy)
4950+
diags = setDefaultIfNotExists(d, diags, mkRebootAfterUpdate, dvRebootAfterUpdate)
4951+
diags = setDefaultIfNotExists(d, diags, mkRebootAfterCreation, dvRebootAfterCreation)
4952+
49174953
return diags
49184954
}
49194955

@@ -4981,6 +5017,23 @@ func vmReadPrimitiveValues(
49815017

49825018
currentTags := d.Get(mkTags).([]interface{})
49835019

5020+
err = d.Set(mkOnBoot, vmConfig.StartOnBoot)
5021+
if err != nil {
5022+
diags = append(diags, diag.FromErr(err)...)
5023+
}
5024+
5025+
if vmConfig.BootOrder == nil {
5026+
err = d.Set(mkBootOrder, nil)
5027+
if err != nil {
5028+
diags = append(diags, diag.FromErr(err)...)
5029+
}
5030+
} else {
5031+
err = d.Set(mkBootOrder, vmConfig.BootOrder.Order)
5032+
if err != nil {
5033+
diags = append(diags, diag.FromErr(err)...)
5034+
}
5035+
}
5036+
49845037
if len(clone) == 0 || len(currentTags) > 0 {
49855038
var tags []string
49865039

0 commit comments

Comments
 (0)