provider/vsphere: more reliable support for network interfaces

[pryorda]

This is to fix the issue with #7673 and #7154. Right now vmware's sdk / govmomi return the nics in reverse order.

[pryorda]

Checking on this?

[chrislovecnm]

@pryorda how can we test this?

[chrislovecnm]

@thetuxkeeper comments??

[pryorda]

@chrislovecnm Add more then one nic to a vmware resource. If you want a module example I can provide one.

[thetuxkeeper]

IMO it's better to not rely on the order of the array. But since my approach hit some walls and I ran out of time, this simpler fix looks good.
I haven't checked if the order is always right with this implementation, but with some test cases, it should at least fail reliably if something is not right.
Test cases where I think we should be quite sure if it works with this implementation:

1. create vm with multiple nics
2. create with no nic, add multiple nics
3. create with multiple (three or more) nics, remove one or less than all nic(s) "in the middle", add one or multiple nic(s) (or multiple add/remove iterations) - idea is trying to force some reordering operations and look if it still fits

[pryorda]

@thetuxkeeper Do you want me to test those? Also anychanges to the network interface are destructive iirc.

[thetuxkeeper]

I think those test cases would be good to see if we can really rely on the order returned by govmomi/vmware and also see when some changes in the future breaks it. At least that were my thoughts when I tried to come up with the scenarios

[pryorda]

I tested your test cases.

1. create vm with multiple nics

Works Perfect. I tested up to 10 nics.

1. create with no nic, add multiple nics

This forces destruction, so the order is correct after.

3 . create with multiple (three or more) nics, remove one or less than all nic(s) "in the middle", add one or multiple nic(s) (or multiple add/remove iterations) - idea is trying to force some reordering operations and look if it still fits -

This forces destruction, so the order is correct after.

            "network_interface": &schema.Schema{
                Type:     schema.TypeList,
                Required: true,
                ForceNew: true,

[sbourlon]

I have the same issue as well with Terraform v0.7.7.

@thetuxkeeper @pryorda Any update on that issue? Let me know if you need testers.

[sbourlon]

I merged this PR into my local master (9c3c5c1), then compiled a dev version and tried it but it didn't work:

Panic:

panic: runtime error: index out of range
2016/10/26 15:44:32 [DEBUG] plugin: terraform:
2016/10/26 15:44:32 [DEBUG] plugin: terraform: goroutine 132 [running]:
2016/10/26 15:44:32 [DEBUG] plugin: terraform: panic(0x25afa00, 0xc42000e110)
2016/10/26 15:44:32 [DEBUG] plugin: terraform:  /usr/local/Cellar/go/1.7.3/libexec/src/runtime/panic.go:500 +0x1a1
2016/10/26 15:44:32 [DEBUG] plugin: terraform: github.com/hashicorp/terraform/builtin/providers/vsphere.resourceVSphereVirtualMachineRead(0xc4204900c0, 0x2b13720, 0xc420163ed0, 0x1, 0x1)
2016/10/26 15:44:32 [DEBUG] plugin: terraform:  /Users/boust06/Documents/Go/src/github.com/hashicorp/terraform/builtin/providers/vsphere/resource_vsphere_virtual_machine.go:1069 +0x3601

I guess https://github.com/hashicorp/terraform/pull/7760/files#diff-9de67d85a10a7cfacde989a1e65c4089R1051 is trying to write in a non-existing index of normNetworkInterfaces.

Merge procedure:

git clone [email protected]:hashicorp/terraform.git
cd terraform
git pull origin pull/7760/head

Git reference:

git describe
v0.7.7-447-g4d42127

Compile procedure:

make
make dev
cp bin/terraform ~/bin

[pryorda]

@mitchellh Do you think we can get this merged in?

[ghost]

This issue exists in v0.7.0-v0.8.4.
👍

[dweomer]

Would love to see this accepted. Is the holdup the lack of a fix on the vmware/govmomi side?

[Freyert]

@pryorda the built in providers have been moved out of master. You will need to remake these changes over on https://github.com/terraform-providers/terraform-provider-vsphere

[apparentlymart]

Hello @pryorda, and thanks for working on this!

As part of the the Terraform 0.10 release earlier this year, all of the Terraform providers were moved to their own repositories in the terraform-providers GitHub organization, and removed from the Terraform Core repository.

Unfortunately due to the fact that new issues and pull requests are being opened constantly, it was not possible for the various provider maintainers to merge all outstanding pull requests before this split, and there is no automatic way to migrate a pull request to a new repository.

As a result, this pull request can sadly no longer be applied as-is, and so I'm going to close it.

The vSphere provider is currently in the process of being revamped somewhat, with the goal of supporting more of the available features. Some improvements to network interfaces were made relatively recently, so unfortunately this means that to re-target this change to the new repository may require some restructuring if the feature was not already implemented as part of the revamp 😖.

Thanks again for working on this, and sorry it was not able to be merged before the provider repository changes.

[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.