From 5c03ec7891b25af8fa866c4517184bedce9d7038 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Thu, 21 Nov 2024 12:32:23 -0500 Subject: [PATCH 1/4] Added support to avoid tag for redeployment --- main.py | 11 ++++-- models/evc.py | 43 +++++++++++++++-------- models/path.py | 7 ++-- openapi.yml | 6 ++++ tests/unit/models/test_evc_deploy.py | 2 +- tests/unit/models/test_link_protection.py | 10 ++++-- 6 files changed, 56 insertions(+), 23 deletions(-) diff --git a/main.py b/main.py index 47dfbaeb..1053f22a 100755 --- a/main.py +++ b/main.py @@ -548,10 +548,14 @@ def delete_metadata(self, request: Request) -> JSONResponse: evc.sync() return JSONResponse("Operation successful") - @rest("/v2/evc/{circuit_id}/redeploy", methods=["PATCH"]) + @rest("/v2/evc/{circuit_id}/redeploy/", methods=["PATCH"]) def redeploy(self, request: Request) -> JSONResponse: """Endpoint to force the redeployment of an EVC.""" circuit_id = request.path_params["circuit_id"] + avoid_vlan = request.query_params.get("avoid_vlan", "false").lower() + if avoid_vlan not in {"true", "false"}: + msg = "Parameter avoid_vlan does not have a valid value." + raise HTTPException(400, detail=msg) log.debug("redeploy /v2/evc/%s/redeploy", circuit_id) try: evc = self.circuits[circuit_id] @@ -563,9 +567,10 @@ def redeploy(self, request: Request) -> JSONResponse: deployed = False if evc.is_enabled(): with evc.lock: - evc.remove_current_flows(sync=False) + path_dict = evc.remove_current_flows( + sync=False, return_path=avoid_vlan == "true") evc.remove_failover_flows(sync=True) - deployed = evc.deploy() + deployed = evc.deploy(path_dict) if deployed: result = {"response": f"Circuit {circuit_id} redeploy received."} status = 202 diff --git a/models/evc.py b/models/evc.py index d13b5a07..71168e39 100644 --- a/models/evc.py +++ b/models/evc.py @@ -579,7 +579,7 @@ def is_using_dynamic_path(self): return True return False - def deploy_to_backup_path(self): + def deploy_to_backup_path(self, path_dict: dict = None): """Deploy the backup path into the datapaths of this circuit. If the backup_path attribute is valid and up, this method will try to @@ -595,17 +595,19 @@ def deploy_to_backup_path(self): success = False if self.backup_path.status is EntityStatus.UP: - success = self.deploy_to_path(self.backup_path) + success = self.deploy_to_path( + self.backup_path, path_dict=path_dict + ) if success: return True if self.dynamic_backup_path or self.is_intra_switch(): - return self.deploy_to_path() + return self.deploy_to_path(path_dict=path_dict) return False - def deploy_to_primary_path(self): + def deploy_to_primary_path(self, path_dict: dict = None): """Deploy the primary path into the datapaths of this circuit. If the primary_path attribute is valid and up, this method will try to @@ -617,10 +619,10 @@ def deploy_to_primary_path(self): return True if self.primary_path.status is EntityStatus.UP: - return self.deploy_to_path(self.primary_path) + return self.deploy_to_path(self.primary_path, path_dict=path_dict) return False - def deploy(self): + def deploy(self, path_dict: dict = None): """Deploy EVC to best path. Best path can be the primary path, if available. If not, the backup @@ -629,9 +631,9 @@ def deploy(self): if self.archived: return False self.enable() - success = self.deploy_to_primary_path() + success = self.deploy_to_primary_path(path_dict) if not success: - success = self.deploy_to_backup_path() + success = self.deploy_to_backup_path(path_dict) if success: emit_event(self._controller, "deployed", @@ -711,13 +713,20 @@ def remove_failover_flows(self, exclude_uni_switches=True, if sync: self.sync() - def remove_current_flows(self, force=True, sync=True): + def remove_current_flows(self, force=True, sync=True, return_path=False): """Remove all flows from current path or path intended for current path if exists.""" - switches = set() + switches, path_dict = set(), {} if not self.current_path and not self.is_intra_switch(): - return + return {} + + if return_path: + for link in self.current_path: + s_vlan = link.metadata.get("s_vlan") + if s_vlan: + path_dict[link.id] = s_vlan.value + current_path = self.current_path for link in current_path: switches.add(link.endpoint_a.switch) @@ -740,10 +749,12 @@ def remove_current_flows(self, force=True, sync=True): current_path.make_vlans_available(self._controller) except KytosTagError as err: log.error(f"Error when removing current path flows: {err}") + return path_dict self.current_path = Path([]) self.deactivate() if sync: self.sync() + return path_dict def remove_path_flows( self, path=None, force=True @@ -827,7 +838,7 @@ def should_deploy(self, path=None): return False # pylint: disable=too-many-branches, too-many-statements - def deploy_to_path(self, path=None): + def deploy_to_path(self, path=None, path_dict=None): """Install the flows for this circuit. Procedures to deploy: @@ -844,10 +855,12 @@ def deploy_to_path(self, path=None): """ self.remove_current_flows(sync=False) use_path = path or Path([]) + if not path_dict: + path_dict = {} tag_errors = [] if self.should_deploy(use_path): try: - use_path.choose_vlans(self._controller) + use_path.choose_vlans(self._controller, path_dict=path_dict) except KytosNoTagAvailableError as e: tag_errors.append(str(e)) use_path = None @@ -856,7 +869,9 @@ def deploy_to_path(self, path=None): if use_path is None: continue try: - use_path.choose_vlans(self._controller) + use_path.choose_vlans( + self._controller, path_dict=path_dict + ) break except KytosNoTagAvailableError as e: tag_errors.append(str(e)) diff --git a/models/path.py b/models/path.py index d7580fa4..f13bdaac 100644 --- a/models/path.py +++ b/models/path.py @@ -36,10 +36,13 @@ def link_affected_by_interface(self, interface=None): return link return None - def choose_vlans(self, controller): + def choose_vlans(self, controller, path_dict: dict = None): """Choose the VLANs to be used for the circuit.""" + path_dict = path_dict if path_dict else {} for link in self: - tag_value = link.get_next_available_tag(controller, link.id) + tag_value = link.get_next_available_tag( + controller, link.id, try_avoid_value=path_dict.get(link.id) + ) tag = TAG('vlan', tag_value) link.add_metadata("s_vlan", tag) diff --git a/openapi.yml b/openapi.yml index af7949fd..d83a7b08 100644 --- a/openapi.yml +++ b/openapi.yml @@ -142,6 +142,12 @@ paths: required: true schema: type: string + - name: avoid_vlan + description: Avoid tags from currently deployed current_path. + in: query + schema: + type: boolean + required: false responses: '202': description: Accepted diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index 0d478464..fce96a10 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -827,7 +827,7 @@ def test_deploy_to_backup_path1( deployed = evc.deploy_to_backup_path() - deploy_to_path_mocked.assert_called_once_with() + deploy_to_path_mocked.assert_called_once_with(path_dict=None) assert deployed is True @patch("httpx.post") diff --git a/tests/unit/models/test_link_protection.py b/tests/unit/models/test_link_protection.py index 5d074feb..a7acae31 100644 --- a/tests/unit/models/test_link_protection.py +++ b/tests/unit/models/test_link_protection.py @@ -449,7 +449,9 @@ async def test_handle_link_up_case_2( current_handle_link_up = evc.handle_link_up(primary_path[0]) assert deploy_mocked.call_count == 0 assert deploy_to_path_mocked.call_count == 1 - deploy_to_path_mocked.assert_called_once_with(evc.primary_path) + deploy_to_path_mocked.assert_called_once_with( + evc.primary_path, path_dict=None + ) assert current_handle_link_up @patch("napps.kytos.mef_eline.models.evc.EVCDeploy.deploy") @@ -514,7 +516,9 @@ async def test_handle_link_up_case_3( assert deploy_mocked.call_count == 0 assert deploy_to_path_mocked.call_count == 1 - deploy_to_path_mocked.assert_called_once_with(evc.backup_path) + deploy_to_path_mocked.assert_called_once_with( + evc.backup_path, path_dict=None + ) assert current_handle_link_up @patch("napps.kytos.mef_eline.models.evc.EVCDeploy.deploy_to_path") @@ -578,7 +582,7 @@ async def test_handle_link_up_case_4(self, *args): current_handle_link_up = evc.handle_link_up(backup_path[0]) assert deploy_to_path_mocked.call_count == 1 - deploy_to_path_mocked.assert_called_once_with() + deploy_to_path_mocked.assert_called_once_with(path_dict=None) assert current_handle_link_up async def test_handle_link_up_case_5(self): From 9d4aaef50f296e433823861a7f09e28f9e1c9200 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Tue, 3 Dec 2024 09:51:49 -0500 Subject: [PATCH 2/4] Renamed variables - Now avoiding feature is true by default. --- main.py | 11 ++++-- models/evc.py | 45 ++++++++++++----------- models/path.py | 7 ++-- tests/unit/models/test_evc_deploy.py | 2 +- tests/unit/models/test_link_protection.py | 10 ++--- 5 files changed, 38 insertions(+), 37 deletions(-) diff --git a/main.py b/main.py index 1053f22a..363ae0de 100755 --- a/main.py +++ b/main.py @@ -548,12 +548,13 @@ def delete_metadata(self, request: Request) -> JSONResponse: evc.sync() return JSONResponse("Operation successful") - @rest("/v2/evc/{circuit_id}/redeploy/", methods=["PATCH"]) + @rest("/v2/evc/{circuit_id}/redeploy", methods=["PATCH"]) def redeploy(self, request: Request) -> JSONResponse: """Endpoint to force the redeployment of an EVC.""" circuit_id = request.path_params["circuit_id"] - avoid_vlan = request.query_params.get("avoid_vlan", "false").lower() - if avoid_vlan not in {"true", "false"}: + try_avoid_same_s_vlan = request.query_params.get("avoid_vlan", "true") + try_avoid_same_s_vlan = try_avoid_same_s_vlan.lower() + if try_avoid_same_s_vlan not in {"true", "false"}: msg = "Parameter avoid_vlan does not have a valid value." raise HTTPException(400, detail=msg) log.debug("redeploy /v2/evc/%s/redeploy", circuit_id) @@ -568,7 +569,9 @@ def redeploy(self, request: Request) -> JSONResponse: if evc.is_enabled(): with evc.lock: path_dict = evc.remove_current_flows( - sync=False, return_path=avoid_vlan == "true") + sync=False, + return_path=try_avoid_same_s_vlan == "true" + ) evc.remove_failover_flows(sync=True) deployed = evc.deploy(path_dict) if deployed: diff --git a/models/evc.py b/models/evc.py index 71168e39..adaa3687 100644 --- a/models/evc.py +++ b/models/evc.py @@ -579,7 +579,7 @@ def is_using_dynamic_path(self): return True return False - def deploy_to_backup_path(self, path_dict: dict = None): + def deploy_to_backup_path(self, old_path_dict: dict = None): """Deploy the backup path into the datapaths of this circuit. If the backup_path attribute is valid and up, this method will try to @@ -595,19 +595,17 @@ def deploy_to_backup_path(self, path_dict: dict = None): success = False if self.backup_path.status is EntityStatus.UP: - success = self.deploy_to_path( - self.backup_path, path_dict=path_dict - ) + success = self.deploy_to_path(self.backup_path, old_path_dict) if success: return True if self.dynamic_backup_path or self.is_intra_switch(): - return self.deploy_to_path(path_dict=path_dict) + return self.deploy_to_path(old_path_dict=old_path_dict) return False - def deploy_to_primary_path(self, path_dict: dict = None): + def deploy_to_primary_path(self, old_path_dict: dict = None): """Deploy the primary path into the datapaths of this circuit. If the primary_path attribute is valid and up, this method will try to @@ -619,10 +617,10 @@ def deploy_to_primary_path(self, path_dict: dict = None): return True if self.primary_path.status is EntityStatus.UP: - return self.deploy_to_path(self.primary_path, path_dict=path_dict) + return self.deploy_to_path(self.primary_path, old_path_dict) return False - def deploy(self, path_dict: dict = None): + def deploy(self, old_path_dict: dict = None): """Deploy EVC to best path. Best path can be the primary path, if available. If not, the backup @@ -631,9 +629,9 @@ def deploy(self, path_dict: dict = None): if self.archived: return False self.enable() - success = self.deploy_to_primary_path(path_dict) + success = self.deploy_to_primary_path(old_path_dict) if not success: - success = self.deploy_to_backup_path(path_dict) + success = self.deploy_to_backup_path(old_path_dict) if success: emit_event(self._controller, "deployed", @@ -713,10 +711,15 @@ def remove_failover_flows(self, exclude_uni_switches=True, if sync: self.sync() - def remove_current_flows(self, force=True, sync=True, return_path=False): + def remove_current_flows( + self, + force=True, + sync=True, + return_path=False + ) -> dict[str, int]: """Remove all flows from current path or path intended for current path if exists.""" - switches, path_dict = set(), {} + switches, old_path_dict = set(), {} if not self.current_path and not self.is_intra_switch(): return {} @@ -725,7 +728,7 @@ def remove_current_flows(self, force=True, sync=True, return_path=False): for link in self.current_path: s_vlan = link.metadata.get("s_vlan") if s_vlan: - path_dict[link.id] = s_vlan.value + old_path_dict[link.id] = s_vlan.value current_path = self.current_path for link in current_path: @@ -749,12 +752,12 @@ def remove_current_flows(self, force=True, sync=True, return_path=False): current_path.make_vlans_available(self._controller) except KytosTagError as err: log.error(f"Error when removing current path flows: {err}") - return path_dict + return old_path_dict self.current_path = Path([]) self.deactivate() if sync: self.sync() - return path_dict + return old_path_dict def remove_path_flows( self, path=None, force=True @@ -838,7 +841,7 @@ def should_deploy(self, path=None): return False # pylint: disable=too-many-branches, too-many-statements - def deploy_to_path(self, path=None, path_dict=None): + def deploy_to_path(self, path=None, old_path_dict: dict = None): """Install the flows for this circuit. Procedures to deploy: @@ -855,12 +858,12 @@ def deploy_to_path(self, path=None, path_dict=None): """ self.remove_current_flows(sync=False) use_path = path or Path([]) - if not path_dict: - path_dict = {} + if not old_path_dict: + old_path_dict = {} tag_errors = [] if self.should_deploy(use_path): try: - use_path.choose_vlans(self._controller, path_dict=path_dict) + use_path.choose_vlans(self._controller, old_path_dict) except KytosNoTagAvailableError as e: tag_errors.append(str(e)) use_path = None @@ -869,9 +872,7 @@ def deploy_to_path(self, path=None, path_dict=None): if use_path is None: continue try: - use_path.choose_vlans( - self._controller, path_dict=path_dict - ) + use_path.choose_vlans(self._controller, old_path_dict) break except KytosNoTagAvailableError as e: tag_errors.append(str(e)) diff --git a/models/path.py b/models/path.py index f13bdaac..f03770cf 100644 --- a/models/path.py +++ b/models/path.py @@ -36,12 +36,13 @@ def link_affected_by_interface(self, interface=None): return link return None - def choose_vlans(self, controller, path_dict: dict = None): + def choose_vlans(self, controller, old_path_dict: dict = None): """Choose the VLANs to be used for the circuit.""" - path_dict = path_dict if path_dict else {} + 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=path_dict.get(link.id) + controller, link.id, + try_avoid_value=old_path_dict.get(link.id) ) tag = TAG('vlan', tag_value) link.add_metadata("s_vlan", tag) diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index fce96a10..85d233aa 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -827,7 +827,7 @@ def test_deploy_to_backup_path1( deployed = evc.deploy_to_backup_path() - deploy_to_path_mocked.assert_called_once_with(path_dict=None) + deploy_to_path_mocked.assert_called_once_with(old_path_dict=None) assert deployed is True @patch("httpx.post") diff --git a/tests/unit/models/test_link_protection.py b/tests/unit/models/test_link_protection.py index a7acae31..8b8d3fe3 100644 --- a/tests/unit/models/test_link_protection.py +++ b/tests/unit/models/test_link_protection.py @@ -449,9 +449,7 @@ async def test_handle_link_up_case_2( current_handle_link_up = evc.handle_link_up(primary_path[0]) assert deploy_mocked.call_count == 0 assert deploy_to_path_mocked.call_count == 1 - deploy_to_path_mocked.assert_called_once_with( - evc.primary_path, path_dict=None - ) + deploy_to_path_mocked.assert_called_once_with(evc.primary_path, None) assert current_handle_link_up @patch("napps.kytos.mef_eline.models.evc.EVCDeploy.deploy") @@ -516,9 +514,7 @@ async def test_handle_link_up_case_3( assert deploy_mocked.call_count == 0 assert deploy_to_path_mocked.call_count == 1 - deploy_to_path_mocked.assert_called_once_with( - evc.backup_path, path_dict=None - ) + deploy_to_path_mocked.assert_called_once_with(evc.backup_path, None) assert current_handle_link_up @patch("napps.kytos.mef_eline.models.evc.EVCDeploy.deploy_to_path") @@ -582,7 +578,7 @@ async def test_handle_link_up_case_4(self, *args): current_handle_link_up = evc.handle_link_up(backup_path[0]) assert deploy_to_path_mocked.call_count == 1 - deploy_to_path_mocked.assert_called_once_with(path_dict=None) + deploy_to_path_mocked.assert_called_once_with(old_path_dict=None) assert current_handle_link_up async def test_handle_link_up_case_5(self): From 7ef3241e6101376fbe03017bb6d0c455ada4d3d3 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Tue, 3 Dec 2024 19:07:06 -0500 Subject: [PATCH 3/4] Changed paramater from avoid_vlan to try_avoid_same_s_vlan --- main.py | 6 ++-- openapi.yml | 2 +- tests/unit/models/test_evc_deploy.py | 41 ++++++++++++++++------------ tests/unit/test_main.py | 4 ++- 4 files changed, 31 insertions(+), 22 deletions(-) diff --git a/main.py b/main.py index 363ae0de..fd552654 100755 --- a/main.py +++ b/main.py @@ -552,10 +552,12 @@ def delete_metadata(self, request: Request) -> JSONResponse: def redeploy(self, request: Request) -> JSONResponse: """Endpoint to force the redeployment of an EVC.""" circuit_id = request.path_params["circuit_id"] - try_avoid_same_s_vlan = request.query_params.get("avoid_vlan", "true") + try_avoid_same_s_vlan = request.query_params.get( + "try_avoid_same_s_vlan", "true" + ) try_avoid_same_s_vlan = try_avoid_same_s_vlan.lower() if try_avoid_same_s_vlan not in {"true", "false"}: - msg = "Parameter avoid_vlan does not have a valid value." + msg = "Parameter try_avoid_same_s_vlan has an invalid value." raise HTTPException(400, detail=msg) log.debug("redeploy /v2/evc/%s/redeploy", circuit_id) try: diff --git a/openapi.yml b/openapi.yml index d83a7b08..2d18c425 100644 --- a/openapi.yml +++ b/openapi.yml @@ -142,7 +142,7 @@ paths: required: true schema: type: string - - name: avoid_vlan + - name: try_avoid_same_s_vlan description: Avoid tags from currently deployed current_path. in: query schema: diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index 157e0bf9..cbbc6dd0 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -838,6 +838,20 @@ def test_remove_current_flows(self, *args): switch_a = Switch("00:00:00:00:00:01") switch_b = Switch("00:00:00:00:00:02") switch_c = Switch("00:00:00:00:00:03") + link_a_b = get_link_mocked( + switch_a=switch_a, + switch_b=switch_b, + endpoint_a_port=9, + endpoint_b_port=10, + metadata={"s_vlan": Mock(value=5)}, + ) + link_b_c = get_link_mocked( + switch_a=switch_b, + switch_b=switch_c, + endpoint_a_port=11, + endpoint_b_port=12, + metadata={"s_vlan": Mock(value=6)}, + ) attributes = { "controller": get_controller_mock(), @@ -846,29 +860,20 @@ def test_remove_current_flows(self, *args): "uni_z": uni_z, "active": True, "enabled": True, - "primary_links": [ - get_link_mocked( - switch_a=switch_a, - switch_b=switch_b, - endpoint_a_port=9, - endpoint_b_port=10, - metadata={"s_vlan": 5}, - ), - get_link_mocked( - switch_a=switch_b, - switch_b=switch_c, - endpoint_a_port=11, - endpoint_b_port=12, - metadata={"s_vlan": 6}, - ), - ], + "primary_links": [link_a_b, link_b_c] + + } + + expected_old_path = { + link_a_b.id: 5, + link_b_c.id: 6 } evc = EVC(**attributes) evc.current_path = evc.primary_links - evc.remove_current_flows() - + old_path = evc.remove_current_flows(return_path=True) + assert old_path == expected_old_path assert send_flow_mods_mocked.call_count == 1 assert evc.is_active() is False flows = [ diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index 4adbc10a..36710624 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -828,7 +828,9 @@ async def test_redeploy_evc(self): url = f"{self.base_endpoint}/v2/evc/1/redeploy" response = await self.api_client.patch(url) evc1.remove_failover_flows.assert_called() - evc1.remove_current_flows.assert_called() + evc1.remove_current_flows.assert_called_with( + sync=False, return_path=True + ) assert response.status_code == 202, response.data async def test_redeploy_evc_disabled(self): From 582a743834e1aadeb1c713bed58c44c62519bbe0 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Tue, 3 Dec 2024 19:18:46 -0500 Subject: [PATCH 4/4] Added more unit tests for `try_avoid_same_s_vlan` --- tests/unit/test_main.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index 36710624..3e2150da 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -833,6 +833,20 @@ async def test_redeploy_evc(self): ) assert response.status_code == 202, response.data + url = f"{self.base_endpoint}/v2/evc/1/redeploy" + url = url + "?try_avoid_same_s_vlan=false" + response = await self.api_client.patch(url) + evc1.remove_current_flows.assert_called_with( + sync=False, return_path=False + ) + + url = f"{self.base_endpoint}/v2/evc/1/redeploy" + url = url + "?try_avoid_same_s_vlan=True" + response = await self.api_client.patch(url) + evc1.remove_current_flows.assert_called_with( + sync=False, return_path=True + ) + async def test_redeploy_evc_disabled(self): """Test endpoint to redeploy an EVC.""" evc1 = MagicMock()