Merge pull request #624 from kytos-ng/release/2023.2.10

release 2023.2.10: fixed request timeout that could indefinitely hang + backported vlan path choose vlan fix

Author: viniarck

CHANGELOG.rst

@@ -6,6 +6,15 @@ All notable changes to the MEF_ELine NApp will be documented in this file.
 [Unreleased]
 ************
 
+[2023.2.10] - 2025-02-12
+************************
+
+Fixed
+=====
+- Parametrized requests timeout when finding paths and when sending flow mods to avoid potentially hanging indefinitely
+- Fixed Path ``choose_vlans`` to be all or nothing, if a path link fails to allocate a vlan, it'll release the allocated vlans.
+
+
 [2023.2.9] - 2025-01-22
 ***********************
 

kytos.json

@@ -3,7 +3,7 @@
   "username": "kytos",
   "name": "mef_eline",
   "description": "NApp to provision circuits from user request",
-  "version": "2023.2.9",
+  "version": "2023.2.10",
   "napp_dependencies": ["kytos/flow_manager", "kytos/pathfinder", "amlight/sndtrace_cp"],
   "license": "MIT",
   "tags": [],

models/evc.py

@@ -11,13 +11,16 @@
 import requests
 from glom import glom
 from requests.exceptions import Timeout
+from tenacity import (retry, retry_if_exception_type, stop_after_attempt,
+                      wait_combine, wait_fixed, wait_random)
 
 from kytos.core import log
 from kytos.core.common import EntityStatus, GenericEntity
 from kytos.core.exceptions import KytosNoTagAvailableError, KytosTagError
 from kytos.core.helpers import get_time, now
 from kytos.core.interface import UNI, Interface, TAGRange
 from kytos.core.link import Link
