dhcp: support multiple segmentations per network

This change makes the DHPC agent to handle multiple segmentation per
network.

For each segmentation a DHCP Process will be started, this has the
benefit to keep the current logic of building a DHCP service per
network domain.

Partial-Bug: #1956435
Partial-Bug: #1764738
Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@industrialdiscipline.com>
Change-Id: I88264ce2303cbaed983d437203232bd1459d58b2

neutron/agent/dhcp/agent.py

@@ -14,6 +14,7 @@
 #    under the License.
 
 import collections
+import copy
 import functools
 import os
 import threading
@@ -198,9 +199,44 @@ class DhcpAgent(manager.Manager):
             eventlet.greenthread.sleep(self.conf.bulk_reload_interval)
 
     def call_driver(self, action, network, **action_kwargs):
+        sid_segment = {}
+        sid_subnets = collections.defaultdict(list)
+        if 'segments' in network and network.segments:
+            # In case of multi-segments network, let's group network per
+            # segments.  We can then create DHPC process per segmentation
+            # id. All subnets on a same network that are sharing the same
+            # segmentation id will be grouped.
+            for segment in network.segments:
+                sid_segment[segment.id] = segment
+            for subnet in network.subnets:
+                sid_subnets[subnet.get('segment_id')].append(subnet)
+        if sid_subnets:
+            ret = []
+            for seg_id, subnets in sid_subnets.items():
+
+                # TODO(sahid): This whole part should be removed in future.
+                segment = sid_segment.get(seg_id)
+                if segment and segment.segment_index == 0:
+                    if action in ['enable', 'disable']:
+                        self._call_driver(
+                            'disable', network, segment=None, block=True)
+
+                net_seg = copy.deepcopy(network)
+                net_seg.subnets = subnets
+                ret.append(self._call_driver(
+                    action, net_seg, segment=sid_segment.get(seg_id),
+                    **action_kwargs))
+            return all(ret)
+        else:
+            # In case subnets are not attached to segments. default behavior.
+            return self._call_driver(
+                action, network, **action_kwargs)
+
+    def _call_driver(self, action, network, segment=None, **action_kwargs):
         """Invoke an action on a DHCP driver instance."""
-        LOG.debug('Calling driver for network: %(net)s action: %(action)s',
+        LOG.debug('Calling driver for network: %(net)s/seg=%(seg)s '
-                  {'net': network.id, 'action': action})
+                  'action: %(action)s',
+                  {'net': network.id, 'action': action, 'seg': segment})
         if self.conf.bulk_reload_interval and action == 'reload_allocations':
             LOG.debug("Call deferred to bulk load")
             self._network_bulk_allocations[network.id] = True
@@ -214,7 +250,8 @@ class DhcpAgent(manager.Manager):
                                           network,
                                           self._process_monitor,
                                           self.dhcp_version,
-                                          self.plugin_rpc)
+                                          self.plugin_rpc,
+                                          segment)
             rv = getattr(driver, action)(**action_kwargs)
             if action == 'get_metadata_bind_interface':
                 return rv

neutron/agent/linux/dhcp.py

@@ -182,12 +182,13 @@ class NetModel(DictModel):
 class DhcpBase(object, metaclass=abc.ABCMeta):
 
     def __init__(self, conf, network, process_monitor,
-                 version=None, plugin=None):
+                 version=None, plugin=None, segment=None):
         self.conf = conf
         self.network = network
         self.process_monitor = process_monitor
         self.device_manager = DeviceManager(self.conf, plugin)
         self.version = version
+        self.segment = segment
 
     @abc.abstractmethod
     def enable(self):
@@ -241,14 +242,43 @@ class DhcpBase(object, metaclass=abc.ABCMeta):
 class DhcpLocalProcess(DhcpBase, metaclass=abc.ABCMeta):
     PORTS = []
 
+    # Track running interfaces.
+    _interfaces = set()
+
     def __init__(self, conf, network, process_monitor, version=None,
-                 plugin=None):
+                 plugin=None, segment=None):
         super(DhcpLocalProcess, self).__init__(conf, network, process_monitor,
-                                               version, plugin)
+                                               version, plugin, segment)
         self.confs_dir = self.get_confs_dir(conf)
-        self.network_conf_dir = os.path.join(self.confs_dir, network.id)
+        if self.segment:
+            # In case of multi-segments support we want a dns process per vlan.
+            self.network_conf_dir = os.path.join(
+                # NOTE(sahid): Path of dhcp conf will be /<segid>/<netid>. We
+                # don't do the opposite so we can clean /<netid>* when calling
+                # disable of the legacy port that is not taking care of
+                # segmentation.
+                self.confs_dir, str(self.segment.segmentation_id), network.id)
+        else:
+            self.network_conf_dir = os.path.join(self.confs_dir, network.id)
+
         fileutils.ensure_tree(self.network_conf_dir, mode=0o755)
 
+    @classmethod
+    def _add_running_interface(cls, interface):
+        """Safe method that add running interface"""
+        cls._interfaces.add(interface)
+
+    @classmethod
+    def _del_running_interface(cls, interface):
+        """Safe method that remove given interface"""
+        if interface in cls._interfaces:
+            cls._interfaces.remove(interface)
+
+    @classmethod
+    def _has_running_interfaces(cls):
+        """Safe method that remove given interface"""
+        return bool(cls._interfaces)
+
     @staticmethod
     def get_confs_dir(conf):
         return os.path.abspath(os.path.normpath(conf.dhcp_confs))
@@ -257,6 +287,14 @@ class DhcpLocalProcess(DhcpBase, metaclass=abc.ABCMeta):
         """Returns the file name for a given kind of config file."""
         return os.path.join(self.network_conf_dir, kind)
 
+    def get_process_uuid(self):
+        if self.segment:
+            # NOTE(sahid): Keep the order to match directory path. This is used
+            # by external_process.ProcessManager to check whether the process
+            # is active.
+            return "%s/%s" % (self.segment.segmentation_id, self.network.id)
+        return self.network.id
+
     def _remove_config_files(self):
         shutil.rmtree(self.network_conf_dir, ignore_errors=True)
 
@@ -287,9 +325,11 @@ class DhcpLocalProcess(DhcpBase, metaclass=abc.ABCMeta):
 
             if self._enable_dhcp():
                 fileutils.ensure_tree(self.network_conf_dir, mode=0o755)
-                interface_name = self.device_manager.setup(self.network)
+                interface_name = self.device_manager.setup(
+                    self.network, self.segment)
                 self.interface_name = interface_name
                 self.spawn_process()
