Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix VMDK upload failure on ESXi host using vspshere_file resource. #1409

Merged
merged 5 commits into from
Feb 9, 2022

Conversation

sumitmaggo
Copy link
Contributor

@sumitmaggo sumitmaggo commented May 15, 2021

Description

Fixes issues where vsphere_file resource fails to upload VMDK file to ESXi host datastore.

VMDK disk upload has been tested with Terraform 0.13 version on ESXi.

Release Note

vsphere_file resource: Fix upload of VMDK to datastore on ESXi host. [GH-1409]

References

Closes #1296
Closes #1243

@sumitmaggo
Copy link
Contributor Author

@bill-rich @koikonom @aareet @sumitAgrawal007 Can you guys please look into this PR and provide your comments? Thanks in advance.

@sumitmaggo
Copy link
Contributor Author

@bill-rich @koikonom @aareet @sumitAgrawal007 Can anyone please review this PR? Its been pending for too long and we are blocked because of it and not able to pick the latest vsphere provider version for ESXi.

@sumitmaggo
Copy link
Contributor Author

@bill-rich @koikonom Can you please help getting this PR reviewed and merged? We need to move to the latest version of vsphere provider and this issue is blocking that.

@sumitmaggo
Copy link
Contributor Author

@redeux Can you please check and review if this fix can also be added as a candidate for next release? This is also needed in addition to #1408 PR changes to move to the latest version of vsphere provider for ESXi deployment. Thanks in advance.

@sumitmaggo
Copy link
Contributor Author

@appilon Can you please review and merge this PR as well? This is also needed in addition to #1408 to allow launching VM on ESXi server.

@tenthirtyam
Copy link
Collaborator

tenthirtyam commented Jan 13, 2022

@sumitmaggo Can you update the description with "Closes #1296' to link the pull request to your issue using the keyword?

Ryan

@sumitmaggo
Copy link
Contributor Author

Thanks @tenthirtyam, updated the description to link it with the issue.

@tenthirtyam tenthirtyam added the bug Type: Bug label Feb 3, 2022
@tenthirtyam tenthirtyam changed the title Address VMDK upload failure on ESXi host using vspshere_file resource. Fix VMDK upload failure on ESXi host using vspshere_file resource. Feb 3, 2022
@tenthirtyam tenthirtyam requested a review from appilon February 3, 2022 21:49
@tenthirtyam tenthirtyam added the needs-review Status: Pull Request Needs Review label Feb 8, 2022
@tenthirtyam tenthirtyam self-assigned this Feb 8, 2022
@tenthirtyam
Copy link
Collaborator

@sumitmaggo - I found the issue and will submit a fix to your branch soon.

A destroy would complete as a false positive when deleting a .`vmdk` and would leave the file since the `DeleteDatastoreFile_Task `could not delete a VMDK.

This change updates the methods used in the `deleteFile` function.
- If the source file is a `.vmdk`, the Delete method uses the correct `DeleteVirtualDisk_Task`.
- If the source file is not a `.vmdk`, the Delete method uses the correct `DeleteDatastoreFile_Task`.

Signed-off-by: Ryan Johnson <[email protected]>
@github-actions github-actions bot added provider Type: Provider size/s Relative Sizing: Small and removed size/xs Relative Sizing: Extra-Small labels Feb 9, 2022
@tenthirtyam
Copy link
Collaborator

@appilon - I've updated @sumitmaggo's pull request to address a failure seen after a successful upload.

A destroy would complete as a false positive when deleting a .vmdk and would leave the file since the DeleteDatastoreFile_Task could not delete a VMDK.

This change updates the methods used in the deleteFile function.

  • If the source file is a .vmdk, the Delete method uses the correct DeleteVirtualDisk_Task.
  • If the source file is not a .vmdk, the Delete method uses the correct DeleteDatastoreFile_Task.

func deleteFile(client *govmomi.Client, f *file) error {
dc, err := getDatacenter(client, f.datacenter)
if err != nil {
return err
}
finder := find.NewFinder(client.Client, true)
finder = finder.SetDatacenter(dc)
ds, err := getDatastore(finder, f.datastore)
if err != nil {
return fmt.Errorf("error %s", err)
}
// If the source file is a VMDK, the Delete method uses the correct DeleteVirtualDisk_Task
if path.Ext(f.destinationFile) == ".vmdk" {
vdm := object.NewVirtualDiskManager(client.Client)
task, err := vdm.DeleteVirtualDisk(context.TODO(), ds.Path(f.destinationFile), dc)
if err != nil {
return err
}
_, err = task.WaitForResult(context.TODO(), nil)
if err != nil {
return err
}
return nil
} else {
// If the source file is not a VMDK, the Delete method uses the correct DeleteDatastoreFile_Task
fm := object.NewFileManager(client.Client)
task, err := fm.DeleteDatastoreFile(context.TODO(), ds.Path(f.destinationFile), dc)
if err != nil {
return err
}
_, err = task.WaitForResult(context.TODO(), nil)
if err != nil {
return err
}
return nil
}
}

The same method is used in the vsphere_virtual_disk resource:

diskPath := ds.Path(vDisk.vmdkPath)
virtualDiskManager := object.NewVirtualDiskManager(client.Client)
task, err := virtualDiskManager.DeleteVirtualDisk(context.TODO(), diskPath, dc)
if err != nil {
return err
}

I've successfully tested these changes on vSphere v7.0.3(c) and v6.7.2, testing the following:

✅ Create vsphere_file resource for a .vmdk; check exists.
✅ Destroy a vsphere_file resource for a .vmdk; check does not exist.
✅ Create vsphere_file resource for a .iso; check exists.
✅ Destroy a vsphere_file resource for a .iso; check does not exist.

Ryan

@appilon appilon merged commit d4b948a into hashicorp:master Feb 9, 2022
@sumitmaggo
Copy link
Contributor Author

Thanks @appilon and @tenthirtyam for the review and merge of this PR.
When can we expect the next vsphere plugin 2.0.3 release?

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 13, 2022
@tenthirtyam tenthirtyam removed the needs-review Status: Pull Request Needs Review label Mar 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Type: Bug provider Type: Provider size/s Relative Sizing: Small
Projects
None yet
3 participants