New subject: Change in vdsm[ovirt-3.5]: net: Force blocking DHCP when restoring networks

5 Apr 2015

Ido Barkan has uploaded a new change for review.
Change subject: network: Force blocking DHCP when restoring networks
......................................................................
network: Force blocking DHCP when restoring networks
This is done right before restoring the network configuration, and
before calling setupNetworks. It forces the configurator to wait for
an IP address to be configured on the devices before restoration is
completed. This prevents VDSM from possibly report missing IP
addresses on interfaces that had been restored right before it was
started.
Change-Id: Ibc8b84a82794ac97eba6d34ec9d54430e387b659
Signed-off-by: Ido Barkan ibarkan@redhat.com
---
M lib/vdsm/utils.py
M tests/functional/dhcp.py
M tests/functional/firewall.py
M tests/functional/networkTests.py
M vdsm/vdsm-restore-net-config
5 files changed, 179 insertions(+), 37 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/29/39529/1

diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py
index 4619b74..8b8e6ec 100644
--- a/lib/vdsm/utils.py
+++ b/lib/vdsm/utils.py
@@ -27,6 +27,7 @@
     Contains a reverse dictionary pointing from error string to its error code.
 """
 from collections import namedtuple, deque
+from contextlib import contextmanager
 from fnmatch import fnmatch
 from SimpleXMLRPCServer import SimpleXMLRPCRequestHandler
 from SimpleXMLRPCServer import SimpleXMLRPCDispatcher
@@ -1289,6 +1290,15 @@
         self._finally.insert(0, (func, args, kwargs))
+@contextmanager
+def running(runnable):
+    runnable.start()
+    try:
+        yield runnable
+    finally:
+        runnable.stop()
+
+
 def get_selinux_enforce_mode():
     """
     Returns the SELinux mode as reported by kernel.
diff --git a/tests/functional/dhcp.py b/tests/functional/dhcp.py
index 28d1220..b913061 100644
--- a/tests/functional/dhcp.py
+++ b/tests/functional/dhcp.py
@@ -19,7 +19,9 @@
import logging
 import os
-from time import sleep
+from signal import SIGKILL, SIGTERM
+from time import sleep, time
+from errno import ENOENT, ESRCH
from nose.plugins.skip import SkipTest
@@ -32,6 +34,7 @@
 _NM_CLI_BINARY = CommandPath('nmcli', '/usr/bin/nmcli')
 _START_CHECK_TIMEOUT = 0.5
 _DHCLIENT_TIMEOUT = 10
+_WAIT_FOR_STOP_TIMEOUT = 2
class DhcpError(Exception):
@@ -42,52 +45,117 @@
     def __init__(self):
         self.proc = None
-    def start(self, interface, dhcpRangeFrom, dhcpRangeTo, router=None):
-        # --dhcp-option=3,<router> advertise specific router (can be None)
-        # -O 6            don't reply with any DNS servers either
-        # -d              do not daemonize and log to stderr
-        # -p 0            disable all the dnsmasq dns functionality
-        self.proc = execCmd([
+    def start(self, interface, dhcp_range_from, dhcp_range_to,
+              dhcpv6_range_from=None, dhcpv6_range_to=None, router=None,
+              bind_dynamic=True):
+        # -p 0                      don't act as a DNS server
+        # --dhcp-option=3,<router>  advertise a specific gateway (or None)
+        # --dhcp-option=6           don't reply with any DNS servers
+        # -d                        don't daemonize and log to stderr
+        # --bind-dynamic            bind only the testing veth iface
+        # (a better, and quiet, version of --bind-interfaces, but not on EL6)
+        command = [
             _DNSMASQ_BINARY.cmd, '--dhcp-authoritative',
-            '-p', '0', '--dhcp-range=' + dhcpRangeFrom + ',' +
-            dhcpRangeTo + ',2m',
-            '--dhcp-option=3' + ',%s' % (router,) if router else '',
-            '-O', '6',
+            '-p', '0',
+            '--dhcp-range={0},{1},2m'.format(dhcp_range_from, dhcp_range_to),
+            '--dhcp-option=3' + (',{0}'.format(router) if router else ''),
+            '--dhcp-option=6',
             '-i', interface, '-I', 'lo', '-d',
-            '--bind-interfaces'], sync=False)
+            '--bind-dynamic' if bind_dynamic else '--bind-interfaces']
+        if dhcpv6_range_from and dhcpv6_range_to:
+            command += ['--dhcp-range={0},{1},2m'.format(dhcpv6_range_from,
+                                                         dhcpv6_range_to)]
+        self.proc = execCmd(command, sync=False)
         sleep(_START_CHECK_TIMEOUT)
         if self.proc.returncode:
