From cb332acb29ec0bc836637098b42ab8692369c6ac Mon Sep 17 00:00:00 2001 From: Sahid Orentino Ferdjaoui Date: Fri, 29 Apr 2022 17:28:03 +0200 Subject: [PATCH] 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 Change-Id: I4a46b034a5feabb914bc6fd121d68e20278230b5 --- neutron/agent/dhcp/agent.py | 4 +++- neutron/agent/linux/dhcp.py | 14 +++++++++++--- neutron/tests/unit/agent/dhcp/test_agent.py | 14 ++++++++------ neutron/tests/unit/agent/linux/test_dhcp.py | 3 +++ ...tract-method-dhcp-cleanup-4fc973915e3723b8.yaml | 5 +++++ 5 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/new-abstract-method-dhcp-cleanup-4fc973915e3723b8.yaml diff --git a/neutron/agent/dhcp/agent.py b/neutron/agent/dhcp/agent.py index 8a1b519bb58..e8d987b46e2 100644 --- a/neutron/agent/dhcp/agent.py +++ b/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 diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index a99e3273492..26bbc7a9c58 100644 --- a/neutron/agent/linux/dhcp.py +++ b/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 diff --git a/neutron/tests/unit/agent/dhcp/test_agent.py b/neutron/tests/unit/agent/dhcp/test_agent.py index c48b5d5dd3e..aa28b93f269 100644 --- a/neutron/tests/unit/agent/dhcp/test_agent.py +++ b/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) diff --git a/neutron/tests/unit/agent/linux/test_dhcp.py b/neutron/tests/unit/agent/linux/test_dhcp.py index a43b6bd4a68..aabebb31377 100644 --- a/neutron/tests/unit/agent/linux/test_dhcp.py +++ b/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 diff --git a/releasenotes/notes/new-abstract-method-dhcp-cleanup-4fc973915e3723b8.yaml b/releasenotes/notes/new-abstract-method-dhcp-cleanup-4fc973915e3723b8.yaml new file mode 100644 index 00000000000..9ed4c58032b --- /dev/null +++ b/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.