From 1feb75fb667a95f701a2ac277b22128fdcf91ca5 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 26 Nov 2024 12:09:35 -0600 Subject: [PATCH 01/10] Make more things Optional Running pyright/mypy, and handling some quick warnings --- src/sdx_pce/topology/temanager.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sdx_pce/topology/temanager.py b/src/sdx_pce/topology/temanager.py index ed8e8ed2..d3484cf8 100644 --- a/src/sdx_pce/topology/temanager.py +++ b/src/sdx_pce/topology/temanager.py @@ -1023,8 +1023,8 @@ def _reserve_vlan( domain: str, port: dict, request_id: str, - tag: str = None, - upstream_egress_vlan: str = None, + tag: Optional[str] = None, + upstream_egress_vlan: Optional[str] = None, ): """ Find unused VLANs for given domain/port and mark them in-use. @@ -1170,7 +1170,7 @@ def delete_connection(self, request_id: str): # Now it is the time to update the bandwidth of the links after breakdowns are successfully generated self.update_link_bandwidth(solution, reduce=False) - def get_connection_solution(self, request_id: str) -> ConnectionSolution: + def get_connection_solution(self, request_id: str) -> Optional[ConnectionSolution]: """ Get a connection solution by request ID. """ From 002541f9519e3996eb1045b886be1babf532abe1 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 26 Nov 2024 13:09:34 -0600 Subject: [PATCH 02/10] Add methods to get/set VLAN tags table --- src/sdx_pce/topology/temanager.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/sdx_pce/topology/temanager.py b/src/sdx_pce/topology/temanager.py index d3484cf8..505b91e5 100644 --- a/src/sdx_pce/topology/temanager.py +++ b/src/sdx_pce/topology/temanager.py @@ -127,6 +127,20 @@ def get_connections(self) -> List[ConnectionRequest]: connections.append(solution.request_id) return connections + @property + def vlan_tags_table(self) -> dict: + """ + Return the current VLAN tags table. + """ + return self._vlan_tags_table + + @vlan_tags_table.setter + def vlan_tags_table(self, table: dict): + """ + Set VLAN tags table. + """ + self._vlan_tags_table = table + def _update_vlan_tags_table(self, domain_name: str, port_map: dict): """ Update VLAN tags table in a non-disruptive way, meaning: only add new From d7505332c246d897eb106f040b367de1582a4b2e Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 26 Nov 2024 13:10:38 -0600 Subject: [PATCH 03/10] Add a TODO item --- src/sdx_pce/topology/temanager.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sdx_pce/topology/temanager.py b/src/sdx_pce/topology/temanager.py index 505b91e5..4e5c554c 100644 --- a/src/sdx_pce/topology/temanager.py +++ b/src/sdx_pce/topology/temanager.py @@ -139,6 +139,7 @@ def vlan_tags_table(self, table: dict): """ Set VLAN tags table. """ + # TODO: validate the table, perhaps? self._vlan_tags_table = table def _update_vlan_tags_table(self, domain_name: str, port_map: dict): From 75b6c6cb3f7351b887f87ff44a7902751bba445e Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 26 Nov 2024 13:23:48 -0600 Subject: [PATCH 04/10] Do some validation --- src/sdx_pce/topology/temanager.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/sdx_pce/topology/temanager.py b/src/sdx_pce/topology/temanager.py index 4e5c554c..5109b5c7 100644 --- a/src/sdx_pce/topology/temanager.py +++ b/src/sdx_pce/topology/temanager.py @@ -139,7 +139,21 @@ def vlan_tags_table(self, table: dict): """ Set VLAN tags table. """ - # TODO: validate the table, perhaps? + # Ensure that the input is in correct shape. + if not isinstance(table, dict): + raise ValidationError(f"table ({table}) is not a dict") + + for domain, ports in table.items(): + if not isinstance(domain, str): + raise ValidationError(f"domain ({domain}) is not a str") + + for port_id, labels in ports.items(): + if not isinstance(port_id, str): + raise ValidationError(f"port_id ({port_id}) is not a str") + + if not isinstance(labels, dict): + raise ValidationError(f"labels ({labels}) is not a dict") + self._vlan_tags_table = table def _update_vlan_tags_table(self, domain_name: str, port_map: dict): From 726f059b8928f825c49968706275dfbdd62dbb68 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 26 Nov 2024 13:29:06 -0600 Subject: [PATCH 05/10] Disallow restoring VLAN table once PCE has made some reservations --- src/sdx_pce/topology/temanager.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/sdx_pce/topology/temanager.py b/src/sdx_pce/topology/temanager.py index 5109b5c7..5492f4e4 100644 --- a/src/sdx_pce/topology/temanager.py +++ b/src/sdx_pce/topology/temanager.py @@ -154,6 +154,18 @@ def vlan_tags_table(self, table: dict): if not isinstance(labels, dict): raise ValidationError(f"labels ({labels}) is not a dict") + # We should allow VLAN table to be restored only during + # startup. If the table has VLANs that are in use, it means + # that we're in the wrong state. + for domain, ports in self._vlan_tags_table.items(): + for port_id, labels in ports.items(): + for vlan, status in labels.items(): + if status is not UNUSED_VLAN: + raise ValidationError( + f"Error: VLAN table is not empty:" + f"(domain: {domain}, port: {port}, vlan: {vlan})" + ) + self._vlan_tags_table = table def _update_vlan_tags_table(self, domain_name: str, port_map: dict): From 31a24e4c0ecd8046a7883f5df0174537030fd2c7 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 26 Nov 2024 13:31:57 -0600 Subject: [PATCH 06/10] Add some tests for VLAN table save/restore --- tests/test_te_manager.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/test_te_manager.py b/tests/test_te_manager.py index 1ba9a1df..3cc94e73 100644 --- a/tests/test_te_manager.py +++ b/tests/test_te_manager.py @@ -1815,3 +1815,26 @@ def _vlan_meets_request(self, requested_vlan: str, assigned_vlan: int) -> bool: return requested_vlan in requested_range raise Exception("invalid state!") + + def test_vlan_tags_table(self): + """ + Test saving/restoring VLAN tags table. + """ + temanager = TEManager(topology_data=None) + + for topology_file in [ + TestData.TOPOLOGY_FILE_AMLIGHT_v2, + TestData.TOPOLOGY_FILE_ZAOXI_v2, + TestData.TOPOLOGY_FILE_SAX_v2, + ]: + temanager.add_topology(json.loads(topology_file.read_text())) + + # Test getter. + table1 = temanager.vlan_tags_table + self.assertIsInstance(table1, dict) + + # Test setter + temanager.vlan_tags_table = table1 + table2 = temanager.vlan_tags_table + self.assertIsInstance(table2, dict) + self.assertEqual(table1, table2) From b3af848c334a43e12deba17c60414bb790ea46a1 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 26 Nov 2024 13:46:02 -0600 Subject: [PATCH 07/10] Test error checks --- tests/test_te_manager.py | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/test_te_manager.py b/tests/test_te_manager.py index 3cc94e73..1b74eaa2 100644 --- a/tests/test_te_manager.py +++ b/tests/test_te_manager.py @@ -7,7 +7,7 @@ from sdx_pce.load_balancing.te_solver import TESolver from sdx_pce.models import ConnectionRequest, ConnectionSolution, TrafficMatrix from sdx_pce.topology.temanager import TEManager -from sdx_pce.utils.exceptions import TEError, UnknownRequestError +from sdx_pce.utils.exceptions import TEError, UnknownRequestError, ValidationError from . import TestData @@ -1838,3 +1838,31 @@ def test_vlan_tags_table(self): table2 = temanager.vlan_tags_table self.assertIsInstance(table2, dict) self.assertEqual(table1, table2) + + def test_vlan_tags_table_error_checks(self): + """ + Test error checks when restoring VLAN tags table. + """ + temanager = TEManager(topology_data=None) + + for topology_file in [ + TestData.TOPOLOGY_FILE_AMLIGHT_v2, + TestData.TOPOLOGY_FILE_ZAOXI_v2, + TestData.TOPOLOGY_FILE_SAX_v2, + ]: + temanager.add_topology(json.loads(topology_file.read_text())) + + # input must be a dict. + with self.assertRaises(ValidationError) as ctx: + temanager.vlan_tags_table = list() + self.assertTrue("table ([]) is not a dict" in str(ctx.exception)) + + # port_id keys in the input must be a string. + with self.assertRaises(ValidationError) as ctx: + temanager.vlan_tags_table = {"domain1": {1: {1: None}}} + self.assertTrue("port_id (1) is not a str" in str(ctx.exception)) + + # the "inner" VLAN allocation table must be a dict. + with self.assertRaises(ValidationError) as ctx: + temanager.vlan_tags_table = {"domain1": {"port1": (1, None)}} + self.assertTrue("labels ((1, None)) is not a dict" in str(ctx.exception)) From 551a053939a2ac4f20d51e6547bbf5029575e849 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 26 Nov 2024 13:57:25 -0600 Subject: [PATCH 08/10] Test that restore works only when no VLANs have been allocated --- tests/test_te_manager.py | 49 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/test_te_manager.py b/tests/test_te_manager.py index 1b74eaa2..a581c8e7 100644 --- a/tests/test_te_manager.py +++ b/tests/test_te_manager.py @@ -1866,3 +1866,52 @@ def test_vlan_tags_table_error_checks(self): with self.assertRaises(ValidationError) as ctx: temanager.vlan_tags_table = {"domain1": {"port1": (1, None)}} self.assertTrue("labels ((1, None)) is not a dict" in str(ctx.exception)) + + def test_vlan_tags_table_ensure_no_existing_allocations(self): + """ + Test that restoring VLAN tables works when there are no + existing allocations. + """ + temanager = TEManager(topology_data=None) + + for topology_file in [ + TestData.TOPOLOGY_FILE_AMLIGHT_v2, + TestData.TOPOLOGY_FILE_ZAOXI_v2, + TestData.TOPOLOGY_FILE_SAX_v2, + ]: + temanager.add_topology(json.loads(topology_file.read_text())) + + connection_request = { + "name": "check-existing-vlan-allocations", + "id": "id-check-existing-vlan-allocations", + "endpoints": [ + {"port_id": "urn:sdx:port:ampath.net:Ampath1:50", "vlan": "100:200"}, + {"port_id": "urn:sdx:port:tenet.ac.za:Tenet01:1", "vlan": "100:200"}, + ], + } + + graph = temanager.generate_graph_te() + traffic_matrix = temanager.generate_traffic_matrix(connection_request) + + print(f"Generated graph: '{graph}', traffic matrix: '{traffic_matrix}'") + self.assertIsNotNone(graph) + self.assertIsNotNone(traffic_matrix) + + solution = TESolver(graph, traffic_matrix).solve() + self.assertIsNotNone(solution) + + print(f"TESolver result: {solution}") + breakdown = temanager.generate_connection_breakdown( + solution, connection_request + ) + + # The VLAN table should have some allocations now, and we + # should not be able to change its state. + with self.assertRaises(ValidationError) as ctx: + temanager.vlan_tags_table = {"domain1": {"port1": {1: None}}} + expected_error = ( + "Error: VLAN table is not empty:" + "(domain: urn:sdx:topology:ampath.net, port: " + "urn:sdx:port:ampath.net:Ampath1:40, vlan: 100)" + ) + self.assertTrue(expected_error in str(ctx.exception)) From 75706fecad28783168e32ff427f29445ee1bd90e Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 26 Nov 2024 13:57:59 -0600 Subject: [PATCH 09/10] Use the correct variable name --- src/sdx_pce/topology/temanager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sdx_pce/topology/temanager.py b/src/sdx_pce/topology/temanager.py index 5492f4e4..abdff027 100644 --- a/src/sdx_pce/topology/temanager.py +++ b/src/sdx_pce/topology/temanager.py @@ -163,7 +163,7 @@ def vlan_tags_table(self, table: dict): if status is not UNUSED_VLAN: raise ValidationError( f"Error: VLAN table is not empty:" - f"(domain: {domain}, port: {port}, vlan: {vlan})" + f"(domain: {domain}, port: {port_id}, vlan: {vlan})" ) self._vlan_tags_table = table From 1e89ea0eede1bb06ed6f64ba012ae110ec48d903 Mon Sep 17 00:00:00 2001 From: Sajith Sasidharan Date: Tue, 26 Nov 2024 14:22:47 -0600 Subject: [PATCH 10/10] Mollify linter: use the darn variable --- tests/test_te_manager.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_te_manager.py b/tests/test_te_manager.py index a581c8e7..b03e6b41 100644 --- a/tests/test_te_manager.py +++ b/tests/test_te_manager.py @@ -1905,6 +1905,9 @@ def test_vlan_tags_table_ensure_no_existing_allocations(self): solution, connection_request ) + print(f"Breakdown: {breakdown}") + self.assertIsNotNone(breakdown) + # The VLAN table should have some allocations now, and we # should not be able to change its state. with self.assertRaises(ValidationError) as ctx: