Merge pull request #234 from lhuard1A/remove_use_of_local_arp_cache

[libvirt cluster] Use net-dhcp-leases to find VMs’ IPs

2 files changed, 56 insertions, 51 deletions

diff --git a/inventory/libvirt/hosts/libvirt_generic.py b/inventory/libvirt/hosts/libvirt_generic.py
index 4652f112e..1c9c17308 100755
--- a/inventory/libvirt/hosts/libvirt_generic.py
+++ b/inventory/libvirt/hosts/libvirt_generic.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python2
 
-"""
+'''
 libvirt external inventory script
 =================================
 
@@ -12,7 +12,7 @@ To use this, copy this file over /etc/ansible/hosts and chmod +x the file.
 This, more or less, allows you to keep one central database containing
 info about all of your managed instances.
 
-"""
+'''
 
 # (c) 2015, Jason DeTiberus <jdetiber@redhat.com>
 #
@@ -36,9 +36,7 @@ info about all of your managed instances.
 import argparse
 import ConfigParser
 import os
-import re
 import sys
-from time import time
 import libvirt
 import xml.etree.ElementTree as ET
 
@@ -49,8 +47,11 @@ except ImportError:
 
 
 class LibvirtInventory(object):
+    ''' libvirt dynamic inventory '''
 
     def __init__(self):
+        ''' Main execution path '''
+
         self.inventory = dict()  # A list of groups and the hosts in that group
         self.cache = dict()  # Details about hosts in the inventory
 
@@ -59,13 +60,15 @@ class LibvirtInventory(object):
         self.parse_cli_args()
 
         if self.args.host:
-            print self.json_format_dict(self.get_host_info(), self.args.pretty)
+            print _json_format_dict(self.get_host_info(), self.args.pretty)
         elif self.args.list:
-            print self.json_format_dict(self.get_inventory(), self.args.pretty)
+            print _json_format_dict(self.get_inventory(), self.args.pretty)
         else:  # default action with no options
-            print self.json_format_dict(self.get_inventory(), self.args.pretty)
+            print _json_format_dict(self.get_inventory(), self.args.pretty)
 
     def read_settings(self):
+        ''' Reads the settings from the libvirt.ini file '''
+
         config = ConfigParser.SafeConfigParser()
         config.read(
             os.path.dirname(os.path.realpath(__file__)) + '/libvirt.ini'
@@ -73,6 +76,8 @@ class LibvirtInventory(object):
         self.libvirt_uri = config.get('libvirt', 'uri')
 
     def parse_cli_args(self):
+        ''' Command line argument processing '''
+
         parser = argparse.ArgumentParser(
             description='Produce an Ansible Inventory file based on libvirt'
         )
@@ -96,25 +101,27 @@ class LibvirtInventory(object):
         self.args = parser.parse_args()
 
     def get_host_info(self):
+        ''' Get variables about a specific host '''
+
         inventory = self.get_inventory()
         if self.args.host in inventory['_meta']['hostvars']:
             return inventory['_meta']['hostvars'][self.args.host]
 
     def get_inventory(self):
+        ''' Construct the inventory '''
+
         inventory = dict(_meta=dict(hostvars=dict()))
 
         conn = libvirt.openReadOnly(self.libvirt_uri)
         if conn is None:
-            print "Failed to open connection to %s" % libvirt_uri
+            print "Failed to open connection to %s" % self.libvirt_uri
             sys.exit(1)
 
         domains = conn.listAllDomains()
         if domains is None:
-            print "Failed to list domains for connection %s" % libvirt_uri
+            print "Failed to list domains for connection %s" % self.libvirt_uri
             sys.exit(1)
 
-        arp_entries = self.parse_arp_entries()
-
         for domain in domains:
             hostvars = dict(libvirt_name=domain.name(),
                             libvirt_id=domain.ID(),
@@ -130,21 +137,30 @@ class LibvirtInventory(object):
             hostvars['libvirt_status'] = 'running'
 
             root = ET.fromstring(domain.XMLDesc())
-            ns = {'ansible': 'https://github.com/ansible/ansible'}
-            for tag_elem in root.findall('./metadata/ansible:tags/ansible:tag', ns):
+            ansible_ns = {'ansible': 'https://github.com/ansible/ansible'}
+            for tag_elem in root.findall('./metadata/ansible:tags/ansible:tag', ansible_ns):
                 tag = tag_elem.text
-                self.push(inventory, "tag_%s" % tag, domain_name)
-                self.push(hostvars, 'libvirt_tags', tag)
+                _push(inventory, "tag_%s" % tag, domain_name)
+                _push(hostvars, 'libvirt_tags', tag)
 
             # TODO: support more than one network interface, also support
             # interface types other than 'network'
             interface = root.find("./devices/interface[@type='network']")
             if interface is not None:
+                source_elem = interface.find('source')
                 mac_elem = interface.find('mac')
-                if mac_elem is not None:
-                    mac = mac_elem.get('address')
-                    if mac in arp_entries:
-                        ip_address = arp_entries[mac]['ip_address']
+                if source_elem is not None and \
+                   mac_elem    is not None:
+                    # Adding this to disable pylint check specifically
+                    # ignoring libvirt-python versions that
+                    # do not include DHCPLeases
+                    # This is needed until we upgrade the build bot to
+                    # RHEL7 (>= 1.2.6 libvirt)
+                    # pylint: disable=no-member
+                    dhcp_leases = conn.networkLookupByName(source_elem.get('network')) \
+                                      .DHCPLeases(mac_elem.get('address'))
+                    if len(dhcp_leases) > 0:
+                        ip_address = dhcp_leases[0]['ipaddr']
                         hostvars['ansible_ssh_host'] = ip_address
                         hostvars['libvirt_ip_address'] = ip_address
 
@@ -152,28 +168,23 @@ class LibvirtInventory(object):
 
         return inventory
 
-    def parse_arp_entries(self):
-        arp_entries = dict()
-        with open('/proc/net/arp', 'r') as f:
-            # throw away the header
-            f.readline()
-
-            for line in f:
-                ip_address, _, _, mac, _, device = line.strip().split()
-                arp_entries[mac] = dict(ip_address=ip_address, device=device)
-
-        return arp_entries
-
-    def push(self, my_dict, key, element):
-        if key in my_dict:
-            my_dict[key].append(element)
-        else:
-            my_dict[key] = [element]
-
-    def json_format_dict(self, data, pretty=False):
-        if pretty:
-            return json.dumps(data, sort_keys=True, indent=2)
-        else:
-            return json.dumps(data)
+def _push(my_dict, key, element):
+    '''
+    Push element to the my_dict[key] list.
+    After having initialized my_dict[key] if it dosn't exist.
+    '''
+
+    if key in my_dict:
+        my_dict[key].append(element)
+    else:
+        my_dict[key] = [element]
+
+def _json_format_dict(data, pretty=False):
+    ''' Serialize data to a JSON formated str '''
+
+    if pretty:
+        return json.dumps(data, sort_keys=True, indent=2)
+    else:
+        return json.dumps(data)
 
 LibvirtInventory()
diff --git a/playbooks/libvirt/openshift-cluster/tasks/launch_instances.yml b/playbooks/libvirt/openshift-cluster/tasks/launch_instances.yml
index 359d0b2f3..8bf1e84ee 100644
--- a/playbooks/libvirt/openshift-cluster/tasks/launch_instances.yml
+++ b/playbooks/libvirt/openshift-cluster/tasks/launch_instances.yml
@@ -58,23 +58,17 @@
     uri: '{{ libvirt_uri }}'
   with_items: instances
 
-- name: Collect MAC addresses of the VMs
-  shell: 'virsh -c {{ libvirt_uri }} dumpxml {{ item }} | xmllint --xpath "string(//domain/devices/interface/mac/@address)" -'
-  register: scratch_mac
-  with_items: instances
-
 - name: Wait for the VMs to get an IP
-  command: "egrep -c '{{ scratch_mac.results | oo_collect('stdout') | join('|') }}' /proc/net/arp"
-  ignore_errors: yes
+  shell: 'virsh net-dhcp-leases openshift-ansible | egrep -c ''{{ instances | join("|") }}'''
   register: nb_allocated_ips
   until: nb_allocated_ips.stdout == '{{ instances | length }}'
   retries: 30
   delay: 1
 
 - name: Collect IP addresses of the VMs
-  shell: "awk '/{{ item.stdout }}/ {print $1}' /proc/net/arp"
+  shell: 'virsh net-dhcp-leases openshift-ansible | awk ''$6 == "{{ item }}" {gsub(/\/.*/, "", $5); print $5}'''
   register: scratch_ip
-  with_items: scratch_mac.results
+  with_items: instances
 
 - set_fact:
     ips: "{{ scratch_ip.results | oo_collect('stdout') }}"