vsphere virtual machine resource resolves multiple networks

[kzittritsch]

I am trying to use the vsphere virtual machine resource but am running into an issue where vsphere is returning multiple networks due to (I think) a * being prefixed to the network label in the provider:
https://github.com/hashicorp/terraform/blob/master/builtin/providers/vsphere/resource_vsphere_virtual_machine.go#L657

// buildNetworkDevice builds VirtualDeviceConfigSpec for Network Device.
func buildNetworkDevice(f *find.Finder, label, adapterType string) (*types.VirtualDeviceConfigSpec, error) {
    network, err := f.Network(context.TODO(), "*"+label)
    if err != nil {
        return nil, err
    }

The terraform error message:

* vsphere_virtual_machine.app_server: path '*QA' resolves to multiple networks

My network_interface configuration inside my virtual machine resource config:

  network_interface {
    label = "QA"
    ipv4_address = "10.37.2.68"
    ipv4_prefix_length = "20"
  }

Using govc I can see how this wildcard resolution is causing a conflict because I have two networks that end in "QA":

ddckevinz@mbp-kevinz terraform $ govc ls /VT/network/ | grep QA
/VT/network/ERPQA
/VT/network/QA

It does not appear that I can provide a full path to the network in the label field (ie supplying label = "/VT/network/QA" results in an error cannot traverse type Network). I threw together a small test to confirm that dropping the * allows the Finder.Network() method to resolve each network individually.

In the current resource, short of renaming my networks in vsphere, is there a way to create a virtual machine with my "QA" network? I am using terraform 0.6.11 and vsphere 5.5 btw.

[chrislovecnm]

Great find btw, and thanks for the time that you took on this issue ... so this smells like API level fun.

I am thinking that we need to test removing the * out of the code, but then you have to fully reference the network name ... Kinda a catch 22.

Let us ping the guys on govomi: Oooh @pietern @dougm @cblomart is there a better way to design this? Let me know if your team wants me to open a issue on govomi.

And @tkak any ideas?

@kzittritsch are you able to help with this??? You are running vSphere 5.5 with what setup (Cluster, etc)? Can you test this with master please for ease of fixing this. What version of govomi are you running?

[xantheran]

I am happy to help test this issue. Give me a couple days to generate some results

[chrislovecnm]

@dkalleg, @kristinn, @tkak we have an interesting feature in our code :) Two main things are being reported:

First code does not support full path notation for a network device. For example '

  network_interface {
    label = "Hosted_Network/VL74_dvPortGroup"
    ipv4_address = "10.X.Y.Y"
    ipv4_prefix_length = "24"
  }

#5843

Secondly our code's search method allows for wildcard search results to be found. You have QA1 an QA2 networks, and you should get this codes results.

@dkalleg, @kristinn, @tkak you guys have any design opinions on how to proceed? Maybe:

  network_interface {
    full_path_label = "Hosted_Network/VL74_dvPortGroup"
    ipv4_address = "10.X.Y.Y"
    ipv4_prefix_length = "24"
  }

I the name full_path_label kinda sucks, and we would not have a breaking fix ...

[chrislovecnm]

@xantheran and @kzittritsch dragged a couple of other active developers in to see if we can come up with a better solution that I am thinkin of ;)

[xantheran]

I am able to test for vsphere 5.5 and 6 in clustered environments as well.

[kristinn]

It would be best to keep the label key and intelligently resolve it to the correct network/vlan.

[xantheran]

I would be happy to help test your branch @chrislovecnm email me the details and we can go from there

[kristinn]

@chrislovecnm Is this solved in the branch you referred to?

[chrislovecnm]

@kristinn no ... that is just ipWait stuff. This is an open issue . I need to keep the correct comments on the correct issue ... <facepalm>

[chrislovecnm]

@markpeek and @frapposelli - this is the open issue that I mentioned. The manner in which we are doing the network lookups uses a wild card. I would like some idea on how to do this w/o a breaking change. Here are some thoughts

1. Force full paths - breaking change (do not like)
2. Allow either full paths or do lookup - non breaking, but more code, and more complexity for the user
3. Allow wildcards in the search - not sure if we can even do that

