Stop dhclient in os-net-config if interface not set for DHCP

As described in https://bugs.launchpad.net/tripleo/+bug/1640598,
there are situations in which the dhclient instance started by
dhcp-all-interfaces runs even after the interface is no longer
configured for DHCP.  This change will terminate the instance
using the '-r' argument if 'BOOTPROTO' is not set to 'dhcp' in
the ifcfg file for the interface.  The dhclient will only be
stopped if a dhclient pid file exists, the pid file will be
removed after stopping dhclient.

Co-Authored-By: Dan Sneddon <dsneddon@redhat.com>
Change-Id: I8a52ef5fb8052f185c01dcc27a1ecd70f5d630c8
Closes-Bug: 1640598

2 files changed, 65 insertions, 0 deletions

diff --git a/os_net_config/impl_ifcfg.py b/os_net_config/impl_ifcfg.py
index 56f2b33..dd3e84f 100644
--- a/os_net_config/impl_ifcfg.py
+++ b/os_net_config/impl_ifcfg.py
@@ -16,6 +16,7 @@
 
 import glob
 import logging
+import os
 import re
 
 import os_net_config
@@ -25,6 +26,9 @@ from os_net_config import utils
 
 logger = logging.getLogger(__name__)
 
+# Import the raw NetConfig object so we can call its methods
+netconfig = os_net_config.NetConfig()
+
 
 def ifcfg_config_path(name):
     return "/etc/sysconfig/network-scripts/ifcfg-%s" % name
@@ -55,6 +59,39 @@ def cleanup_pattern():
     return "/etc/sysconfig/network-scripts/ifcfg-*"
 
 
+def dhclient_path():
+    if os.path.exists("/usr/sbin/dhclient"):
+        return "/usr/sbin/dhclient"
+    elif os.path.exists("/sbin/dhclient"):
+        return "/sbin/dhclient"
+    else:
+        raise RuntimeError("Could not find dhclient")
+
+
+def stop_dhclient_process(interface):
+    """Stop a DHCP process when no longer needed.
+
+    This method exists so that it may be stubbed out for unit tests.
+    :param interface: The interface on which to stop dhclient.
+    """
+    pid_file = '/var/run/dhclient-%s.pid' % (interface)
+    try:
+        dhclient = dhclient_path()
+    except RuntimeError as err:
+        logger.info('Exception when stopping dhclient: %s' % err)
+        return
+
+    if os.path.exists(pid_file):
+        msg = 'Stopping %s on interface %s' % (dhclient, interface)
+        netconfig.execute(msg, dhclient, '-r', '-pf',
+                          pid_file, interface)
+        try:
+            os.unlink(pid_file)
+        except OSError as err:
+            logger.error('Could not remove dhclient pid file \'%s\': %s' %
+                         (pid_file, err))
+
+
 class IfcfgNetConfig(os_net_config.NetConfig):
     """Configure network interfaces using the ifcfg format."""
 
@@ -629,6 +666,7 @@ class IfcfgNetConfig(os_net_config.NetConfig):
         ivs_interfaces = []  # ivs internal ports
         nfvswitch_interfaces = []       # nfvswitch physical interfaces
         nfvswitch_internal_ifaces = []  # nfvswitch internal/management ports
+        stop_dhclient_interfaces = []
 
         for interface_name, iface_data in self.interface_data.iteritems():
             route_data = self.route_data.get(interface_name, '')
@@ -652,6 +690,8 @@ class IfcfgNetConfig(os_net_config.NetConfig):
                 update_files[interface_path] = iface_data
                 update_files[route_path] = route_data
                 update_files[route6_path] = route6_data
+                if "BOOTPROTO=dhcp" not in iface_data:
+                    stop_dhclient_interfaces.append(interface_name)
             else:
                 logger.info('No changes required for interface: %s' %
                             interface_name)
@@ -880,6 +920,10 @@ class IfcfgNetConfig(os_net_config.NetConfig):
             for bridge in restart_bridges:
                 self.ifup(bridge, iftype='bridge')
 
+            # If dhclient is running and dhcp not set, stop dhclient
+            for interface in stop_dhclient_interfaces:
+                stop_dhclient_process(interface)
+
             for interface in restart_interfaces:
                 self.ifup(interface)
 
diff --git a/os_net_config/tests/test_impl_ifcfg.py b/os_net_config/tests/test_impl_ifcfg.py
index 8586daa..228004b 100644
--- a/os_net_config/tests/test_impl_ifcfg.py
+++ b/os_net_config/tests/test_impl_ifcfg.py
@@ -843,6 +843,7 @@ class TestIfcfgNetConfigApply(base.TestCase):
         self.temp_cleanup_file = tempfile.NamedTemporaryFile(delete=False)
         self.ifup_interface_names = []
         self.ovs_appctl_cmds = []
+        self.stop_dhclient_interfaces = []
 
         def test_ifcfg_path(name):
             return self.temp_ifcfg_file.name
@@ -864,6 +865,11 @@ class TestIfcfgNetConfigApply(base.TestCase):
             return self.temp_cleanup_file.name
         self.stubs.Set(impl_ifcfg, 'cleanup_pattern', test_cleanup_pattern)
 
+        def test_stop_dhclient_process(interface):
+            self.stop_dhclient_interfaces.append(interface)
+        self.stubs.Set(impl_ifcfg, 'stop_dhclient_process',
+                       test_stop_dhclient_process)
+
         def test_execute(*args, **kwargs):
             if args[0] == '/sbin/ifup':
                 self.ifup_interface_names.append(args[1])
@@ -913,6 +919,21 @@ class TestIfcfgNetConfigApply(base.TestCase):
         route_data = utils.get_file_data(self.temp_route_file.name)
         self.assertEqual("", route_data)
 
+    def test_dhclient_stop_on_iface_activate(self):
+        self.stop_dhclient_interfaces = []
+        v4_addr = objects.Address('192.168.1.2/24')
+        interface = objects.Interface('em1', addresses=[v4_addr])
+        interface2 = objects.Interface('em2', use_dhcp=True)
+        interface3 = objects.Interface('em3', use_dhcp=False)
+        self.provider.add_interface(interface)
+        self.provider.add_interface(interface2)
+        self.provider.add_interface(interface3)
+        self.provider.apply()
+        # stop dhclient on em1 due to static IP and em3 due to no IP
+        self.assertIn('em1', self.stop_dhclient_interfaces)
+        self.assertIn('em3', self.stop_dhclient_interfaces)
+        self.assertNotIn('em2', self.stop_dhclient_interfaces)
+
     def test_apply_noactivate(self):
         interface = objects.Interface('em1')
         bridge = objects.OvsBridge('br-ctlplane', use_dhcp=True,