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

Add support for using guest.ipAddress for older versions of VM Tools #684

Merged
merged 3 commits into from
Feb 8, 2019

Conversation

cbascom
Copy link

@cbascom cbascom commented Jan 8, 2019

Description

There are two parts to this change:

  • Add a new waiter that waits for guest.ipAddress to be populated. This waiter is controlled via the wait_for_guest_ip_timeout property that is disabled by default. This waiter is intended to be used in place of the wait_for_guest_net_timeout waiter.
  • Regardless of which waiter is enabled, the code that populates default_ip_address and guest_ip_addresses will first look at guest.net and guest.ipStack to find the IP address. Only if no IP address is found via that method will it then fall back to looking at the guest.ipAddress property instead.

Use Case

The VMs I am creating unfortunately have an older version of VM Tools that does not populate the guest.net or guest.ipStack properties. However, the VMs I am creating do populate the guest.ipAddress property which can be used for this same purpose. This change allows me to just wait for guest.ipAddress and use that IP address to proceed with provisioning.

Todo

  • Tests
  • Documentation updates

I wanted to see if there was a chance of getting something like this in before I spent time adding those as well.

Older versions of VMTools do not populate the IpStack and Net propreties
in GuestInfo which is what is currently used to find the VM's IP
address. With this change we now have the option to wait for the IpAddress
property instead which does get populated with older versions of
VMTools.

In order to ensure that we wait for the guest IpAddress property to be
populated, a new wait_for_guest_ip_timeout configuration option has been
added. This option is disabled by default but can be enabled by setting
the timeout to the number of minutes to wait for the guest IpAddress
property to be populated.
@ghost ghost added the size/m Relative Sizing: Medium label Jan 8, 2019
@bill-rich
Copy link
Contributor

Hi @cbascom!

Thanks for asking about this. Which version is guest.ipAddress supported in? I don't know right off where the cutoff is. I'd like to support as much as possible with the vSphere provider, but we can't go too far back without adding a lot of potential issues and complexity.

@cbascom
Copy link
Author

cbascom commented Jan 14, 2019

@bill-rich The version of VMTools I am forced to deal with for now is 9.4.6.33107 (build-1770165). Based on the VMware docs I have looked at, I think it has been available from the beginning. The guest.ipStack that is currently being used wasn't added until API version 4.1. FWIW, The guest.ipAddress field is what the vSphere client uses to display the primary IP address of the VM I believe.

I understand about not wanting to go too far back, if this is a no go I will just have to maintain my fork for the time being. If you have any other ideas on how I can work around being stuck with the older VMTools, I'm all ears - this was the only way I have come up with so far that gets me what I need. Thanks!

Some VMs have default IP addresses that show up before DHCP has given an
actual IP address. The new ignored_guest_ips attribute allows the user
to configure one or more IP addresses that should be ignored by the IP
address waiters.
Copy link
Contributor

@bill-rich bill-rich left a comment

Choose a reason for hiding this comment

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

Just have the one question on the additional IP func, and the documentation comment. I think adding older tools support will be ok. I'm going to run the changes through acceptance tests and make sure everything works as expected.

//
// The timeout is specified in minutes. If zero or a negative value is passed,
// the waiter returns without error immediately.
func WaitForGuestIP(client *govmomi.Client, vm *object.VirtualMachine, timeout int, ignoredGuestIPs []interface{}) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do differently than WaitForGuestNet with wait_for_guest_net_routable set to false?

Copy link
Author

Choose a reason for hiding this comment

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

The only real difference is that it waits for the string guest.ipAddress property instead of the ArrayOfGuestNicInfo guest.net property. I thought about just adding guest.ipAddress to the list of properties that WaitForGuestNet monitors with a new case statement in the switch, but then it becomes a race between which one gets populated first in newer VMTools versions I believe. This seemed like a potential backwards compatibility problem, so I chose to make them completely independent waiters to be safe.

vsphere/resource_vsphere_virtual_machine.go Show resolved Hide resolved
Added doc for the new wait_for_guest_ip_timeout and ignored_guest_ips
properties. Updated docs for the existing wait_for_guest_net_timeout and
wait_for_guest_net_routable properties to document their relationship
with these new properties.
@ghost ghost added the documentation Type: Documentation label Feb 8, 2019
Copy link
Contributor

@bill-rich bill-rich left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for all your work and patience on this. The waiter related tests were run and all passed.

@bill-rich bill-rich merged commit 3051961 into hashicorp:master Feb 8, 2019
@ghost ghost locked and limited conversation to collaborators Apr 18, 2020
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
documentation Type: Documentation size/m Relative Sizing: Medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants