Skip to content

Commit

Permalink
fix: Differences between creation and import state (#2127)
Browse files Browse the repository at this point in the history
There are differences between the state in the r/vsphere_virtual_machine when it is imported and when it is created/cloned.

- 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<number of the diks>" 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.

Signed-off-by: Vasil Atanasov <[email protected]>
  • Loading branch information
vasilsatanasov authored Feb 12, 2024
1 parent 4644a86 commit 609d0ae
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 4 deletions.
19 changes: 15 additions & 4 deletions vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
20 changes: 20 additions & 0 deletions vsphere/resource_vsphere_virtual_machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions vsphere/resource_vsphere_virtual_machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit 609d0ae

Please sign in to comment.