Ondřej Svoboda has uploaded a new change for review.

Change subject: networkTests: remove a broken approach to unmanaging devices by 
NetworkManager
......................................................................

networkTests: remove a broken approach to unmanaging devices by NetworkManager

In DHCP tests which use a veth pair to simulate DHCP communication (with
dnsmasq and dhclient on the respective sides) it was previously enough to set
an address on the client side to stop NetworkManager from running its own
dhclient on it.

Now this approach is useless. On the server side, NetworkManager automatically
takes the veth up, so we cannot even assign an address to it, and the test is
skipped.

Let's remove the broken code now.

In the following patches we will inform NetworkManager to "unmanage" both
veth sides by passing an ifcfg file with NM_CONTROLLED=no in it.

Change-Id: Idba14753bf9cd37ec1659a49ba7e13b9478f3913
Signed-off-by: Ondřej Svoboda <[email protected]>
---
M tests/functional/dhcp.py
M tests/functional/networkTests.py
2 files changed, 41 insertions(+), 86 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/41/37041/1

diff --git a/tests/functional/dhcp.py b/tests/functional/dhcp.py
index ffbed51..cb08774 100644
--- a/tests/functional/dhcp.py
+++ b/tests/functional/dhcp.py
@@ -32,7 +32,6 @@
 _DNSMASQ_BINARY = CommandPath('dnsmasq', '/usr/sbin/dnsmasq')
 _DHCLIENT_BINARY = CommandPath('dhclient', '/usr/sbin/dhclient',
                                '/sbin/dhclient')
-_NM_CLI_BINARY = CommandPath('nmcli', '/usr/bin/nmcli')
 _START_CHECK_TIMEOUT = 0.5
 _DHCLIENT_TIMEOUT = 10
 _WAIT_FOR_STOP_TIMEOUT = 2
@@ -157,28 +156,6 @@
             else:
                 raise
         return executable == _DHCLIENT_BINARY.cmd
-
-
-def addNMplaceholderConnection(interface, connection):
-    """Creating our own 'connection' with a static address prevents Network
-    Manager from running dhclient on the interface.
-
-    And so it does not interfere with dhclient we are going to run."""
-    rc, out, err = execCmd([_NM_CLI_BINARY.cmd, 'connection', 'add',
-                            'type', 'ethernet', 'ifname', interface,
-                            'con-name', connection, 'autoconnect', 'yes',
-                            'ip4', '12.34.56.78'])
-
-    if rc:
-        raise SkipTest('Could not add a placeholder NM connection.')
-
-
-def removeNMplaceholderConnection(connection):
-    rc, out, err = execCmd([_NM_CLI_BINARY.cmd, 'connection',
-                            'delete', connection])
-
-    if rc:
-        raise DhcpError('Could not remove the placeholder NM connection.')
 
 
 def delete_dhclient_leases(iface, dhcpv4=False, dhcpv6=False):
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index a9e019f..21adf6d 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -125,22 +125,6 @@
 
 
 @contextmanager
-def avoidAnotherDhclient(interface):
-    """Makes sure no other dhclient is run automatically on the interface."""
-    has_nm = pgrep('NetworkManager')
-
-    if has_nm:
-        connectionName = 'placeholder-' + interface
-        dhcp.addNMplaceholderConnection(interface, connectionName)
-
-    try:
-        yield
-    finally:
-        if has_nm:
-            dhcp.removeNMplaceholderConnection(connectionName)
-
-
-@contextmanager
 def firewallDhcp(interface):
     """ Adds and removes firewall rules for DHCP"""
     firewall.allowDhcp(interface)
@@ -1793,23 +1777,21 @@
         dhcpv4_ifaces = set()
         dhcpv6_ifaces = set()
         with vethIf() as (server, client):
-            with avoidAnotherDhclient(client):
+            veth.setIP(server, IP_ADDRESS, IP_CIDR)
+            veth.setIP(server, IPv6_ADDRESS, IPv6_CIDR, 6)
+            veth.setLinkUp(server)
 
-                veth.setIP(server, IP_ADDRESS, IP_CIDR)
-                veth.setIP(server, IPv6_ADDRESS, IPv6_CIDR, 6)
-                veth.setLinkUp(server)
+            with dnsmasqDhcp(server, el6):
 
-                with dnsmasqDhcp(server, el6):
-
-                    with namedTemporaryDir(dir='/var/lib/dhclient') as dir:
-                        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])
-                        except dhcp.ProcessCannotBeKilled:
-                            raise SkipTest('dhclient could not be killed')
+                with namedTemporaryDir(dir='/var/lib/dhclient') as dir:
+                    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])
+                    except dhcp.ProcessCannotBeKilled:
+                        raise SkipTest('dhclient could not be killed')
 
         if family == 4:
             self.assertIn(client, dhcpv4_ifaces,
@@ -2069,39 +2051,35 @@
                     .split('\0')[-1] for pid in pids]
 
         with vethIf() as (server, client):
-            with avoidAnotherDhclient(client):
-                veth.setIP(server, IP_ADDRESS, IP_CIDR)
-                veth.setLinkUp(server)
-                with dnsmasqDhcp(server):
-                    with namedTemporaryDir(dir='/var/lib/dhclient') as dhdir:
-                        # Start a non-vdsm owned dhclient for the 'client'
-                        # iface
-                        dhclient_runner = dhcp.DhclientRunner(
-                            client, 4, dhdir, 'default')
-                        with running(dhclient_runner):
-                            # 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)
+            veth.setIP(server, IP_ADDRESS, IP_CIDR)
+            veth.setLinkUp(server)
+            with dnsmasqDhcp(server):
+                with namedTemporaryDir(dir='/var/lib/dhclient') as dhdir:
+                    # Start a non-vdsm owned dhclient for the 'client' iface
+                    dhclient_runner = dhcp.DhclientRunner(
+                        client, 4, dhdir, 'default')
+                    with running(dhclient_runner):
+                        # 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
-                            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')
+                        # 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
             self.vdsm_net.setupNetworks(


-- 
To view, visit http://gerrit.ovirt.org/37041
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idba14753bf9cd37ec1659a49ba7e13b9478f3913
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to