+                self._add_running_interface(self.interface_name)
             return True
         except exceptions.ProcessExecutionError as error:
             LOG.debug("Spawning DHCP process for network %s failed; "
@@ -299,7 +339,7 @@ class DhcpLocalProcess(DhcpBase, metaclass=abc.ABCMeta):
     def _get_process_manager(self, cmd_callback=None):
         return external_process.ProcessManager(
             conf=self.conf,
-            uuid=self.network.id,
+            uuid=self.get_process_uuid(),
             namespace=self.network.namespace,
             service=DNSMASQ_SERVICE_NAME,
             default_cmd_callback=cmd_callback,
@@ -312,22 +352,27 @@ class DhcpLocalProcess(DhcpBase, metaclass=abc.ABCMeta):
         self._get_process_manager().disable()
         if block:
             common_utils.wait_until_true(lambda: not self.active)
+        self._del_running_interface(self.interface_name)
         if not retain_port:
             self._destroy_namespace_and_port()
             self._remove_config_files()
 
     def _destroy_namespace_and_port(self):
+        segmentation_id = (
+            self.segment.segmentation_id if self.segment else None)
         try:
-            self.device_manager.destroy(self.network, self.interface_name)
+            self.device_manager.destroy(
+                self.network, self.interface_name, segmentation_id)
         except RuntimeError:
             LOG.warning('Failed trying to delete interface: %s',
                         self.interface_name)
-
+        if not self._has_running_interfaces():
-        try:
+            # Delete nm only if we don't serve different segmentation id.
-            ip_lib.delete_network_namespace(self.network.namespace)
+            try:
-        except RuntimeError:
+                ip_lib.delete_network_namespace(self.network.namespace)
-            LOG.warning('Failed trying to delete namespace: %s',
+            except RuntimeError:
-                        self.network.namespace)
+                LOG.warning('Failed trying to delete namespace: %s',
+                            self.network.namespace)
 
     def _get_value_from_conf_file(self, kind, converter=None):
         """A helper function to read a value from one of the state files."""
@@ -540,7 +585,7 @@ class Dnsmasq(DhcpLocalProcess):
 
         pm.enable(reload_cfg=reload_with_HUP, ensure_active=True)
 
-        self.process_monitor.register(uuid=self.network.id,
+        self.process_monitor.register(uuid=self.get_process_uuid(),
                                       service_name=DNSMASQ_SERVICE_NAME,
                                       monitored_process=pm)
 
@@ -1428,12 +1473,14 @@ class DeviceManager(object):
         """Return interface(device) name for use by the DHCP process."""
         return self.driver.get_device_name(port)
 
-    def get_device_id(self, network):
+    def get_device_id(self, network, segment=None):
         """Return a unique DHCP device ID for this host on the network."""
         # There could be more than one dhcp server per network, so create
         # a device id that combines host and network ids
+        segmentation_id = segment.segmentation_id if segment else None
         return common_utils.get_dhcp_agent_device_id(network.id,
-                                                     self.conf.host)
+                                                     self.conf.host,
+                                                     segmentation_id)
 
     def _set_default_route_ip_version(self, network, device_name, ip_version):
         device = ip_lib.IPDevice(device_name, namespace=network.namespace)
@@ -1634,11 +1681,11 @@ class DeviceManager(object):
                   {'dhcp_port': dhcp_port,
                    'updated_dhcp_port': updated_dhcp_port})
 
-    def setup_dhcp_port(self, network):
+    def setup_dhcp_port(self, network, segment=None):
         """Create/update DHCP port for the host if needed and return port."""
 
         # The ID that the DHCP port will have (or already has).
-        device_id = self.get_device_id(network)
+        device_id = self.get_device_id(network, segment)
 
         # Get the set of DHCP-enabled local subnets on this network.
         dhcp_subnets = {subnet.id: subnet for subnet in network.subnets
@@ -1715,10 +1762,10 @@ class DeviceManager(object):
                          namespace=network.namespace,
                          mtu=network.get('mtu'))
 
-    def setup(self, network):
+    def setup(self, network, segment=None):
         """Create and initialize a device for network's DHCP on this host."""
         try:
-            port = self.setup_dhcp_port(network)
+            port = self.setup_dhcp_port(network, segment)
         except Exception:
             with excutils.save_and_reraise_exception():
                 # clear everything out so we don't leave dangling interfaces
@@ -1803,7 +1850,7 @@ class DeviceManager(object):
         """Unplug device settings for the network's DHCP on this host."""
         self.driver.unplug(device_name, namespace=network.namespace)
 
-    def destroy(self, network, device_name):
+    def destroy(self, network, device_name, segment=None):
         """Destroy the device used for the network's DHCP on this host."""
         if device_name:
             self.unplug(device_name, network)
@@ -1811,7 +1858,7 @@ class DeviceManager(object):
             LOG.debug('No interface exists for network %s', network.id)
 
         self.plugin.release_dhcp_port(network.id,
-                                      self.get_device_id(network))
+                                      self.get_device_id(network, segment))
 
     def fill_dhcp_udp_checksums(self, namespace):
         """Ensure DHCP reply packets always have correct UDP checksums."""

neutron/tests/fullstack/agents/dhcp_agent.py

@@ -60,11 +60,12 @@ def monkeypatch_dhcplocalprocess_init():
     original_init = linux_dhcp.DhcpLocalProcess.__init__
 
     def new_init(self, conf, network, process_monitor, version=None,
-                 plugin=None):
+                 plugin=None, segment=None):
         network_copy = copy.deepcopy(network)
         network_copy.id = "%s%s" % (network.id, cfg.CONF.test_namespace_suffix)
         original_init(
-            self, conf, network_copy, process_monitor, version, plugin)
+            self, conf, network_copy, process_monitor, version, plugin,
+            segment)
         self.network = network
 
     linux_dhcp.DhcpLocalProcess.__init__ = new_init

