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

Adds support for building with the VMWare Workstation Pro API #161

Draft
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

niwamo
Copy link

@niwamo niwamo commented Nov 16, 2023

As described in #157:

Creates a new driver for vmrest, the API that ships with VMWare Workstation Pro. Due to limitations in the API, only the vmware-vmx builder can be implemented.

Checks have been added to the vmware-iso builder to throw an explanatory error if a user mistakenly tries to use the vmrest remote_type. Additional checks have been added to throw errors if incompatible configurations are set while using the vmrest remote_type.

The vmware-vmx documentation was also updated.

A test for the new driver has been provided in #157.

Closes #157

@niwamo niwamo requested a review from a team as a code owner November 16, 2023 18:56
@hashicorp-cla
Copy link

hashicorp-cla commented Nov 16, 2023

CLA assistant check
All committers have signed the CLA.

Updated expected error value in "Invalid remote type" and updated
expected default in "Set default values". Note: driver_config.go was
updated to only set exsi-relevant defaults is esx5 is selected
Fixes all issues in driver_vmrest.go identified by golangci-lint
Fixes the two-line discrepancy identified by CI
@niwamo
Copy link
Author

niwamo commented Nov 17, 2023

All issues identified by the first round of checks should be addressed. There were three categories of issues:

  1. Tests for driver_config.go that I failed to update, including an error message that was intentionally changed, and the adjustment to only set esxi-relevant defaults if esxi is selected; Both have been fixed
  2. A small number of linter-identified issues in driver_vmrest.go. All good catches; All now addressed.
  3. Exactly two lines with discrepancies between docs and web-docs; Now fixed

I've also re-run my live test of the vmrest builder to ensure changes did not affect functionality

### SSH key pair automation
### Building with the VMWare Workstation Pro API (vmrest)

VMWare Workstation Pro ships with a 'vmrest' executable enabling programmatic
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"VMware" for all occurrences of "VMWare", please.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review! I've corrected "VMWare" to "VMware" in all instances where I wrote the former.

(Note: I did not make the change for instances of "VMWare" that preexisted my contribution)

@niwamo
Copy link
Author

niwamo commented Nov 28, 2023

@tenthirtyam Since you were kind enough to review - do you know if there are additional steps I should be taking and/or reviewers I should be talking to? This has been slow to take off, and I want to make sure I'm doing my due diligence. The members of my team that need this functionality are already using a fork, but it would be nice to have this in the official plugin.

@nywilken
Copy link
Member

nywilken commented Dec 5, 2023

@tenthirtyam we will throw some time on your calendar to sync up on this change before starting to review.

@tenthirtyam tenthirtyam marked this pull request as draft May 9, 2024 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for the VMware Workstation Pro API with the vmware-vmx builder
4 participants