I do like the second options, but we would need to document the heck out of it.

@dkalleg @tkak @kristinn @stack72 @jen20 any ideas / feedback would be appreciated!

[kzittritsch]

Thanks everyone for taking a look at this. Why not just remove the "forced" wildcard prefix and, if a user would like to wildcard searching, they can prefix/append it on their label param in their .tf file? The provider should not force a wildcard prefix. However this would break some users who are dependent on the wildcard.

[chrislovecnm]

@markpeek and @frapposelli are you guys going to take a stab at this one?

[sbrinkerhoff]

I am running into the same issue @kzittritsch seems to be having. Is there a plan to remove this wildcard from the provider?

[sizgiyaev]

Is there any solution for the issue?

[chrislovecnm]

It is an open issue that requires a coding change.

[simone84]

Hi @chrislovecnm ,

Do you have any update?

Cheers
Simone

[ddcchloel]

I am by no means a Golang expert, or a govmomi expert for that matter, but I propose something like this:

--- a/builtin/providers/vsphere/resource_vsphere_virtual_machine.go
+++ b/builtin/providers/vsphere/resource_vsphere_virtual_machine.go
@@ -1465,6 +1465,9 @@ func addCdrom(client *govmomi.Client, vm *object.VirtualMachine, datacenter *obj
 // buildNetworkDevice builds VirtualDeviceConfigSpec for Network Device.
 func buildNetworkDevice(f *find.Finder, label, adapterType string, macAddress string) (*types.VirtualDeviceConfigSpec, error) {
        network, err := f.Network(context.TODO(), "*"+label)
+       if _, ok := err.(*find.MultipleFoundError); ok {
+               network, err = f.Network(context.TODO(), label)
+       }
        if err != nil {
                return nil, err
        }

This will retry after stripping the leading * but only in those cases where it would otherwise error out on MultipleFoundError. Only users who are currently erroring out would ever traverse this code path, and existing functional tf code would continue to function exactly the same way. (Assuming of course that I have the right error type.)

I would submit this myself as a pull request but I feel these cases ought to have good testing around them and I'm not sure where in the code base that happens.

[ddcchloel]

There seems to be an assumption that the prepended * is needed in order for a label like netname to match a network whose inventory path is subfolder/netname. I don't think that this is actually what this * does (Cf. #8584).

Govmomi find seems designed to work like unix ls, where * doesn't glob across /s. Prepending a * does allow people to incompletely specify the network label (etname or /netname), at the cost of failing when the glob matches more than one network. It would be interesting to test what the actual behavior is for all these cases. Either way, retrying without the * when the * matches more than one thing (or nothing) should preserve existing workflows.

Anyway, in govmomi 0.13 it seems that the semantics of find have been expanded to do a recursive search for netname as long as the path string argument doesn't contain a /. This suggests that specifying netname would/should be enough to locate subfolder/netname. It is not clear to me whether updating the provider to use 0.13 or 0.14 is in the cards any time soon.

Since datacenters define a namespace for objects, I think we should aim to refer to the object name netname and not the inventory path subfolder/netname when identifying objects, including in the resource param, which suggests a path forward in resolving #5843.

Short of an update of govmomi, I propose a sequence of similar retries (e.g., *label --> label --> */label --> */*/label), maybe testing for / characters, as a short-term workaround to approximate the future govmomi 0.13 behavior, and deprecate using inventory paths as a resource param in favor of the object name.

[joshmullis]

I was able to work around this using a partial path gathered from govc ls

Network-Non-Production/BU_FOLDER/ClusterName/MY_NETWORK_LABEL

[rismoney]

i am not sure how to work around this issue. If I use path from govc ls, I get cannot traverse type Network.

[joshmullis]

@rismoney did you leave off the / at the beginning of the path?

[rismoney]

Thanks for the tip @joshmullis

In toying more with govc, I ultimately realized why i was getting issue. my network paths are not totally unique-
I have VM Network 1.1.1.x and also vds_VM Network 1.1.1.x for example, so with the wildcard issue noted above, both were returning against VM Network 1.1.1.x

My workaround is going to be a rename of the port groups (as I am not holding my breath on a year old issue)

[ghost]

I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.