New subject: Change in vdsm[master]: clean dhclient processes that are left after functional tests

Wednesday, 22 October 2014

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 <ibarkan(a)redhat.com&gt;
---
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 <ibarkan(a)redhat.com&gt;

    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Change in vdsm[master]: context manager for dhclient in functional tests