From 3feb16f3816c2754a544e58508f15380d1f59483 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Wed, 24 Jul 2024 09:09:34 -0300 Subject: [PATCH 01/34] feat: validate shared proxy port --- exceptions.py | 5 +++++ main.py | 10 ++++++++-- managers/int.py | 50 ++++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/exceptions.py b/exceptions.py index 6ddc1e5..0cdf26e 100644 --- a/exceptions.py +++ b/exceptions.py @@ -53,6 +53,11 @@ class ProxyPortSameSourceIntraEVC(ProxyPortError): """ +class ProxyPortShared(ProxyPortError): + """ProxyPortShared. A shared proxy port isn't supported for now. + Each uni should have its own proxy port""" + + class EVCHasNoINT(EVCError): """Exception in case the EVC doesn't have INT enabled.""" diff --git a/main.py b/main.py index cbe01c5..73ff6a2 100644 --- a/main.py +++ b/main.py @@ -24,6 +24,7 @@ FlowsNotFound, ProxyPortNotFound, ProxyPortSameSourceIntraEVC, + ProxyPortShared, ProxyPortStatusNotUP, UnrecoverableError, ) @@ -115,7 +116,12 @@ async def enable_telemetry(self, request: Request) -> JSONResponse: await self.int_manager.enable_int(evcs, force) except (EVCNotFound, FlowsNotFound, ProxyPortNotFound) as exc: raise HTTPException(404, detail=str(exc)) - except (EVCHasINT, ProxyPortStatusNotUP, ProxyPortSameSourceIntraEVC) as exc: + except ( + EVCHasINT, + ProxyPortStatusNotUP, + ProxyPortSameSourceIntraEVC, + ProxyPortShared, + ) as exc: raise HTTPException(409, detail=str(exc)) except RetryError as exc: exc_error = str(exc.last_attempt.exception()) @@ -232,7 +238,7 @@ async def redeploy_telemetry(self, request: Request) -> JSONResponse: await self.int_manager.redeploy_int(evcs) except (EVCNotFound, FlowsNotFound, ProxyPortNotFound) as exc: raise HTTPException(404, detail=str(exc)) - except (EVCHasNoINT, ProxyPortSameSourceIntraEVC) as exc: + except (EVCHasNoINT, ProxyPortSameSourceIntraEVC, ProxyPortShared) as exc: raise HTTPException(409, detail=str(exc)) except RetryError as exc: exc_error = str(exc.last_attempt.exception()) diff --git a/managers/int.py b/managers/int.py index 9a34226..ed32dff 100644 --- a/managers/int.py +++ b/managers/int.py @@ -31,6 +31,7 @@ ProxyPortDestNotFound, ProxyPortNotFound, ProxyPortSameSourceIntraEVC, + ProxyPortShared, ) @@ -273,17 +274,22 @@ async def handle_pp_metadata_added(self, intf: Interface) -> None: "EVCs to be safe, and then try to enable again with the updated " f" proxy port {pp}, EVC ids: {list(affected_evcs)}" ) - await self.disable_int(affected_evcs, force=True) + await self.disable_int( + affected_evcs, force=True, reason="proxy_port_metadata_added" + ) try: await self.enable_int(affected_evcs, force=True) - except ProxyPortSameSourceIntraEVC as exc: + except (ProxyPortSameSourceIntraEVC, ProxyPortShared) as exc: + # TODO update metdata msg = ( - f"Validation error when updating interface {intf} proxy port {pp}" + f"Validation error when updating interface {intf}" f" EVC ids: {list(affected_evcs)}, exception {str(exc)}" ) log.error(msg) - async def disable_int(self, evcs: dict[str, dict], force=False) -> None: + async def disable_int( + self, evcs: dict[str, dict], force=False, reason="disabled" + ) -> None: """Disable INT on EVCs. evcs is a dict of prefetched EVCs from mef_eline based on evc_ids. @@ -302,7 +308,7 @@ async def disable_int(self, evcs: dict[str, dict], force=False) -> None: "telemetry": { "enabled": False, "status": "DOWN", - "status_reason": ["disabled"], + "status_reason": [reason], "status_updated_at": datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%S"), } } @@ -499,6 +505,35 @@ def _validate_intra_evc_different_proxy_ports(self, evc: dict) -> None: evc["id"], "intra EVC UNIs must use different proxy ports" ) + def _validate_dedicated_proxy_port_evcs(self, evcs: dict[str, dict]): + """Validate that a proxy port is dedicated for the given EVCs. + + https://github.com/kytos-ng/telemetry_int/issues/110 + """ + seen_src_unis: dict[str, str] = {} + for evc in evcs.values(): + pp_a, unia_id = evc["uni_a"]["proxy_port"], evc["uni_a"]["interface_id"] + pp_z, uniz_id = evc["uni_z"]["proxy_port"], evc["uni_z"]["interface_id"] + for cur_uni_id, cur_src_id in self.unis_src.items(): + for uni_id, pp in zip((unia_id, uniz_id), (pp_a, pp_z)): + if uni_id != cur_uni_id and cur_src_id == pp.source.id: + msg = ( + f"UNI {uni_id} must use another dedicated proxy port. " + f"UNI {cur_uni_id} is using {pp}" + ) + raise ProxyPortShared(evc["id"], msg) + + # This is needed to validate the EVCs of the current request + # since self.uni_src only gets updated when a EVC gets enabled + for uni_id, pp in zip((unia_id, uniz_id), (pp_a, pp_z)): + if (found := seen_src_unis.get(pp.source.id)) and found != uni_id: + msg = ( + f"UNI {uni_id} must use another dedicated proxy port. " + f"UNI {found} would use {pp}" + ) + raise ProxyPortShared(evc["id"], msg) + seen_src_unis[pp_a.source.id] = uni_id + async def handle_failover_flows( self, evcs_content: dict[str, dict], event_name: str ) -> None: @@ -630,6 +665,7 @@ def _validate_map_enable_evcs( ) self._validate_intra_evc_different_proxy_ports(evc) + self._validate_dedicated_proxy_port_evcs(evcs) return evcs def _validate_has_int(self, evcs: dict[str, dict]): @@ -662,6 +698,10 @@ def _discard_pps_evc_ids(self, evcs: dict[str, dict]) -> None: pp_z = self.get_proxy_port_or_raise(uni_z["interface_id"], evc_id) pp_a.evc_ids.discard(evc_id) pp_z.evc_ids.discard(evc_id) + if not pp_a.evc_ids: + self.unis_src.pop(evc["uni_a"]["interface_id"], None) + if not pp_z.evc_ids: + self.unis_src.pop(evc["uni_z"]["interface_id"], None) def evc_compare( self, stored_int_flows: dict, stored_mef_flows: dict, evcs: dict From afcdb21760a40b19554b48a09a0a41a77da51a62 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Thu, 25 Jul 2024 11:18:38 -0300 Subject: [PATCH 02/34] tests: covered with unit tests --- tests/unit/test_int_manager.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/unit/test_int_manager.py b/tests/unit/test_int_manager.py index 488ceba..191372c 100644 --- a/tests/unit/test_int_manager.py +++ b/tests/unit/test_int_manager.py @@ -4,6 +4,7 @@ from unittest.mock import AsyncMock, MagicMock from napps.kytos.telemetry_int.exceptions import ProxyPortSameSourceIntraEVC +from napps.kytos.telemetry_int.exceptions import ProxyPortShared from napps.kytos.telemetry_int.managers.int import INTManager from napps.kytos.telemetry_int import exceptions from kytos.core.common import EntityStatus @@ -440,6 +441,37 @@ def test_validate_intra_evc_different_proxy_ports(self) -> None: with pytest.raises(ProxyPortSameSourceIntraEVC): int_manager._validate_intra_evc_different_proxy_ports(evc) + def test_validate_dedicated_proxy_port_evcs(self) -> None: + """Test _validate_intra_evc_different_proxy_ports.""" + pp_a, pp_z, controller = MagicMock(), MagicMock(), MagicMock() + evc = { + "id": "some_id", + "uni_a": {"proxy_port": pp_a, "interface_id": "00:00:00:00:00:00:00:01:1"}, + "uni_z": {"proxy_port": pp_z, "interface_id": "00:00:00:00:00:00:00:01:2"}, + } + + int_manager = INTManager(controller) + int_manager._validate_dedicated_proxy_port_evcs({evc["id"]: evc}) + + source = MagicMock() + pp_a.source, pp_z.source = source, source + with pytest.raises(ProxyPortShared): + int_manager._validate_dedicated_proxy_port_evcs({evc["id"]: evc}) + + def test_validate_dedicated_proxy_port_evcs_existing(self) -> None: + """Test _validate_intra_evc_different_proxy_ports existing.""" + pp_a, pp_z, controller = MagicMock(), MagicMock(), MagicMock() + evc = { + "id": "some_id", + "uni_a": {"proxy_port": pp_a, "interface_id": "00:00:00:00:00:00:00:01:1"}, + "uni_z": {"proxy_port": pp_z, "interface_id": "00:00:00:00:00:00:00:01:2"}, + } + + int_manager = INTManager(controller) + int_manager.unis_src["00:00:00:00:00:00:00:01:3"] = pp_a.source.id + with pytest.raises(ProxyPortShared): + int_manager._validate_dedicated_proxy_port_evcs({evc["id"]: evc}) + async def test__remove_int_flows_by_cookies( self, inter_evc_evpl_flows_data ) -> None: From 707aef367f5d44227af8b5ab50f51d5e6acd6038 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Thu, 25 Jul 2024 11:19:05 -0300 Subject: [PATCH 03/34] chore: updated batch to keep at 100 flows / sec at most like mef_eline for now --- settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/settings.py b/settings.py index a29bafb..b2e9cc0 100644 --- a/settings.py +++ b/settings.py @@ -13,12 +13,12 @@ # BATCH_INTERVAL: time interval between batch requests that will be sent to # flow_manager (in seconds) - zero enable sending all the requests in a row -BATCH_INTERVAL = 0.2 +BATCH_INTERVAL = 0.5 # BATCH_SIZE: size of a batch request that will be sent to flow_manager, in # number of FlowMod requests. Use 0 (zero) to disable BATCH mode, i.e. sends # everything at a glance -BATCH_SIZE = 200 +BATCH_SIZE = 50 # Fallback to mef_eline by removing INT flows if an external loop goes down. If # the loop goes UP again and the EVC is active, it'll install INT flows From 144601f6e37067b60d638fafb0f7ad46e4d54090 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Thu, 25 Jul 2024 11:21:29 -0300 Subject: [PATCH 04/34] updated changelog --- CHANGELOG.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 4310db8..08f40e5 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -20,6 +20,7 @@ Added Changed ======= - Only raise ``FlowsNotFound`` when an EVC is active and flows aren't found. Update status and status_reason accordingly too when installing flows. +- Validate to support only a single proxy port per UNI for now. Fixed ===== From 9f3e6a37130f816d4444a36e922ea80a45da5d50 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Fri, 26 Jul 2024 10:22:33 -0300 Subject: [PATCH 05/34] feat: added proxy_port_shared error via metadata and covered with test --- managers/int.py | 12 +++++++++++- tests/unit/test_int_manager.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/managers/int.py b/managers/int.py index ed32dff..7233002 100644 --- a/managers/int.py +++ b/managers/int.py @@ -280,12 +280,22 @@ async def handle_pp_metadata_added(self, intf: Interface) -> None: try: await self.enable_int(affected_evcs, force=True) except (ProxyPortSameSourceIntraEVC, ProxyPortShared) as exc: - # TODO update metdata msg = ( f"Validation error when updating interface {intf}" f" EVC ids: {list(affected_evcs)}, exception {str(exc)}" ) log.error(msg) + metadata = { + "telemetry": { + "enabled": False, + "status": "DOWN", + "status_reason": ["proxy_port_shared"], + "status_updated_at": datetime.utcnow().strftime( + "%Y-%m-%dT%H:%M:%S" + ), + } + } + api.add_evcs_metadata(affected_evcs, metadata) async def disable_int( self, evcs: dict[str, dict], force=False, reason="disabled" diff --git a/tests/unit/test_int_manager.py b/tests/unit/test_int_manager.py index 191372c..6fbeded 100644 --- a/tests/unit/test_int_manager.py +++ b/tests/unit/test_int_manager.py @@ -324,6 +324,37 @@ async def test_handle_pp_metadata_added_no_affected(self, monkeypatch): assert not int_manager.disable_int.call_count assert not int_manager.enable_int.call_count + async def test_handle_pp_metadata_added_exc_port_shared(self, monkeypatch): + """Test handle_pp_metadata_added exception port shared.""" + log_mock = MagicMock() + monkeypatch.setattr("napps.kytos.telemetry_int.managers.int.log", log_mock) + int_manager = INTManager(MagicMock()) + api_mock, intf_mock, pp_mock = AsyncMock(), MagicMock(), MagicMock() + intf_mock.id = "some_intf_id" + source_id, source_port = "some_source_id", 2 + intf_mock.metadata = {"proxy_port": source_port} + evc_id = "3766c105686748" + int_manager.unis_src[intf_mock.id] = source_id + int_manager.srcs_pp[source_id] = pp_mock + pp_mock.evc_ids = {evc_id} + + assert "proxy_port" in intf_mock.metadata + monkeypatch.setattr("napps.kytos.telemetry_int.managers.int.api", api_mock) + api_mock.get_evcs.return_value = {evc_id: {}} + int_manager.disable_int = AsyncMock() + int_manager.enable_int = AsyncMock() + int_manager.enable_int.side_effect = ProxyPortShared(evc_id, "shared") + + await int_manager.handle_pp_metadata_added(intf_mock) + assert api_mock.get_evcs.call_count == 1 + assert api_mock.get_evcs.call_count == 1 + assert api_mock.get_evcs.call_args[1] == {"metadata.telemetry.enabled": "true"} + assert int_manager.disable_int.call_count == 1 + assert int_manager.enable_int.call_count == 1 + + assert api_mock.add_evcs_metadata.call_count == 1 + assert log_mock.error.call_count == 1 + async def test_disable_int_metadata(self, monkeypatch) -> None: """Test disable INT metadata args.""" controller = MagicMock() From 452faad3786983c832df4dbeb82a49bf37cb65d8 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Fri, 26 Jul 2024 12:17:16 -0300 Subject: [PATCH 06/34] fix: fixed validation bug --- managers/int.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/managers/int.py b/managers/int.py index 7233002..7239c00 100644 --- a/managers/int.py +++ b/managers/int.py @@ -295,7 +295,7 @@ async def handle_pp_metadata_added(self, intf: Interface) -> None: ), } } - api.add_evcs_metadata(affected_evcs, metadata) + await api.add_evcs_metadata(affected_evcs, metadata) async def disable_int( self, evcs: dict[str, dict], force=False, reason="disabled" @@ -542,7 +542,7 @@ def _validate_dedicated_proxy_port_evcs(self, evcs: dict[str, dict]): f"UNI {found} would use {pp}" ) raise ProxyPortShared(evc["id"], msg) - seen_src_unis[pp_a.source.id] = uni_id + seen_src_unis[pp.source.id] = uni_id async def handle_failover_flows( self, evcs_content: dict[str, dict], event_name: str From 2fd8b433dcdbcaa4e3d6b39f67b9cfdde8921e04 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Fri, 26 Jul 2024 12:43:08 -0300 Subject: [PATCH 07/34] Revert "chore: updated batch to keep at 100 flows / sec at most like mef_eline for now" This reverts commit 707aef367f5d44227af8b5ab50f51d5e6acd6038. --- settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/settings.py b/settings.py index b2e9cc0..a29bafb 100644 --- a/settings.py +++ b/settings.py @@ -13,12 +13,12 @@ # BATCH_INTERVAL: time interval between batch requests that will be sent to # flow_manager (in seconds) - zero enable sending all the requests in a row -BATCH_INTERVAL = 0.5 +BATCH_INTERVAL = 0.2 # BATCH_SIZE: size of a batch request that will be sent to flow_manager, in # number of FlowMod requests. Use 0 (zero) to disable BATCH mode, i.e. sends # everything at a glance -BATCH_SIZE = 50 +BATCH_SIZE = 200 # Fallback to mef_eline by removing INT flows if an external loop goes down. If # the loop goes UP again and the EVC is active, it'll install INT flows From 25ac989435bf4b09cd76c43409ec68130fc6f6fc Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Mon, 29 Jul 2024 11:00:09 -0300 Subject: [PATCH 08/34] fix: fixed batch flows slicing --- managers/int.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/managers/int.py b/managers/int.py index 7239c00..f3c090e 100644 --- a/managers/int.py +++ b/managers/int.py @@ -828,8 +828,8 @@ async def _send_flows( batch_size = len(flows) for i in range(0, len(flows), batch_size): - flows = flows[i : i + batch_size] - if not flows: + flows_batch = flows[i : i + batch_size] + if not flows_batch: continue if i > 0: @@ -839,7 +839,7 @@ async def _send_flows( content={ "dpid": dpid, "force": True, - "flow_dict": {"flows": flows}, + "flow_dict": {"flows": flows_batch}, }, ) await self.controller.buffers.app.aput(event) From 71783e82a0ed4099d91de3602614118e45c8d0a2 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Mon, 29 Jul 2024 11:00:29 -0300 Subject: [PATCH 09/34] updated changelog --- CHANGELOG.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 08f40e5..29ab1b2 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -25,6 +25,7 @@ Changed Fixed ===== - Only redeploy if INT has been enabled before +- Fixed batched flows slicing [2023.2.0] - 2024-02-16 *********************** From 520403a490fcb85053b37c1066d165be187acb8c Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Mon, 29 Jul 2024 16:30:22 -0300 Subject: [PATCH 10/34] feat: added has_instruction_and_action_type func helper --- tests/unit/test_utils.py | 49 ++++++++++++++++++++++++++++++++++++++++ utils.py | 16 +++++++++++++ 2 files changed, 65 insertions(+) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index e5f0cde..81dfb1d 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -164,6 +164,55 @@ def test_add_to_apply_actions() -> None: assert instructions[0]["actions"][0] == new_instruction +@pytest.mark.parametrize( + "instructions,instruction_type,action_type,expected", + [ + ( + [ + { + "instruction_type": "apply_actions", + "actions": [{"action_type": "push_int"}], + }, + {"instruction_type": "goto_table", "table_id": 2}, + ], + "apply_actions", + "push_int", + True, + ), + ( + [ + {"instruction_type": "goto_table", "table_id": 2}, + ], + "apply_actions", + "push_int", + False, + ), + ( + [ + { + "instruction_type": "apply_actions", + "actions": [{"action_type": "push_int"}], + }, + {"instruction_type": "goto_table", "table_id": 2}, + ], + "apply_actions", + "pop_int", + False, + ), + ], +) +def test_has_instruction_and_action_type( + instructions, instruction_type, action_type, expected +) -> None: + """Test add to apply actions.""" + assert ( + utils.has_instruction_and_action_type( + instructions, instruction_type, action_type + ) + == expected + ) + + def test_set_owner() -> None: """Test set_owner.""" flow = { diff --git a/utils.py b/utils.py index 91e7272..44b8bd8 100644 --- a/utils.py +++ b/utils.py @@ -59,6 +59,22 @@ def add_to_apply_actions( return instructions +def has_instruction_and_action_type( + instructions: list[dict], instruction_type: str, action_type: str +) -> bool: + """Check if any of the instructions has a given type and action type.""" + for instruction in instructions: + if ( + instruction["instruction_type"] != instruction_type + or "actions" not in instruction + ): + continue + for action in instruction["actions"]: + if "action_type" in action and action["action_type"] == action_type: + return True + return False + + def get_cookie(evc_id: str, cookie_prefix: int) -> int: """Return the cookie integer from evc id. From cffcf80371848d32172d4081ed5be725c34bcfe5 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Mon, 29 Jul 2024 16:42:40 -0300 Subject: [PATCH 11/34] fix: optimized away redudant push_int flows when handling failover events --- managers/int.py | 17 ++++- tests/unit/test_flow_builder_failover.py | 92 ------------------------ 2 files changed, 14 insertions(+), 95 deletions(-) diff --git a/managers/int.py b/managers/int.py index f3c090e..283363e 100644 --- a/managers/int.py +++ b/managers/int.py @@ -627,9 +627,20 @@ async def handle_failover_flows( log.info( f"Handling {event_name} flows install on EVC ids: {to_install.keys()}" ) - await self._install_int_flows( - self.flow_builder.build_int_flows(to_install, new_flows) - ) + built_flows = self.flow_builder.build_int_flows(to_install, new_flows) + built_flows = { + cookie: [ + flow + for flow in flows + if not utils.has_instruction_and_action_type( + flow.get("flow", {}).get("instructions", []), + "apply_actions", + "push_int", + ) + ] + for cookie, flows in built_flows.items() + } + await self._install_int_flows(built_flows) def _validate_map_enable_evcs( self, diff --git a/tests/unit/test_flow_builder_failover.py b/tests/unit/test_flow_builder_failover.py index 9ad222e..0336c07 100644 --- a/tests/unit/test_flow_builder_failover.py +++ b/tests/unit/test_flow_builder_failover.py @@ -154,52 +154,6 @@ async def test_handle_failover_link_down() -> None: expected_built_flows = { "12307967605643950656": [ - { - "flow": { - "match": { - "in_port": 1, - "dl_vlan": 100, - "dl_type": 2048, - "nw_proto": 6, - }, - "cookie": 12163852417568094784, - "owner": "telemetry_int", - "table_group": "evpl", - "table_id": 0, - "priority": 20100, - "instructions": [ - { - "instruction_type": "apply_actions", - "actions": [{"action_type": "push_int"}], - }, - {"instruction_type": "goto_table", "table_id": 2}, - ], - }, - "switch": "00:00:00:00:00:00:00:01", - }, - { - "flow": { - "match": { - "in_port": 1, - "dl_vlan": 100, - "dl_type": 2048, - "nw_proto": 17, - }, - "cookie": 12163852417568094784, - "owner": "telemetry_int", - "table_group": "evpl", - "table_id": 0, - "priority": 20100, - "instructions": [ - { - "instruction_type": "apply_actions", - "actions": [{"action_type": "push_int"}], - }, - {"instruction_type": "goto_table", "table_id": 2}, - ], - }, - "switch": "00:00:00:00:00:00:00:01", - }, { "flow": { "match": {"in_port": 1, "dl_vlan": 100}, @@ -222,52 +176,6 @@ async def test_handle_failover_link_down() -> None: }, "switch": "00:00:00:00:00:00:00:01", }, - { - "flow": { - "match": { - "in_port": 1, - "dl_vlan": 100, - "dl_type": 2048, - "nw_proto": 6, - }, - "cookie": 12163852417568094784, - "owner": "telemetry_int", - "table_group": "evpl", - "table_id": 0, - "priority": 20100, - "instructions": [ - { - "instruction_type": "apply_actions", - "actions": [{"action_type": "push_int"}], - }, - {"instruction_type": "goto_table", "table_id": 2}, - ], - }, - "switch": "00:00:00:00:00:00:00:03", - }, - { - "flow": { - "match": { - "in_port": 1, - "dl_vlan": 100, - "dl_type": 2048, - "nw_proto": 17, - }, - "cookie": 12163852417568094784, - "owner": "telemetry_int", - "table_group": "evpl", - "table_id": 0, - "priority": 20100, - "instructions": [ - { - "instruction_type": "apply_actions", - "actions": [{"action_type": "push_int"}], - }, - {"instruction_type": "goto_table", "table_id": 2}, - ], - }, - "switch": "00:00:00:00:00:00:00:03", - }, { "flow": { "match": {"in_port": 1, "dl_vlan": 100}, From 77aea00d3678f378a624d8c044a4ca77d1b2210c Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Fri, 2 Aug 2024 14:53:50 -0300 Subject: [PATCH 12/34] feat: included UNI proxy_port configuration endpoints --- openapi.yml | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/openapi.yml b/openapi.yml index a498e18..045a40a 100644 --- a/openapi.yml +++ b/openapi.yml @@ -170,6 +170,112 @@ paths: description: Internal Server Error '503': description: Service unavailable + /v1/uni/proxy_port: + get: + summary: List the existing UNIs and the configured proxy ports + operationId: list_proxy_ports + responses: + '200': + description: List the existing configured proxy ports + content: + application/json: + schema: + items: + $ref: '#/components/schemas/ProxyPortListResp' + '500': + description: Internal Server Error + '503': + description: Service unavailable + /v1/uni/{interface_id}/proxy_port/{port_number}: + post: + summary: Set proxy port metadata on a UNI + description: Set proxy port metadata on a UNI. Each UNI should use a different proxy port. If the proxy port is being updated, INT will be redeployed. If the proxy port is being set for the first time, you will need to enable INT on EVC afterwards. + operationId: set_proxy_port + parameters: + - name: interface_id + schema: + type: string + required: true + description: The interface ID + in: path + - name: port_number + schema: + type: integer + required: true + description: The proxy port number + in: path + - name: force + schema: + type: boolean + required: false + description: If force is set, it will also accept a proxy_port that isn't UP + in: query + responses: + '200': + description: Operation sucessful + content: + application/json: + schema: + type: string + example: Operation sucessful + '404': + description: Interface id or proxy port not found + '409': + description: Proxy port conflict + '500': + description: Internal Server Error + '503': + description: Service unavailable + delete: + summary: Delete proxy port metadata on a UNI + description: Delete proxy port metadata on a UNI. It will only delete the metadata if no INT EVCs are using this port, unless you use the force option. When metadata is removed, INT will be disabled. + operationId: del_proxy_port + parameters: + - name: interface_id + schema: + type: string + required: true + description: The interface ID + in: path + - name: port_number + schema: + type: integer + required: true + description: The proxy port number + in: path + - name: force + schema: + type: boolean + in: query + description: Force metadata deletion even if there are INT EVCs using the proxy port + responses: + '200': + description: Operation sucessful. + content: + application/json: + schema: + type: string + example: Operation sucessful + '404': + description: Interface id or proxy port not found + '409': + description: Proxy port deletion conflict + content: + application/json: + schema: + type: object + properties: + description: + type: string + evc_ids: + type: array + items: + type: string + '500': + description: Internal Server Error + '503': + description: Service unavailable + components: @@ -192,3 +298,10 @@ components: type: array items: type: string + ProxyPortListResp: # Can be referenced via '#/components/schemas/ProxyPortListResp' + type: object + properties: + interface_id: + type: string + proxy_port: + type: integer From dbbd54f62ad9e66f2484914d6c56202779a3b6de Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Tue, 6 Aug 2024 11:36:07 -0300 Subject: [PATCH 13/34] feat: introduced api helper methods to add and delete intf proxy_port metadata --- kytos_api_helper.py | 51 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/kytos_api_helper.py b/kytos_api_helper.py index 28f3860..1c20e21 100644 --- a/kytos_api_helper.py +++ b/kytos_api_helper.py @@ -166,3 +166,54 @@ async def add_evcs_metadata( f"Failed to add_evc_metadata for EVC ids {list(evcs.keys())} " f"status code {response.status_code}, response text: {response.text}" ) + + +@retry( + stop=stop_after_attempt(5), + wait=wait_combine(wait_fixed(3), wait_random(min=2, max=7)), + before_sleep=before_sleep, + retry=retry_if_exception_type(httpx.RequestError), +) +async def add_proxy_port_metadata(intf_id: str, port_no: int) -> dict: + """Add proxy_port metadata.""" + async with httpx.AsyncClient(base_url=settings.topology_url) as client: + response = await client.post( + f"/interfaces/{intf_id}/metadata", + timeout=10, + json={"proxy_port": port_no}, + ) + if response.is_success: + return response.json() + if response.status_code == 404: + raise ValueError(f"interface_id {intf_id} not found") + if response.is_server_error: + raise httpx.RequestError(response.text) + raise UnrecoverableError( + f"Failed to add_proxy_port {port_no} metadata for intf_id {intf_id} " + f"status code {response.status_code}, response text: {response.text}" + ) + + +@retry( + stop=stop_after_attempt(5), + wait=wait_combine(wait_fixed(3), wait_random(min=2, max=7)), + before_sleep=before_sleep, + retry=retry_if_exception_type(httpx.RequestError), +) +async def delete_proxy_port_metadata(intf_id: str) -> dict: + """Delete proxy_port metadata.""" + async with httpx.AsyncClient(base_url=settings.topology_url) as client: + response = await client.delete( + f"/interfaces/{intf_id}/metadata/proxy_port", + timeout=10, + ) + if response.is_success: + return response.json() + if response.status_code == 404: + raise ValueError(f"interface_id {intf_id} or metadata proxy_port not found") + if response.is_server_error: + raise httpx.RequestError(response.text) + raise UnrecoverableError( + f"Failed to delete_proxy_port metadata for intf_id {intf_id} " + f"status code {response.status_code}, response text: {response.text}" + ) From e45a2678e7544b26c79b37368b980277cf99e761 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Tue, 6 Aug 2024 11:38:53 -0300 Subject: [PATCH 14/34] feat: augmented int_manager get_proxy_port_or_raise to support new_port_number --- managers/int.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/managers/int.py b/managers/int.py index 283363e..398b2f9 100644 --- a/managers/int.py +++ b/managers/int.py @@ -4,7 +4,7 @@ import copy from collections import defaultdict from datetime import datetime -from typing import Literal +from typing import Literal, Optional from pyof.v0x04.controller2switch.table_mod import Table @@ -433,25 +433,29 @@ async def install_int_flows( api.add_evcs_metadata(active_evcs, metadata, force), ) - def get_proxy_port_or_raise(self, intf_id: str, evc_id: str) -> ProxyPort: - """Return a ProxyPort assigned to a UNI or raise.""" + def get_proxy_port_or_raise( + self, intf_id: str, evc_id: str, new_port_number: Optional[int] = None + ) -> ProxyPort: + """Return a ProxyPort assigned to a UNI or raise. + + new_port_number can be set and used to validate a new port_number. + """ interface = self.controller.get_interface_by_id(intf_id) if not interface: raise ProxyPortNotFound(evc_id, f"UNI interface {intf_id} not found") - if "proxy_port" not in interface.metadata: + if new_port_number is None and "proxy_port" not in interface.metadata: raise ProxyPortNotFound( evc_id, f"proxy_port metadata not found in {intf_id}" ) - source_intf = interface.switch.get_interface_by_port_no( - interface.metadata.get("proxy_port") - ) + port_no = new_port_number or interface.metadata.get("proxy_port") + source_intf = interface.switch.get_interface_by_port_no(port_no) if not source_intf: raise ProxyPortNotFound( evc_id, - f"proxy_port of {intf_id} source interface not found", + f"proxy_port {port_no} of {intf_id} source interface not found", ) pp = self.srcs_pp.get(source_intf.id) @@ -462,8 +466,8 @@ def get_proxy_port_or_raise(self, intf_id: str, evc_id: str) -> ProxyPort: if not pp.destination: raise ProxyPortDestNotFound( evc_id, - f"proxy_port of {intf_id} isn't looped or destination interface " - "not found", + f"proxy_port {port_no} of {intf_id} isn't looped or " + "destination interface not found", ) return pp From 72404a0547952dd948d2f0ab02bfd39fb8570511 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Tue, 6 Aug 2024 11:39:54 -0300 Subject: [PATCH 15/34] feat: introduced v1/uni proxy_port endpoints removed old unused TODO methods --- main.py | 122 ++++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 101 insertions(+), 21 deletions(-) diff --git a/main.py b/main.py index 73ff6a2..732deca 100644 --- a/main.py +++ b/main.py @@ -13,6 +13,7 @@ from tenacity import RetryError from kytos.core import KytosEvent, KytosNApp, log, rest +from kytos.core.common import EntityStatus from kytos.core.helpers import alisten_to, avalidate_openapi_request, load_spec from kytos.core.rest_api import HTTPException, JSONResponse, Request, aget_json_or_400 @@ -22,6 +23,7 @@ EVCHasNoINT, EVCNotFound, FlowsNotFound, + ProxyPortError, ProxyPortNotFound, ProxyPortSameSourceIntraEVC, ProxyPortShared, @@ -297,6 +299,105 @@ async def evc_compare(self, _request: Request) -> JSONResponse: ] return JSONResponse(response) + @rest("v1/uni/{interface_id}/proxy_port", methods=["DELETE"]) + async def delete_proxy_port_metadata(self, request: Request) -> JSONResponse: + """Delete proxy port metadata.""" + intf_id = request.path_params["interface_id"] + if ( + (intf := self.controller.get_interface_by_id(intf_id)) + and "proxy_port" not in intf.metadata + ): + return JSONResponse("Operation successful") + + qparams = request.query_params + force = False if qparams.get("force", "false").lower() == "false" else True + try: + pp = self.int_manager.get_proxy_port_or_raise(intf_id, "no_evc_id") + if pp.evc_ids and not force: + return JSONResponse( + { + "status_code": 409, + "code": 409, + "description": f"{pp} is in use on {len(pp.evc_ids)} EVCs", + "evc_ids": sorted(pp.evc_ids), + }, + status_code=409, + ) + except ProxyPortError as exc: + raise HTTPException(404, detail=exc.message) + + try: + await api.delete_proxy_port_metadata(intf_id) + return JSONResponse("Operation successful") + except ValueError as exc: + raise HTTPException(404, detail=str(exc)) + except UnrecoverableError as exc: + raise HTTPException(500, detail=str(exc)) + + @rest("v1/uni/{interface_id}/proxy_port/{port_number:int}", methods=["POST"]) + async def add_proxy_port_metadata(self, request: Request) -> JSONResponse: + """Add proxy port metadata.""" + intf_id = request.path_params["interface_id"] + port_no = request.path_params["port_number"] + if ( + (intf := self.controller.get_interface_by_id(intf_id)) + and "proxy_port" in intf.metadata + and intf.metadata["proxy_port"] == port_no + ): + return JSONResponse("Operation successful") + + qparams = request.query_params + force = False if qparams.get("force", "false").lower() == "false" else True + try: + pp = self.int_manager.get_proxy_port_or_raise(intf_id, "no_evc_id", port_no) + if pp.status != EntityStatus.UP and not force: + raise HTTPException(409, detail=f"{pp} status isn't UP") + self.int_manager._validate_dedicated_proxy_port_evcs( + { + "no_evc_id": { + "uni_a": {"interface_id": intf_id, "proxy_port": pp}, + "uni_z": {"interface_id": intf_id, "proxy_port": pp}, + } + } + ) + except ProxyPortShared as exc: + raise HTTPException(409, detail=exc.message) + except ProxyPortError as exc: + raise HTTPException(404, detail=exc.message) + + try: + await api.add_proxy_port_metadata(intf_id, port_no) + return JSONResponse("Operation successful") + except ValueError as exc: + raise HTTPException(404, detail=str(exc)) + except UnrecoverableError as exc: + raise HTTPException(500, detail=str(exc)) + + @rest("v1/uni/proxy_port") + async def list_uni_proxy_ports(self, _request: Request) -> JSONResponse: + """List configured UNI proxy ports.""" + interfaces_proxy_ports = [] + for switch in self.controller.switches.copy().values(): + for intf in switch.interfaces.copy().values(): + if "proxy_port" in intf.metadata: + payload = { + "interface_id": intf.id, + "interface_status": intf.status.value, + "interface_status_reason": sorted(intf.status_reason), + "proxy_port_number": intf.metadata["proxy_port"], + "proxy_port_status": "DOWN", + "proxy_port_status_reason": [], + } + try: + pp = self.int_manager.get_proxy_port_or_raise( + intf.id, "no_evc_id" + ) + payload["proxy_port_status"] = pp.status.value + except ProxyPortError as exc: + payload["proxy_port_status_reason"] = [exc.message] + interfaces_proxy_ports.append(payload) + return JSONResponse(interfaces_proxy_ports) + @alisten_to("kytos/mef_eline.evcs_loaded") async def on_mef_eline_evcs_loaded(self, event: KytosEvent) -> None: """Handle kytos/mef_eline.evcs_loaded.""" @@ -549,24 +650,3 @@ async def on_intf_metadata_removed(self, event: KytosEvent) -> None: async def on_intf_metadata_added(self, event: KytosEvent) -> None: """On interface metadata added.""" await self.int_manager.handle_pp_metadata_added(event.content["interface"]) - - # Event-driven methods: future - def listen_for_new_evcs(self): - """Change newly created EVC to INT-enabled EVC based on the metadata field - (future)""" - pass - - def listen_for_evc_change(self): - """Change newly created EVC to INT-enabled EVC based on the - metadata field (future)""" - pass - - def listen_for_path_changes(self): - """Change EVC's new path to INT-enabled EVC based on the metadata field - when there is a path change. (future)""" - pass - - def listen_for_topology_changes(self): - """If the topology changes, make sure it is not the loop ports. - If so, update proxy ports""" - pass From 81f72cae7c52211bb4eac97c0e8dce552ba1ee8c Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Tue, 6 Aug 2024 11:40:20 -0300 Subject: [PATCH 16/34] feat: included topology_url on settings.py --- settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/settings.py b/settings.py index a29bafb..8d8ca0d 100644 --- a/settings.py +++ b/settings.py @@ -3,6 +3,7 @@ KYTOS_API = "http://0.0.0.0:8181/api" mef_eline_api = f"{KYTOS_API}/kytos/mef_eline/v2" flow_manager_api = f"{KYTOS_API}/kytos/flow_manager/v2" +topology_url = f"{KYTOS_API}/kytos/topology/v3" INT_COOKIE_PREFIX = 0xA8 MEF_COOKIE_PREFIX = 0xAA IPv4 = 2048 From 4ed727c681d919f2716d1e2b676ba9d97cd2b343 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Wed, 7 Aug 2024 17:54:54 -0300 Subject: [PATCH 17/34] refactor: refactored dedicated proxy_port validation --- main.py | 20 +++++++------------- managers/int.py | 24 ++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/main.py b/main.py index 732deca..f1d735b 100644 --- a/main.py +++ b/main.py @@ -310,7 +310,7 @@ async def delete_proxy_port_metadata(self, request: Request) -> JSONResponse: return JSONResponse("Operation successful") qparams = request.query_params - force = False if qparams.get("force", "false").lower() == "false" else True + force = True if qparams.get("force", "false").lower() == "true" else False try: pp = self.int_manager.get_proxy_port_or_raise(intf_id, "no_evc_id") if pp.evc_ids and not force: @@ -339,27 +339,21 @@ async def add_proxy_port_metadata(self, request: Request) -> JSONResponse: """Add proxy port metadata.""" intf_id = request.path_params["interface_id"] port_no = request.path_params["port_number"] + qparams = request.query_params + if not (intf := self.controller.get_interface_by_id(intf_id)): + raise HTTPException(404, detail=f"Interface id {intf_id} not found") if ( - (intf := self.controller.get_interface_by_id(intf_id)) - and "proxy_port" in intf.metadata + "proxy_port" in intf.metadata and intf.metadata["proxy_port"] == port_no ): return JSONResponse("Operation successful") - qparams = request.query_params - force = False if qparams.get("force", "false").lower() == "false" else True + force = True if qparams.get("force", "false").lower() == "true" else False try: pp = self.int_manager.get_proxy_port_or_raise(intf_id, "no_evc_id", port_no) if pp.status != EntityStatus.UP and not force: raise HTTPException(409, detail=f"{pp} status isn't UP") - self.int_manager._validate_dedicated_proxy_port_evcs( - { - "no_evc_id": { - "uni_a": {"interface_id": intf_id, "proxy_port": pp}, - "uni_z": {"interface_id": intf_id, "proxy_port": pp}, - } - } - ) + self.int_manager._validate_new_dedicated_proxy_port(intf, port_no) except ProxyPortShared as exc: raise HTTPException(409, detail=exc.message) except ProxyPortError as exc: diff --git a/managers/int.py b/managers/int.py index 398b2f9..85fd1b6 100644 --- a/managers/int.py +++ b/managers/int.py @@ -195,6 +195,7 @@ async def handle_pp_link_up(self, link: Link) -> None: async def handle_pp_metadata_removed(self, intf: Interface) -> None: """Handle proxy port metadata removed.""" + # TODO yes, this has to disable too, then it'll be ok if "proxy_port" in intf.metadata: return try: @@ -256,8 +257,6 @@ async def handle_pp_metadata_added(self, intf: Interface) -> None: return async with self._intf_meta_lock: - pp.source = cur_source_intf - evcs = await api.get_evcs( **{ "metadata.telemetry.enabled": "true", @@ -519,8 +518,29 @@ def _validate_intra_evc_different_proxy_ports(self, evc: dict) -> None: evc["id"], "intra EVC UNIs must use different proxy ports" ) + def _validate_new_dedicated_proxy_port( + self, uni: Interface, new_port_no: int + ) -> None: + """This is for validating a future proxy port. + Only a dedicated proxy port per UNI is supported at the moment. + + https://github.com/kytos-ng/telemetry_int/issues/110 + """ + for intf in uni.switch.interfaces.copy().values(): + if ( + intf != uni + and "proxy_port" in intf.metadata + and intf.metadata["proxy_port"] == new_port_no + ): + msg = ( + f"UNI {uni.id} must use another dedicated proxy port. " + f"UNI {intf.id} is already using proxy_port number {new_port_no}" + ) + raise ProxyPortShared("no_evc_id", msg) + def _validate_dedicated_proxy_port_evcs(self, evcs: dict[str, dict]): """Validate that a proxy port is dedicated for the given EVCs. + Only a dedicated proxy port per UNI is supported at the moment. https://github.com/kytos-ng/telemetry_int/issues/110 """ From 146a887f28684f0ff57644a8d4e54b429aa28ea4 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Wed, 7 Aug 2024 18:24:25 -0300 Subject: [PATCH 18/34] updated openapi.yml spec accordingly --- main.py | 20 ++++++++++++-------- openapi.yml | 33 ++++++++++++++++++++++++--------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/main.py b/main.py index f1d735b..8cad77f 100644 --- a/main.py +++ b/main.py @@ -375,20 +375,24 @@ async def list_uni_proxy_ports(self, _request: Request) -> JSONResponse: for intf in switch.interfaces.copy().values(): if "proxy_port" in intf.metadata: payload = { - "interface_id": intf.id, - "interface_status": intf.status.value, - "interface_status_reason": sorted(intf.status_reason), - "proxy_port_number": intf.metadata["proxy_port"], - "proxy_port_status": "DOWN", - "proxy_port_status_reason": [], + "uni": { + "id": intf.id, + "status": intf.status.value, + "status_reason": sorted(intf.status_reason) + }, + "proxy_port": { + "port_number": intf.metadata["proxy_port"], + "status": "DOWN", + "status_reason": [] + } } try: pp = self.int_manager.get_proxy_port_or_raise( intf.id, "no_evc_id" ) - payload["proxy_port_status"] = pp.status.value + payload["proxy_port"]["status"] = pp.status.value except ProxyPortError as exc: - payload["proxy_port_status_reason"] = [exc.message] + payload["proxy_port"]["status_reason"] = [exc.message] interfaces_proxy_ports.append(payload) return JSONResponse(interfaces_proxy_ports) diff --git a/openapi.yml b/openapi.yml index 045a40a..e56c344 100644 --- a/openapi.yml +++ b/openapi.yml @@ -226,6 +226,7 @@ paths: description: Internal Server Error '503': description: Service unavailable + /v1/uni/{interface_id}/proxy_port: delete: summary: Delete proxy port metadata on a UNI description: Delete proxy port metadata on a UNI. It will only delete the metadata if no INT EVCs are using this port, unless you use the force option. When metadata is removed, INT will be disabled. @@ -237,16 +238,11 @@ paths: required: true description: The interface ID in: path - - name: port_number - schema: - type: integer - required: true - description: The proxy port number - in: path - name: force schema: type: boolean in: query + required: false description: Force metadata deletion even if there are INT EVCs using the proxy port responses: '200': @@ -301,7 +297,26 @@ components: ProxyPortListResp: # Can be referenced via '#/components/schemas/ProxyPortListResp' type: object properties: - interface_id: - type: string + uni: + type: object + properties: + id: + type: string + status: + type: string + status_reason: + type: array + items: + type: string proxy_port: - type: integer + type: object + properties: + port_number: + type: integer + status: + type: string + status_reason: + type: array + items: + type: string + From 599e7c3b8b445dce952e2e101ae4d3c242353ec9 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Wed, 7 Aug 2024 18:44:11 -0300 Subject: [PATCH 19/34] refactor! changed handling removed proxy_port metadata to disable INT --- managers/int.py | 23 +++++++---------------- tests/unit/test_int_manager.py | 12 +++--------- 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/managers/int.py b/managers/int.py index 85fd1b6..e825d3a 100644 --- a/managers/int.py +++ b/managers/int.py @@ -195,7 +195,6 @@ async def handle_pp_link_up(self, link: Link) -> None: async def handle_pp_metadata_removed(self, intf: Interface) -> None: """Handle proxy port metadata removed.""" - # TODO yes, this has to disable too, then it'll be ok if "proxy_port" in intf.metadata: return try: @@ -212,27 +211,19 @@ async def handle_pp_metadata_removed(self, intf: Interface) -> None: "metadata.telemetry.status": "UP", } ) - to_deactivate = { + affected_evcs = { evc_id: evc for evc_id, evc in evcs.items() if evc_id in pp.evc_ids } - if not to_deactivate: + if not affected_evcs: return log.info( - f"Handling interface metadata removed on {intf}, removing INT flows " - f"falling back to mef_eline, EVC ids: {list(to_deactivate)}" + f"Handling interface metadata removed on {intf}, it'll disable INT " + f"falling back to mef_eline, EVC ids: {list(affected_evcs)}" + ) + await self.disable_int( + affected_evcs, force=True, reason="proxy_port_metadata_removed" ) - metadata = { - "telemetry": { - "enabled": True, - "status": "DOWN", - "status_reason": ["proxy_port_metadata_removed"], - "status_updated_at": datetime.utcnow().strftime( - "%Y-%m-%dT%H:%M:%S" - ), - } - } - await self.remove_int_flows(to_deactivate, metadata) async def handle_pp_metadata_added(self, intf: Interface) -> None: """Handle proxy port metadata added. diff --git a/tests/unit/test_int_manager.py b/tests/unit/test_int_manager.py index 6fbeded..355a0d0 100644 --- a/tests/unit/test_int_manager.py +++ b/tests/unit/test_int_manager.py @@ -223,7 +223,7 @@ async def test_handle_pp_metadata_removed(self, monkeypatch): assert "proxy_port" not in intf_mock.metadata monkeypatch.setattr("napps.kytos.telemetry_int.managers.int.api", api_mock) api_mock.get_evcs.return_value = {evc_id: {}} - int_manager.remove_int_flows = AsyncMock() + int_manager.disable_int = AsyncMock() await int_manager.handle_pp_metadata_removed(intf_mock) assert api_mock.get_evcs.call_count == 1 @@ -232,15 +232,9 @@ async def test_handle_pp_metadata_removed(self, monkeypatch): "metadata.telemetry.enabled": "true", "metadata.telemetry.status": "UP", } - assert int_manager.remove_int_flows.call_count == 1 - args = int_manager.remove_int_flows.call_args[0] + assert int_manager.disable_int.call_count == 1 + args = int_manager.disable_int.call_args[0] assert evc_id in args[0] - assert "telemetry" in args[1] - telemetry = args[1]["telemetry"] - assert telemetry["enabled"] - assert telemetry["status"] == "DOWN" - assert telemetry["status_reason"] == ["proxy_port_metadata_removed"] - assert "status_updated_at" in telemetry async def test_handle_pp_metadata_added(self, monkeypatch): """Test handle_pp_metadata_added.""" From d91665653e9445b1a78f0e9b661ee8de27624a62 Mon Sep 17 00:00:00 2001 From: David Ramirez Date: Thu, 8 Aug 2024 02:38:21 +0000 Subject: [PATCH 20/34] remove batching from telemetry_int --- managers/int.py | 35 ++++++++++++----------------------- settings.py | 9 --------- 2 files changed, 12 insertions(+), 32 deletions(-) diff --git a/managers/int.py b/managers/int.py index 283363e..36f61ad 100644 --- a/managers/int.py +++ b/managers/int.py @@ -828,29 +828,18 @@ async def _install_int_flows( async def _send_flows( self, switch_flows: dict[str, list[dict]], cmd: Literal["install", "delete"] ): - """Send batched flows by dpid to flow_manager. - - The flows will be batched per dpid based on settings.BATCH_SIZE and will wait - for settings.BATCH_INTERVAL per batch iteration. + """ + Send batched flows by dpid to flow_manager. """ for dpid, flows in switch_flows.items(): - batch_size = settings.BATCH_SIZE - if batch_size <= 0: - batch_size = len(flows) - - for i in range(0, len(flows), batch_size): - flows_batch = flows[i : i + batch_size] - if not flows_batch: - continue - - if i > 0: - await asyncio.sleep(settings.BATCH_INTERVAL) - event = KytosEvent( - f"kytos.flow_manager.flows.single.{cmd}", - content={ - "dpid": dpid, - "force": True, - "flow_dict": {"flows": flows_batch}, - }, + if flows: + await self.controller.buffers.app.aput( + KytosEvent( + f"kytos.flow_manager.flows.single.{cmd}", + content={ + "dpid": dpid, + "force": True, + "flow_dict": {"flows": flows}, + }, + ) ) - await self.controller.buffers.app.aput(event) diff --git a/settings.py b/settings.py index a29bafb..0376baf 100644 --- a/settings.py +++ b/settings.py @@ -11,15 +11,6 @@ TABLE_GROUP_ALLOWED = {"evpl", "epl"} -# BATCH_INTERVAL: time interval between batch requests that will be sent to -# flow_manager (in seconds) - zero enable sending all the requests in a row -BATCH_INTERVAL = 0.2 - -# BATCH_SIZE: size of a batch request that will be sent to flow_manager, in -# number of FlowMod requests. Use 0 (zero) to disable BATCH mode, i.e. sends -# everything at a glance -BATCH_SIZE = 200 - # Fallback to mef_eline by removing INT flows if an external loop goes down. If # the loop goes UP again and the EVC is active, it'll install INT flows FALLBACK_TO_MEF_LOOP_DOWN = True From 244d875095814dcc957b176a863ca9b6eeb83ac8 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Thu, 8 Aug 2024 12:46:10 -0300 Subject: [PATCH 21/34] fix: enhanced discard_pps_evc_ids to also handle metadata removed completely --- managers/int.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/managers/int.py b/managers/int.py index e825d3a..441f77d 100644 --- a/managers/int.py +++ b/managers/int.py @@ -313,11 +313,7 @@ async def disable_int( } } await self.remove_int_flows(evcs, metadata, force=force) - try: - self._discard_pps_evc_ids(evcs) - except ProxyPortError: - if not force: - raise + self._discard_pps_evc_ids(evcs) async def remove_int_flows( self, evcs: dict[str, dict], metadata: dict, force=False @@ -730,14 +726,20 @@ def _discard_pps_evc_ids(self, evcs: dict[str, dict]) -> None: """ for evc_id, evc in evcs.items(): uni_a, uni_z = utils.get_evc_unis(evc) - pp_a = self.get_proxy_port_or_raise(uni_a["interface_id"], evc_id) - pp_z = self.get_proxy_port_or_raise(uni_z["interface_id"], evc_id) - pp_a.evc_ids.discard(evc_id) - pp_z.evc_ids.discard(evc_id) - if not pp_a.evc_ids: - self.unis_src.pop(evc["uni_a"]["interface_id"], None) - if not pp_z.evc_ids: - self.unis_src.pop(evc["uni_z"]["interface_id"], None) + try: + pp_a = self.srcs_pp[self.unis_src[uni_a["interface_id"]]] + pp_a.evc_ids.discard(evc_id) + if not pp_a.evc_ids: + self.unis_src.pop(evc["uni_a"]["interface_id"], None) + except KeyError: + pass + try: + pp_z = self.srcs_pp[self.unis_src[uni_z["interface_id"]]] + pp_z.evc_ids.discard(evc_id) + if not pp_z.evc_ids: + self.unis_src.pop(evc["uni_z"]["interface_id"], None) + except KeyError: + pass def evc_compare( self, stored_int_flows: dict, stored_mef_flows: dict, evcs: dict From c79920e64b459c00698370b1ae7acc10778059db Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Fri, 9 Aug 2024 12:00:15 -0300 Subject: [PATCH 22/34] chore: include term-missing on setup.py cov --- setup.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 7a69268..140ff58 100644 --- a/setup.py +++ b/setup.py @@ -115,7 +115,10 @@ class TestCoverage(Test): def run(self): """Run tests quietly and display coverage report.""" - cmd = "python3 -m pytest --cov=. tests/ %s" % self.get_args() + cmd = ( + "python3 -m pytest --cov=. --cov-report term-missing tests/ %s" + % self.get_args() + ) try: check_call(cmd, shell=True) except CalledProcessError as exc: From 681c4680aee1c666bf2e63bfe86e0fa1fd345d1d Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Fri, 9 Aug 2024 12:00:51 -0300 Subject: [PATCH 23/34] tests: increased unit test cov --- tests/unit/test_int_manager.py | 8 +- tests/unit/test_kytos_api_helper.py | 26 +++ tests/unit/test_main.py | 253 +++++++++++++++++++++++++++- 3 files changed, 279 insertions(+), 8 deletions(-) diff --git a/tests/unit/test_int_manager.py b/tests/unit/test_int_manager.py index 355a0d0..ba0ac00 100644 --- a/tests/unit/test_int_manager.py +++ b/tests/unit/test_int_manager.py @@ -577,11 +577,11 @@ def test__discard_pps_evc_ids(self): controller = get_controller_mock() int_manager = INTManager(controller) pp = MagicMock() - mock = MagicMock() - int_manager.get_proxy_port_or_raise = mock - mock.return_value = pp + int_manager.unis_src[intf_id_a] = "a" + int_manager.unis_src[intf_id_z] = "z" + int_manager.srcs_pp[int_manager.unis_src[intf_id_a]] = pp + int_manager.srcs_pp[int_manager.unis_src[intf_id_z]] = pp int_manager._discard_pps_evc_ids(evcs) - assert int_manager.get_proxy_port_or_raise.call_count == 2 assert pp.evc_ids.discard.call_count == 2 pp.evc_ids.discard.assert_called_with(evc_id) diff --git a/tests/unit/test_kytos_api_helper.py b/tests/unit/test_kytos_api_helper.py index d5b8075..b943ad6 100644 --- a/tests/unit/test_kytos_api_helper.py +++ b/tests/unit/test_kytos_api_helper.py @@ -4,6 +4,8 @@ from unittest.mock import AsyncMock, MagicMock from napps.kytos.telemetry_int.kytos_api_helper import ( add_evcs_metadata, + add_proxy_port_metadata, + delete_proxy_port_metadata, get_evc, get_stored_flows, get_evcs, @@ -105,3 +107,27 @@ async def test_add_evcs_metadata(monkeypatch): {"some_id": {"id": "some_id"}}, {"some_key": "some_val"} ) assert data == resp + + +async def test_add_proxy_port_metadata(monkeypatch): + """test add_proxy_port_metadata.""" + aclient_mock, awith_mock = AsyncMock(), MagicMock() + resp = "Operation successful" + aclient_mock.post.return_value = Response(201, json=resp, request=MagicMock()) + awith_mock.return_value.__aenter__.return_value = aclient_mock + monkeypatch.setattr("httpx.AsyncClient", awith_mock) + intf_id, port_no = "00:00:00:00:00:00:00:01:1", 7 + data = await add_proxy_port_metadata(intf_id, port_no) + assert data + + +async def test_delete_proxy_port_metadata(monkeypatch): + """test delete_proxy_port_metadata.""" + aclient_mock, awith_mock = AsyncMock(), MagicMock() + resp = "Operation successful" + aclient_mock.post.return_value = Response(201, json=resp, request=MagicMock()) + awith_mock.return_value.__aenter__.return_value = aclient_mock + monkeypatch.setattr("httpx.AsyncClient", awith_mock) + intf_id = "00:00:00:00:00:00:00:01:1" + data = await delete_proxy_port_metadata(intf_id) + assert data diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index 10e1c07..7a99c7c 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -1,12 +1,15 @@ """Test Main methods.""" -import pytest - from unittest.mock import AsyncMock, MagicMock, patch -from napps.kytos.telemetry_int.main import Main + +import pytest from napps.kytos.telemetry_int import utils -from kytos.lib.helpers import get_controller_mock, get_test_client +from napps.kytos.telemetry_int.exceptions import EVCError, ProxyPortShared +from napps.kytos.telemetry_int.main import Main + +from kytos.core.common import EntityStatus from kytos.core.events import KytosEvent +from kytos.lib.helpers import get_controller_mock, get_test_client class TestMain: @@ -239,6 +242,224 @@ async def test_get_evc_compare_missing_some_int_flows(self, monkeypatch) -> None assert data[0]["compare_reason"] == ["missing_some_int_flows"] assert data[0]["name"] == "evc" + async def test_delete_proxy_port_metadata(self, monkeypatch) -> None: + """Test delete proxy_port metadata.""" + api_mock = AsyncMock() + monkeypatch.setattr( + "napps.kytos.telemetry_int.main.api", + api_mock, + ) + intf_id, port_number = "00:00:00:00:00:00:00:01:1", 7 + endpoint = f"{self.base_endpoint}/uni/{intf_id}/proxy_port" + self.napp.controller.get_interface_by_id = MagicMock() + pp = MagicMock() + pp.evc_ids = set() + self.napp.int_manager.get_proxy_port_or_raise = MagicMock() + self.napp.int_manager.get_proxy_port_or_raise.return_value = pp + intf_mock = MagicMock() + intf_mock.metadata = {"proxy_port": port_number} + self.napp.controller.get_interface_by_id = MagicMock() + self.napp.controller.get_interface_by_id.return_value = intf_mock + response = await self.api_client.delete(endpoint) + assert response.status_code == 200 + data = response.json() + assert data == "Operation successful" + assert api_mock.delete_proxy_port_metadata.call_count == 1 + + async def test_delete_proxy_port_metadata_force(self, monkeypatch) -> None: + """Test delete proxy_port metadata force.""" + api_mock = AsyncMock() + monkeypatch.setattr( + "napps.kytos.telemetry_int.main.api", + api_mock, + ) + intf_id, port_number = "00:00:00:00:00:00:00:01:1", 7 + endpoint = f"{self.base_endpoint}/uni/{intf_id}/proxy_port" + self.napp.controller.get_interface_by_id = MagicMock() + pp = MagicMock() + pp.evc_ids = set(["some_id"]) + self.napp.int_manager.get_proxy_port_or_raise = MagicMock() + self.napp.int_manager.get_proxy_port_or_raise.return_value = pp + intf_mock = MagicMock() + intf_mock.metadata = {"proxy_port": port_number} + self.napp.controller.get_interface_by_id = MagicMock() + self.napp.controller.get_interface_by_id.return_value = intf_mock + response = await self.api_client.delete(endpoint) + assert response.status_code == 409 + data = response.json()["description"] + assert "is in use on 1" in data + assert not api_mock.delete_proxy_port_metadata.call_count + + endpoint = f"{self.base_endpoint}/uni/{intf_id}/proxy_port?force=true" + response = await self.api_client.delete(endpoint) + assert response.status_code == 200 + data = response.json() + assert "Operation successful" in data + assert api_mock.delete_proxy_port_metadata.call_count == 1 + + async def test_delete_proxy_port_metadata_early_ret(self, monkeypatch) -> None: + """Test delete proxy_port metadata early ret.""" + api_mock = AsyncMock() + monkeypatch.setattr( + "napps.kytos.telemetry_int.main.api", + api_mock, + ) + intf_id = "00:00:00:00:00:00:00:01:1" + endpoint = f"{self.base_endpoint}/uni/{intf_id}/proxy_port" + self.napp.controller.get_interface_by_id = MagicMock() + pp = MagicMock() + pp.evc_ids = set(["some_id"]) + self.napp.int_manager.get_proxy_port_or_raise = MagicMock() + self.napp.int_manager.get_proxy_port_or_raise.return_value = pp + intf_mock = MagicMock() + intf_mock.metadata = {} + self.napp.controller.get_interface_by_id = MagicMock() + self.napp.controller.get_interface_by_id.return_value = intf_mock + response = await self.api_client.delete(endpoint) + assert response.status_code == 200 + data = response.json() + assert "Operation successful" in data + assert not api_mock.delete_proxy_port_metadata.call_count + + async def test_add_proxy_port_metadata(self, monkeypatch) -> None: + """Test add proxy_port metadata.""" + api_mock = AsyncMock() + monkeypatch.setattr( + "napps.kytos.telemetry_int.main.api", + api_mock, + ) + intf_id, port_number = "00:00:00:00:00:00:00:01:1", 7 + endpoint = f"{self.base_endpoint}/uni/{intf_id}/proxy_port/{port_number}" + self.napp.controller.get_interface_by_id = MagicMock() + pp = MagicMock() + pp.status = EntityStatus.UP + self.napp.int_manager.get_proxy_port_or_raise = MagicMock() + self.napp.int_manager.get_proxy_port_or_raise.return_value = pp + response = await self.api_client.post(endpoint) + assert response.status_code == 200 + data = response.json() + assert data == "Operation successful" + assert api_mock.add_proxy_port_metadata.call_count == 1 + + async def test_add_proxy_port_metadata_early_ret(self, monkeypatch) -> None: + """Test add proxy_port metadata early ret.""" + api_mock = AsyncMock() + monkeypatch.setattr( + "napps.kytos.telemetry_int.main.api", + api_mock, + ) + intf_id, port_number = "00:00:00:00:00:00:00:01:1", 7 + endpoint = f"{self.base_endpoint}/uni/{intf_id}/proxy_port/{port_number}" + intf_mock = MagicMock() + intf_mock.metadata = {"proxy_port": port_number} + self.napp.controller.get_interface_by_id = MagicMock() + self.napp.controller.get_interface_by_id.return_value = intf_mock + response = await self.api_client.post(endpoint) + assert response.status_code == 200 + data = response.json() + assert data == "Operation successful" + assert not api_mock.add_proxy_port_metadata.call_count + + async def test_add_proxy_port_metadata_conflict(self, monkeypatch) -> None: + """Test add proxy_port metadata conflict.""" + api_mock = AsyncMock() + monkeypatch.setattr( + "napps.kytos.telemetry_int.main.api", + api_mock, + ) + intf_id, port_number = "00:00:00:00:00:00:00:01:1", 7 + endpoint = f"{self.base_endpoint}/uni/{intf_id}/proxy_port/{port_number}" + self.napp.controller.get_interface_by_id = MagicMock() + pp = MagicMock() + pp.status = EntityStatus.UP + self.napp.int_manager.get_proxy_port_or_raise = MagicMock() + self.napp.int_manager.get_proxy_port_or_raise.side_effect = ProxyPortShared( + "no_evc_id", "boom" + ) + response = await self.api_client.post(endpoint) + assert response.status_code == 409 + + async def test_add_proxy_port_metadata_force(self, monkeypatch) -> None: + """Test add proxy_port metadata force.""" + api_mock = AsyncMock() + monkeypatch.setattr( + "napps.kytos.telemetry_int.main.api", + api_mock, + ) + intf_id, port_number = "00:00:00:00:00:00:00:01:1", 7 + force = "true" + endpoint = ( + f"{self.base_endpoint}/uni/{intf_id}/proxy_port/{port_number}?force={force}" + ) + self.napp.controller.get_interface_by_id = MagicMock() + pp = MagicMock() + # despite proxy port down, with force true the request shoudl succeed + pp.status = EntityStatus.DOWN + self.napp.int_manager.get_proxy_port_or_raise = MagicMock() + self.napp.int_manager.get_proxy_port_or_raise.return_value = pp + response = await self.api_client.post(endpoint) + assert response.status_code == 200 + data = response.json() + assert data == "Operation successful" + assert api_mock.add_proxy_port_metadata.call_count == 1 + + force = "false" + endpoint = ( + f"{self.base_endpoint}/uni/{intf_id}/proxy_port/{port_number}?force={force}" + ) + response = await self.api_client.post(endpoint) + assert response.status_code == 409 + assert "isn't UP" in response.json()["description"] + + async def test_list_proxy_port(self) -> None: + """Test list proxy port.""" + endpoint = f"{self.base_endpoint}/uni/proxy_port" + response = await self.api_client.get(endpoint) + assert response.status_code == 200 + data = response.json() + assert not data + + sw1, intf_mock = MagicMock(), MagicMock() + intf_mock.metadata = {"proxy_port": 1} + intf_mock.status.value = "UP" + intf_mock.id = "1" + sw1.interfaces = {"intf1": intf_mock} + self.napp.controller.switches = {"sw1": sw1} + response = await self.api_client.get(endpoint) + assert response.status_code == 200 + data = response.json() + expected = [ + { + "proxy_port": { + "port_number": 1, + "status": "DOWN", + "status_reason": ["UNI interface 1 not found"], + }, + "uni": {"id": "1", "status": "UP", "status_reason": []}, + } + ] + assert data == expected + + pp = MagicMock() + self.napp.int_manager.get_proxy_port_or_raise = MagicMock() + self.napp.int_manager.get_proxy_port_or_raise.return_value = pp + pp.status.value = "UP" + + response = await self.api_client.get(endpoint) + assert response.status_code == 200 + data = response.json() + expected = [ + { + "proxy_port": { + "port_number": 1, + "status": "UP", + "status_reason": [], + }, + "uni": {"id": "1", "status": "UP", "status_reason": []}, + } + ] + assert data == expected + async def test_on_table_enabled(self) -> None: """Test on_table_enabled.""" assert self.napp.int_manager.flow_builder.table_group == {"evpl": 2, "epl": 3} @@ -269,6 +490,16 @@ async def test_on_evc_deployed(self) -> None: assert self.napp.int_manager.enable_int.call_count == 1 assert self.napp.int_manager.redeploy_int.call_count == 1 + async def test_on_evc_deployed_error(self, monkeypatch) -> None: + """Test on_evc_deployed error.""" + content = {"metadata": {"telemetry_request": {}}, "id": "some_id"} + self.napp.int_manager.enable_int = AsyncMock() + self.napp.int_manager.enable_int.side_effect = EVCError("no_id", "boom") + log_mock = MagicMock() + monkeypatch.setattr("napps.kytos.telemetry_int.main.log", log_mock) + await self.napp.on_evc_deployed(KytosEvent(content=content)) + assert log_mock.error.call_count == 1 + async def test_on_evc_deleted(self) -> None: """Test on_evc_deleted.""" content = {"metadata": {"telemetry": {"enabled": True}}, "id": "some_id"} @@ -329,6 +560,20 @@ async def test_on_evc_redeployed_link(self) -> None: await self.napp.on_evc_redeployed_link(KytosEvent(content=content)) assert self.napp.int_manager.redeploy_int.call_count == 1 + async def test_on_evc_redeployed_link_error(self, monkeypatch) -> None: + """Test on redeployed_link_down|redeployed_link_up error.""" + content = { + "enabled": True, + "metadata": {"telemetry": {"enabled": True}}, + "id": "some_id", + } + log_mock = MagicMock() + monkeypatch.setattr("napps.kytos.telemetry_int.main.log", log_mock) + self.napp.int_manager.redeploy_int = AsyncMock() + self.napp.int_manager.redeploy_int.side_effect = EVCError("no_id", "boom") + await self.napp.on_evc_redeployed_link(KytosEvent(content=content)) + assert log_mock.error.call_count == 1 + async def test_on_evc_error_redeployed_link_down(self) -> None: """Test error_redeployed_link_down.""" content = { From 32e94e8378aacd4cfbb7726a4935c386b77b3c04 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Fri, 9 Aug 2024 12:01:13 -0300 Subject: [PATCH 24/34] chore: linter fixes --- main.py | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/main.py b/main.py index 8cad77f..2f8ef50 100644 --- a/main.py +++ b/main.py @@ -32,8 +32,6 @@ ) from .managers.int import INTManager -# pylint: disable=fixme - class Main(KytosNApp): """Main class of kytos/telemetry NApp. @@ -304,13 +302,12 @@ async def delete_proxy_port_metadata(self, request: Request) -> JSONResponse: """Delete proxy port metadata.""" intf_id = request.path_params["interface_id"] if ( - (intf := self.controller.get_interface_by_id(intf_id)) - and "proxy_port" not in intf.metadata - ): + intf := self.controller.get_interface_by_id(intf_id) + ) and "proxy_port" not in intf.metadata: return JSONResponse("Operation successful") qparams = request.query_params - force = True if qparams.get("force", "false").lower() == "true" else False + force = qparams.get("force", "false").lower() == "true" try: pp = self.int_manager.get_proxy_port_or_raise(intf_id, "no_evc_id") if pp.evc_ids and not force: @@ -342,13 +339,10 @@ async def add_proxy_port_metadata(self, request: Request) -> JSONResponse: qparams = request.query_params if not (intf := self.controller.get_interface_by_id(intf_id)): raise HTTPException(404, detail=f"Interface id {intf_id} not found") - if ( - "proxy_port" in intf.metadata - and intf.metadata["proxy_port"] == port_no - ): + if "proxy_port" in intf.metadata and intf.metadata["proxy_port"] == port_no: return JSONResponse("Operation successful") - force = True if qparams.get("force", "false").lower() == "true" else False + force = qparams.get("force", "false").lower() == "true" try: pp = self.int_manager.get_proxy_port_or_raise(intf_id, "no_evc_id", port_no) if pp.status != EntityStatus.UP and not force: @@ -378,13 +372,13 @@ async def list_uni_proxy_ports(self, _request: Request) -> JSONResponse: "uni": { "id": intf.id, "status": intf.status.value, - "status_reason": sorted(intf.status_reason) + "status_reason": sorted(intf.status_reason), }, "proxy_port": { "port_number": intf.metadata["proxy_port"], "status": "DOWN", - "status_reason": [] - } + "status_reason": [], + }, } try: pp = self.int_manager.get_proxy_port_or_raise( From 95d6013c15b64c7df26f6e78ac9c395bed4bebb7 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Fri, 9 Aug 2024 12:06:11 -0300 Subject: [PATCH 25/34] updated changelog --- CHANGELOG.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 29ab1b2..2ae7600 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,6 +16,7 @@ Added - Handled ``kytos/mef_eline.deployed`` event. - Handled ``kytos/mef_eline.(failover_link_down|failover_old_path|failover_deployed)`` events. - Added UI for telemetry_int. +- Included the endpoints to create, update, delete and list proxy_port metadata, and updated OpenAPI spec. These endpoints should be used to manage the proxy_port metadata instead of directly on topology endpoints since these endpoints provide extra validations. Changed ======= From 7f4980ddbf8e59c7e0081abe0dc3242ec9ed5be6 Mon Sep 17 00:00:00 2001 From: David Ramirez Date: Mon, 12 Aug 2024 09:54:28 -0400 Subject: [PATCH 26/34] Updated changelog --- CHANGELOG.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 29ab1b2..81ee986 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -22,6 +22,10 @@ Changed - Only raise ``FlowsNotFound`` when an EVC is active and flows aren't found. Update status and status_reason accordingly too when installing flows. - Validate to support only a single proxy port per UNI for now. +Removed +======= +- Removed client side batching with ``BATCH_INTERVAL`` and ``BATCH_SIZE``, now replaced with pacing in ``flow_manager`` + Fixed ===== - Only redeploy if INT has been enabled before From 554af55324b2dc12875a2714c305506e71fe454d Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Wed, 14 Aug 2024 11:07:17 -0300 Subject: [PATCH 27/34] ui: included new k-button and request for listing proxy ports k-info-panel --- ui/k-toolbar/main.kytos | 57 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/ui/k-toolbar/main.kytos b/ui/k-toolbar/main.kytos index 78c9232..af81c30 100644 --- a/ui/k-toolbar/main.kytos +++ b/ui/k-toolbar/main.kytos @@ -1,11 +1,14 @@ @@ -14,8 +17,12 @@ module.exports = { data: function() { return { telemetryINT_Data: [], + proxy_ports: [], } }, + mounted() { + this.$kytos.eventBus.$on('kytos/telemetry_int/refetch-proxy-ports', this.get_proxy_ports) + }, methods: { //API request for EVC data get_EVCs() { @@ -38,6 +45,41 @@ module.exports = { self.$kytos.eventBus.$emit("setNotification", notification) }); }, + //API request for getting Proxy Ports + get_proxy_ports() { + const self = this + const request = $.ajax({ + type:"GET", + dataType: "json", + url: this.$kytos_server_api + "kytos/telemetry_int/v1/uni/proxy_port", + async: true}); + request.done(function(data) { + self.set_proxy_ports(data) + self.display_proxy_port_info_panel() + }); + request.fail(function(data) { + let notification = { + icon: 'cog', + title: 'Could not retrieve proxy ports', + description: data.status.toString() + ": " + data.responseJSON.description + } + self.$kytos.eventBus.$emit("setNotification", notification) + }); + }, + set_proxy_ports(data){ + this.proxy_ports = data.map((obj) => + ( + { + uni_id: obj.uni.id, + uni_status: obj.uni.status, + uni_status_reason: obj.uni.status_reason.join(",") || "N/A", + proxy_port_port_number: obj.proxy_port.port_number, + proxy_port_status: obj.proxy_port.status, + proxy_port_status_reason: obj.proxy_port.status_reason.join(",") || "N/A" + } + ) + ) + }, /** * @param {Object} EVC_Data - The EVC data from mef_eline. * @description Extracts only the required information for the telemetry_int table from the mef_eline EVC data. @@ -78,10 +120,21 @@ module.exports = { } this.$kytos.eventBus.$emit("showInfoPanel", content) }, + // Display list_proxy_ports k-info-panel + display_proxy_port_info_panel() { + const content = { + "component": 'kytos-telemetry_int-k-info-panel-list_proxy_ports', + "content": this.proxy_ports, + "icon": "info-circle", + "title": "Proxy Ports", + "subtitle": "by kytos/telemetry_int" + } + this.$kytos.eventBus.$emit("showInfoPanel", content) + }, //Formats status reason data for table splitStatusReasons(statusReasons) { return statusReasons.toString() } } } - \ No newline at end of file + From 4c7f00aaaa7400066a2ba04d742ff0472d9c54a2 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Wed, 14 Aug 2024 11:16:41 -0300 Subject: [PATCH 28/34] refactor: refactored proxy_port deletion to also handle proxy ports configured but without EVCs enabled yet --- main.py | 14 +++++++------ managers/int.py | 53 +++++++++++++++++++++++++++++-------------------- 2 files changed, 39 insertions(+), 28 deletions(-) diff --git a/main.py b/main.py index 2f8ef50..f6ee38e 100644 --- a/main.py +++ b/main.py @@ -301,15 +301,17 @@ async def evc_compare(self, _request: Request) -> JSONResponse: async def delete_proxy_port_metadata(self, request: Request) -> JSONResponse: """Delete proxy port metadata.""" intf_id = request.path_params["interface_id"] - if ( - intf := self.controller.get_interface_by_id(intf_id) - ) and "proxy_port" not in intf.metadata: + intf = self.controller.get_interface_by_id(intf_id) + if not intf: + raise HTTPException(404, detail=f"Interface id {intf_id} not found") + if "proxy_port" not in intf.metadata: return JSONResponse("Operation successful") qparams = request.query_params force = qparams.get("force", "false").lower() == "true" + try: - pp = self.int_manager.get_proxy_port_or_raise(intf_id, "no_evc_id") + pp = self.int_manager.srcs_pp[self.int_manager.unis_src[intf_id]] if pp.evc_ids and not force: return JSONResponse( { @@ -320,8 +322,8 @@ async def delete_proxy_port_metadata(self, request: Request) -> JSONResponse: }, status_code=409, ) - except ProxyPortError as exc: - raise HTTPException(404, detail=exc.message) + except KeyError: + pass try: await api.delete_proxy_port_metadata(intf_id) diff --git a/managers/int.py b/managers/int.py index 441f77d..80db0f8 100644 --- a/managers/int.py +++ b/managers/int.py @@ -61,26 +61,36 @@ def load_uni_src_proxy_ports(self, evcs: dict[str, dict]) -> None: uni_a = self.controller.get_interface_by_id(uni_a_id) uni_z = self.controller.get_interface_by_id(uni_z_id) if uni_a and "proxy_port" in uni_a.metadata: - src_a = uni_a.switch.get_interface_by_port_no( + if src_a := uni_a.switch.get_interface_by_port_no( uni_a.metadata["proxy_port"] - ) - self.unis_src[uni_a.id] = src_a.id - try: - pp = self.get_proxy_port_or_raise(uni_a.id, evc_id) - except ProxyPortDestNotFound: - pp = self.srcs_pp[src_a.id] - pp.evc_ids.add(evc_id) + ): + self.unis_src[uni_a.id] = src_a.id + try: + pp = self.get_proxy_port_or_raise(uni_a.id, evc_id) + except ProxyPortDestNotFound: + pp = self.srcs_pp[src_a.id] + pp.evc_ids.add(evc_id) + else: + log.error( + f"Failed to load proxy_port {uni_a.metadata['proxy_port']} " + f"of UNI {uni_a_id}. You need to set a correct proxy_port value" + ) if uni_z and "proxy_port" in uni_z.metadata: - src_z = uni_z.switch.get_interface_by_port_no( + if src_z := uni_z.switch.get_interface_by_port_no( uni_z.metadata["proxy_port"] - ) - self.unis_src[uni_z.id] = src_z.id - try: - pp = self.get_proxy_port_or_raise(uni_z.id, evc_id) - except ProxyPortDestNotFound: - pp = self.srcs_pp[src_z.id] - pp.evc_ids.add(evc_id) + ): + self.unis_src[uni_z.id] = src_z.id + try: + pp = self.get_proxy_port_or_raise(uni_z.id, evc_id) + except ProxyPortDestNotFound: + pp = self.srcs_pp[src_z.id] + pp.evc_ids.add(evc_id) + else: + log.error( + f"Failed to load proxy_port {uni_z.metadata['proxy_port']} " + f"of UNI {uni_z_id}. You need to set a correct proxy_port value" + ) async def handle_pp_link_down(self, link: Link) -> None: """Handle proxy_port link_down.""" @@ -452,8 +462,7 @@ def get_proxy_port_or_raise( if not pp.destination: raise ProxyPortDestNotFound( evc_id, - f"proxy_port {port_no} of {intf_id} isn't looped or " - "destination interface not found", + f"proxy_port {port_no} of UNI {intf_id} isn't looped" ) return pp @@ -520,8 +529,8 @@ def _validate_new_dedicated_proxy_port( and intf.metadata["proxy_port"] == new_port_no ): msg = ( - f"UNI {uni.id} must use another dedicated proxy port. " - f"UNI {intf.id} is already using proxy_port number {new_port_no}" + f"UNI {uni.id} must use another dedicated proxy_port. " + f"UNI {intf.id} is already using proxy_port {new_port_no}" ) raise ProxyPortShared("no_evc_id", msg) @@ -682,7 +691,7 @@ def _validate_map_enable_evcs( dest_status = pp_a.status if pp_a.destination else None raise ProxyPortStatusNotUP( evc_id, - f"proxy_port of {uni_a['interface_id']} isn't UP. " + f"proxy_port of UNI {uni_a['interface_id']} isn't UP. " f"source {pp_a.source.id} status {pp_a.source.status}, " f"destination {dest_id} status {dest_status}", ) @@ -691,7 +700,7 @@ def _validate_map_enable_evcs( dest_status = pp_z.status if pp_z.destination else None raise ProxyPortStatusNotUP( evc_id, - f"proxy_port of {uni_z['interface_id']} isn't UP." + f"proxy_port of UNI {uni_z['interface_id']} isn't UP." f"source {pp_z.source.id} status {pp_z.source.status}, " f"destination {dest_id} status {dest_status}", ) From 59188693bf6f50559599b4a204f317b6babf8110 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Wed, 14 Aug 2024 11:18:10 -0300 Subject: [PATCH 29/34] chore: refactored unit tests accordingly --- tests/unit/test_int_manager.py | 2 +- tests/unit/test_main.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_int_manager.py b/tests/unit/test_int_manager.py index ba0ac00..e6f4137 100644 --- a/tests/unit/test_int_manager.py +++ b/tests/unit/test_int_manager.py @@ -45,7 +45,7 @@ def test_get_proxy_port_or_raise(self) -> None: mock_interface_a.metadata = {"proxy_port": 5} with pytest.raises(exceptions.ProxyPortDestNotFound) as exc: int_manager.get_proxy_port_or_raise(intf_id, evc_id) - assert "destination interface not found" in str(exc) + assert "isn't looped" in str(exc) mock_interface_b = get_interface_mock("s1-eth5", 5, mock_switch_a) mock_interface_b.metadata = {"looped": {"port_numbers": [5, 6]}} diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index 7a99c7c..32763b9 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -274,12 +274,13 @@ async def test_delete_proxy_port_metadata_force(self, monkeypatch) -> None: api_mock, ) intf_id, port_number = "00:00:00:00:00:00:00:01:1", 7 + src_id = "00:00:00:00:00:00:00:01:7" endpoint = f"{self.base_endpoint}/uni/{intf_id}/proxy_port" self.napp.controller.get_interface_by_id = MagicMock() pp = MagicMock() pp.evc_ids = set(["some_id"]) - self.napp.int_manager.get_proxy_port_or_raise = MagicMock() - self.napp.int_manager.get_proxy_port_or_raise.return_value = pp + self.napp.int_manager.unis_src[intf_id] = src_id + self.napp.int_manager.srcs_pp[src_id] = pp intf_mock = MagicMock() intf_mock.metadata = {"proxy_port": port_number} self.napp.controller.get_interface_by_id = MagicMock() From 3a1ecc4a865304c23f9ade4631da0630f7bbb410 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Wed, 14 Aug 2024 11:23:10 -0300 Subject: [PATCH 30/34] chore: removed console log debug --- ui/k-info-panel/show_telemetry_int_data.kytos | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ui/k-info-panel/show_telemetry_int_data.kytos b/ui/k-info-panel/show_telemetry_int_data.kytos index 0c50613..ca8ab0f 100644 --- a/ui/k-info-panel/show_telemetry_int_data.kytos +++ b/ui/k-info-panel/show_telemetry_int_data.kytos @@ -394,8 +394,6 @@ content: { handler: function() { this.currentTableData = this.content - console.log(this.content) - console.log(this.currentTableData) }, deep: true } @@ -509,4 +507,4 @@ color: #ffc5c5; background: #be0000; } - \ No newline at end of file + From 2063e21b43b2ea241b63c5f298d91e880467fcf3 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Thu, 15 Aug 2024 17:07:25 -0300 Subject: [PATCH 31/34] refactor: extracted json_err_description_or_text method --- ui/k-info-panel/show_telemetry_int_data.kytos | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/ui/k-info-panel/show_telemetry_int_data.kytos b/ui/k-info-panel/show_telemetry_int_data.kytos index ca8ab0f..1153ae1 100644 --- a/ui/k-info-panel/show_telemetry_int_data.kytos +++ b/ui/k-info-panel/show_telemetry_int_data.kytos @@ -187,7 +187,7 @@ let notification = { icon: 'cog', title: 'Could not enable INT on EVCs', - description: data.status.toString() + ": " + data.responseJSON.description + description: self.json_err_description_or_text(data) } self.get_EVCs() self.$kytos.eventBus.$emit("setNotification", notification) @@ -224,7 +224,7 @@ let notification = { icon: 'cog', title: 'Could not disable INT on EVCs', - description: data.status.toString() + ": " + data.responseJSON.description + description: self.json_err_description_or_text(data) } self.get_EVCs() self.$kytos.eventBus.$emit("setNotification", notification) @@ -258,7 +258,7 @@ let notification = { icon: 'cog', title: 'Could not redeploy INT on EVCs', - description: data.status.toString() + ": " + data.responseJSON.description + description: self.json_err_description_or_text(data) } self.get_EVCs() self.$kytos.eventBus.$emit("setNotification", notification) @@ -279,7 +279,7 @@ let notification = { icon: 'cog', title: 'Could not retrieve EVCs', - description: data.status.toString() + ": " + data.responseJSON.description + description: self.json_err_description_or_text(data) } self.$kytos.eventBus.$emit("setNotification", notification) }); @@ -329,6 +329,11 @@ */ this.showDelModal = false this.disableSelectedEVCs() + }, + // Map request fail data response, either get json description or text + json_err_description_or_text(data) { + const content = data.responseJSON ? data.responseJSON.description : data.responseText + return data.status.toString() + ": " + content } }, computed: { From 0cd942fc0a6f4072b4fad8172e91b9227b4121d3 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Thu, 15 Aug 2024 17:14:36 -0300 Subject: [PATCH 32/34] feat: added list_proxy_port k-info-panel --- ui/k-info-panel/list_proxy_ports.kytos | 472 +++++++++++++++++++++++++ 1 file changed, 472 insertions(+) create mode 100644 ui/k-info-panel/list_proxy_ports.kytos diff --git a/ui/k-info-panel/list_proxy_ports.kytos b/ui/k-info-panel/list_proxy_ports.kytos new file mode 100644 index 0000000..6eef6fd --- /dev/null +++ b/ui/k-info-panel/list_proxy_ports.kytos @@ -0,0 +1,472 @@ + + + From 1816a334b94dff8725dbcaf1dd66066d1f398188 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Thu, 15 Aug 2024 17:45:23 -0300 Subject: [PATCH 33/34] chore: reverted extracted err extract funcs due to obj lifetime context --- ui/k-info-panel/list_proxy_ports.kytos | 10 ++++------ ui/k-info-panel/show_telemetry_int_data.kytos | 17 ++++++++--------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/ui/k-info-panel/list_proxy_ports.kytos b/ui/k-info-panel/list_proxy_ports.kytos index 6eef6fd..d008572 100644 --- a/ui/k-info-panel/list_proxy_ports.kytos +++ b/ui/k-info-panel/list_proxy_ports.kytos @@ -190,10 +190,11 @@ self.$kytos.eventBus.$emit("setNotification", notification) }) request.fail(function(data) { + const content = data.responseJSON ? data.responseJSON.description : data.responseText let notification = { icon: 'cog', title: 'Could not set proxy_port', - description: self.json_err_description_or_text(data) + description: data.status.toString() + ": " + content } self.$kytos.eventBus.$emit("setNotification", notification) }) @@ -219,10 +220,11 @@ self.$kytos.eventBus.$emit("setNotification", notification) }) request.fail(function(data) { + const content = data.responseJSON ? data.responseJSON.description : data.responseText let notification = { icon: 'cog', title: `Could not delete proxy_port of UNI ${uni_intf}`, - description: self.json_err_description_or_text(data) + description: data.status.toString() + ": " + content } self.$kytos.eventBus.$emit("setNotification", notification) }) @@ -247,10 +249,6 @@ self.fetched_interfaces = data['interfaces'] } }) - }, - json_err_description_or_text(data) { - const content = data.responseJSON ? data.responseJSON.description : data.responseText - return data.status.toString() + ": " + content } }, computed: { diff --git a/ui/k-info-panel/show_telemetry_int_data.kytos b/ui/k-info-panel/show_telemetry_int_data.kytos index 1153ae1..ab09780 100644 --- a/ui/k-info-panel/show_telemetry_int_data.kytos +++ b/ui/k-info-panel/show_telemetry_int_data.kytos @@ -184,10 +184,11 @@ self.$kytos.eventBus.$emit("setNotification", notification) }); request.fail(function(data) { + const content = data.responseJSON ? data.responseJSON.description : data.responseText let notification = { icon: 'cog', title: 'Could not enable INT on EVCs', - description: self.json_err_description_or_text(data) + description: data.status.toString() + ": " + content } self.get_EVCs() self.$kytos.eventBus.$emit("setNotification", notification) @@ -221,10 +222,11 @@ self.$kytos.eventBus.$emit("setNotification", notification) }); request.fail(function(data) { + const content = data.responseJSON ? data.responseJSON.description : data.responseText let notification = { icon: 'cog', title: 'Could not disable INT on EVCs', - description: self.json_err_description_or_text(data) + description: data.status.toString() + ": " + content } self.get_EVCs() self.$kytos.eventBus.$emit("setNotification", notification) @@ -255,10 +257,11 @@ self.$kytos.eventBus.$emit("setNotification", notification) }); request.fail(function(data) { + const content = data.responseJSON ? data.responseJSON.description : data.responseText let notification = { icon: 'cog', title: 'Could not redeploy INT on EVCs', - description: self.json_err_description_or_text(data) + description: data.status.toString() + ": " + content } self.get_EVCs() self.$kytos.eventBus.$emit("setNotification", notification) @@ -276,10 +279,11 @@ self.extract_TelemetryINTData(data) }); request.fail(function(data) { + const content = data.responseJSON ? data.responseJSON.description : data.responseText let notification = { icon: 'cog', title: 'Could not retrieve EVCs', - description: self.json_err_description_or_text(data) + description: data.status.toString() + ": " + content } self.$kytos.eventBus.$emit("setNotification", notification) }); @@ -329,11 +333,6 @@ */ this.showDelModal = false this.disableSelectedEVCs() - }, - // Map request fail data response, either get json description or text - json_err_description_or_text(data) { - const content = data.responseJSON ? data.responseJSON.description : data.responseText - return data.status.toString() + ": " + content } }, computed: { From 1cf9b2005af210b81dab82d0b01d656f097eed01 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Thu, 15 Aug 2024 17:50:07 -0300 Subject: [PATCH 34/34] updated changelog --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 3e2a47d..9e874bd 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,7 +15,7 @@ Added - Handled ``kytos/mef_eline.uni_active_updated`` event. - Handled ``kytos/mef_eline.deployed`` event. - Handled ``kytos/mef_eline.(failover_link_down|failover_old_path|failover_deployed)`` events. -- Added UI for telemetry_int. +- Added UI for telemetry_int to list EVCs and to configure proxy ports - Included the endpoints to create, update, delete and list proxy_port metadata, and updated OpenAPI spec. These endpoints should be used to manage the proxy_port metadata instead of directly on topology endpoints since these endpoints provide extra validations. Changed