+from kytos.core.retry import before_sleep
 from kytos.core.tag_ranges import range_difference
 from napps.kytos.mef_eline import controllers, settings
 from napps.kytos.mef_eline.exceptions import (ActivationError,
@@ -1270,21 +1273,33 @@ def _install_uni_flows(self, path=None, skip_in=False, skip_out=False):
             self._send_flow_mods(dpid, flows)
 
     @staticmethod
-    def _send_flow_mods(dpid, flow_mods, command='flows', force=False):
+    @retry(
+        stop=stop_after_attempt(3),
+        wait=wait_combine(wait_fixed(3), wait_random(min=0, max=5)),
+        retry=retry_if_exception_type(FlowModException),
+        before_sleep=before_sleep,
+        reraise=True
+    )
+    def _send_flow_mods(dpid, flow_mods, command='flows', force=False,
+                        timeout=30):
         """Send a flow_mod list to a specific switch.
 
         Args:
             dpid(str): The target of flows (i.e. Switch.id).
             flow_mods(dict): Python dictionary with flow_mods.
             command(str): By default is 'flows'. To remove a flow is 'remove'.
             force(bool): True to send via consistency check in case of errors
+            timeout(int): request timeout
 
         """
 
         endpoint = f"{settings.MANAGER_URL}/{command}/{dpid}"
 
         data = {"flows": flow_mods, "force": force}
-        response = requests.post(endpoint, json=data)
+        try:
+            response = requests.post(endpoint, json=data, timeout=timeout)
+        except requests.exceptions.RequestException as exc:
+            raise FlowModException(str(exc)) from exc
         if response.status_code >= 400:
             raise FlowModException(str(response.text))
 

models/path.py

@@ -1,10 +1,14 @@
 """Classes related to paths"""
 import requests
+from tenacity import (retry, retry_if_exception_type, stop_after_attempt,
+                      wait_combine, wait_fixed, wait_random)
 
 from kytos.core import log
 from kytos.core.common import EntityStatus, GenericEntity
+from kytos.core.exceptions import KytosNoTagAvailableError
 from kytos.core.interface import TAG
 from kytos.core.link import Link
+from kytos.core.retry import before_sleep
 from napps.kytos.mef_eline import settings
 from napps.kytos.mef_eline.exceptions import InvalidPath
 
@@ -34,20 +38,29 @@ def link_affected_by_interface(self, interface=None):
         return None
 
     def choose_vlans(self, controller, old_path_dict: dict = None):
-        """Choose the VLANs to be used for the circuit."""
+        """Choose the VLANs to be used for the circuit.
+        If any of the links next tag isn't available, it'll release
+        all vlans of the path that have been allocated, all or nothing.
+        """
         old_path_dict = old_path_dict if old_path_dict else {}
-        for link in self:
-            tag_value = link.get_next_available_tag(
-                controller, link.id,
-                try_avoid_value=old_path_dict.get(link.id)
-            )
-            tag = TAG('vlan', tag_value)
-            link.add_metadata("s_vlan", tag)
+        try:
+            for link in self:
+                tag_value = link.get_next_available_tag(
+                    controller, link.id,
+                    try_avoid_value=old_path_dict.get(link.id)
+                )
+                tag = TAG('vlan', tag_value)
+                link.add_metadata("s_vlan", tag)
+        except KytosNoTagAvailableError as exc:
+            self.make_vlans_available(controller)
+            raise exc
 
     def make_vlans_available(self, controller):
         """Make the VLANs used in a path available when undeployed."""
         for link in self:
             tag = link.get_metadata("s_vlan")
+            if not tag:
+                continue
             conflict_a, conflict_b = link.make_tags_available(
                 controller, tag.value, link.id, tag.tag_type,
                 check_order=False
@@ -126,7 +139,14 @@ def set_controller(cls, controller=None):
         cls.controller = controller
 
     @staticmethod
-    def get_paths(circuit, max_paths=2, **kwargs) -> list[dict]:
+    @retry(
+        stop=stop_after_attempt(3),
+        wait=wait_combine(wait_fixed(3), wait_random(min=0, max=5)),
+        retry=retry_if_exception_type(requests.exceptions.RequestException),
+        before_sleep=before_sleep,
+        reraise=True
+    )
+    def get_paths(circuit, max_paths=2, timeout=30, **kwargs) -> list[dict]:
         """Get a valid path for the circuit from the Pathfinder."""
         endpoint = settings.PATHFINDER_URL
         spf_attribute = kwargs.get("spf_attribute") or settings.SPF_ATTRIBUTE
@@ -137,7 +157,7 @@ def get_paths(circuit, max_paths=2, **kwargs) -> list[dict]:
             "spf_attribute": spf_attribute
         }
         request_data.update(kwargs)
-        api_reply = requests.post(endpoint, json=request_data)
+        api_reply = requests.post(endpoint, timeout=timeout, json=request_data)
 
         if api_reply.status_code != getattr(requests.codes, "ok"):
             log.error(
@@ -167,8 +187,15 @@ def get_best_path(cls, circuit):
     @classmethod
     def get_best_paths(cls, circuit, **kwargs):
         """Return the best paths available for a circuit, if they exist."""
-        for path in cls.get_paths(circuit, **kwargs):
-            yield cls.create_path(path["hops"])
+        try:
+            for path in cls.get_paths(circuit, **kwargs):
+                yield cls.create_path(path["hops"])
+        except requests.exceptions.RequestException as err:
+            log.error(
+                f"{circuit} failed to get paths from pathfinder. Error {err}"
+            )
+            return
+            yield
 
     @classmethod
     def get_disjoint_paths(

tests/unit/models/test_evc_deploy.py

@@ -123,7 +123,7 @@ def test_send_flow_mods_case1(self, requests_mock):
         expected_data = {"flows": flow_mods, "force": False}
         assert requests_mock.post.call_count == 1
         requests_mock.post.assert_called_once_with(
-            expected_endpoint, json=expected_data
+            expected_endpoint, json=expected_data, timeout=30
         )
 
     @patch("napps.kytos.mef_eline.models.evc.requests")
@@ -142,7 +142,7 @@ def test_send_flow_mods_case2(self, requests_mock):
         expected_data = {"flows": flow_mods, "force": True}
         assert requests_mock.post.call_count == 1
         requests_mock.post.assert_called_once_with(
-            expected_endpoint, json=expected_data
+            expected_endpoint, json=expected_data, timeout=30
         )
 
     @patch("napps.kytos.mef_eline.models.evc.requests")

tests/unit/models/test_path.py

@@ -4,6 +4,7 @@
 import pytest
 from napps.kytos.mef_eline import settings
 
+from kytos.core.exceptions import KytosNoTagAvailableError
 from kytos.core.common import EntityStatus
 from kytos.core.link import Link
 from kytos.core.switch import Switch
@@ -120,6 +121,43 @@ def test_compare_same_paths(self):
         path_2 = Path(links)
         assert path_1 == path_2
 
+    def test_choose_vlans(self) -> None:
+        """Test choose vlans."""
+        controller = MagicMock()
+        link1 = get_link_mocked(status=EntityStatus.UP)
+        link2 = get_link_mocked(status=EntityStatus.UP)
+        link1.id = "def"
+        link2.id = "abc"
+        links = [link1, link2]
+        path = Path(links)
+        path.make_vlans_available = MagicMock()
+        path.choose_vlans(controller)
+        assert link1.get_next_available_tag.call_count == 1
+        assert link1.add_metadata.call_count == 1
+        assert link2.get_next_available_tag.call_count == 1
+        assert link2.add_metadata.call_count == 1
+        assert not path.make_vlans_available.call_count
+
+    def test_choose_vlans_tags_not_available(self) -> None:
+        """Test choose vlans rollback if tags not available."""
+        controller = MagicMock()
+        link1 = get_link_mocked(status=EntityStatus.UP)
+        link2 = get_link_mocked(status=EntityStatus.UP)
+        link1.id = "def"
+        link2.id = "abc"
+        links = [link1, link2]
+        path = Path(links)
+        path.make_vlans_available = MagicMock()
+        exc = KytosNoTagAvailableError(link2)
+        link2.get_next_available_tag.side_effect = exc
+        with pytest.raises(KytosNoTagAvailableError):
+            path.choose_vlans(controller)
+        assert link1.get_next_available_tag.call_count == 1
+        assert link1.add_metadata.call_count == 1
+        assert link2.get_next_available_tag.call_count == 1
+        assert not link2.add_metadata.call_count
+        assert path.make_vlans_available.call_count == 1
+
     def test_compare_different_paths(self):
         """Test compare paths with different links."""
         links_1 = [
@@ -416,6 +454,7 @@ def test_get_best_paths(self, mock_requests_post):
         )
         expected_call = call(
             "http://localhost:8181/api/kytos/pathfinder/v3/",
+            timeout=30,
             json={
                 **{
                     "source": circuit.uni_a.interface.id,
@@ -667,6 +706,7 @@ def test_get_disjoint_paths(self, mock_requests_post, mock_shared):
         max_paths = 10
         expected_call = call(
             "http://localhost:8181/api/kytos/pathfinder/v3/",
+            timeout=30,
             json={
                 **{
                     "source": evc.uni_a.interface.id,