dhcp: add/use cleanup stale devices API

This is adding new API for the dhcp driver to clean stale
devices. Previously it was not necessary since a dhcp port was related
to a nemaspace and when the namespace got deleted, the device was also
removed.

Now with multisegments we can have more than one dhcp port per
namespace based on segmenation id so we should ensure to remove the
stale device.

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

neutron/agent/dhcp/agent.py

@@ -685,10 +685,12 @@ class DhcpAgent(manager.Manager):
     def _port_delete(self, payload):
         port_id = payload['port_id']
         port = self.cache.get_port_by_id(port_id)
+        network = self.cache.get_network_by_id(payload['network_id'])
         self.cache.add_to_deleted_ports(port_id)
         if not port:
+            # Let's ensure that we clean namespace from stale devices
+            self.call_driver('clean_devices', network)
             return
-        network = self.cache.get_network_by_id(port.network_id)
         self.cache.remove_port(port)
         if self._is_port_on_this_agent(port):
             # the agent's port has been deleted. disable the service

neutron/agent/linux/dhcp.py

@@ -233,6 +233,10 @@ class DhcpBase(object, metaclass=abc.ABCMeta):
         """True if the metadata-proxy should be enabled for the network."""
         raise NotImplementedError()
 
+    @abc.abstractmethod
+    def clean_devices(self, network):
+        """Request to clean unnecessary devices for the network"""
+
 
 class DhcpLocalProcess(DhcpBase, metaclass=abc.ABCMeta):
     PORTS = []
@@ -358,6 +362,10 @@ class DhcpLocalProcess(DhcpBase, metaclass=abc.ABCMeta):
     def spawn_process(self):
         pass
 
+    def clean_devices(self, network):
+        return self.device_manager.cleanup_stale_devices(
+            network, dhcp_port=None)
+
 
 class Dnsmasq(DhcpLocalProcess):
     # The ports that need to be opened when security policies are active
@@ -1676,7 +1684,7 @@ class DeviceManager(object):
         else:
             network.ports.append(port)
 
-    def _cleanup_stale_devices(self, network, dhcp_port):
+    def cleanup_stale_devices(self, network, dhcp_port):
         """Unplug unrelated or stale devices found in the namespace."""
         LOG.debug("Cleaning stale devices for network %s", network.id)
         skip_dev_name = (self.driver.get_device_name(dhcp_port)
@@ -1715,7 +1723,7 @@ class DeviceManager(object):
             with excutils.save_and_reraise_exception():
                 # clear everything out so we don't leave dangling interfaces
                 # if setup never succeeds in the future.
-                self._cleanup_stale_devices(network, dhcp_port=None)
+                self.cleanup_stale_devices(network, dhcp_port=None)
         self._update_dhcp_port(network, port)
         interface_name = self.get_interface_name(network, port)
 
@@ -1783,7 +1791,7 @@ class DeviceManager(object):
                             namespace=network.namespace)
 
         self._set_default_route(network, interface_name)
-        self._cleanup_stale_devices(network, port)
+        self.cleanup_stale_devices(network, port)
 
         return interface_name
 

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

@@ -1422,8 +1422,8 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
             self.dhcp._process_resource_update()
             self.cache.assert_has_calls(
                 [mock.call.get_port_by_id(fake_port2.id),
-                 mock.call.add_to_deleted_ports(fake_port2.id),
                  mock.call.get_network_by_id(fake_network.id),
+                 mock.call.add_to_deleted_ports(fake_port2.id),
                  mock.call.remove_port(fake_port2)])
             self.call_driver.assert_has_calls(
                 [mock.call.call_driver('reload_allocations', fake_network)])
@@ -1441,8 +1441,8 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
             self.dhcp._process_resource_update()
             self.cache.assert_has_calls(
                 [mock.call.get_port_by_id(fake_port2.id),
-                 mock.call.add_to_deleted_ports(fake_port2.id),
                  mock.call.get_network_by_id(fake_network.id),
+                 mock.call.add_to_deleted_ports(fake_port2.id),
                  mock.call.remove_port(fake_port2)])
             self.call_driver.assert_has_calls(
                 [mock.call.call_driver('reload_allocations', fake_network)])
@@ -1452,12 +1452,14 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
         payload = dict(port_id='unknown', network_id='unknown',
                        priority=FAKE_PRIORITY)
         self.cache.get_port_by_id.return_value = None
+        self.cache.get_network_by_id.return_value = fake_network
 
         self.dhcp.port_delete_end(None, payload)
         self.dhcp._process_resource_update()
 
         self.cache.assert_has_calls([mock.call.get_port_by_id('unknown')])
-        self.assertEqual(self.call_driver.call_count, 0)
+        self.call_driver.assert_has_calls(
+            [mock.call.call_driver('clean_devices', fake_network)])
 
     def test_port_delete_end_agents_port(self):
         port = dhcp.DictModel(copy.deepcopy(fake_port1))
@@ -1882,7 +1884,7 @@ class TestDeviceManager(base.BaseTestCase):
 
         dh = dhcp.DeviceManager(cfg.CONF, plugin)
         dh._set_default_route = mock.Mock()
-        dh._cleanup_stale_devices = mock.Mock()
+        dh.cleanup_stale_devices = mock.Mock()
         interface_name = dh.setup(net)
 
         self.assertEqual('tap12345678-12', interface_name)
@@ -1960,7 +1962,7 @@ class TestDeviceManager(base.BaseTestCase):
             net = copy.deepcopy(fake_network)
             plugin.create_dhcp_port.side_effect = exceptions.Conflict()
             dh = dhcp.DeviceManager(cfg.CONF, plugin)
-            clean = mock.patch.object(dh, '_cleanup_stale_devices').start()
+            clean = mock.patch.object(dh, 'cleanup_stale_devices').start()
             with testtools.ExpectedException(exceptions.Conflict):
                 dh.setup(net)
             clean.assert_called_once_with(net, dhcp_port=None)
@@ -1993,7 +1995,7 @@ class TestDeviceManager(base.BaseTestCase):
         self.mock_driver.get_device_name.return_value = 'tap12345678-12'
         dh = dhcp.DeviceManager(cfg.CONF, plugin)
         dh._set_default_route = mock.Mock()
-        dh._cleanup_stale_devices = mock.Mock()
+        dh.cleanup_stale_devices = mock.Mock()
         dh.driver = mock.Mock()
         dh.driver.plug.side_effect = OSError()
         net = copy.deepcopy(fake_network)

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

@@ -1142,6 +1142,9 @@ class TestDhcpBase(TestBase):
             def reload_allocations(self):
                 pass
 
+            def clean_devices(self):
+                pass
+
             @property
             def active(self):
                 return True

releasenotes/notes/new-abstract-method-dhcp-cleanup-4fc973915e3723b8.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    Introducing `clean_devices`, a new DHCP driver's API that can be
+    called to clean stale devices.