-            raise DhcpError('Failed to start dnsmasq DHCP server.' +
-                            ''.join(self.proc.stderr))
+            raise DhcpError('Failed to start dnsmasq DHCP server.\n%s\n%s' %
+                            (''.join(self.proc.stderr), ' '.join(command)))
def stop(self):
         self.proc.kill()
         logging.debug(''.join(self.proc.stderr))
-def runDhclient(interface, tmpDir, dateFormat):
+class ProcessCannotBeKilled(Exception):
+    pass
+
+
+class DhclientRunner(object):
     """On the interface, dhclient is run to obtain a DHCP lease.
-    In the working directory (tmpDir), which is managed by the caller,
-    a lease file is created and a path to it is returned.
-    dhclient accepts the following dateFormats: 'default' and 'local'.
+    In the working directory (tmp_dir), which is managed by the caller.
+    dhclient accepts the following date_formats: 'default' and 'local'.
     """
-    confFile = os.path.join(tmpDir, 'test.conf')
-    pidFile = os.path.join(tmpDir, 'test.pid')
-    leaseFile = os.path.join(tmpDir, 'test.lease')
+    def __init__(self, interface, family, tmp_dir, date_format,
+                 default_route=False):
+        self._interface = interface
+        self._family = family
+        self._date_format = date_format
+        self._conf_file = os.path.join(tmp_dir, 'test.conf')
+        self._pid_file = os.path.join(tmp_dir, 'test.pid')
+        self.pid = None
+        self.lease_file = os.path.join(tmp_dir, 'test.lease')
+        cmd = [_DHCLIENT_BINARY.cmd, '-' + str(family), '-1', '-v',
+               '-timeout', str(_DHCLIENT_TIMEOUT), '-cf', self._conf_file,
+               '-pf', self._pid_file, '-lf', self.lease_file
+               ]
+        if not default_route:
+            # Instruct Fedora/EL's dhclient-script not to set gateway on iface
+            cmd += ['-e', 'DEFROUTE=no']
+        self._cmd = cmd + [self._interface]
-    with open(confFile, 'w') as f:
-        f.write('db-time-format {0};'.format(dateFormat))
+    def _create_conf(self):
+        with open(self._conf_file, 'w') as f:
+            if self._date_format:
+                f.write('db-time-format {0};'.format(self._date_format))
-    rc, out, err = execCmd([_DHCLIENT_BINARY.cmd, '-lf', leaseFile,
-                            '-pf', pidFile, '-timeout', str(_DHCLIENT_TIMEOUT),
-                            '-1', '-cf', confFile, interface])
+    def start(self):
+        self._create_conf()
+        rc, out, err = execCmd(self._cmd)
-    if rc:  # == 2
-        logging.debug(''.join(err))
-        raise DhcpError('dhclient failed to obtain a lease: %d', rc)
+        if rc:  # == 2
+            logging.debug(''.join(err))
+            raise DhcpError('dhclient failed to obtain a lease: %d', rc)
-    return leaseFile
+        with open(self._pid_file) as pid_file:
+            self.pid = int(pid_file.readline())
+
+    def stop(self):
+        if self._try_kill(SIGTERM):
+            return
+        if self._try_kill(SIGKILL):
+            return
+        raise ProcessCannotBeKilled('cmd=%s, pid=%s' % (' '.join(self._cmd),
+                                                        self.pid))
+
+    def _try_kill(self, signal, timeout=_WAIT_FOR_STOP_TIMEOUT):
+        now = time()
+        deadline = now + timeout
+        while now < deadline:
+            try:
+                os.kill(self.pid, signal)
+            except OSError as err:
+                if err.errno != ESRCH:
+                    raise
+                return True  # no such process
+
+            sleep(0.5)
+            if not self._is_running():
+                return True
+            now = time()
+
+        return False
+
+    def _is_running(self):
+        executable_link = '/proc/{0}/exe'.format(self.pid)
+        try:
+            executable = os.readlink(executable_link)
+        except OSError as err:
+            if err.errno == ENOENT:
+                return False  # no such pid
+            else:
+                raise
+        return executable == _DHCLIENT_BINARY.cmd
def addNMplaceholderConnection(interface, connection):
diff --git a/tests/functional/firewall.py b/tests/functional/firewall.py
index ee7bb9f..b703603 100644
--- a/tests/functional/firewall.py
+++ b/tests/functional/firewall.py
@@ -48,10 +48,9 @@
             _execCmdChecker([_FIREWALLD_BINARY.cmd, '--zone=trusted',
                             '--change-interface=' + veth])
         else:
