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

provider/vsphere: read gateway information #6522

Merged
merged 6 commits into from
May 11, 2016
Merged

provider/vsphere: read gateway information #6522

merged 6 commits into from
May 11, 2016

Conversation

thetuxkeeper
Copy link
Contributor

The gateway was not read from the VM before and could have forced redeploying the VM when setting a static gateway.

@chrislovecnm
Copy link
Contributor

Should one of our test caught this? We don't need another test, but I am thinking we should improve the test.

@thetuxkeeper
Copy link
Contributor Author

The TestAccVSphereVirtualMachine_ipv4Andipv6 test was failing for me

=== RUN   TestAccVSphereVirtualMachine_ipv4Andipv6
--- FAIL: TestAccVSphereVirtualMachine_ipv4Andipv6 (96.15s)
        testing.go:172: Step 0 error: Check failed: Check 10/12 error: vsphere_virtual_machine.ipv4ipv6: Attribute 'network_interface.0.ipv4_gateway' expected "10.30.8.1", got ""
FAIL

@@ -795,6 +795,19 @@ func resourceVSphereVirtualMachineRead(d *schema.ResourceData, meta interface{})
networkInterfaces = append(networkInterfaces, networkInterface)
}
}
for _, v := range mvm.Guest.IpStack {
for _, route := range v.IpRouteConfig.IpRoute {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to check for nil on these?? Can IpRouteConfig == nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I think we need some more checks here. Guest, IpStack, IpRouteConfig and IpRoute could be nil

@thetuxkeeper
Copy link
Contributor Author

@chrislovecnm Fixed. Please review again :)

@chrislovecnm
Copy link
Contributor

@thetuxkeeper travis is not happy:

builtin/providers/vsphere/resource_vsphere_virtual_machine.go:802: invalid operation: route.Gateway.Device != nil (mismatched types string and nil)

@thetuxkeeper
Copy link
Contributor Author

oh, I missed that. I thought I checked it. go is not nice here ;)
Will check it later.

@thetuxkeeper
Copy link
Contributor Author

@chrislovecnm Fixed the nil/empty check
I pushed the fixes for the tests, though you said to put it in an additional/WIP PR. But there are not much changes. I can remove it again if you want (and put it as an additional PR).
before 0fac39c

=== RUN   TestAccVSphereVirtualMachine_basic
--- FAIL: TestAccVSphereVirtualMachine_basic (112.18s)
        testing.go:172: Step 0 error: After applying this step, the plan was not empty:

                DIFF:

                UPDATE: vsphere_virtual_machine.foo
                  network_interface.0.ipv4_gateway: "10.30.8.1" => ""

                STATE:

                vsphere_virtual_machine.foo:
                  ID = terraform-test
                  cluster = dc1.boerse-go.de
                  datacenter = dc1
                  disk.# = 2
                  disk.0.bootable = false
                  disk.0.datastore = DS1
                  disk.0.iops = 500
                  disk.0.size = 0
                  disk.0.template = terraform-test-folder/centos-terraform-template
                  disk.0.type = eager_zeroed
                  disk.0.vmdk =
                  disk.1.bootable = false
                  disk.1.datastore =
                  disk.1.iops = 500
                  disk.1.size = 1
                  disk.1.template =
                  disk.1.type = eager_zeroed
                  disk.1.vmdk =
                  domain = vsphere.local
                  gateway = 10.30.8.1
                  linked_clone = false
                  memory = 4096
                  memory_reservation = 4096
                  name = terraform-test
                  network_interface.# = 1
                  network_interface.0.adapter_type =
                  network_interface.0.ip_address =
                  network_interface.0.ipv4_address = 10.30.8.250
                  network_interface.0.ipv4_gateway = 10.30.8.1
                  network_interface.0.ipv4_prefix_length = 24
                  network_interface.0.ipv6_address = fe80::250:56ff:fe8e:3807
                  network_interface.0.ipv6_gateway =
                  network_interface.0.ipv6_prefix_length = 64
                  network_interface.0.label = cld_tst1_access
                  network_interface.0.subnet_mask =
                  resource_pool = dc1.boerse-go.de/Resources/test_server
                  skip_customization = false
                  time_zone = Etc/UTC
                  vcpu = 2
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/vsphere        112.191s

after 0fac39c:

TF_ACC=1 go test ./builtin/providers/vsphere -v -run=TestAccVSphereVirtualMachine_basic -timeout 120m
=== RUN   TestAccVSphereVirtualMachine_basic
--- PASS: TestAccVSphereVirtualMachine_basic (105.38s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/vsphere        105.394s

@chrislovecnm
Copy link
Contributor

@stack72 @jen20 @phinze ~ can someone take a find review and get the code in??

Thanks!!!!

@thetuxkeeper
Copy link
Contributor Author

Like I discovered in #6590 the ipv6_gateway is not covered by this PR yet. I'll try to push an additional patch. It should be quite the same as with ipv4, so it shouldn't add too much more to review.

@chrislovecnm
Copy link
Contributor

@thetuxkeeper you want to fix ipv6?

@thetuxkeeper
Copy link
Contributor Author

@chrislovecnm : yes. just pushed it.

@jen20
Copy link
Contributor

jen20 commented May 11, 2016

This seems good to me - I have no way of verifying whether the acceptance test is passing but I am happy to take @thetuxkeeper's word for it! Thanks all!

@jen20 jen20 merged commit 5984f84 into hashicorp:master May 11, 2016
@chrislovecnm
Copy link
Contributor

@jen20 much thanks!!

@thetuxkeeper
Copy link
Contributor Author

thetuxkeeper commented May 11, 2016

@jen20 : Thanks! I hope I can prove that I'm worth your trust. :)
But I think I missed something. I broke the DHCP tests. Since the gateway is read from vSphere, but not configured.
I think we have to change the ipv*_gateway settings to Computed: true?

@ghost
Copy link

ghost commented Apr 25, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants