From 93f7f3d1b84221c72380b306777a04bf1c4a8acc Mon Sep 17 00:00:00 2001 From: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Date: Sun, 17 Aug 2025 18:33:14 -0400 Subject: [PATCH] update acceptance test Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> --- fwprovider/test/resource_vm_disks_test.go | 196 ++++++++++++++++++++++ proxmoxtf/resource/vm/disk/disk.go | 8 +- proxmoxtf/resource/vm/disk/disk_test.go | 25 +-- 3 files changed, 211 insertions(+), 18 deletions(-) diff --git a/fwprovider/test/resource_vm_disks_test.go b/fwprovider/test/resource_vm_disks_test.go index 825c8ea9..0714a2a3 100644 --- a/fwprovider/test/resource_vm_disks_test.go +++ b/fwprovider/test/resource_vm_disks_test.go @@ -246,6 +246,202 @@ func TestAccResourceVMDisks(t *testing.T) { RefreshState: true, }, }}, + {"disk ordering consistency", []resource.TestStep{ + { + Config: te.RenderConfig(` + resource "proxmox_virtual_environment_vm" "test_disk_ordering" { + node_name = "{{.NodeName}}" + started = false + name = "test-disk-ordering" + + disk { + datastore_id = "local-lvm" + interface = "virtio2" + size = 8 + } + disk { + datastore_id = "local-lvm" + interface = "scsi1" + size = 15 + } + disk { + datastore_id = "local-lvm" + interface = "sata0" + size = 12 + } + disk { + datastore_id = "local-lvm" + interface = "scsi0" + size = 10 + } + disk { + datastore_id = "local-lvm" + interface = "virtio0" + size = 20 + } + disk { + datastore_id = "local-lvm" + interface = "scsi3" + size = 5 + } + disk { + datastore_id = "local-lvm" + interface = "virtio1" + size = 18 + } + disk { + datastore_id = "local-lvm" + interface = "scsi2" + size = 25 + } + }`), + Check: ResourceAttributes("proxmox_virtual_environment_vm.test_disk_ordering", map[string]string{ + "disk.0.interface": "virtio2", + "disk.0.size": "8", + "disk.1.interface": "scsi1", + "disk.1.size": "15", + "disk.2.interface": "sata0", + "disk.2.size": "12", + "disk.3.interface": "scsi0", + "disk.3.size": "10", + "disk.4.interface": "virtio0", + "disk.4.size": "20", + "disk.5.interface": "scsi3", + "disk.5.size": "5", + "disk.6.interface": "virtio1", + "disk.6.size": "18", + "disk.7.interface": "scsi2", + "disk.7.size": "25", + }), + }, + { + Config: te.RenderConfig(` + resource "proxmox_virtual_environment_vm" "test_disk_ordering" { + node_name = "{{.NodeName}}" + started = false + name = "test-disk-ordering" + + disk { + datastore_id = "local-lvm" + interface = "virtio2" + size = 8 + } + disk { + datastore_id = "local-lvm" + interface = "scsi1" + size = 15 + } + disk { + datastore_id = "local-lvm" + interface = "sata0" + size = 12 + } + disk { + datastore_id = "local-lvm" + interface = "scsi0" + size = 10 + } + disk { + datastore_id = "local-lvm" + interface = "virtio0" + size = 20 + } + disk { + datastore_id = "local-lvm" + interface = "scsi3" + size = 5 + } + disk { + datastore_id = "local-lvm" + interface = "virtio1" + size = 18 + } + disk { + datastore_id = "local-lvm" + interface = "scsi2" + size = 25 + } + }`), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + Check: ResourceAttributes("proxmox_virtual_environment_vm.test_disk_ordering", map[string]string{ + "disk.0.interface": "virtio2", + "disk.0.size": "8", + "disk.1.interface": "scsi1", + "disk.1.size": "15", + "disk.2.interface": "sata0", + "disk.2.size": "12", + "disk.3.interface": "scsi0", + "disk.3.size": "10", + "disk.4.interface": "virtio0", + "disk.4.size": "20", + "disk.5.interface": "scsi3", + "disk.5.size": "5", + "disk.6.interface": "virtio1", + "disk.6.size": "18", + "disk.7.interface": "scsi2", + "disk.7.size": "25", + }), + }, + { + // Third apply to ensure consistency across multiple applies + Config: te.RenderConfig(` + resource "proxmox_virtual_environment_vm" "test_disk_ordering" { + node_name = "{{.NodeName}}" + started = false + name = "test-disk-ordering" + + disk { + datastore_id = "local-lvm" + interface = "virtio2" + size = 8 + } + disk { + datastore_id = "local-lvm" + interface = "scsi1" + size = 15 + } + disk { + datastore_id = "local-lvm" + interface = "sata0" + size = 12 + } + disk { + datastore_id = "local-lvm" + interface = "scsi0" + size = 10 + } + disk { + datastore_id = "local-lvm" + interface = "virtio0" + size = 20 + } + disk { + datastore_id = "local-lvm" + interface = "scsi3" + size = 5 + } + disk { + datastore_id = "local-lvm" + interface = "virtio1" + size = 18 + } + disk { + datastore_id = "local-lvm" + interface = "scsi2" + size = 25 + } + }`), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + }, + }}, {"adding disks", []resource.TestStep{ { Config: te.RenderConfig(` diff --git a/proxmoxtf/resource/vm/disk/disk.go b/proxmoxtf/resource/vm/disk/disk.go index ca5369ff..3e6ee446 100644 --- a/proxmoxtf/resource/vm/disk/disk.go +++ b/proxmoxtf/resource/vm/disk/disk.go @@ -10,7 +10,6 @@ import ( "context" "errors" "fmt" - "maps" "regexp" "slices" "strings" @@ -560,11 +559,8 @@ func Read( } } - // Sort keys to ensure deterministic ordering - currentKeys := slices.Collect(maps.Keys(currentDiskMap)) - slices.SortFunc(currentKeys, utils.CompareWithPrefix) - - diskList = utils.OrderedListFromMapByKeyValues(diskMap, currentKeys) + disks := utils.ListResourcesAttributeValue(currentDiskList, mkDiskInterface) + diskList = utils.OrderedListFromMapByKeyValues(diskMap, disks) } else { diskList = utils.OrderedListFromMap(diskMap) } diff --git a/proxmoxtf/resource/vm/disk/disk_test.go b/proxmoxtf/resource/vm/disk/disk_test.go index 21c86b05..e1b88b1d 100644 --- a/proxmoxtf/resource/vm/disk/disk_test.go +++ b/proxmoxtf/resource/vm/disk/disk_test.go @@ -20,9 +20,9 @@ import ( ) // TestDiskOrderingDeterministic tests that disk ordering is deterministic -// when reading VM disk configuration. This test addresses the issue where -// disk interfaces (scsi0, scsi1, etc.) would randomly reorder on each -// terraform apply due to Go's non-deterministic map iteration. +// and preserves the order from currentDiskList. This test addresses the issue where +// disk interfaces would randomly reorder on each terraform apply due to +// Go's non-deterministic map iteration. func TestDiskOrderingDeterministic(t *testing.T) { t.Parallel() @@ -96,20 +96,21 @@ func TestDiskOrderingDeterministic(t *testing.T) { "Disk ordering should be deterministic across multiple calls. Iteration %d differs from first result", i) } - // Verify the specific ordering - should be scsi0, then scsi1 (numerical order) + // Verify the specific ordering - should preserve currentDiskList order: scsi1, then scsi0 require.Len(t, expectedResult, 2, "Should have 2 disks") disk0 := expectedResult[0].(map[string]interface{}) disk1 := expectedResult[1].(map[string]interface{}) - require.Equal(t, "scsi0", disk0[mkDiskInterface], "First disk should be scsi0") - require.Equal(t, 50, disk0[mkDiskSize], "First disk should have size 50") + require.Equal(t, "scsi1", disk0[mkDiskInterface], "First disk should be scsi1 (as in currentDiskList)") + require.Equal(t, 150, disk0[mkDiskSize], "First disk should have size 150") - require.Equal(t, "scsi1", disk1[mkDiskInterface], "Second disk should be scsi1") - require.Equal(t, 150, disk1[mkDiskSize], "Second disk should have size 150") + require.Equal(t, "scsi0", disk1[mkDiskInterface], "Second disk should be scsi0 (as in currentDiskList)") + require.Equal(t, 50, disk1[mkDiskSize], "Second disk should have size 50") } -// TestDiskOrderingVariousInterfaces tests deterministic ordering with various disk interfaces. +// TestDiskOrderingVariousInterfaces tests deterministic ordering with various disk interfaces, +// ensuring the order from currentDiskList is preserved. func TestDiskOrderingVariousInterfaces(t *testing.T) { t.Parallel() @@ -183,13 +184,13 @@ func TestDiskOrderingVariousInterfaces(t *testing.T) { "Disk ordering should be deterministic for various interfaces. Iteration %d differs", i) } - // Verify logical ordering: sata1, scsi0, virtio0, virtio2 + // Verify ordering preserves currentDiskList order: virtio2, scsi0, sata1, virtio0 require.Len(t, expectedResult, 4) - expectedOrder := []string{"sata1", "scsi0", "virtio0", "virtio2"} + expectedOrder := []string{"virtio2", "scsi0", "sata1", "virtio0"} for i, expectedInterface := range expectedOrder { disk := expectedResult[i].(map[string]interface{}) require.Equal(t, expectedInterface, disk[mkDiskInterface], - "Disk at position %d should be %s", i, expectedInterface) + "Disk at position %d should be %s (as in currentDiskList)", i, expectedInterface) } }