-            raise SkipTest('No firewall service detected.')
+            logging.info('No firewall service detected.')
     except FirewallError as e:
-        raise SkipTest('Failed to allow dhcp traffic in firewall because of '
-                       '%s' % e)
+        raise SkipTest('Failed to allow DHCP traffic in firewall: %s' % e)
def stopAllowingDhcp(veth):
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 075fac4..ef02d3f 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -41,7 +41,7 @@
                             getLinks, routeShowTable)
from vdsm.constants import EXT_BRCTL, EXT_IFUP
-from vdsm.utils import RollbackContext, execCmd
+from vdsm.utils import RollbackContext, execCmd, running
 from vdsm.netinfo import (bridges, operstate, prefix2netmask, getRouteDeviceTo,
                           getDhclientIfaces, BONDING_SLAVES, BONDING_MASTERS,
                           NET_CONF_PREF)
@@ -63,6 +63,8 @@
 IP_GATEWAY = '240.0.0.254'
 DHCP_RANGE_FROM = '240.0.0.10'
 DHCP_RANGE_TO = '240.0.0.100'
+DHCPv6_RANGE_FROM = 'fdb3:84e5:4ff4:55e3::a'
+DHCPv6_RANGE_TO = 'fdb3:84e5:4ff4:55e3::64'
 CUSTOM_PROPS = {'linux': 'rules', 'vdsm': 'as well'}
IPv6_ADDRESS = 'fdb3:84e5:4ff4:55e3::1/64'
@@ -91,12 +93,18 @@
@contextmanager
-def dnsmasqDhcp(interface):
-    """Manages the life cycle of dnsmasq as a DHCP server."""
+def dnsmasqDhcp(interface, el6=False):
+    """Manages the life cycle of dnsmasq as a DHCP server.
+
+    'el6' parameter serves to disable DHCPv6 functionality on EL6 where it is
+    not supported, and avoids warning on --bind-interfaces switch elsewhere."""
     dhcpServer = dhcp.Dnsmasq()
     try:
+        dhcpv6_range_from, dhcpv6_range_to = (
+            (None, None) if el6 else (DHCPv6_RANGE_FROM, DHCPv6_RANGE_TO))
         dhcpServer.start(interface, DHCP_RANGE_FROM, DHCP_RANGE_TO,
-                         router=IP_GATEWAY)
+                         dhcpv6_range_from, dhcpv6_range_to, router=IP_GATEWAY,
+                         bind_dynamic=not el6)
     except dhcp.DhcpError as e:
         raise SkipTest(e)
