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