Change in vdsm[master]: net: IP DHCP detection

edwardh at redhat.com edwardh at redhat.com
Thu Feb 4 07:41:46 UTC 2016


Edward Haas has uploaded a new change for review.

Change subject: net: IP DHCP detection
......................................................................

net: IP DHCP detection

Improve and fix DHCP status detection.

There have been several points covered in this patch:
- dhcpv6 is now correctly detected (directly from kernel).
- dhcp detection simplified by checking ip interface flags (through
  netlink). This method is also faster and reduces complexity.
- Code cleanup.

Change-Id: I4d0396610822e3a41d44f37346ee1aad71d569d1
Signed-off-by: Edward Haas <edwardh at redhat.com>
---
M lib/vdsm/netinfo/__init__.py
M lib/vdsm/netinfo/dhcp.py
M tests/functional/networkTests.py
M tests/netinfoTests.py
M vdsm_hooks/ovs/ovs_after_get_caps.py
5 files changed, 57 insertions(+), 214 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/76/53076/1

diff --git a/lib/vdsm/netinfo/__init__.py b/lib/vdsm/netinfo/__init__.py
index b73bea6..0a6dbad 100644
--- a/lib/vdsm/netinfo/__init__.py
+++ b/lib/vdsm/netinfo/__init__.py
@@ -34,8 +34,8 @@
 from .addresses import getIpAddrs, getIpInfo
 from . import bonding
 from . import bridges
-from .dhcp import (get_dhclient_ifaces, propose_updates_to_reported_dhcp,
-                   update_reported_dhcp, dhcp_used)
+from .dhcp import (propose_updates_to_reported_dhcp, update_reported_dhcp,
+                   dhcp_status, dhcp_faked_status)
 from .dns import get_host_nameservers
 from .misc import getIfaceCfg, ipv6_supported
 from .mtus import getMtu
@@ -65,15 +65,12 @@
                   'vlans': {}, 'dnss': get_host_nameservers()}
     paddr = bonding.permanent_address()
     ipaddrs = getIpAddrs()
-    dhcpv4_ifaces, dhcpv6_ifaces = get_dhclient_ifaces()
     routes = get_routes()
-    running_config = RunningConfig()
 
     if vdsmnets is None:
         libvirt_nets = networks()
-        networking['networks'] = libvirtNets2vdsm(libvirt_nets, running_config,
-                                                  routes, ipaddrs,
-                                                  dhcpv4_ifaces, dhcpv6_ifaces)
+        networking['networks'] = libvirtNets2vdsm(libvirt_nets, routes,
+                                                  ipaddrs)
     else:
         networking['networks'] = vdsmnets
 
@@ -88,8 +85,7 @@
             devinfo = networking['vlans'][dev.name] = vlans.info(dev)
         else:
             continue
-        devinfo.update(_devinfo(dev, routes, ipaddrs, dhcpv4_ifaces,
-                                dhcpv6_ifaces))
+        devinfo.update(_devinfo(dev, routes, ipaddrs))
         if dev.isBOND():
             bonding.bondOptsCompat(devinfo)
 
@@ -122,40 +118,40 @@
     return netinfo_data
 
 
-def libvirtNets2vdsm(nets, running_config=None, routes=None, ipAddrs=None,
-                     dhcpv4_ifaces=None, dhcpv6_ifaces=None):
-    if running_config is None:
-        running_config = RunningConfig()
+def libvirtNets2vdsm(nets, routes=None, ipAddrs=None):
+    running_config = RunningConfig()
     if routes is None:
         routes = get_routes()
     if ipAddrs is None:
         ipAddrs = getIpAddrs()
-    if dhcpv4_ifaces is None or dhcpv6_ifaces is None:
-        dhcpv4_ifaces, dhcpv6_ifaces = get_dhclient_ifaces()
+
     d = {}
     for net, netAttr in nets.iteritems():
         try:
             # Pass the iface if the net is _not_ bridged, the bridge otherwise
             d[net] = _getNetInfo(netAttr.get('iface', net), netAttr['bridged'],
-                                 routes, ipAddrs, dhcpv4_ifaces, dhcpv6_ifaces,
+                                 routes, ipAddrs,
                                  running_config.networks.get(net, None))
         except KeyError:
             continue  # Do not report missing libvirt networks.
     return d
 
 
-def _devinfo(link, routes, ipaddrs, dhcpv4_ifaces, dhcpv6_ifaces):
+def _devinfo(link, routes, ipaddrs):
     gateway = get_gateway(routes, link.name)
     ipv4addr, ipv4netmask, ipv4addrs, ipv6addrs = getIpInfo(
         link.name, ipaddrs, gateway)
+
+    is_dhcpv4, is_dhcpv6 = dhcp_status(ipaddrs, link.name)
+
     info = {'addr': ipv4addr,
             'cfg': getIfaceCfg(link.name),
             'ipv4addrs': ipv4addrs,
             'ipv6addrs': ipv6addrs,
             'gateway': gateway,
             'ipv6gateway': get_gateway(routes, link.name, family=6),
-            'dhcpv4': link.name in dhcpv4_ifaces,  # to be refined if a network
-            'dhcpv6': link.name in dhcpv6_ifaces,  # is not configured for DHCP
+            'dhcpv4': is_dhcpv4,
+            'dhcpv6': is_dhcpv6,
             'mtu': link.mtu,
             'netmask': ipv4netmask}
     if 'BOOTPROTO' not in info['cfg']:
@@ -206,8 +202,7 @@
     return nets
 
 
-def _getNetInfo(iface, bridged, routes, ipaddrs, dhcpv4_ifaces, dhcpv6_ifaces,
-                net_attrs):
+def _getNetInfo(iface, bridged, routes, ipaddrs, net_attrs):
     """Returns a dictionary of properties about the network's interface status.
     Raises a KeyError if the iface does not exist."""
     data = {}
@@ -224,11 +219,13 @@
         gateway = get_gateway(routes, iface)
         ipv4addr, ipv4netmask, ipv4addrs, ipv6addrs = getIpInfo(
             iface, ipaddrs, gateway)