@@ -176,6 +184,12 @@
             if state != originalState:
                 raise OperStateChangedError('%s operstate changed: %s -> %r' %
                                             (device, originalState, changes))
+
+
+def _system_is_el6():
+    # REQUIRED_FOR: el6
+    return (caps.getos() in (caps.OSName.RHEVH, caps.OSName.RHEL)
+            and caps.osversion()['version'].startswith('6'))
@expandPermutations
@@ -1483,6 +1497,41 @@
             self.vdsm_net.save_config()
@cleanupNet
+    @RequireVethMod
+    @ValidateRunningAsRoot
+    def testRestoreToBlockingDHCP(self):
+        def _get_blocking_dhcp(net_name):
+            self.vdsm_net.refreshNetinfo()
+            return self.vdsm_net.config.networks[net_name].get('blockingdhcp')
+
+        with vethIf() as (server, client):
+            veth.setIP(server, IP_ADDRESS, IP_CIDR)
+            veth.setLinkUp(server)
+            dhcp_async_net = {'nic': client, 'bridged': False,
+                              'bootproto': 'dhcp', 'blockingdhcp': False}
+            status, msg = self.vdsm_net.setupNetworks(
+                {NETWORK_NAME: dhcp_async_net}, {}, NOCHK)
+            self.assertEquals(status, SUCCESS, msg)
+
+            self.assertNetworkExists(NETWORK_NAME)
+            self.assertFalse(_get_blocking_dhcp(NETWORK_NAME))
+
+            self.vdsm_net.save_config()
+
+            with dnsmasqDhcp(server, _system_is_el6()):
+                with namedTemporaryDir(dir='/var/lib/dhclient') as dhdir:
+                    dhclient_runner = dhcp.DhclientRunner(client, 4, dhdir,
+                                                          'default')
+                    with running(dhclient_runner):
+                        self.vdsm_net.restoreNetConfig()
+                        self.assertTrue(_get_blocking_dhcp(NETWORK_NAME))
+
+            # cleanup
+            status, msg = self.vdsm_net.setupNetworks(
+                {NETWORK_NAME: {'remove': True}}, {}, NOCHK)
+            self.assertEquals(status, SUCCESS, msg)
+
+    @cleanupNet
     @permutations([[True], [False]])
     @RequireDummyMod
     @ValidateRunningAsRoot
diff --git a/vdsm/vdsm-restore-net-config b/vdsm/vdsm-restore-net-config
index d7cb8c7..515a03a 100755
--- a/vdsm/vdsm-restore-net-config
+++ b/vdsm/vdsm-restore-net-config
@@ -73,11 +73,27 @@
     persistentConfig = PersistentConfig()
     nets, bonds = _filter_nets_bonds(persistentConfig.networks,
                                      persistentConfig.bonds)
+    _convert_to_blocking_dhcp(nets)
     logging.debug('Calling setupNetworks with networks (%s) and bond (%s).',
                   nets, bonds)
     setupNetworks(nets, bonds, connectivityCheck=False, _inRollback=True)
+def _convert_to_blocking_dhcp(networks):
+    """
+    This function changes DHCP configuration, if present, to be blocking.
+
+    This is done right before restoring the network configuration, and forces
+    the configurator to wait for an IP address to be configured on the devices
+    before restoration is completed. This prevents VDSM to possibly report
+    missing IP address on interfaces that had been restored right before it was
+    started.
+    """
+    for net, net_attr in networks.iteritems():
+        if net_attr.get('bootproto') == 'dhcp':
+            net_attr['blockingdhcp'] = True
+
+
 def _filter_nets_bonds(nets, bonds):
     """Returns only nets and bonds that can be configured with the devices
     present in the system"""
-- 
To view, visit https://gerrit.ovirt.org/39529
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibc8b84a82794ac97eba6d34ec9d54430e387b659
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Ido Barkan ibarkan@redhat.com

    

Change in vdsm[ovirt-3.5]: network: Force blocking DHCP when restoring networks