Ido Barkan has uploaded a new change for review. Change subject: context manager for dhclient in functional tests ......................................................................
context manager for dhclient in functional tests the context manager kills dhclient process that outlive the functional test Change-Id: I458aa38415c697d3863e173444ff921d759166a2 Signed-off-by: ibarkan <[email protected]> --- M tests/functional/dhcp.py M tests/functional/networkTests.py M tests/tcTests.py 3 files changed, 80 insertions(+), 41 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/66/34366/1 diff --git a/tests/functional/dhcp.py b/tests/functional/dhcp.py index 28d1220..9895827 100644 --- a/tests/functional/dhcp.py +++ b/tests/functional/dhcp.py @@ -16,9 +16,11 @@ # # Refer to the README and COPYING files for full details of the license # +from contextlib import contextmanager import logging import os +from signal import SIGKILL from time import sleep from nose.plugins.skip import SkipTest @@ -65,29 +67,54 @@ logging.debug(''.join(self.proc.stderr)) -def runDhclient(interface, tmpDir, dateFormat): +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, tmp_dir, date_format): + self._interface = interface + 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') + self._create_conf() - 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: + 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 run(self): + rc, out, err = execCmd([_DHCLIENT_BINARY.cmd, '-v', '-lf', + self.lease_file, '-pf', self._pid_file, + '-timeout', str(_DHCLIENT_TIMEOUT), '-1', + '-cf', self._conf_file, self._interface]) - 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_fd: + self.pid = int(pid_fd.read()) + + def kill(self): + if self.pid: + try: + os.kill(self.pid, SIGKILL) + except OSError: + pass # No such process + + +@contextmanager +def dhclient(interface, tmpDir, dateFormat): + dhclient_runner = _DhclientRunner(interface, tmpDir, dateFormat) + try: + dhclient_runner.run() + yield dhclient_runner + finally: + dhclient_runner.kill() def addNMplaceholderConnection(interface, connection): diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index 34741ab..d1fa007 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -1991,9 +1991,9 @@ with dnsmasqDhcp(server): with namedTemporaryDir(dir='/var/lib/dhclient') as dir: - leaseFile = dhcp.runDhclient(client, dir, dateFormat) - - dhcp4 = getDhclientIfaces([leaseFile]) + with dhcp.dhclient(client, dir, dateFormat) as \ + _dh_client: + dhcp4 = getDhclientIfaces([_dh_client.lease_file]) self.assertIn(client, dhcp4, 'Test iface not found in a lease file.') @@ -2259,6 +2259,11 @@ """When asked to setupNetwork on top of an interface with a running dhclient process, Vdsm is expected to stop that dhclient and start owning the interface. BZ#1100264""" + def _get_dhclient_ifaces(): + pids = pgrep('dhclient') + return [open('/proc/%s/cmdline' % pid).read().strip('\0') + .split('\0')[-1] for pid in pids] + with vethIf() as (server, client): with avoidAnotherDhclient(client): veth.setIP(server, IP_ADDRESS, IP_CIDR) @@ -2267,27 +2272,34 @@ with namedTemporaryDir(dir='/var/lib/dhclient') as dhdir: # Start a non-vdsm owned dhclient for the 'client' # iface - dhcp.runDhclient(client, dhdir, 'default') - # Set up a network over it and wait for dhcp success - status, msg = self.vdsm_net.setupNetworks( - {NETWORK_NAME: {'nic': client, 'bridged': False, - 'bootproto': 'dhcp', 'blockingdhcp': True}}, {}, - NOCHK) - self.assertEquals(status, SUCCESS, msg) - self.assertNetworkExists(NETWORK_NAME) + dhcp._runDhclient(client, dhdir, 'default') + with dhcp.dhclient(client, dhdir, 'default'): + # Set up a network over it and wait for dhcp + # success + status, msg = self.vdsm_net.setupNetworks( + { + NETWORK_NAME: { + 'nic': client, 'bridged': False, + 'bootproto': 'dhcp', + 'blockingdhcp': True + } + }, + {}, + NOCHK) + self.assertEquals(status, SUCCESS, msg) + self.assertNetworkExists(NETWORK_NAME) - # Verify that dhclient is running for the device - pids = pgrep('dhclient') - ifaces = [open('/proc/%s/cmdline' % pid).read(). - strip('\0').split('\0')[-1] for pid in pids] - vdsm_dhclient = [iface for iface in ifaces if - iface == client] - self.assertEqual(len(vdsm_dhclient), 1, - 'There should be one and only one ' - 'running dhclient for the device') + # Verify that dhclient is running for the device + ifaces = _get_dhclient_ifaces() + vdsm_dhclient = [iface for iface in ifaces if + iface == client] + self.assertEqual(len(vdsm_dhclient), 1, + 'There should be one and only ' + 'one running dhclient for the ' + 'device') # cleanup - status, msg = self.vdsm_net.setupNetworks( + self.vdsm_net.setupNetworks( {NETWORK_NAME: {'remove': True}}, {}, NOCHK) @cleanupNet diff --git a/tests/tcTests.py b/tests/tcTests.py index 45ae29a..67a9db0 100644 --- a/tests/tcTests.py +++ b/tests/tcTests.py @@ -354,11 +354,11 @@ when it is attached to an active device. """ - ####[ Ethernet ]### + # ###[ Ethernet ]### # dst = 00:1c:c0:d0:44:dc # src = 00:21:5c:4d:42:75 # type = IPv4 - ####[ IP ]### + # ###[ IP ]### # version = 4L # ihl = 5L # tos = 0x0 @@ -372,13 +372,13 @@ # src = 192.168.0.52 # dst = 192.168.0.3 # \options \ - ####[ ICMP ]### + # ###[ ICMP ]### # type = echo-request # code = 0 # chksum = 0x2875 # id = 0x0 # seq = 0x0 - ####[ Raw ]### + # ###[ Raw ]### # load = '\x01#Eg\x89' _ICMP = unhexlify( '001cc0d044dc''00215c4d4275''0800' # Ethernet -- To view, visit http://gerrit.ovirt.org/34366 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I458aa38415c697d3863e173444ff921d759166a2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