+
+        is_dhcpv4, is_dhcpv6 = dhcp_faked_status(iface, ipaddrs, net_attrs)
+
         data.update({'iface': iface, 'bridged': bridged,
                      'addr': ipv4addr, 'netmask': ipv4netmask,
-                     'dhcpv4': dhcp_used(iface, dhcpv4_ifaces, net_attrs),
-                     'dhcpv6': dhcp_used(iface, dhcpv6_ifaces, net_attrs,
-                                         family=6),
+                     'dhcpv4': is_dhcpv4,
+                     'dhcpv6': is_dhcpv6,
                      'ipv4addrs': ipv4addrs,
                      'ipv6addrs': ipv6addrs,
                      'gateway': gateway,
diff --git a/lib/vdsm/netinfo/dhcp.py b/lib/vdsm/netinfo/dhcp.py
index 6877893..71d114b 100644
--- a/lib/vdsm/netinfo/dhcp.py
+++ b/lib/vdsm/netinfo/dhcp.py
@@ -17,8 +17,7 @@
 #
 # Refer to the README and COPYING files for full details of the license
 from __future__ import absolute_import
-from datetime import datetime, timedelta
-from glob import iglob
+from datetime import datetime
 import six
 
 # possible names of dhclient's lease files (e.g. as NetworkManager's slave)
@@ -26,88 +25,6 @@
     '/var/lib/dhclient/dhclient*.lease*',  # iproute2 configurator, initscripts
     '/var/lib/NetworkManager/dhclient*-*.lease',
 ]
-
-
-def get_dhclient_ifaces(lease_files_globs=DHCLIENT_LEASES_GLOBS):
-    """Return a pair of sets containing ifaces configured using dhclient (-6)
-
-    dhclient stores DHCP leases to file(s) whose names can be specified
-    by the lease_files_globs parameter (an iterable of glob strings).
-    """
-    dhcpv4_ifaces, dhcpv6_ifaces = set(), set()
-
-    for lease_files_glob in lease_files_globs:
-        for lease_path in iglob(lease_files_glob):
-            with open(lease_path) as lease_file:
-                found_dhcpv4, found_dhcpv6 = _parse_lease_file(lease_file)
-                dhcpv4_ifaces.update(found_dhcpv4)
-                dhcpv6_ifaces.update(found_dhcpv6)
-
-    return dhcpv4_ifaces, dhcpv6_ifaces
-
-
-def _parse_lease_file(lease_file):
-    IFACE = '  interface "'
-    IFACE_END = '";\n'
-    EXPIRE = '  expire '  # DHCPv4
-    STARTS = '      starts '  # DHCPv6
-    MAX_LIFE = '      max-life '
-    VALUE_END = ';\n'
-
-    family = None
-    iface = None
-    lease6_starts = None
-    dhcpv4_ifaces, dhcpv6_ifaces = set(), set()
-
-    for line in lease_file:
-        if line == 'lease {\n':
-            family = 4
-            iface = None
-            continue
-
-        elif line == 'lease6 {\n':
-            family = 6
-            iface = None
-            continue
-
-        if family and line.startswith(IFACE) and line.endswith(IFACE_END):
-            iface = line[len(IFACE):-len(IFACE_END)]
-
-        elif family == 4:
-            if line.startswith(EXPIRE):
-                end = line.find(';')
-                if end == -1:
-                    continue  # the line should always contain a ;
-
-                expiry_time = _parse_expiry_time(line[len(EXPIRE):end])
-                if expiry_time is not None and datetime.utcnow() > expiry_time:
-                    family = None
-                    continue
-
-            elif line == '}\n':
-                family = None
-                if iface:
-                    dhcpv4_ifaces.add(iface)
-
-        elif family == 6:
-            if line.startswith(STARTS) and line.endswith(VALUE_END):
-                timestamp = float(line[len(STARTS):-len(VALUE_END)])
-                lease6_starts = datetime.utcfromtimestamp(timestamp)
-
-            elif (lease6_starts and line.startswith(MAX_LIFE) and
-                    line.endswith(VALUE_END)):
-                seconds = float(line[len(MAX_LIFE):-len(VALUE_END)])
-                max_life = timedelta(seconds=seconds)
-                if datetime.utcnow() > lease6_starts + max_life:
-                    family = None
-                    continue
-
-            elif line == '}\n':
-                family = None
-                if iface:
-                    dhcpv6_ifaces.add(iface)
-
-    return dhcpv4_ifaces, dhcpv6_ifaces
 
 
 def propose_updates_to_reported_dhcp(network_info, networking):
@@ -164,14 +81,24 @@
         return datetime.strptime(expiry_time, '%w %Y/%m/%d %H:%M:%S')
 
 
-def dhcp_used(iface, ifaces_with_active_leases, net_attrs, family=4):
+def dhcp_status(ipaddrs, dev):
+    is_dhcpv4 = False
+    is_dhcpv6 = False
+    for ipaddr in ipaddrs[dev]:
+        if ipaddr['family'] == 'inet':
+            is_dhcpv4 |= 'permanent' not in ipaddr['flags']
+        elif ipaddr['family'] == 'inet6':
+            is_dhcpv6 |= 'permanent' not in ipaddr['flags']
+    return is_dhcpv4, is_dhcpv6
+
+
+def dhcp_faked_status(iface, ipaddrs, net_attrs):
+    # If running config exists for this network, fake dhcp status
+    # and report the request (not the actual)
+    # Engine expects this behaviour.
     if net_attrs is None:
-        return iface in ifaces_with_active_leases
+        is_dhcpv4, is_dhcpv6 = dhcp_status(ipaddrs, iface)
     else:
-        try:
-            if family == 4:
-                return net_attrs['bootproto'] == 'dhcp'
-            else:
-                return net_attrs['dhcpv6']
-        except KeyError:
-            return False
+        is_dhcpv4 = (net_attrs.get('bootproto', None) == 'dhcp')
+        is_dhcpv6 = net_attrs.get('dhcpv6', False)
+    return is_dhcpv4, is_dhcpv6
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 3bfba16..7bdcbc1 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -38,7 +38,6 @@
 from vdsm.netinfo.bridges import bridges
 from vdsm.netinfo.misc import NET_CONF_PREF
 from vdsm.netinfo.mtus import DEFAULT_MTU
-from vdsm.netinfo.dhcp import get_dhclient_ifaces
 from vdsm.netinfo.nics import operstate, OPERSTATE_UNKNOWN, OPERSTATE_UP
 from vdsm.netinfo.routes import getRouteDeviceTo
 from vdsm.netlink import monitor
@@ -2144,8 +2143,6 @@
     @cleanupNet
     @RequireVethMod
     def testDhclientLeases(self, family, dateFormat):
-        dhcpv4_ifaces = set()
-        dhcpv6_ifaces = set()
         with veth_pair() as (server, client):
             addrAdd(server, IP_ADDRESS, IP_CIDR)
             addrAdd(server, IPv6_ADDRESS, IPv6_CIDR, 6)
@@ -2157,18 +2154,17 @@
                     dhclient_runner = dhcp.DhclientRunner(
                         client, family, dir, dateFormat)
                     try:
-                        with running(dhclient_runner) as dhc:
-                            dhcpv4_ifaces, dhcpv6_ifaces = \
-                                get_dhclient_ifaces([dhc.lease_file])
+                        with running(dhclient_runner):
+                            ipaddrs = vdsm.netinfo.addresses.getIpAddrs()
+                            is_dhcpv4, is_dhcpv6 = (
+                                vdsm.netinfo.dhcp.dhcp_status(ipaddrs, client))
                     except dhcp.ProcessCannotBeKilled:
                         raise SkipTest('dhclient could not be killed')
 
         if family == 4:
-            self.assertIn(client, dhcpv4_ifaces,
-                          '{0} not found in a lease file.'.format(client))
+            self.assertTrue(is_dhcpv4)
         else:
-            self.assertIn(client, dhcpv6_ifaces,
-                          '{0} not found in a lease file.'.format(client))
+            self.assertTrue(is_dhcpv6)
 
     def testGetRouteDeviceTo(self):
         with dummyIf(1) as nics:
diff --git a/tests/netinfoTests.py b/tests/netinfoTests.py
index c506674..ff01105 100644
--- a/tests/netinfoTests.py
+++ b/tests/netinfoTests.py
@@ -19,15 +19,13 @@
 # Refer to the README and COPYING files for full details of the license
 #
 import os
-from datetime import datetime
 from functools import partial
 import io
 import netaddr
-import time
 
 from vdsm import ipwrapper
 from vdsm import netinfo
-from vdsm.netinfo import addresses, bonding, dns, dhcp, misc, nics, routes, get
+from vdsm.netinfo import addresses, bonding, dns, misc, nics, routes, get
 from vdsm.netlink import addr as nl_addr
 from vdsm.utils import random_iface_name
 
@@ -233,74 +231,6 @@
                     misc.getIfaceCfg(deviceName)['GATEWAY'], '1.1.1.1')
                 self.assertEqual(
                     misc.getIfaceCfg(deviceName)['NETMASK'], '255.255.0.0')
