r/virtual_machine: Network fixes #181
Conversation
Along with 1f4c85f, this corrects a couple of issues and adds tests for one that we can test for: Fixes #165, which was an issue where network searches in govmomi were not working for networks with unescaped slashes in their names. We also have a test for this now as well where we reproduce by adding a host-level portgroup, which the API does not escape slashes for. Fixes #161, which was an issue with network searches where panics could happen on trying to load a network backed by a 3rd party hardware DVS. The latest govmomi has commits that properly use base types for the respective items.
vancluever
added
bug
| os.Getenv("VSPHERE_IPV4_GATEWAY"), | ||
| os.Getenv("VSPHERE_DATASTORE"), | ||
| os.Getenv("VSPHERE_TEMPLATE"), | ||
| os.Getenv("VSPHERE_USE_LINKED_CLONE"), |
There was a problem hiding this comment.
minor/not a blocker feels like this lot would make a nice test helper object
There was a problem hiding this comment.
Yeah, things are getting a little ridiculous here 😉
Definitely could be something we could assemble into an object, although I'm a little resistant to the idea of breaking the variable configuration off, which is kind of what this would do, as right now all config fixtures are pretty easy to read and look like real Terraform config rather than chunks, which was a problem with this test code before (in a different way, though, one that made a lot less sense than what would be accomplished with a proper test fixture generator. 🙂 )
| iops = 500 | ||
| } | ||
|
|
||
| depends_on = ["vsphere_host_port_group.pg"] |
There was a problem hiding this comment.
I'd have thought this should be implied by network_interface.0.label being set to "${vsphere_host_port_group.pg.0.name}"?
There was a problem hiding this comment.
We wait on all resources here because while we are only referencing the first resource for the network name, the VM could be created on any of the three hosts, hence it's important to ensure that the network is available on all three before proceeding with creation of the VM.
|
Thanks for the review @tombuildsstuff! Merging now |
ghost
locked and limited conversation to collaborators
An update of govmomi corrects a couple of recent issues with handling networks in
vsphere_virtual_machine:Fixes #165, which was an issue where network searches in govmomi were
not working for networks with unescaped slashes in their names. We also
have a test for this now as well where we reproduce by adding a
host-level portgroup, which the API does not escape slashes for.
Fixes #161, which was an issue with network searches where panics could
happen on trying to load a network backed by a 3rd party hardware DVS.
The latest govmomi has commits that properly use base types for the
respective items.