diff --git a/.vscode/extensions.json b/.vscode/extensions.json index 1dcb293b..2f4ff167 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -1,8 +1,11 @@ { "recommendations": [ + "britesnow.vscode-toggle-quotes", "davidanson.vscode-markdownlint", + "EditorConfig.editorconfig", "golang.go", "hashicorp.terraform", "joshbolduc.commitlint", + "PKief.material-icon-theme", ] } diff --git a/.vscode/settings.json b/.vscode/settings.json index 6cc92b9e..adf2923f 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -95,4 +95,19 @@ ], "go.lintOnSave": "workspace", "go.testEnvFile": "${workspaceFolder}/testacc.env", + "material-icon-theme.folders.associations": { + ".github/workflows": "robot", + "fwprovider": "middleware", + "proxmox": "api", + "proxmoxtf": "container", + "structure": "utils", + "access": "admin", + "firewall": "secure", + "nodes": "server", + "storage": "database", + "pools": "batch", + "ssh": "connection", + "version": "include", + "datasource": "database", + }, } diff --git a/fwprovider/tests/resource_vm_test.go b/fwprovider/tests/resource_vm_test.go index 1f26ccb9..b377bfd0 100644 --- a/fwprovider/tests/resource_vm_test.go +++ b/fwprovider/tests/resource_vm_test.go @@ -573,6 +573,42 @@ func TestAccResourceVMDisks(t *testing.T) { RefreshState: true, }, }}, + {"multiple disks", []resource.TestStep{ + { + Config: te.renderConfig(` + resource "proxmox_virtual_environment_vm" "test_disk4" { + node_name = "{{.NodeName}}" + started = false + name = "test-disk4" + template = "true" + + disk { + file_format = "raw" + datastore_id = "local-lvm" + interface = "virtio0" + size = 8 + } + disk { + file_format = "raw" + datastore_id = "local-lvm" + interface = "scsi0" + size = 8 + } + }`), + 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`, + }), + }, + { + RefreshState: true, + }, + }}, {"clone disk with overrides", []resource.TestStep{ { SkipFunc: func() (bool, error) { @@ -610,18 +646,16 @@ func TestAccResourceVMDisks(t *testing.T) { //size = 10 } }`), - Check: resource.ComposeTestCheckFunc( - testResourceAttributes("proxmox_virtual_environment_vm.test_disk3", map[string]string{ - "disk.0.datastore_id": "local-lvm", - "disk.0.discard": "on", - "disk.0.file_format": "raw", - "disk.0.interface": "scsi0", - "disk.0.iothread": "true", - "disk.0.path_in_datastore": `vm-\d+-disk-\d+`, - "disk.0.size": "8", - "disk.0.ssd": "true", - }), - ), + Check: testResourceAttributes("proxmox_virtual_environment_vm.test_disk3", map[string]string{ + "disk.0.datastore_id": "local-lvm", + "disk.0.discard": "on", + "disk.0.file_format": "raw", + "disk.0.interface": "scsi0", + "disk.0.iothread": "true", + "disk.0.path_in_datastore": `vm-\d+-disk-\d+`, + "disk.0.size": "8", + "disk.0.ssd": "true", + }), }, { RefreshState: true, diff --git a/proxmox/nodes/storage/download_url.go b/proxmox/nodes/storage/download_url.go index ff44c9a7..5266edba 100644 --- a/proxmox/nodes/storage/download_url.go +++ b/proxmox/nodes/storage/download_url.go @@ -34,7 +34,7 @@ func (c *Client) DownloadFileByURL( taskErr := c.Tasks().WaitForTask(ctx, *resBody.TaskID, int(uploadTimeout), 5) if taskErr != nil { err = fmt.Errorf( - "error download file to datastore %s: failed waiting for url download - %w", + "error download file to datastore %s: failed waiting for url download: %w", c.StorageName, taskErr, ) diff --git a/proxmoxtf/resource/vm/disk/disk.go b/proxmoxtf/resource/vm/disk/disk.go index 7b64c174..a9913e04 100644 --- a/proxmoxtf/resource/vm/disk/disk.go +++ b/proxmoxtf/resource/vm/disk/disk.go @@ -12,6 +12,7 @@ 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" @@ -24,18 +25,6 @@ import ( // GetInfo returns the disk information for a VM. func GetInfo(resp *vms.GetResponseData, d *schema.ResourceData) vms.CustomStorageDevices { - currentDisk := d.Get(MkDisk) - - currentDiskList := currentDisk.([]interface{}) - currentDiskMap := map[string]map[string]interface{}{} - - for _, v := range currentDiskList { - diskMap := v.(map[string]interface{}) - diskInterface := diskMap[mkDiskInterface].(string) - - currentDiskMap[diskInterface] = diskMap - } - storageDevices := vms.CustomStorageDevices{} storageDevices["ide0"] = resp.IDEDevice0 @@ -82,11 +71,14 @@ func GetInfo(resp *vms.GetResponseData, d *schema.ResourceData) vms.CustomStorag storageDevices["virtio14"] = resp.VirtualIODevice14 storageDevices["virtio15"] = resp.VirtualIODevice15 + currentDisk := d.Get(MkDisk) + + diskMap := utils.MapResourceList(currentDisk.([]interface{}), mkDiskInterface) + for k, v := range storageDevices { - if v != nil { - if currentDiskMap[k] != nil { - if currentDiskMap[k][mkDiskFileID] != nil { - fileID := currentDiskMap[k][mkDiskFileID].(string) + if v != nil && diskMap[k] != nil { + if d, ok := diskMap[k].(map[string]interface{}); ok { + if fileID, ok := d[mkDiskFileID].(string); ok && fileID != "" { v.FileID = &fileID } } @@ -749,8 +741,17 @@ func Read( } if !isClone || len(currentDiskList) > 0 { - orderedDiskList := utils.OrderedListFromMap(diskMap) - err := d.Set(MkDisk, orderedDiskList) + var diskList []interface{} + + if len(currentDiskList) > 0 { + resMap := utils.MapResourceList(currentDiskList, mkDiskInterface) + interfaces := maps.Keys[map[string]interface{}](resMap) + diskList = utils.OrderedListFromMapByKeyValues(diskMap, interfaces) + } else { + diskList = utils.OrderedListFromMap(diskMap) + } + + err := d.Set(MkDisk, diskList) diags = append(diags, diag.FromErr(err)...) } diff --git a/proxmoxtf/resource/vm/disk/schema.go b/proxmoxtf/resource/vm/disk/schema.go index 12c1c858..f7730af2 100644 --- a/proxmoxtf/resource/vm/disk/schema.go +++ b/proxmoxtf/resource/vm/disk/schema.go @@ -5,6 +5,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/bpg/terraform-provider-proxmox/proxmoxtf/resource/validators" + "github.com/bpg/terraform-provider-proxmox/proxmoxtf/structure" ) const ( @@ -71,6 +72,10 @@ func Schema() map[string]*schema.Schema { }, }, nil }, + DiffSuppressFunc: structure.SuppressIfListsOfMapsAreEqualIgnoringOrderByKey( + mkDiskInterface, mkDiskPathInDatastore, + ), + DiffSuppressOnRefresh: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ mkDiskInterface: { diff --git a/proxmoxtf/resource/vm/vm.go b/proxmoxtf/resource/vm/vm.go index 7cfe194b..66f70dc1 100644 --- a/proxmoxtf/resource/vm/vm.go +++ b/proxmoxtf/resource/vm/vm.go @@ -3723,7 +3723,7 @@ func vmReadCustom( diags = append(diags, diag.FromErr(err)...) } - allDiskInfo := disk.GetInfo(vmConfig, d) // from the cloned VM + allDiskInfo := disk.GetInfo(vmConfig, d) diags = append(diags, disk.Read(ctx, d, allDiskInfo, vmID, api, nodeName, len(clone) > 0)...) diff --git a/proxmoxtf/structure/schema.go b/proxmoxtf/structure/schema.go index 16f5b4e3..182fd8ee 100644 --- a/proxmoxtf/structure/schema.go +++ b/proxmoxtf/structure/schema.go @@ -12,8 +12,9 @@ import ( "sort" "strings" - "github.com/bpg/terraform-provider-proxmox/utils" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + + "github.com/bpg/terraform-provider-proxmox/utils" ) // MergeSchema merges the map[string]*schema.Schema from src into dst. @@ -127,9 +128,15 @@ func SuppressIfListsAreEqualIgnoringOrder(key, _, _ string, d *schema.ResourceDa // elements. // It will be called for each resource attribute, so it is not super efficient. It is // recommended to use it only for small lists / small resources. -// Note: The order of the attributes within the resource is still important. +// The keyAttr is the attribute that will be used as the key to compare the resources. +// The ignoreKeys are the keys that will be ignored when comparing the resources. Include computed read-only +// attributes here. +// // Ref: https://github.com/hashicorp/terraform-plugin-sdk/issues/477 -func SuppressIfListsOfMapsAreEqualIgnoringOrderByKey(keyAttr string) schema.SchemaDiffSuppressFunc { +func SuppressIfListsOfMapsAreEqualIgnoringOrderByKey( + keyAttr string, + ignoreKeys ...string, +) schema.SchemaDiffSuppressFunc { // the attr is a path to the item's attribute, not the list itself, e.g. "numa.0.device" return func(attr, _, _ string, d *schema.ResourceData) bool { lastDotIndex := strings.LastIndex(attr, ".") @@ -147,22 +154,34 @@ func SuppressIfListsOfMapsAreEqualIgnoringOrderByKey(keyAttr string) schema.Sche return false } - oldArray := oldData.([]interface{}) - newArray := newData.([]interface{}) + oldArray, ok := oldData.([]interface{}) + if !ok { + return false + } + + newArray, ok := newData.([]interface{}) + if !ok { + return false + } if len(oldArray) != len(newArray) { return false } - oldKeys := utils.MapResourceList(oldArray, keyAttr) - newKeys := utils.MapResourceList(newArray, keyAttr) + oldMap := utils.MapResourceList(oldArray, keyAttr) + newMap := utils.MapResourceList(newArray, keyAttr) - for k, v := range oldKeys { - if _, ok := newKeys[k]; !ok { + for k, v := range oldMap { + if _, ok := newMap[k]; !ok { return false } - if !reflect.DeepEqual(v, newKeys[k]) { + for _, ignoreKey := range ignoreKeys { + delete(v.(map[string]interface{}), ignoreKey) + delete(newMap[k].(map[string]interface{}), ignoreKey) + } + + if !reflect.DeepEqual(v, newMap[k]) { return false } } diff --git a/utils/maps.go b/utils/maps.go index f902b28e..d3fcbfe5 100644 --- a/utils/maps.go +++ b/utils/maps.go @@ -32,8 +32,9 @@ func MapResourceList(resourceList []interface{}, attrName string) map[string]int } r := resource.(map[string]interface{}) - key := r[attrName].(string) - m[key] = r + if key, ok := r[attrName].(string); ok { + m[key] = r + } } return m