-
-    def testGetDhclientIfaces(self):
-        LEASES = (
-            'lease {{\n'
-            '  interface "valid";\n'
-            '  expire {active_datetime:%w %Y/%m/%d %H:%M:%S};\n'
-            '}}\n'
-            'lease {{\n'
-            '  interface "valid2";\n'
-            '  expire epoch {active:.0f}; # Sat Jan 31 20:04:20 2037\n'
-            '}}\n'                   # human-readable date is just a comment
-            'lease {{\n'
-            '  interface "valid3";\n'
-            '  expire never;\n'
-            '}}\n'
-            'lease {{\n'
-            '  interface "expired";\n'
-            '  expire {expired_datetime:%w %Y/%m/%d %H:%M:%S};\n'
-            '}}\n'
-            'lease {{\n'
-            '  interface "expired2";\n'
-            '  expire epoch {expired:.0f}; # Fri Jan 31 20:04:20 2014\n'
-            '}}\n'
-            'lease6 {{\n'
-            '  interface "valid4";\n'
-            '  ia-na [some MAC address] {{\n'
-            '    iaaddr [some IPv6 address] {{\n'
-            '      starts {now:.0f};\n'
-            '      max-life 60;\n'  # the lease has a minute left
-            '    }}\n'
-            '  }}\n'
-            '}}\n'
-            'lease6 {{\n'
-            '  interface "expired3";\n'
-            '  ia-na [some MAC address] {{\n'
-            '    iaaddr [some IPv6 address] {{\n'
-            '      starts {expired:.0f};\n'
-            '      max-life 30;\n'  # the lease expired half a minute ago
-            '    }}\n'
-            '  }}\n'
-            '}}\n'
-        )
-
-        with namedTemporaryDir() as tmp_dir:
-            lease_file = os.path.join(tmp_dir, 'test.lease')
-            with open(lease_file, 'w') as f:
-                now = time.time()
-                last_minute = now - 60
-                next_minute = now + 60
-
-                f.write(LEASES.format(
-                    active_datetime=datetime.utcfromtimestamp(next_minute),
-                    active=next_minute,
-                    expired_datetime=datetime.utcfromtimestamp(last_minute),
-                    expired=last_minute,
-                    now=now
-                ))
-
-            dhcpv4_ifaces, dhcpv6_ifaces = \
-                dhcp.get_dhclient_ifaces([lease_file])
-
-        self.assertIn('valid', dhcpv4_ifaces)
-        self.assertIn('valid2', dhcpv4_ifaces)
-        self.assertIn('valid3', dhcpv4_ifaces)
-        self.assertNotIn('expired', dhcpv4_ifaces)
-        self.assertNotIn('expired2', dhcpv4_ifaces)
-        self.assertIn('valid4', dhcpv6_ifaces)
-        self.assertNotIn('expired3', dhcpv6_ifaces)
 
     @brokentest("Skipped becasue it breaks randomly on the CI")
     @MonkeyPatch(bonding, 'BONDING_DEFAULTS', bonding.BONDING_DEFAULTS
diff --git a/vdsm_hooks/ovs/ovs_after_get_caps.py b/vdsm_hooks/ovs/ovs_after_get_caps.py
index 203a869..ecc07c1 100755
--- a/vdsm_hooks/ovs/ovs_after_get_caps.py
+++ b/vdsm_hooks/ovs/ovs_after_get_caps.py
@@ -52,11 +52,12 @@
     return out
 
 
-def _get_net_info(attrs, interface, dhcpv4ifaces, dhcpv6ifaces, routes):
+def _get_net_info(interface, routes):
     mtu = mtus.getMtu(interface)
-    addr, netmask, ipv4addrs, ipv6addrs = addresses.getIpInfo(interface)
-    dhcpv4 = dhcp.dhcp_used(interface, dhcpv4ifaces, attrs)
-    dhcpv6 = dhcp.dhcp_used(interface, dhcpv6ifaces, attrs, family=6)
+    ipaddrs = addresses.getIpAddrs()
+    addr, netmask, ipv4addrs, ipv6addrs = addresses.getIpInfo(interface,
+                                                              ipaddrs)
+    dhcpv4, dhcpv6 = dhcp.dhcp_status(ipaddrs, interface)
     gateway = netinfo_routes.get_gateway(routes, interface)
     ipv6gateway = netinfo_routes.get_gateway(routes, interface, family=6)
     return {
@@ -89,12 +90,10 @@
 
 def networks_caps(running_config):
     ovs_networks_caps = {}
-    dhcpv4ifaces, dhcpv6ifaces = dhcp.get_dhclient_ifaces()
     routes = netinfo_routes.get_routes()
     for network, attrs in iter_ovs_nets(running_config.networks):
         interface = network if 'vlan' in attrs else BRIDGE_NAME
-        net_info = _get_net_info(attrs, interface, dhcpv4ifaces, dhcpv6ifaces,
-                                 routes)
+        net_info = _get_net_info(interface, routes)
         net_info['iface'] = network
         net_info['bridged'] = True
         net_info['ports'] = _get_ports(network, attrs)
@@ -105,12 +104,10 @@
 
 def bridges_caps(running_config):
     ovs_bridges_caps = {}
-    dhcpv4ifaces, dhcpv6ifaces = dhcp.get_dhclient_ifaces()
     routes = netinfo_routes.get_routes()
     for network, attrs in iter_ovs_nets(running_config.networks):
         interface = network if 'vlan' in attrs else BRIDGE_NAME
-        net_info = _get_net_info(attrs, interface, dhcpv4ifaces, dhcpv6ifaces,
-                                 routes)
+        net_info = _get_net_info(interface, routes)
         net_info['bridged'] = True
         net_info['ports'] = _get_ports(network, attrs)
         # TODO netinfo._bridge_options does not work here
@@ -122,13 +119,11 @@
 
 def vlans_caps(running_config):
     ovs_vlans_caps = {}
-    dhcpv4ifaces, dhcpv6ifaces = dhcp.get_dhclient_ifaces()
     routes = netinfo_routes.get_routes()
     for network, attrs in iter_ovs_nets(running_config.networks):
         vlan = attrs.get('vlan')
         if vlan is not None:
-            net_info = _get_net_info(attrs, network, dhcpv4ifaces,
-                                     dhcpv6ifaces, routes)
+            net_info = _get_net_info(network, routes)
             iface = attrs.get('bonding') or attrs.get('nic')
             net_info['iface'] = iface
             net_info['bridged'] = True
@@ -157,12 +152,10 @@
 
 def bondings_caps(running_config):
     ovs_bonding_caps = {}
-    dhcpv4ifaces, dhcpv6ifaces = dhcp.get_dhclient_ifaces()
     routes = netinfo_routes.get_routes()
     for bonding, attrs in iter_ovs_bonds(running_config.bonds):
         options = get_bond_options(attrs.get('options'), keep_custom=True)
-        net_info = _get_net_info(attrs, bonding, dhcpv4ifaces, dhcpv6ifaces,
-                                 routes)
+        net_info = _get_net_info(bonding, routes)
         net_info['slaves'] = attrs.get('nics')
         net_info['active_slave'] = _get_active_slave(bonding)
         net_info['opts'] = options


-- 
To view, visit https://gerrit.ovirt.org/53076
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4d0396610822e3a41d44f37346ee1aad71d569d1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh at redhat.com>


More information about the vdsm-patches mailing list