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

VMware: Fixed vmware fact gathering when no physical interfaces have IP Addresses #42600

Merged
merged 2 commits into from Oct 26, 2018

Conversation

bandit145
Copy link
Contributor

…enter

SUMMARY

Fixes #42597

This changes the way the guest IP address is reported, using guest.ipAddress ensures that if vmware-tools is reporting it then it will get returned (In this specific case no IP for a specific physical interface but a virtual interface exists that vmware-tools detected and reports).

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

vmware_guest_facts

ANSIBLE VERSION
2.x
ADDITIONAL INFORMATION

@ansibot
Copy link
Contributor

ansibot commented Jul 10, 2018

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:community This issue/PR relates to code supported by the Ansible community. vmware VMware community labels Jul 10, 2018
@bandit145 bandit145 changed the title Fixed vmware fact gathering when no phyiscal interfaces have IP Addresses Fixed vmware fact gathering when no physical interfaces have IP Addresses Jul 10, 2018
@gundalow gundalow removed the needs_triage Needs a first human triage before being processed. label Jul 11, 2018
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jul 19, 2018
@ansibot ansibot added support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed support:community This issue/PR relates to code supported by the Ansible community. labels Sep 18, 2018
@Akasurde Akasurde changed the title Fixed vmware fact gathering when no physical interfaces have IP Addresses VMware: Fixed vmware fact gathering when no physical interfaces have IP Addresses Oct 19, 2018
@ansibot ansibot added the core_review In order to be merged, this PR must follow the core review workflow. label Oct 19, 2018
@Akasurde
Copy link
Member

@pdellaert @dericcrago @Im0 @tchernomax @ckotte Could you please review this ?

@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 26, 2018
else:
facts['ipv4'] = ipaddress
if vm.guest.ipAddress:
if '::' in vm.guest.ipAddress:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit of a bad way of checking wether it is IPv6 or IPv4: :: only occurs if one of the hexadectets is 0000. You can easily end up with 2001:db8:1:2:3:4:5:6, which does not contain ::.

quick fix: check for : in ipaddress.
Clean fix: use the ipaddress python module (python 3.3+ native https://docs.python.org/3/library/ipaddress.html; or https://pypi.org/project/ipaddress/ for 2.6, 2.7 and 3.2, which is a backport (so same functionality).

This is the interesting part:

ipaddress.ip_address(address)
Return an IPv4Address or IPv6Address object depending on the IP address passed as argument. Either IPv4 or IPv6 addresses may be supplied; integers less than 2**32 will be considered to be IPv4 by default. A ValueError is raised if address does not represent a valid IPv4 or IPv6 address.

>>>ipaddress.ip_address('192.168.0.1')
IPv4Address('192.168.0.1')
>>> ipaddress.ip_address('2001:db8::')
IPv6Address('2001:db8::')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll throw the quick fix in for now, I'll leave deciding on adding a python2 dep to @Akasurde .

Copy link
Contributor

@pdellaert pdellaert left a comment

Choose a reason for hiding this comment

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

LGTM

@Akasurde Akasurde merged commit 055ee04 into ansible:devel Oct 26, 2018
@Akasurde
Copy link
Member

@bandit145 Thanks for the contribution. @pdellaert thanks for the valuable review.

@bandit145 bandit145 deleted the fix_vmware_fact_gather branch October 26, 2018 18:17
Tomorrow9 pushed a commit to Tomorrow9/ansible that referenced this pull request Dec 4, 2018
…IP Addresses (ansible#42600)

* Changed vmware_guest_facts to accuretly reflect ip as displayed in vcenter
* fixed ipv6 check
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. cloud core_review In order to be merged, this PR must follow the core review workflow. new_contributor This PR is the first contribution by a new community member. support:core This issue/PR relates to code supported by the Ansible Engineering Team. vmware VMware community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VMware: vmware_guest_facts does not report IP address of F5 Big IP load balancer
6 participants