Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

r/virtual_machine: Device cull range fix, and handle zero-NIC templates #269

Merged
merged 1 commit into from
Dec 4, 2017

Conversation

vancluever
Copy link
Contributor

There was some issues with our device culling/ranging logic in certain
scenarios:

  • On templates with no NICs, unitRange was not producing a proper
    result. This was the result of these values being pointers because we
    work with the unit number value directly in the virtual device, which is
    a pointer. This fixes things so that if a zero-length device list is
    passed to unit range, we return 0 immediately (as there are no devices
    to range over).
  • On templates where the NIC count was less than the amount of NICs
    needed by configuration, the device cull range expression (which is used
    to handle the exact opposite scenario) was running into out-of-range
    issues. This has been fixed by asserting that the old device list is
    indeed longer than the new device list so that there's no way that
    referencing old[len(new):] will fail. This has been extended to CD
    devices too, which has the same logic, even though it's currently masked
    by the fact that we only allow one cdrom device.

Fixes #267.

There was some issues with our device culling/ranging logic in certain
scenarios:

* On templates with no NICs, unitRange was not producing a proper
result. This was the result of these values being pointers because we
work with the unit number value directly in the virtual device, which is
a pointer. This fixes things so that if a zero-length device list is
passed to unit range, we return 0 immediately (as there are no devices
to range over).
* On templates where the NIC count was less than the amount of NICs
needed by configuration, the device cull range expression (which is used
to handle the exact opposite scenario) was running into out-of-range
issues. This has been fixed by asserting that the old device list is
indeed longer than the new device list so that there's no way that
referencing old[len(new):] will fail. This has been extended to CD
devices too, which has the same logic, even though it's currently masked
by the fact that we only allow one cdrom device.
@vancluever
Copy link
Contributor Author

Just waiting for some clone tests to finish and will add the results when hey are done.

@vancluever
Copy link
Contributor Author

Test results:

--- PASS: TestAccResourceVSphereVirtualMachine/clone_from_template (125.25s)
--- PASS: TestAccResourceVSphereVirtualMachine/clone,_multi-nic_(template_should_have_one) (120.24s)
--- PASS: TestAccResourceVSphereVirtualMachine/clone_with_different_timezone (127.92s)
--- PASS: TestAccResourceVSphereVirtualMachine/clone_with_bad_timezone (0.01s)
--- PASS: TestAccResourceVSphereVirtualMachine/clone_with_different_hostname (120.12s)
--- PASS: TestAccResourceVSphereVirtualMachine/clone_with_extra_disks (294.94s)

Copy link
Member

@mbfrahry mbfrahry left a comment

Choose a reason for hiding this comment

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

LGTM!

@vancluever
Copy link
Contributor Author

Thanks @mbfrahry!

@vancluever vancluever merged commit 132ec89 into master Dec 4, 2017
@vancluever vancluever deleted the b-GH267 branch December 5, 2017 13:42
@ghost ghost locked and limited conversation to collaborators Apr 19, 2020
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
bug Type: Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Terraform crashes when creating vSphere VM with 1.x vSphere provider
2 participants