From 4356a760322207b23b60da3bffe4ccf5c11fd9c1 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Tue, 10 Sep 2024 12:48:27 -0300 Subject: [PATCH 1/4] hotfix: increased acquired lock scope when handling link up|down --- main.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/main.py b/main.py index cd07b0d..c2e66b4 100644 --- a/main.py +++ b/main.py @@ -52,7 +52,6 @@ def setup(self): DEFAULT_LINK_UP_TIMER) self._links_lock = Lock() - self._links_notify_lock = defaultdict(Lock) # to keep track of potential unorded scheduled interface events self._intfs_lock = defaultdict(Lock) self._intfs_updated_at = {} @@ -989,7 +988,7 @@ def handle_interface_down(self, event): ): return self._intfs_updated_at[interface.id] = event.timestamp - interface.deactivate() + interface.deactivate() self.handle_interface_link_down(interface, event) @listen_to('.*.switch.interface.deleted') @@ -1075,7 +1074,7 @@ def handle_interface_link_up(self, interface, event): ): return self._intfs_updated_at[interface.id] = event.timestamp - self.handle_link_up(interface) + self.handle_link_up(interface) @tenacity.retry( stop=stop_after_attempt(3), @@ -1111,7 +1110,7 @@ def notify_link_up_if_status(self, link, reason="link up") -> None: time.sleep(self.link_up_timer) if link.status != EntityStatus.UP: return - with self._links_notify_lock[link.id]: + with self._links_lock: notified_at = link.get_metadata("notified_up_at") if ( notified_at @@ -1145,7 +1144,11 @@ def handle_link_up(self, interface): link.extend_metadata(metadata) link.activate() self.notify_topology_update() - self.notify_link_up_if_status(link, "link up") + event = KytosEvent( + name="kytos/topology.notify_link_up_if_status", + content={"reason": "link up", "link": link} + ) + self.controller.buffers.app.put(event) @listen_to('.*.switch.interface.link_down') def on_interface_link_down(self, event): @@ -1165,7 +1168,7 @@ def handle_interface_link_down(self, interface, event): ): return self._intfs_updated_at[interface.id] = event.timestamp - self.handle_link_down(interface) + self.handle_link_down(interface) def handle_link_down(self, interface): """Notify a link is down.""" From 034045653d2735400c887cbc5016e540ba491201 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Tue, 10 Sep 2024 12:48:56 -0300 Subject: [PATCH 2/4] feat: included link changed status log info for correlation --- main.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/main.py b/main.py index c2e66b4..092db32 100644 --- a/main.py +++ b/main.py @@ -1312,6 +1312,8 @@ def notify_link_status_change(self, link, reason='not given'): (not link.status_reason and link.status == EntityStatus.UP) and link_id not in self.link_up ): + log.info(f"{link} changed status {link.status}, " + f"reason: {reason}") self.link_up.add(link_id) event = KytosEvent( name='kytos/topology.link_up', @@ -1324,6 +1326,8 @@ def notify_link_status_change(self, link, reason='not given'): (link.status_reason or link.status != EntityStatus.UP) and link_id in self.link_up ): + log.info(f"{link} changed status {link.status}, " + f"reason: {reason}") self.link_up.remove(link_id) event = KytosEvent( name='kytos/topology.link_down', From 932c49bc4ddd3e980667ba7faf5cdfc2526f4106 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Tue, 10 Sep 2024 12:50:47 -0300 Subject: [PATCH 3/4] release: bumped 2024.1.2 --- CHANGELOG.rst | 11 +++++++++++ kytos.json | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 013659b..688010d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,6 +7,17 @@ All notable changes to the ``topology`` project will be documented in this file. [UNRELEASED] - Under development ******************************** +[2024.1.2] - 2024-09-10 +*********************** + +Fixed +===== +- Fixed link down thread safety notification issue + +Added +===== +- Included link changed status log info for correlation + [2024.1.1] - 2024-08-23 *********************** diff --git a/kytos.json b/kytos.json index fc6ed1c..c76599e 100644 --- a/kytos.json +++ b/kytos.json @@ -3,7 +3,7 @@ "username": "kytos", "name": "topology", "description": "Manage the network topology.", - "version": "2024.1.1", + "version": "2024.1.2", "napp_dependencies": ["kytos/of_core", "kytos/of_lldp"], "license": "MIT", "tags": ["topology", "rest"], From c3f81591f2b402fe60404865982a9470698a425c Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Tue, 10 Sep 2024 13:37:01 -0300 Subject: [PATCH 4/4] hotfix: updated unit test accordingly --- tests/unit/test_main.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index 8386249..ded4912 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -1245,12 +1245,10 @@ def test_interface_deleted(self, mock_handle_interface_link_down): mock_handle_interface_link_down.assert_called() @patch('napps.kytos.topology.main.Main._get_link_from_interface') - @patch('napps.kytos.topology.main.Main.notify_link_up_if_status') @patch('napps.kytos.topology.main.Main.notify_topology_update') def test_interface_link_up(self, *args): """Test interface link_up.""" (mock_notify_topology_update, - mock_notify_link_up_if_status, mock_link_from_interface) = args tnow = time.time() @@ -1270,7 +1268,9 @@ def test_interface_link_up(self, *args): mock_notify_topology_update.assert_called() mock_link.extend_metadata.assert_called() mock_link.activate.assert_called() - mock_notify_link_up_if_status.assert_called() + assert self.napp.controller.buffers.app.put.call_count == 2 + ev = "kytos/topology.notify_link_up_if_status" + assert self.napp.controller.buffers.app.put.call_args[0][0].name == ev @patch('napps.kytos.topology.main.Main._get_link_from_interface') @patch('napps.kytos.topology.main.Main.notify_topology_update') @@ -1375,12 +1375,10 @@ def test_handle_link_down_not_active_last_status(self, *args): mock_status_change.assert_called() @patch('napps.kytos.topology.main.Main._get_link_from_interface') - @patch('napps.kytos.topology.main.Main.notify_link_up_if_status') @patch('napps.kytos.topology.main.Main.notify_topology_update') def test_handle_link_up(self, *args): """Test handle link up.""" (mock_notify_topology_update, - mock_notify_link_up_if_status, mock_link_from_interface) = args mock_interface = create_autospec(Interface) @@ -1389,8 +1387,10 @@ def test_handle_link_up(self, *args): mock_link_from_interface.return_value = mock_link self.napp.handle_link_up(mock_interface) mock_interface.activate.assert_not_called() - assert mock_notify_link_up_if_status.call_count == 1 mock_notify_topology_update.assert_called() + assert self.napp.controller.buffers.app.put.call_count == 2 + ev = "kytos/topology.notify_link_up_if_status" + assert self.napp.controller.buffers.app.put.call_args[0][0].name == ev @patch('time.sleep') @patch('napps.kytos.topology.main.Main._get_link_from_interface')