neutron/tests/unit/agent/dhcp/test_agent.py

@@ -336,20 +336,23 @@ class TestDhcpAgent(base.BaseTestCase):
                     spawn_n.assert_called_once_with(mocks['_process_loop'])
 
     def test_call_driver(self):
-        network = mock.Mock()
+        network = mock.MagicMock()
         network.id = '1'
+        network.segments = None
         dhcp = dhcp_agent.DhcpAgent(cfg.CONF)
         self.assertTrue(dhcp.call_driver('foo', network))
         self.driver.assert_called_once_with(cfg.CONF,
                                             mock.ANY,
                                             mock.ANY,
                                             mock.ANY,
-                                            mock.ANY)
+                                            mock.ANY,
+                                            None)
 
     def _test_call_driver_failure(self, exc=None,
                                   trace_level='exception', expected_sync=True):
-        network = mock.Mock()
+        network = mock.MagicMock()
         network.id = '1'
+        network.segments = None
         self.driver.return_value.foo.side_effect = exc or Exception
         dhcp = dhcp_agent.DhcpAgent(HOSTNAME)
         with mock.patch.object(dhcp,
@@ -359,7 +362,8 @@ class TestDhcpAgent(base.BaseTestCase):
                                                 mock.ANY,
                                                 mock.ANY,
                                                 mock.ANY,
-                                                mock.ANY)
+                                                mock.ANY,
+                                                None)
             self.assertEqual(expected_sync, schedule_resync.called)
 
     def test_call_driver_ip_address_generation_failure(self):
@@ -387,7 +391,8 @@ class TestDhcpAgent(base.BaseTestCase):
             expected_sync=False)
 
     def test_call_driver_get_metadata_bind_interface_returns(self):
-        network = mock.Mock()
+        network = mock.MagicMock()
+        network.segments = None
         self.driver().get_metadata_bind_interface.return_value = 'iface0'
         agent = dhcp_agent.DhcpAgent(cfg.CONF)
         self.assertEqual(

neutron/tests/unit/agent/linux/test_dhcp.py

@@ -1198,7 +1198,8 @@ class TestDhcpLocalProcess(TestBase):
 
             self.mock_mgr.assert_has_calls(
                 [mock.call(self.conf, None),
-                 mock.call().setup(mock.ANY)])
+                 mock.call().setup(mock.ANY, None),
+                 mock.call().setup(mock.ANY, None)])
             self.assertEqual(2, mocks['interface_name'].__set__.call_count)
             ensure_dir.assert_has_calls([
                 mock.call(
@@ -1223,7 +1224,7 @@ class TestDhcpLocalProcess(TestBase):
                             'delete_network_namespace') as delete_ns:
                 lp.disable()
             lp.device_manager.destroy.assert_called_once_with(
-                network, 'tap0')
+                network, 'tap0', None)
             self._assert_disabled(lp)
 
         delete_ns.assert_called_with('qdhcp-ns')
@@ -1265,7 +1266,8 @@ class TestDhcpLocalProcess(TestBase):
                         'delete_network_namespace') as delete_ns:
             lp.disable(retain_port=False)
 
-        expected = [mock.call.DeviceManager().destroy(mock.ANY, mock.ANY),
+        expected = [mock.call.DeviceManager().destroy(mock.ANY, mock.ANY,
+                                                      mock.ANY),
                     mock.call.rmtree(mock.ANY, ignore_errors=True)]
         parent.assert_has_calls(expected)
         delete_ns.assert_called_with('qdhcp-ns')
@@ -3353,7 +3355,7 @@ class TestDeviceManager(TestConfBase):
                                                     reserved_port_2]
 
         with testtools.ExpectedException(oslo_messaging.RemoteError):
-            dh.setup_dhcp_port(fake_network)
+            dh.setup_dhcp_port(fake_network, None)
 
 
 class TestDictModel(base.BaseTestCase):