Merge pull request #211 from kytos-ng/hotfix/link_status_thread_safety

hotfix 2024.1.2: increased acquired lock scope when handling link up|down

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
 ***********************
 

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"],

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."""
@@ -1309,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',
@@ -1321,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',

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')