From 3180f81b4a14868bb0076ec8ab6299b3f6945510 Mon Sep 17 00:00:00 2001 From: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Date: Tue, 14 May 2024 22:31:41 -0400 Subject: [PATCH] fix(vm): yet another fix for disk reordering (#1297) Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> --- fwprovider/tests/resource_vm_test.go | 11 ++++------ proxmoxtf/resource/vm/disk/disk.go | 6 ++---- proxmoxtf/resource/vm/vm.go | 5 +---- proxmoxtf/structure/schema.go | 4 ++-- utils/maps.go | 31 +++++++++++++++++++++------- utils/maps_test.go | 16 +++++++------- 6 files changed, 42 insertions(+), 31 deletions(-) diff --git a/fwprovider/tests/resource_vm_test.go b/fwprovider/tests/resource_vm_test.go index b4e9f502..c90ce7c3 100644 --- a/fwprovider/tests/resource_vm_test.go +++ b/fwprovider/tests/resource_vm_test.go @@ -580,13 +580,10 @@ func TestAccResourceVMDisks(t *testing.T) { } }`), Check: testResourceAttributes("proxmox_virtual_environment_vm.test_disk4", map[string]string{ - // I'd love to test this, but the order of the list items is not guaranteed, apparently - // resource_vm_test.go:669: Step 1/2 error: Check failed: proxmox_virtual_environment_vm.test_disk4: Attribute "disk.1.interface" value: expected 'virtio0' to match 'scsi0' - // --- FAIL: TestAccResourceVMDisks/multiple_disks (5.24s) - // "disk.1.interface": "scsi0", - // "disk.1.path_in_datastore": `vm-\d+-disk-0`, - // "disk.0.interface": "virtio0", - // "disk.0.path_in_datastore": `vm-\d+-disk-1`, + "disk.0.interface": "virtio0", + "disk.0.path_in_datastore": `vm-\d+-disk-1`, + "disk.1.interface": "scsi0", + "disk.1.path_in_datastore": `vm-\d+-disk-0`, }), }, { diff --git a/proxmoxtf/resource/vm/disk/disk.go b/proxmoxtf/resource/vm/disk/disk.go index 09107e22..0cf7086f 100644 --- a/proxmoxtf/resource/vm/disk/disk.go +++ b/proxmoxtf/resource/vm/disk/disk.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "golang.org/x/exp/maps" "github.com/bpg/terraform-provider-proxmox/proxmox" "github.com/bpg/terraform-provider-proxmox/proxmox/nodes/vms" @@ -73,7 +72,7 @@ func GetInfo(resp *vms.GetResponseData, d *schema.ResourceData) vms.CustomStorag currentDisk := d.Get(MkDisk) - diskMap := utils.MapResourceList(currentDisk.([]interface{}), mkDiskInterface) + diskMap := utils.MapResourcesByAttribute(currentDisk.([]interface{}), mkDiskInterface) for k, v := range storageDevices { if v != nil && diskMap[k] != nil { @@ -624,8 +623,7 @@ func Read( var diskList []interface{} if len(currentDiskList) > 0 { - resMap := utils.MapResourceList(currentDiskList, mkDiskInterface) - interfaces := maps.Keys[map[string]interface{}](resMap) + interfaces := utils.ListResourcesAttributeValue(currentDiskList, mkDiskInterface) diskList = utils.OrderedListFromMapByKeyValues(diskMap, interfaces) } else { diskList = utils.OrderedListFromMap(diskMap) diff --git a/proxmoxtf/resource/vm/vm.go b/proxmoxtf/resource/vm/vm.go index bebf2c5f..c3369fd4 100644 --- a/proxmoxtf/resource/vm/vm.go +++ b/proxmoxtf/resource/vm/vm.go @@ -17,8 +17,6 @@ import ( "strings" "time" - "golang.org/x/exp/maps" - "github.com/google/go-cmp/cmp" "github.com/google/uuid" "github.com/hashicorp/terraform-plugin-log/tflog" @@ -3664,8 +3662,7 @@ func vmReadCustom( var numaList []interface{} if len(currentNUMAList) > 0 { - resMap := utils.MapResourceList(currentNUMAList, mkNUMADevice) - devices := maps.Keys[map[string]interface{}](resMap) + devices := utils.ListResourcesAttributeValue(currentNUMAList, mkNUMADevice) numaList = utils.OrderedListFromMapByKeyValues(numaMap, devices) } else { numaList = utils.OrderedListFromMap(numaMap) diff --git a/proxmoxtf/structure/schema.go b/proxmoxtf/structure/schema.go index 182fd8ee..76d6f294 100644 --- a/proxmoxtf/structure/schema.go +++ b/proxmoxtf/structure/schema.go @@ -168,8 +168,8 @@ func SuppressIfListsOfMapsAreEqualIgnoringOrderByKey( return false } - oldMap := utils.MapResourceList(oldArray, keyAttr) - newMap := utils.MapResourceList(newArray, keyAttr) + oldMap := utils.MapResourcesByAttribute(oldArray, keyAttr) + newMap := utils.MapResourcesByAttribute(newArray, keyAttr) for k, v := range oldMap { if _, ok := newMap[k]; !ok { diff --git a/utils/maps.go b/utils/maps.go index d3fcbfe5..ba17b9df 100644 --- a/utils/maps.go +++ b/utils/maps.go @@ -18,12 +18,29 @@ func OrderedListFromMap(inputMap map[string]interface{}) []interface{} { return OrderedListFromMapByKeyValues(inputMap, keyList) } -// MapResourceList generates a list of strings from a Terraform resource list (list of maps). -// The list is generated from the value of the specified attribute. -// -// "Map" in this context is a functional programming term, not a Go map. -// "Resource" in this context is a Terraform resource, i.e. a map of attributes. -func MapResourceList(resourceList []interface{}, attrName string) map[string]interface{} { +// ListResourcesAttributeValue generates a list of strings from a Terraform resource list (which is list of maps). +// The list is generated by extracting a specific key attribute from each resource. If the attribute is not found in a +// resource, it is skipped. +func ListResourcesAttributeValue(resourceList []interface{}, keyAttr string) []string { + var l []string + + for _, resource := range resourceList { + if resource == nil { + continue + } + + r := resource.(map[string]interface{}) + if value, ok := r[keyAttr].(string); ok { + l = append(l, value) + } + } + + return l +} + +// MapResourcesByAttribute generates a map of resources from a resource list, using a specified attribute as the key +// and the resource as the value. If the attribute is not found in a resource, it is skipped. +func MapResourcesByAttribute(resourceList []interface{}, keyAttr string) map[string]interface{} { m := make(map[string]interface{}, len(resourceList)) for _, resource := range resourceList { @@ -32,7 +49,7 @@ func MapResourceList(resourceList []interface{}, attrName string) map[string]int } r := resource.(map[string]interface{}) - if key, ok := r[attrName].(string); ok { + if key, ok := r[keyAttr].(string); ok { m[key] = r } } diff --git a/utils/maps_test.go b/utils/maps_test.go index 13d17833..399681b7 100644 --- a/utils/maps_test.go +++ b/utils/maps_test.go @@ -23,7 +23,7 @@ func TestOrderedListFromMap(t *testing.T) { result := OrderedListFromMap(inputMap) if !reflect.DeepEqual(result, expected) { - t.Errorf("MapResourceList() = %v, want %v", result, expected) + t.Errorf("ListResourcesAttributeValue() = %v, want %v", result, expected) } } @@ -35,18 +35,20 @@ func TestMapResourceList(t *testing.T) { map[string]interface{}{"name": "resource2", "attr": "value2"}, nil, map[string]interface{}{"name": "resource3", "attr": "value3"}, + map[string]interface{}{"name": "resource4", "attr": "value4"}, } - expected := map[string]interface{}{ - "value1": map[string]interface{}{"name": "resource1", "attr": "value1"}, - "value2": map[string]interface{}{"name": "resource2", "attr": "value2"}, - "value3": map[string]interface{}{"name": "resource3", "attr": "value3"}, + expected := []string{ + "value1", + "value2", + "value3", + "value4", } - result := MapResourceList(resourceList, "attr") + result := ListResourcesAttributeValue(resourceList, "attr") if !reflect.DeepEqual(result, expected) { - t.Errorf("MapResourceList() = %v, want %v", result, expected) + t.Errorf("ListResourcesAttributeValue() = %v, want %v", result, expected) } }