From 0613b1ab2c50fe9c173e3fd26cf7e9457cd96f10 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Wed, 20 Jul 2022 16:18:27 +0200 Subject: [PATCH] Port update will trigger less notifications to the DHCP agents After port update, DHCP agent will be notified about changes only if one of the port's attributes related somehow to the DHCP will change. Such fields are: * fixed_ips, * MAC address, * dns_domain, * dns_name, * dns_assignment, * extra_dhcp_opts. In other cases there is no reason to send notifications to the agent. This will results with less notifications to the DHCP agent and less possibilities to race condition between DHCP and L2 agents while switching ports from the DOWN to ACTIVE status and sending notifications to nova. Conflicts: neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py Closes-Bug: #1982367 Change-Id: If7990bdec435af76ad2e88fd4ea2bc24a255fd5a (cherry picked from commit 06ddcaf4368ce054a8d396e70880f7dd1abe0562) (cherry picked from commit 14ff8dca34f185eca1ffb50da7e81d7e6eccc3d8) --- .../rpc/agentnotifiers/dhcp_rpc_agent_api.py | 27 ++++++------- .../agentnotifiers/test_dhcp_rpc_agent_api.py | 40 +++++++++++++++---- 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py b/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py index 29b61b2c9ba..b8d01bb1dfe 100644 --- a/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py +++ b/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py @@ -298,26 +298,23 @@ class DhcpAgentNotifyAPI(object): payload = kwargs[resource] data = {resource: payload} if resource == resources.PORT: - if self._only_status_changed(kwargs.get('original_port'), - kwargs.get('port')): + if not self._notification_is_needed(kwargs.get('original_port'), + kwargs.get('port')): # don't waste time updating the DHCP agent for status updates return self.notify(context, data, method_name) - def _only_status_changed(self, orig, new): - # a status change will manifest as a bumped revision number, a new - # updated_at timestamp, and a new status. If that's all that changed, - # return True, else False + def _notification_is_needed(self, orig, new): + # notification to the DHCP agent should be send only if DHCP related + # attributes of the port were changed, like: fixed_ips, mac_address, + # extra_dhcp_opts, dns_name, dns_assignment or dns_domain. + # Otherwise there is no need to waste DHCP agent's time for doing + # updates. if not orig or not new: - return False - if set(orig.keys()) != set(new.keys()): - return False - for k in orig.keys(): - if k in ('status', 'updated_at', 'revision_number'): - continue - if orig[k] != new[k]: - return False - return True + return True + return any(orig.get(k) != new.get(k) for k in ( + 'fixed_ips', 'mac_address', 'extra_dhcp_opts', + 'dns_name', 'dns_assignment', 'dns_domain')) def _send_dhcp_notification(self, resource, event, trigger, payload=None): action = payload.action.split('_')[0] diff --git a/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py b/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py index 0b7650a7c7e..1a956433d26 100644 --- a/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py +++ b/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py @@ -281,17 +281,41 @@ class TestDhcpAgentNotifyAPI(base.BaseTestCase): context=mock.Mock(), **kwargs) self.assertEqual([res], self.notifier._unsubscribed_resources) - def test__only_status_changed(self): + def test__notification_is_needed(self): p1 = {'id': 1, 'status': 'DOWN', 'updated_at': '10:00:00', - 'revision_number': 1} + 'revision_number': 1, + 'fixed_ips': [{'ip_address': '10.0.0.10', 'subnet_id': 'aaa'}], + 'mac_address': 'aa:bb:cc:dd:ee:ff'} p2 = dict(p1) p2['status'] = 'ACTIVE' p2['revision_number'] = 2 p2['updated_at'] = '10:00:01' - self.assertTrue(self.notifier._only_status_changed(p1, p2)) + self.assertFalse(self.notifier._notification_is_needed(p1, p2)) + p2['name'] = 'test' - self.assertFalse(self.notifier._only_status_changed(p1, p2)) - p1['name'] = 'test' - self.assertTrue(self.notifier._only_status_changed(p1, p2)) - p1['name'] = 'test1' - self.assertFalse(self.notifier._only_status_changed(p1, p2)) + self.assertFalse(self.notifier._notification_is_needed(p1, p2)) + p2.pop('name') + + p2['mac_address'] = '11:22:33:44:55:66' + self.assertTrue(self.notifier._notification_is_needed(p1, p2)) + p2['mac_address'] = p1['mac_address'] + + p2['fixed_ips'] = [{'ip_address': '10.0.0.11', 'subnet_id': 'aaa'}] + self.assertTrue(self.notifier._notification_is_needed(p1, p2)) + p2['fixed_ips'] = p1['fixed_ips'] + + p2['extra_dhcp_opts'] = 'some-test-opt' + self.assertTrue(self.notifier._notification_is_needed(p1, p2)) + p2.pop('extra_dhcp_opts') + + p2['dns_name'] = 'test-dns-name' + self.assertTrue(self.notifier._notification_is_needed(p1, p2)) + p2.pop('dns_name') + + p2['dns_assignment'] = 'test-dns-assignment' + self.assertTrue(self.notifier._notification_is_needed(p1, p2)) + p2.pop('dns_assignment') + + p2['dns_domain'] = 'test-dns-domain' + self.assertTrue(self.notifier._notification_is_needed(p1, p2)) + p2.pop('dns_domain')