From fa837837e56296b00b7593f6374c4c5641771169 Mon Sep 17 00:00:00 2001 From: Vasil Atanasov Date: Tue, 30 Jan 2024 15:18:48 +0200 Subject: [PATCH] fix: Differences between creation and import state There are differences between the state in the r/vsphere_virtual_machine when it is imported and when it is created/cloned. Fixed several of them which maked sense for me. - datastore_cluster_id - adding the property to the state when the VM is imported in order to keep the pervious behaviour. - disk.label / disk.keep_on_remove - the logic was to assign a computed label in format "disk" instead of the actual label added by the system and in such cases the disk was marked as keep_on_remove. Changed this to set the actual label and not setting the keep_on_remove property (defaulting to folse). If the label is not present - keeping the old logic. - Not sure how to procede with sata_controller_count - looks like this should be a computed proprty, but this is a breaking change which I would avoid introducung. added sata_controller_count attribute to ignore state attributes since e2e tests are failing localy Fixes #1706 Signed-off-by: Vasil Atanasov --- .../virtual_machine_disk_subresource.go | 19 ++++++++++++++---- vsphere/resource_vsphere_virtual_machine.go | 20 +++++++++++++++++++ .../resource_vsphere_virtual_machine_test.go | 1 + 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go b/vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go index b36048999..3054dc909 100644 --- a/vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go +++ b/vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go @@ -1234,14 +1234,25 @@ func DiskImportOperation(d *schema.ResourceData, l object.VirtualDeviceList) err // the device address. m["key"] = (i + 1) * -1 m["device_address"] = addr - // Assign a computed label. This label *needs* be the label this disk is - // assigned in config, or you risk service interruptions or data corruption. - m["label"] = fmt.Sprintf("disk%d", i) + // Assign disk label coming from the configuration + // If there is no info about the label - assign computed label + deviceLabel := "" + deviceDescription := device.GetVirtualDevice().DeviceInfo.GetDescription() + if deviceDescription != nil { + deviceLabel = deviceDescription.Label + } + keepAfterRemove := false + if len(deviceLabel) > 0 { + m["label"] = deviceLabel + } else { + keepAfterRemove = true + m["label"] = fmt.Sprintf("disk%d", i) + } // Set keep_on_remove to ensure that if labels are assigned incorrectly, // all that happens is that the disk is removed. The comments above // regarding the risk of incorrect label assignment are still true, but // this greatly reduces the risk of data loss. - m["keep_on_remove"] = true + m["keep_on_remove"] = keepAfterRemove curSet = append(curSet, m) } diff --git a/vsphere/resource_vsphere_virtual_machine.go b/vsphere/resource_vsphere_virtual_machine.go index 27571d988..0d600609d 100644 --- a/vsphere/resource_vsphere_virtual_machine.go +++ b/vsphere/resource_vsphere_virtual_machine.go @@ -493,6 +493,26 @@ func resourceVSphereVirtualMachineRead(d *schema.ResourceData, meta interface{}) _ = d.Set("datastore_id", ds.Reference().Value) _ = d.Set("vmx_path", dp.Path) + isImported := d.Get("imported").(bool) + if isImported { + dsProps, err := datastore.Properties(ds) + if err != nil { + return fmt.Errorf("could not read properties for datastore: %s", ds.Name()) + } + if dsProps.Parent.Type == "StoragePod" { + cluster, err := storagepod.FromID(client, dsProps.Parent.Value) + if err != nil { + return fmt.Errorf("could not read managed object reference (datastore cluster): %s", err) + } + + if cluster == nil { + return fmt.Errorf("could not read managed object reference (datastore cluster): %s", dsProps.Parent.Value) + } + + d.Set("datastore_cluster_id", cluster.Reference().Value) + } + + } // Read general VM config info if err := flattenVirtualMachineConfigInfo(d, vprops.Config, client); err != nil { return fmt.Errorf("error reading virtual machine configuration: %s", err) diff --git a/vsphere/resource_vsphere_virtual_machine_test.go b/vsphere/resource_vsphere_virtual_machine_test.go index 2614c115c..abac5ff48 100644 --- a/vsphere/resource_vsphere_virtual_machine_test.go +++ b/vsphere/resource_vsphere_virtual_machine_test.go @@ -2399,6 +2399,7 @@ func TestAccResourceVSphereVirtualMachine_cloneImport(t *testing.T) { "clone", "cdrom", "wait_for_guest_net_timeout", + "sata_controller_count", }, ImportStateIdFunc: func(s *terraform.State) (string, error) { vm, err := testGetVirtualMachine(s, "vm")