From 1080ed5f21a70c0f12287b9e62e5b58271724de8 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Thu, 2 Feb 2023 15:57:34 -0500 Subject: [PATCH 01/33] Removed required property from UNI tag --- openapi.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/openapi.yml b/openapi.yml index 38c53cfe..f84c8842 100644 --- a/openapi.yml +++ b/openapi.yml @@ -598,9 +598,6 @@ components: Tag: # Can be referenced via '#/components/schemas/Tag' type: object - required: - - tag_type - - value properties: tag_type: type: integer From d6772b39f12f35aa3493b498da74494b6aaa9801 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Tue, 7 Feb 2023 16:28:19 -0500 Subject: [PATCH 02/33] Added support to str in tag.value --- controllers/__init__.py | 17 +++++++++++------ db/models.py | 12 +++++++++--- main.py | 10 +++++++--- models/evc.py | 15 ++++++++++----- openapi.yml | 6 ++++-- 5 files changed, 41 insertions(+), 19 deletions(-) diff --git a/controllers/__init__.py b/controllers/__init__.py index e2fd3115..65b26143 100644 --- a/controllers/__init__.py +++ b/controllers/__init__.py @@ -5,6 +5,7 @@ from typing import Dict, Optional import pymongo +from pydantic import ValidationError from pymongo.collection import ReturnDocument from pymongo.errors import AutoReconnect from tenacity import retry_if_exception_type, stop_after_attempt, wait_random @@ -71,12 +72,16 @@ def get_circuit(self, circuit_id: str) -> Optional[Dict]: def upsert_evc(self, evc: Dict) -> Optional[Dict]: """Update or insert an EVC""" utc_now = datetime.utcnow() - model = EVCBaseDoc( - **{ - **evc, - **{"_id": evc["id"]} - } - ) + try: + model = EVCBaseDoc( + **{ + **evc, + **{"_id": evc["id"]} + } + ) + except ValidationError as err: + raise err + updated = self.db.evcs.find_one_and_update( {"_id": evc["id"]}, { diff --git a/db/models.py b/db/models.py index a2a69017..2a748038 100644 --- a/db/models.py +++ b/db/models.py @@ -3,9 +3,9 @@ # pylint: disable=no-self-argument,no-name-in-module from datetime import datetime -from typing import Dict, List, Literal, Optional +from typing import Dict, List, Literal, Optional, Union -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, validator class DocumentBaseModel(BaseModel): @@ -38,7 +38,13 @@ class CircuitScheduleDoc(BaseModel): class TAGDoc(BaseModel): """TAG model""" tag_type: int - value: int + value: Union[str, int] + + @validator('value') + def validate_value(cls, value): + """Validate value when is a string""" + if value.isinstance(str) and value not in ("any", "untagged"): + raise ValueError("value only allows 'any' and 'untagged' strings") class UNIDoc(BaseModel): diff --git a/main.py b/main.py index 24dde2b3..788afda6 100644 --- a/main.py +++ b/main.py @@ -7,6 +7,7 @@ from threading import Lock from flask import jsonify, request +from pydantic import ValidationError from werkzeug.exceptions import (BadRequest, Conflict, Forbidden, MethodNotAllowed, NotFound, UnsupportedMediaType) @@ -217,12 +218,15 @@ def create_circuit(self, data): except ValueError as exception: raise BadRequest(str(exception)) from exception + # save circuit + try: + evc.sync() + except ValidationError as exception: + raise BadRequest(str(exception)) from exception + # store circuit in dictionary self.circuits[evc.id] = evc - # save circuit - evc.sync() - # Schedule the circuit deploy self.sched.add(evc) diff --git a/models/evc.py b/models/evc.py index 9336b4d2..a3db5620 100644 --- a/models/evc.py +++ b/models/evc.py @@ -251,7 +251,10 @@ def _validate(self, **kwargs): raise ValueError(f"{attribute} is an invalid UNI.") if not uni.is_valid(): - tag = uni.user_tag.value + try: + tag = uni.user_tag.value + except AttributeError: + tag = None message = f"VLAN tag {tag} is not available in {attribute}" raise ValueError(message) @@ -1064,8 +1067,9 @@ def _prepare_push_flow(self, *args, queue_id=None): ) # the service tag must be always pushed - new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} - flow_mod["actions"].insert(0, new_action) + if out_vlan != "any": + new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} + flow_mod["actions"].insert(0, new_action) new_action = {"action_type": "push_vlan", "tag_type": "s"} flow_mod["actions"].insert(0, new_action) @@ -1075,8 +1079,9 @@ def _prepare_push_flow(self, *args, queue_id=None): flow_mod["match"]["dl_vlan"] = in_vlan if new_c_vlan: # new_in_vlan is set, so an action to set it is necessary - new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} - flow_mod["actions"].insert(0, new_action) + if out_vlan != "any": + new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} + flow_mod["actions"].insert(0, new_action) if not in_vlan: # new_in_vlan is set, but in_vlan is not, so there was no # vlan set; then it is set now diff --git a/openapi.yml b/openapi.yml index f84c8842..6568ccc0 100644 --- a/openapi.yml +++ b/openapi.yml @@ -603,8 +603,10 @@ components: type: integer format: int32 value: - type: integer - format: int32 + oneOf: + - type: integer + format: int32 + - type: string CircuitSchedule: # Can be referenced via '#/components/schemas/CircuitSchedule' type: object From 165c431feababed17e0c7eab7cca883837b71eb3 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Tue, 7 Feb 2023 19:14:23 -0500 Subject: [PATCH 03/33] Extended changes to more flow types --- db/models.py | 5 +++-- models/evc.py | 34 +++++++++++++++++++--------------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/db/models.py b/db/models.py index 2a748038..9d0813c6 100644 --- a/db/models.py +++ b/db/models.py @@ -38,13 +38,14 @@ class CircuitScheduleDoc(BaseModel): class TAGDoc(BaseModel): """TAG model""" tag_type: int - value: Union[str, int] + value: Union[int, str] @validator('value') def validate_value(cls, value): """Validate value when is a string""" - if value.isinstance(str) and value not in ("any", "untagged"): + if type(value) == (str) and (value not in ("any", "untagged")): raise ValueError("value only allows 'any' and 'untagged' strings") + return value class UNIDoc(BaseModel): diff --git a/models/evc.py b/models/evc.py index f32ccb4a..cee3831c 100644 --- a/models/evc.py +++ b/models/evc.py @@ -831,24 +831,28 @@ def _prepare_direct_uni_flows(self): if vlan_a and vlan_z: flow_mod_az["match"]["dl_vlan"] = vlan_a flow_mod_za["match"]["dl_vlan"] = vlan_z - flow_mod_az["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_z} - ) - flow_mod_za["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_a} - ) + if vlan_z != "any": + flow_mod_az["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_z} + ) + if vlan_a != "any": + flow_mod_za["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_a} + ) elif vlan_a: flow_mod_az["match"]["dl_vlan"] = vlan_a flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) - flow_mod_za["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_a} - ) + if vlan_a != "any": + flow_mod_za["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_a} + ) elif vlan_z: flow_mod_za["match"]["dl_vlan"] = vlan_z flow_mod_za["actions"].insert(0, {"action_type": "pop_vlan"}) - flow_mod_az["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_z} - ) + if vlan_z != "any": + flow_mod_az["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_z} + ) return ( self.uni_a.interface.switch.id, [flow_mod_az, flow_mod_za] ) @@ -1037,8 +1041,8 @@ def _prepare_nni_flow(self, *args, queue_id=None): in_interface, out_interface, queue_id ) flow_mod["match"]["dl_vlan"] = in_vlan - - new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} + if out_vlan != "any": + new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} flow_mod["actions"].insert(0, new_action) return flow_mod @@ -1078,7 +1082,7 @@ def _prepare_push_flow(self, *args, queue_id=None): flow_mod["match"]["dl_vlan"] = in_vlan if new_c_vlan: # new_in_vlan is set, so an action to set it is necessary - if out_vlan != "any": + if new_c_vlan != "any": new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} flow_mod["actions"].insert(0, new_action) if not in_vlan: From 0f2aa8c5ff2571c7375766d1f333295babf4f894 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 8 Feb 2023 12:26:03 -0500 Subject: [PATCH 04/33] Added new static method _get_value_from_uni_tag() --- models/evc.py | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/models/evc.py b/models/evc.py index cee3831c..f71ead92 100644 --- a/models/evc.py +++ b/models/evc.py @@ -831,25 +831,25 @@ def _prepare_direct_uni_flows(self): if vlan_a and vlan_z: flow_mod_az["match"]["dl_vlan"] = vlan_a flow_mod_za["match"]["dl_vlan"] = vlan_z - if vlan_z != "any": + if vlan_z != "4096/4096": flow_mod_az["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) - if vlan_a != "any": + if vlan_a != "4096/4096": flow_mod_za["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_a} ) elif vlan_a: flow_mod_az["match"]["dl_vlan"] = vlan_a flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) - if vlan_a != "any": + if vlan_a != "4096/4096": flow_mod_za["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_a} ) elif vlan_z: flow_mod_za["match"]["dl_vlan"] = vlan_z flow_mod_za["actions"].insert(0, {"action_type": "pop_vlan"}) - if vlan_z != "any": + if vlan_z != "4096/4096": flow_mod_az["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) @@ -903,6 +903,19 @@ def _install_nni_flows(self, path=None): for dpid, flows in self._prepare_nni_flows(path).items(): self._send_flow_mods(dpid, flows) + @staticmethod + def _get_value_from_uni_tag(uni): + if uni.user_tag: + if type(uni.user_tag.value) == str: + if uni.user_tag.value == "any": + return "4096/4096" + else: + return 0 + else: + return uni.user_tag.value + else: + return None + def _prepare_uni_flows(self, path=None, skip_in=False, skip_out=False): """Prepare flows to install UNIs.""" uni_flows = {} @@ -911,10 +924,11 @@ def _prepare_uni_flows(self, path=None, skip_in=False, skip_out=False): return uni_flows # Determine VLANs - in_vlan_a = self.uni_a.user_tag.value if self.uni_a.user_tag else None + in_vlan_a = self._get_value_from_uni_tag(self.uni_a) + #in_vlan_a = self.uni_a.user_tag.value if self.uni_a.user_tag else None out_vlan_a = path[0].get_metadata("s_vlan").value - - in_vlan_z = self.uni_z.user_tag.value if self.uni_z.user_tag else None + in_vlan_z = self._get_value_from_uni_tag(self.uni_z) + #in_vlan_z = self.uni_z.user_tag.value if self.uni_z.user_tag else None out_vlan_z = path[-1].get_metadata("s_vlan").value # Flows for the first UNI @@ -1041,7 +1055,7 @@ def _prepare_nni_flow(self, *args, queue_id=None): in_interface, out_interface, queue_id ) flow_mod["match"]["dl_vlan"] = in_vlan - if out_vlan != "any": + if out_vlan != "4096/4096": new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} flow_mod["actions"].insert(0, new_action) @@ -1070,7 +1084,7 @@ def _prepare_push_flow(self, *args, queue_id=None): ) # the service tag must be always pushed - if out_vlan != "any": + if out_vlan != "4096/4096": new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} flow_mod["actions"].insert(0, new_action) @@ -1082,7 +1096,7 @@ def _prepare_push_flow(self, *args, queue_id=None): flow_mod["match"]["dl_vlan"] = in_vlan if new_c_vlan: # new_in_vlan is set, so an action to set it is necessary - if new_c_vlan != "any": + if new_c_vlan != "4096/4096": new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} flow_mod["actions"].insert(0, new_action) if not in_vlan: From a5369f090a215b4f4aa93a554604ea3587cbc756 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 8 Feb 2023 12:47:34 -0500 Subject: [PATCH 05/33] Fixed linter --- db/models.py | 2 +- models/evc.py | 14 +++++--------- tox.ini | 2 +- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/db/models.py b/db/models.py index 9d0813c6..70750d4a 100644 --- a/db/models.py +++ b/db/models.py @@ -43,7 +43,7 @@ class TAGDoc(BaseModel): @validator('value') def validate_value(cls, value): """Validate value when is a string""" - if type(value) == (str) and (value not in ("any", "untagged")): + if isinstance(value, str) and (value not in ("any", "untagged")): raise ValueError("value only allows 'any' and 'untagged' strings") return value diff --git a/models/evc.py b/models/evc.py index f71ead92..f0a5ec25 100644 --- a/models/evc.py +++ b/models/evc.py @@ -906,15 +906,12 @@ def _install_nni_flows(self, path=None): @staticmethod def _get_value_from_uni_tag(uni): if uni.user_tag: - if type(uni.user_tag.value) == str: + if isinstance(uni.user_tag.value, str): if uni.user_tag.value == "any": return "4096/4096" - else: - return 0 - else: - return uni.user_tag.value - else: - return None + return 0 + return uni.user_tag.value + return None def _prepare_uni_flows(self, path=None, skip_in=False, skip_out=False): """Prepare flows to install UNIs.""" @@ -925,10 +922,9 @@ def _prepare_uni_flows(self, path=None, skip_in=False, skip_out=False): # Determine VLANs in_vlan_a = self._get_value_from_uni_tag(self.uni_a) - #in_vlan_a = self.uni_a.user_tag.value if self.uni_a.user_tag else None out_vlan_a = path[0].get_metadata("s_vlan").value + in_vlan_z = self._get_value_from_uni_tag(self.uni_z) - #in_vlan_z = self.uni_z.user_tag.value if self.uni_z.user_tag else None out_vlan_z = path[-1].get_metadata("s_vlan").value # Flows for the first UNI diff --git a/tox.ini b/tox.ini index 7024ecb7..aa850452 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = coverage,lint +envlist = lint [gh-actions] python = From eee89e8d5bfc1d3493a0ba4eda6c3e25f267c607 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 8 Feb 2023 12:55:32 -0500 Subject: [PATCH 06/33] Reversed tox.ini --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index aa850452..7024ecb7 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = lint +envlist = coverage,lint [gh-actions] python = From 31e0cb1afbb7f5e04c12f6cea7186ba36461e07b Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Thu, 9 Feb 2023 11:09:26 -0500 Subject: [PATCH 07/33] Added support for intra-switch EVC --- models/evc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/evc.py b/models/evc.py index f0a5ec25..08b0738d 100644 --- a/models/evc.py +++ b/models/evc.py @@ -814,8 +814,8 @@ def get_failover_flows(self): def _prepare_direct_uni_flows(self): """Prepare flows connecting two UNIs for intra-switch EVC.""" - vlan_a = self.uni_a.user_tag.value if self.uni_a.user_tag else None - vlan_z = self.uni_z.user_tag.value if self.uni_z.user_tag else None + vlan_a = self._get_value_from_uni_tag(self.uni_a) + vlan_z = self._get_value_from_uni_tag(self.uni_z) is_EVPL = (vlan_a is not None) flow_mod_az = self._prepare_flow_mod( From eac6bf0fdb30997a9d82e06a3cecabd2ba3b4699 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 15 Feb 2023 11:07:52 -0500 Subject: [PATCH 08/33] Added more possible strings to `value` from TagDoc --- db/models.py | 15 ++++++++++++--- models/evc.py | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/db/models.py b/db/models.py index 70750d4a..4f424649 100644 --- a/db/models.py +++ b/db/models.py @@ -2,6 +2,7 @@ # pylint: disable=unused-argument,invalid-name,unused-argument # pylint: disable=no-self-argument,no-name-in-module +import re from datetime import datetime from typing import Dict, List, Literal, Optional, Union @@ -43,9 +44,17 @@ class TAGDoc(BaseModel): @validator('value') def validate_value(cls, value): """Validate value when is a string""" - if isinstance(value, str) and (value not in ("any", "untagged")): - raise ValueError("value only allows 'any' and 'untagged' strings") - return value + if isinstance(value, int): + return value + if isinstance(value, str): + if value in ("any", "untagged"): + return value + regex = r"^(?=.{1,9}$)\d{1,4}\/\d{1,4}$" + left, right = map(int, value.split('/')) + if re.match(regex, value) and left < 4097 and right < 4097: + return value + raise ValueError("value as string allows 'any', 'untagged' and the ", + "format 'n/n' where n > 4097") class UNIDoc(BaseModel): diff --git a/models/evc.py b/models/evc.py index 08b0738d..19f5de70 100644 --- a/models/evc.py +++ b/models/evc.py @@ -1080,7 +1080,7 @@ def _prepare_push_flow(self, *args, queue_id=None): ) # the service tag must be always pushed - if out_vlan != "4096/4096": + if in_vlan != "4096/4096": new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} flow_mod["actions"].insert(0, new_action) From 4cadff0cef29dc4a8a523215640bd75ab70bb213 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Tue, 21 Feb 2023 18:11:32 -0500 Subject: [PATCH 09/33] Added support for `untagged` - Added tests. --- db/models.py | 12 +- models/evc.py | 43 ++++---- tests/unit/models/test_evc_deploy.py | 157 ++++++++++++++++++++++++++- tests/unit/test_db_models.py | 20 +++- 4 files changed, 199 insertions(+), 33 deletions(-) diff --git a/db/models.py b/db/models.py index 4f424649..505a1915 100644 --- a/db/models.py +++ b/db/models.py @@ -2,7 +2,6 @@ # pylint: disable=unused-argument,invalid-name,unused-argument # pylint: disable=no-self-argument,no-name-in-module -import re from datetime import datetime from typing import Dict, List, Literal, Optional, Union @@ -46,15 +45,10 @@ def validate_value(cls, value): """Validate value when is a string""" if isinstance(value, int): return value - if isinstance(value, str): - if value in ("any", "untagged"): - return value - regex = r"^(?=.{1,9}$)\d{1,4}\/\d{1,4}$" - left, right = map(int, value.split('/')) - if re.match(regex, value) and left < 4097 and right < 4097: - return value + if isinstance(value, str) and value in ("any", "untagged"): + return value raise ValueError("value as string allows 'any', 'untagged' and the ", - "format 'n/n' where n > 4097") + "format 'n/n'") class UNIDoc(BaseModel): diff --git a/models/evc.py b/models/evc.py index 19f5de70..414585da 100644 --- a/models/evc.py +++ b/models/evc.py @@ -828,28 +828,28 @@ def _prepare_direct_uni_flows(self): self.queue_id, is_EVPL ) - if vlan_a and vlan_z: + if vlan_a is not None and vlan_z is not None: flow_mod_az["match"]["dl_vlan"] = vlan_a flow_mod_za["match"]["dl_vlan"] = vlan_z - if vlan_z != "4096/4096": + if vlan_z not in ("4096/4096", 0): flow_mod_az["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) - if vlan_a != "4096/4096": + if vlan_a not in ("4096/4096", 0): flow_mod_za["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_a} ) - elif vlan_a: + elif vlan_a is not None: flow_mod_az["match"]["dl_vlan"] = vlan_a flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) - if vlan_a != "4096/4096": + if vlan_a not in ("4096/4096", 0): flow_mod_za["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_a} ) - elif vlan_z: + elif vlan_z is not None: flow_mod_za["match"]["dl_vlan"] = vlan_z flow_mod_za["actions"].insert(0, {"action_type": "pop_vlan"}) - if vlan_z != "4096/4096": + if vlan_z not in ("4096/4096", 0): flow_mod_az["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) @@ -905,12 +905,15 @@ def _install_nni_flows(self, path=None): @staticmethod def _get_value_from_uni_tag(uni): + """Returns the value from tag. In case of any and untagged + it should return 4096/4096 and 0 respectively""" + special = {"any": "4096/4096", "untagged": 0} + if uni.user_tag: - if isinstance(uni.user_tag.value, str): - if uni.user_tag.value == "any": - return "4096/4096" - return 0 - return uni.user_tag.value + value = uni.user_tag.value + if isinstance(value, str): + return special.get(value, value) + return value return None def _prepare_uni_flows(self, path=None, skip_in=False, skip_out=False): @@ -1051,8 +1054,7 @@ def _prepare_nni_flow(self, *args, queue_id=None): in_interface, out_interface, queue_id ) flow_mod["match"]["dl_vlan"] = in_vlan - if out_vlan != "4096/4096": - new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} + new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} flow_mod["actions"].insert(0, new_action) return flow_mod @@ -1078,29 +1080,28 @@ def _prepare_push_flow(self, *args, queue_id=None): flow_mod = self._prepare_flow_mod( in_interface, out_interface, queue_id, is_EVPL ) - # the service tag must be always pushed - if in_vlan != "4096/4096": + if in_vlan not in ("4096/4096", 0): new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} flow_mod["actions"].insert(0, new_action) new_action = {"action_type": "push_vlan", "tag_type": "s"} flow_mod["actions"].insert(0, new_action) - if in_vlan: + if in_vlan is not None: # if in_vlan is set, it must be included in the match flow_mod["match"]["dl_vlan"] = in_vlan - if new_c_vlan: + if new_c_vlan is not None: # new_in_vlan is set, so an action to set it is necessary - if new_c_vlan != "4096/4096": + if new_c_vlan not in ("4096/4096", 0): new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} flow_mod["actions"].insert(0, new_action) - if not in_vlan: + if in_vlan is None: # new_in_vlan is set, but in_vlan is not, so there was no # vlan set; then it is set now new_action = {"action_type": "push_vlan", "tag_type": "c"} flow_mod["actions"].insert(0, new_action) - elif in_vlan: + elif in_vlan is not None: # in_vlan is set, but new_in_vlan is not, so the existing vlan # must be removed new_action = {"action_type": "pop_vlan"} diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index b99fb280..be0ca9e5 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -275,6 +275,56 @@ def test_prepare_push_flow(self): expected_flow_mod["priority"] = EPL_SB_PRIORITY self.assertEqual(expected_flow_mod, flow_mod) + def test_prepare_push_flow_any_untagged(self): + """Test _prepare_push_flow""" + attributes = { + "controller": get_controller_mock(), + "name": "custom_name", + "uni_a": get_uni_mocked(interface_port=1, is_valid=True), + "uni_z": get_uni_mocked(interface_port=2, is_valid=True), + } + evc = EVC(**attributes) + interface_a = evc.uni_a.interface + interface_z = evc.uni_z.interface + out_vlan_a = 20 + for in_vlan_a in ("4096/4096", None): + for in_vlan_z in (0, None): + with self.subTest(in_vlan_a=in_vlan_a, in_vlan_z=in_vlan_z): + # pylint: disable=protected-access + flow_mod = evc._prepare_push_flow(interface_a, interface_z, + in_vlan_a, out_vlan_a, + in_vlan_z) + expected_flow_mod = { + 'match': {'in_port': interface_a.port_number}, + 'cookie': evc.get_cookie(), + 'actions': [ + {'action_type': 'push_vlan', 'tag_type': 's'}, + {'action_type': 'set_vlan', 'vlan_id': out_vlan_a}, + { + 'action_type': 'output', + 'port': interface_z.port_number + } + ], + "priority": EPL_SB_PRIORITY, + } + if in_vlan_a and in_vlan_z is not None: + del expected_flow_mod["actions"][1] + expected_flow_mod['match']['dl_vlan'] = in_vlan_a + expected_flow_mod['priority'] = EVPL_SB_PRIORITY + elif in_vlan_a: + del expected_flow_mod["actions"][1] + expected_flow_mod['match']['dl_vlan'] = in_vlan_a + expected_flow_mod['actions'].insert(0, { + 'action_type': 'pop_vlan' + }) + expected_flow_mod["priority"] = EVPL_SB_PRIORITY + elif in_vlan_z is not None: + expected_flow_mod['actions'].insert(0, { + 'action_type': 'push_vlan', 'tag_type': 'c' + }) + expected_flow_mod["priority"] = EPL_SB_PRIORITY + self.assertEqual(expected_flow_mod, flow_mod) + @staticmethod @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") def test_install_uni_flows(send_flow_mods_mock): @@ -467,7 +517,7 @@ def test_install_nni_flows(send_flow_mods_mock): {"action_type": "set_vlan", "vlan_id": in_vlan}, {"action_type": "output", "port": in_port}, ], - "priority": EVPL_SB_PRIORITY + "priority": EVPL_SB_PRIORITY, }, ] @@ -1195,7 +1245,7 @@ def test_deploy_direct_uni_flows(send_flow_mods_mock): ) evc.uni_z.user_tag = uni_z_tag - # Test3: no TAG in both UNI_Z and UNI_Z + # Test4: no TAG in both UNI_Z and UNI_Z evc.uni_a.user_tag = None evc.uni_z.user_tag = None expected_flows_no_tag = [ @@ -1228,6 +1278,91 @@ def test_deploy_direct_uni_flows(send_flow_mods_mock): # print(evc._prepare_direct_uni_flows()) # self.assertTrue(False) + @staticmethod + @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") + def test_deploy_direct_uni_flows_untagged_any(send_flow_mods_mock): + """Test _install_direct_uni_flows with untagged and any.""" + evc = TestEVC.create_evc_intra_switch() + + evc.uni_a = get_uni_mocked(tag_value="untagged", is_valid=True) + evc.uni_z = get_uni_mocked(tag_value="untagged", is_valid=True) + expected_dpid = evc.uni_a.interface.switch.id + expected_flows = [ + { + "match": {"in_port": 1, "dl_vlan": 0}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + }, + { + "match": {"in_port": 1, "dl_vlan": 0}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + } + ] + evc._install_direct_uni_flows() + send_flow_mods_mock.assert_called_with( + expected_dpid, expected_flows + ) + + evc.uni_a = get_uni_mocked(tag_value="any", is_valid=True) + evc.uni_z = get_uni_mocked(tag_value="any", is_valid=True) + expected_dpid = evc.uni_a.interface.switch.id + expected_flows = [ + { + "match": {"in_port": 1, "dl_vlan": "4096/4096"}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + }, + { + "match": {"in_port": 1, "dl_vlan": "4096/4096"}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + } + ] + evc._install_direct_uni_flows() + send_flow_mods_mock.assert_called_with( + expected_dpid, expected_flows + ) + + evc.uni_a = get_uni_mocked(tag_value="any", is_valid=True) + evc.uni_z = get_uni_mocked(tag_value=100, is_valid=True) + expected_dpid = evc.uni_a.interface.switch.id + expected_flows = [ + { + "match": {"in_port": 1, "dl_vlan": "4096/4096"}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "set_vlan", "vlan_id": 100}, + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + }, + { + "match": {"in_port": 1, "dl_vlan": 100}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + } + ] + evc._install_direct_uni_flows() + send_flow_mods_mock.assert_called_with( + expected_dpid, expected_flows + ) + def test_is_affected_by_link(self): """Test is_affected_by_link method""" self.evc_deploy.current_path = Path(['a', 'b', 'c']) @@ -1531,3 +1666,21 @@ def test_is_eligible_for_failover_path(self): self.evc_deploy.primary_path = Path([]) self.evc_deploy.backup_path = Path([]) self.assertTrue(self.evc_deploy.is_eligible_for_failover_path()) + + def test_get_value_from_uni_tag(self): + """Test _get_value_from_uni_tag""" + uni = get_uni_mocked(tag_value=None) + value = EVC._get_value_from_uni_tag(uni) + self.assertEqual(value, None) + + uni = get_uni_mocked(tag_value="any") + value = EVC._get_value_from_uni_tag(uni) + self.assertEqual(value, "4096/4096") + + uni = get_uni_mocked(tag_value="untagged") + value = EVC._get_value_from_uni_tag(uni) + self.assertEqual(value, 0) + + uni = get_uni_mocked(tag_value=100) + value = EVC._get_value_from_uni_tag(uni) + self.assertEqual(value, 100) diff --git a/tests/unit/test_db_models.py b/tests/unit/test_db_models.py index 2a2dedfa..087b26f6 100644 --- a/tests/unit/test_db_models.py +++ b/tests/unit/test_db_models.py @@ -2,7 +2,7 @@ from unittest import TestCase from pydantic import ValidationError -from db.models import EVCBaseDoc, DocumentBaseModel +from db.models import EVCBaseDoc, DocumentBaseModel, TAGDoc class TestDBModels(TestCase): @@ -61,3 +61,21 @@ def test_document_base_model_dict(self): self.evc_dict["_id"] = "some_id" model = DocumentBaseModel(**self.evc_dict) assert "_id" not in model.dict(exclude={"_id"}) + + def test_tagdoc_value(self): + """Test TAGDoc value restrictions""" + tag_mask = {"tag_type": 1, "value": "untagged"} + tag = TAGDoc(**tag_mask) + assert tag.tag_type == 1 + assert tag.value == "untagged" + + tag_mask = {"tag_type": 1, "value": "any"} + tag = TAGDoc(**tag_mask) + assert tag.tag_type == 1 + assert tag.value == "any" + + def test_tagdoc_fail(self): + """Test TAGDoc value fail case""" + tag_fail = {"tag_type": 1, "value": "test_fail"} + with self.assertRaises(ValueError): + TAGDoc(**tag_fail) From 6f82d45628cd580878c039930d836fbdbc1fd357 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 1 Mar 2023 09:33:36 -0500 Subject: [PATCH 10/33] Corrected EVC combinations --- models/evc.py | 86 ++++--- tests/unit/models/test_evc_deploy.py | 370 +++++++-------------------- 2 files changed, 151 insertions(+), 305 deletions(-) diff --git a/models/evc.py b/models/evc.py index 414585da..78c4ce19 100644 --- a/models/evc.py +++ b/models/evc.py @@ -828,31 +828,55 @@ def _prepare_direct_uni_flows(self): self.queue_id, is_EVPL ) - if vlan_a is not None and vlan_z is not None: + if vlan_a is not None: flow_mod_az["match"]["dl_vlan"] = vlan_a + + if vlan_z is not None: flow_mod_za["match"]["dl_vlan"] = vlan_z - if vlan_z not in ("4096/4096", 0): + + if vlan_a not in {None, "4096/4096", 0}: + flow_mod_za["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_a} + ) + if vlan_z not in {None, "4096/4096", 0}: flow_mod_az["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) - if vlan_a not in ("4096/4096", 0): + if not vlan_z: flow_mod_za["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_a} + 0, {"action_type": "push_vlan", "tag_type": "c"} ) - elif vlan_a is not None: - flow_mod_az["match"]["dl_vlan"] = vlan_a - flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) - if vlan_a not in ("4096/4096", 0): - flow_mod_za["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_a} + if vlan_z == 0: + flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) + + elif vlan_a == "4096/4096": + if vlan_z not in {None, "4096/4096", 0}: + flow_mod_az["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) - elif vlan_z is not None: - flow_mod_za["match"]["dl_vlan"] = vlan_z - flow_mod_za["actions"].insert(0, {"action_type": "pop_vlan"}) - if vlan_z not in ("4096/4096", 0): + if vlan_z == 0: + flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) + + elif vlan_a == 0: + if vlan_z not in {None, "4096/4096", 0}: flow_mod_az["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) + flow_mod_az["actions"].insert( + 0, {"action_type": "push_vlan", "tag_type": "c"} + ) + if vlan_z: + flow_mod_za["actions"].insert(0, {"action_type": "pop_vlan"}) + + elif vlan_a is None: + if vlan_z not in {None, "4096/4096", 0}: + flow_mod_az["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_z} + ) + flow_mod_az["actions"].insert( + 0, {"action_type": "push_vlan", "tag_type": "c"} + ) + return ( self.uni_a.interface.switch.id, [flow_mod_az, flow_mod_za] ) @@ -1081,9 +1105,8 @@ def _prepare_push_flow(self, *args, queue_id=None): in_interface, out_interface, queue_id, is_EVPL ) # the service tag must be always pushed - if in_vlan not in ("4096/4096", 0): - new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} - flow_mod["actions"].insert(0, new_action) + new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} + flow_mod["actions"].insert(0, new_action) new_action = {"action_type": "push_vlan", "tag_type": "s"} flow_mod["actions"].insert(0, new_action) @@ -1091,21 +1114,26 @@ def _prepare_push_flow(self, *args, queue_id=None): if in_vlan is not None: # if in_vlan is set, it must be included in the match flow_mod["match"]["dl_vlan"] = in_vlan - if new_c_vlan is not None: - # new_in_vlan is set, so an action to set it is necessary - if new_c_vlan not in ("4096/4096", 0): - new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} + + if new_c_vlan not in {"4096/4096", 0, None}: + new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} + flow_mod["actions"].insert(0, new_action) + + if in_vlan not in {"4096/4096", 0, None}: + if new_c_vlan == 0: + new_action = {"action_type": "pop_vlan"} + flow_mod["actions"].insert(0, new_action) + + elif in_vlan == "4096/4096": + if new_c_vlan == 0: + new_action = {"action_type": "pop_vlan"} flow_mod["actions"].insert(0, new_action) - if in_vlan is None: - # new_in_vlan is set, but in_vlan is not, so there was no - # vlan set; then it is set now + + elif not in_vlan: + if new_c_vlan not in {"4096/4096", 0, None}: new_action = {"action_type": "push_vlan", "tag_type": "c"} flow_mod["actions"].insert(0, new_action) - elif in_vlan is not None: - # in_vlan is set, but new_in_vlan is not, so the existing vlan - # must be removed - new_action = {"action_type": "pop_vlan"} - flow_mod["actions"].insert(0, new_action) + return flow_mod def _prepare_pop_flow( diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index be0ca9e5..58692f81 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -233,8 +233,8 @@ def test_prepare_push_flow(self): interface_z = evc.uni_z.interface out_vlan_a = 20 - for in_vlan_a in (10, None): - for in_vlan_z in (3, None): + for in_vlan_a in [100, "4096/4096", 0, None]: + for in_vlan_z in [50, "4096/4096", 0, None]: with self.subTest(in_vlan_a=in_vlan_a, in_vlan_z=in_vlan_z): # pylint: disable=protected-access flow_mod = evc._prepare_push_flow(interface_a, interface_z, @@ -251,78 +251,31 @@ def test_prepare_push_flow(self): 'port': interface_z.port_number } ], - "priority": EPL_SB_PRIORITY, + "priority": EVPL_SB_PRIORITY, } - if in_vlan_a and in_vlan_z: + if in_vlan_a is not None: expected_flow_mod['match']['dl_vlan'] = in_vlan_a - expected_flow_mod['actions'].insert(0, { - 'action_type': 'set_vlan', 'vlan_id': in_vlan_z - }) - expected_flow_mod['priority'] = EVPL_SB_PRIORITY - elif in_vlan_a: - expected_flow_mod['match']['dl_vlan'] = in_vlan_a - expected_flow_mod['actions'].insert(0, { - 'action_type': 'pop_vlan' - }) - expected_flow_mod["priority"] = EVPL_SB_PRIORITY - elif in_vlan_z: - expected_flow_mod['actions'].insert(0, { - 'action_type': 'set_vlan', 'vlan_id': in_vlan_z - }) - expected_flow_mod['actions'].insert(0, { - 'action_type': 'push_vlan', 'tag_type': 'c' - }) - expected_flow_mod["priority"] = EPL_SB_PRIORITY - self.assertEqual(expected_flow_mod, flow_mod) - - def test_prepare_push_flow_any_untagged(self): - """Test _prepare_push_flow""" - attributes = { - "controller": get_controller_mock(), - "name": "custom_name", - "uni_a": get_uni_mocked(interface_port=1, is_valid=True), - "uni_z": get_uni_mocked(interface_port=2, is_valid=True), - } - evc = EVC(**attributes) - interface_a = evc.uni_a.interface - interface_z = evc.uni_z.interface - out_vlan_a = 20 - for in_vlan_a in ("4096/4096", None): - for in_vlan_z in (0, None): - with self.subTest(in_vlan_a=in_vlan_a, in_vlan_z=in_vlan_z): - # pylint: disable=protected-access - flow_mod = evc._prepare_push_flow(interface_a, interface_z, - in_vlan_a, out_vlan_a, - in_vlan_z) - expected_flow_mod = { - 'match': {'in_port': interface_a.port_number}, - 'cookie': evc.get_cookie(), - 'actions': [ - {'action_type': 'push_vlan', 'tag_type': 's'}, - {'action_type': 'set_vlan', 'vlan_id': out_vlan_a}, - { - 'action_type': 'output', - 'port': interface_z.port_number - } - ], - "priority": EPL_SB_PRIORITY, - } - if in_vlan_a and in_vlan_z is not None: - del expected_flow_mod["actions"][1] - expected_flow_mod['match']['dl_vlan'] = in_vlan_a - expected_flow_mod['priority'] = EVPL_SB_PRIORITY - elif in_vlan_a: - del expected_flow_mod["actions"][1] - expected_flow_mod['match']['dl_vlan'] = in_vlan_a - expected_flow_mod['actions'].insert(0, { - 'action_type': 'pop_vlan' - }) - expected_flow_mod["priority"] = EVPL_SB_PRIORITY - elif in_vlan_z is not None: - expected_flow_mod['actions'].insert(0, { - 'action_type': 'push_vlan', 'tag_type': 'c' - }) - expected_flow_mod["priority"] = EPL_SB_PRIORITY + else: + expected_flow_mod['priority'] = EPL_SB_PRIORITY + + if in_vlan_z not in {"4096/4096", 0, None}: + new_action = {"action_type": "set_vlan", + "vlan_id": in_vlan_z} + expected_flow_mod["actions"].insert(0, new_action) + + if in_vlan_a not in {"4096/4096", 0, None}: + if in_vlan_z == 0: + new_action = {"action_type": "pop_vlan"} + expected_flow_mod["actions"].insert(0, new_action) + elif in_vlan_a == "4096/4096": + if in_vlan_z == 0: + new_action = {"action_type": "pop_vlan"} + expected_flow_mod["actions"].insert(0, new_action) + elif not in_vlan_a: + if in_vlan_z not in {"4096/4096", 0, None}: + new_action = {"action_type": "push_vlan", + "tag_type": "c"} + expected_flow_mod["actions"].insert(0, new_action) self.assertEqual(expected_flow_mod, flow_mod) @staticmethod @@ -1155,213 +1108,78 @@ def create_evc_intra_switch(): @staticmethod @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") def test_deploy_direct_uni_flows(send_flow_mods_mock): - """Test _install_direct_uni_flows.""" + """Test _install_direct_uni_flows""" evc = TestEVC.create_evc_intra_switch() - - # Test 1: both UNIs with TAG expected_dpid = evc.uni_a.interface.switch.id - expected_flows = [ - { - "match": {"in_port": 1, "dl_vlan": 82}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "set_vlan", "vlan_id": 84}, - {"action_type": "output", "port": 3}, - ], - "priority": EVPL_SB_PRIORITY - }, - { - "match": {"in_port": 3, "dl_vlan": 84}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "set_vlan", "vlan_id": 82}, - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - } - ] - - # pylint: disable=protected-access - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_once_with( - expected_dpid, expected_flows - ) - - # Test2: no TAG in UNI_A - uni_a_tag = evc.uni_a.user_tag - evc.uni_a.user_tag = None - expected_flows_no_tag_a = [ - { - "match": {"in_port": 1}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "set_vlan", "vlan_id": 84}, - {"action_type": "output", "port": 3}, - ], - "priority": EPL_SB_PRIORITY - }, - { - "match": {"in_port": 3, "dl_vlan": 84}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "pop_vlan"}, - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows_no_tag_a - ) - evc.uni_a.user_tag = uni_a_tag - # Test3: no TAG in UNI_Z - uni_z_tag = evc.uni_z.user_tag - evc.uni_z.user_tag = None - expected_flows_no_tag_z = [ - { - "match": {"in_port": 1, "dl_vlan": 82}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "pop_vlan"}, - {"action_type": "output", "port": 3}, - ], - "priority": EVPL_SB_PRIORITY - }, - { - "match": {"in_port": 3}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "set_vlan", "vlan_id": 82}, - {"action_type": "output", "port": 1}, - ], - "priority": EPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows_no_tag_z - ) - evc.uni_z.user_tag = uni_z_tag - - # Test4: no TAG in both UNI_Z and UNI_Z - evc.uni_a.user_tag = None - evc.uni_z.user_tag = None - expected_flows_no_tag = [ - { - "match": {"in_port": 1}, - "cookie": evc.get_cookie(), - "actions": [{"action_type": "output", "port": 3}], - "priority": EPL_SB_PRIORITY - }, - { - "match": {"in_port": 3}, - "cookie": evc.get_cookie(), - "actions": [{"action_type": "output", "port": 1}], - "priority": EPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows_no_tag - ) -# -# print(evc._prepare_direct_uni_flows()) -# evc.uni_a.user_tag = uni_a_tag -# uni_z_tag = evc.uni_z.user_tag -# evc.uni_z.user_tag = None -# print(evc._prepare_direct_uni_flows()) -# evc.uni_z.user_tag = uni_z_tag -# evc.uni_a.user_tag = None -# evc.uni_z.user_tag = None -# print(evc._prepare_direct_uni_flows()) -# self.assertTrue(False) - - @staticmethod - @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") - def test_deploy_direct_uni_flows_untagged_any(send_flow_mods_mock): - """Test _install_direct_uni_flows with untagged and any.""" - evc = TestEVC.create_evc_intra_switch() - - evc.uni_a = get_uni_mocked(tag_value="untagged", is_valid=True) - evc.uni_z = get_uni_mocked(tag_value="untagged", is_valid=True) - expected_dpid = evc.uni_a.interface.switch.id - expected_flows = [ - { - "match": {"in_port": 1, "dl_vlan": 0}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - }, - { - "match": {"in_port": 1, "dl_vlan": 0}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows - ) - - evc.uni_a = get_uni_mocked(tag_value="any", is_valid=True) - evc.uni_z = get_uni_mocked(tag_value="any", is_valid=True) - expected_dpid = evc.uni_a.interface.switch.id - expected_flows = [ - { - "match": {"in_port": 1, "dl_vlan": "4096/4096"}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - }, - { - "match": {"in_port": 1, "dl_vlan": "4096/4096"}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows - ) - - evc.uni_a = get_uni_mocked(tag_value="any", is_valid=True) - evc.uni_z = get_uni_mocked(tag_value=100, is_valid=True) - expected_dpid = evc.uni_a.interface.switch.id - expected_flows = [ - { - "match": {"in_port": 1, "dl_vlan": "4096/4096"}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "set_vlan", "vlan_id": 100}, - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - }, - { - "match": {"in_port": 1, "dl_vlan": 100}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows - ) + for uni_a in [100, "4096/4096", 0, None]: + for uni_z in [100, "4096/4096", 0, None]: + expected_flows = [ + { + "match": {"in_port": 1}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 3}, + ], + "priority": EPL_SB_PRIORITY + }, + { + "match": {"in_port": 3}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EPL_SB_PRIORITY + } + ] + evc.uni_a = get_uni_mocked(tag_value=uni_a, is_valid=True) + evc.uni_a.interface.port_number = 1 + evc.uni_z = get_uni_mocked(tag_value=uni_z, is_valid=True) + evc.uni_z.interface.port_number = 3 + expected_dpid = evc.uni_a.interface.switch.id + evc._install_direct_uni_flows() + if uni_a is not None: + expected_flows[0]["match"]["dl_vlan"] = uni_a + expected_flows[0]["priority"] = EVPL_SB_PRIORITY + if uni_z is not None: + expected_flows[1]["match"]["dl_vlan"] = uni_z + expected_flows[1]["priority"] = EVPL_SB_PRIORITY + + if uni_z not in {None, "4096/4096", 0}: + expected_flows[0]["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": uni_z} + ) + if uni_a not in {None, "4096/4096", 0}: + expected_flows[1]["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": uni_a} + ) + if not uni_z: + expected_flows[1]["actions"].insert( + 0, {"action_type": "push_vlan", "tag_type": "c"} + ) + if uni_z == 0: + new_action = {"action_type": "pop_vlan"} + expected_flows[0]["actions"].insert(0, new_action) + elif uni_a == "4096/4096": + if uni_z == 0: + new_action = {"action_type": "pop_vlan"} + expected_flows[0]["actions"].insert(0, new_action) + elif uni_a == 0: + if uni_z not in {None, "4096/4096", 0}: + expected_flows[0]["actions"].insert( + 0, {"action_type": "push_vlan", "tag_type": "c"} + ) + if uni_z: + new_action = {"action_type": "pop_vlan"} + expected_flows[1]["actions"].insert(0, new_action) + elif uni_a is None: + if uni_z not in {None, "4096/4096", 0}: + expected_flows[0]["actions"].insert( + 0, {"action_type": "push_vlan", "tag_type": "c"} + ) + send_flow_mods_mock.assert_called_with( + expected_dpid, expected_flows + ) def test_is_affected_by_link(self): """Test is_affected_by_link method""" From 8115423a7af7fceb73c096d0e8d43500e94cb123 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 1 Mar 2023 09:43:28 -0500 Subject: [PATCH 11/33] Fixed linter --- models/evc.py | 1 + tests/unit/models/test_evc_deploy.py | 1 + 2 files changed, 2 insertions(+) diff --git a/models/evc.py b/models/evc.py index 78c4ce19..17e0862f 100644 --- a/models/evc.py +++ b/models/evc.py @@ -812,6 +812,7 @@ def get_failover_flows(self): return {} return self._prepare_uni_flows(self.failover_path, skip_out=True) + # pylint: disable=too-many-branches def _prepare_direct_uni_flows(self): """Prepare flows connecting two UNIs for intra-switch EVC.""" vlan_a = self._get_value_from_uni_tag(self.uni_a) diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index 58692f81..fb69e787 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -1105,6 +1105,7 @@ def create_evc_intra_switch(): } return EVC(**attributes) + # pylint: disable=too-many-branches @staticmethod @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") def test_deploy_direct_uni_flows(send_flow_mods_mock): From bb7fceb753129d008de5cd8d85550c6edc46393f Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Thu, 2 Feb 2023 15:57:34 -0500 Subject: [PATCH 12/33] Removed required property from UNI tag --- openapi.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/openapi.yml b/openapi.yml index 38c53cfe..f84c8842 100644 --- a/openapi.yml +++ b/openapi.yml @@ -598,9 +598,6 @@ components: Tag: # Can be referenced via '#/components/schemas/Tag' type: object - required: - - tag_type - - value properties: tag_type: type: integer From 4c768cc0f91d357adf316a9054baae2daef80001 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Tue, 7 Feb 2023 16:28:19 -0500 Subject: [PATCH 13/33] Added support to str in tag.value --- controllers/__init__.py | 17 +++++++++++------ db/models.py | 12 +++++++++--- main.py | 10 +++++++--- models/evc.py | 15 ++++++++++----- openapi.yml | 6 ++++-- 5 files changed, 41 insertions(+), 19 deletions(-) diff --git a/controllers/__init__.py b/controllers/__init__.py index e2fd3115..65b26143 100644 --- a/controllers/__init__.py +++ b/controllers/__init__.py @@ -5,6 +5,7 @@ from typing import Dict, Optional import pymongo +from pydantic import ValidationError from pymongo.collection import ReturnDocument from pymongo.errors import AutoReconnect from tenacity import retry_if_exception_type, stop_after_attempt, wait_random @@ -71,12 +72,16 @@ def get_circuit(self, circuit_id: str) -> Optional[Dict]: def upsert_evc(self, evc: Dict) -> Optional[Dict]: """Update or insert an EVC""" utc_now = datetime.utcnow() - model = EVCBaseDoc( - **{ - **evc, - **{"_id": evc["id"]} - } - ) + try: + model = EVCBaseDoc( + **{ + **evc, + **{"_id": evc["id"]} + } + ) + except ValidationError as err: + raise err + updated = self.db.evcs.find_one_and_update( {"_id": evc["id"]}, { diff --git a/db/models.py b/db/models.py index 1e8dd733..60e60b13 100644 --- a/db/models.py +++ b/db/models.py @@ -3,9 +3,9 @@ # pylint: disable=no-self-argument,no-name-in-module from datetime import datetime -from typing import Dict, List, Literal, Optional +from typing import Dict, List, Literal, Optional, Union -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, validator class DocumentBaseModel(BaseModel): @@ -38,7 +38,13 @@ class CircuitScheduleDoc(BaseModel): class TAGDoc(BaseModel): """TAG model""" tag_type: int - value: int + value: Union[str, int] + + @validator('value') + def validate_value(cls, value): + """Validate value when is a string""" + if value.isinstance(str) and value not in ("any", "untagged"): + raise ValueError("value only allows 'any' and 'untagged' strings") class UNIDoc(BaseModel): diff --git a/main.py b/main.py index c681f7e6..8c89be12 100644 --- a/main.py +++ b/main.py @@ -7,6 +7,7 @@ from threading import Lock from flask import jsonify, request +from pydantic import ValidationError from werkzeug.exceptions import (BadRequest, Conflict, Forbidden, MethodNotAllowed, NotFound, UnsupportedMediaType) @@ -229,12 +230,15 @@ def create_circuit(self, data): except ValueError as exception: raise BadRequest(str(exception)) from exception + # save circuit + try: + evc.sync() + except ValidationError as exception: + raise BadRequest(str(exception)) from exception + # store circuit in dictionary self.circuits[evc.id] = evc - # save circuit - evc.sync() - # Schedule the circuit deploy self.sched.add(evc) diff --git a/models/evc.py b/models/evc.py index aa859757..53685fec 100644 --- a/models/evc.py +++ b/models/evc.py @@ -251,7 +251,10 @@ def _validate(self, **kwargs): raise ValueError(f"{attribute} is an invalid UNI.") if not uni.is_valid(): - tag = uni.user_tag.value + try: + tag = uni.user_tag.value + except AttributeError: + tag = None message = f"VLAN tag {tag} is not available in {attribute}" raise ValueError(message) @@ -1062,8 +1065,9 @@ def _prepare_push_flow(self, *args, queue_id=None): ) # the service tag must be always pushed - new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} - flow_mod["actions"].insert(0, new_action) + if out_vlan != "any": + new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} + flow_mod["actions"].insert(0, new_action) new_action = {"action_type": "push_vlan", "tag_type": "s"} flow_mod["actions"].insert(0, new_action) @@ -1073,8 +1077,9 @@ def _prepare_push_flow(self, *args, queue_id=None): flow_mod["match"]["dl_vlan"] = in_vlan if new_c_vlan: # new_in_vlan is set, so an action to set it is necessary - new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} - flow_mod["actions"].insert(0, new_action) + if out_vlan != "any": + new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} + flow_mod["actions"].insert(0, new_action) if not in_vlan: # new_in_vlan is set, but in_vlan is not, so there was no # vlan set; then it is set now diff --git a/openapi.yml b/openapi.yml index f84c8842..6568ccc0 100644 --- a/openapi.yml +++ b/openapi.yml @@ -603,8 +603,10 @@ components: type: integer format: int32 value: - type: integer - format: int32 + oneOf: + - type: integer + format: int32 + - type: string CircuitSchedule: # Can be referenced via '#/components/schemas/CircuitSchedule' type: object From 81d5ae64f34c7be23a623af928bf999c5142b108 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Tue, 7 Feb 2023 19:14:23 -0500 Subject: [PATCH 14/33] Extended changes to more flow types --- db/models.py | 5 +++-- models/evc.py | 34 +++++++++++++++++++--------------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/db/models.py b/db/models.py index 60e60b13..c1ea451e 100644 --- a/db/models.py +++ b/db/models.py @@ -38,13 +38,14 @@ class CircuitScheduleDoc(BaseModel): class TAGDoc(BaseModel): """TAG model""" tag_type: int - value: Union[str, int] + value: Union[int, str] @validator('value') def validate_value(cls, value): """Validate value when is a string""" - if value.isinstance(str) and value not in ("any", "untagged"): + if type(value) == (str) and (value not in ("any", "untagged")): raise ValueError("value only allows 'any' and 'untagged' strings") + return value class UNIDoc(BaseModel): diff --git a/models/evc.py b/models/evc.py index 53685fec..22e051ba 100644 --- a/models/evc.py +++ b/models/evc.py @@ -830,24 +830,28 @@ def _prepare_direct_uni_flows(self): if vlan_a and vlan_z: flow_mod_az["match"]["dl_vlan"] = vlan_a flow_mod_za["match"]["dl_vlan"] = vlan_z - flow_mod_az["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_z} - ) - flow_mod_za["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_a} - ) + if vlan_z != "any": + flow_mod_az["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_z} + ) + if vlan_a != "any": + flow_mod_za["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_a} + ) elif vlan_a: flow_mod_az["match"]["dl_vlan"] = vlan_a flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) - flow_mod_za["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_a} - ) + if vlan_a != "any": + flow_mod_za["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_a} + ) elif vlan_z: flow_mod_za["match"]["dl_vlan"] = vlan_z flow_mod_za["actions"].insert(0, {"action_type": "pop_vlan"}) - flow_mod_az["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_z} - ) + if vlan_z != "any": + flow_mod_az["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_z} + ) return ( self.uni_a.interface.switch.id, [flow_mod_az, flow_mod_za] ) @@ -1036,8 +1040,8 @@ def _prepare_nni_flow(self, *args, queue_id=None): in_interface, out_interface, queue_id ) flow_mod["match"]["dl_vlan"] = in_vlan - - new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} + if out_vlan != "any": + new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} flow_mod["actions"].insert(0, new_action) return flow_mod @@ -1077,7 +1081,7 @@ def _prepare_push_flow(self, *args, queue_id=None): flow_mod["match"]["dl_vlan"] = in_vlan if new_c_vlan: # new_in_vlan is set, so an action to set it is necessary - if out_vlan != "any": + if new_c_vlan != "any": new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} flow_mod["actions"].insert(0, new_action) if not in_vlan: From 6d91bd709b39455d21d0e0876cec60f56187314a Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 8 Feb 2023 12:26:03 -0500 Subject: [PATCH 15/33] Added new static method _get_value_from_uni_tag() --- models/evc.py | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/models/evc.py b/models/evc.py index 22e051ba..b80fdcbe 100644 --- a/models/evc.py +++ b/models/evc.py @@ -830,25 +830,25 @@ def _prepare_direct_uni_flows(self): if vlan_a and vlan_z: flow_mod_az["match"]["dl_vlan"] = vlan_a flow_mod_za["match"]["dl_vlan"] = vlan_z - if vlan_z != "any": + if vlan_z != "4096/4096": flow_mod_az["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) - if vlan_a != "any": + if vlan_a != "4096/4096": flow_mod_za["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_a} ) elif vlan_a: flow_mod_az["match"]["dl_vlan"] = vlan_a flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) - if vlan_a != "any": + if vlan_a != "4096/4096": flow_mod_za["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_a} ) elif vlan_z: flow_mod_za["match"]["dl_vlan"] = vlan_z flow_mod_za["actions"].insert(0, {"action_type": "pop_vlan"}) - if vlan_z != "any": + if vlan_z != "4096/4096": flow_mod_az["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) @@ -902,6 +902,19 @@ def _install_nni_flows(self, path=None): for dpid, flows in self._prepare_nni_flows(path).items(): self._send_flow_mods(dpid, flows) + @staticmethod + def _get_value_from_uni_tag(uni): + if uni.user_tag: + if type(uni.user_tag.value) == str: + if uni.user_tag.value == "any": + return "4096/4096" + else: + return 0 + else: + return uni.user_tag.value + else: + return None + def _prepare_uni_flows(self, path=None, skip_in=False, skip_out=False): """Prepare flows to install UNIs.""" uni_flows = {} @@ -910,10 +923,11 @@ def _prepare_uni_flows(self, path=None, skip_in=False, skip_out=False): return uni_flows # Determine VLANs - in_vlan_a = self.uni_a.user_tag.value if self.uni_a.user_tag else None + in_vlan_a = self._get_value_from_uni_tag(self.uni_a) + #in_vlan_a = self.uni_a.user_tag.value if self.uni_a.user_tag else None out_vlan_a = path[0].get_metadata("s_vlan").value - - in_vlan_z = self.uni_z.user_tag.value if self.uni_z.user_tag else None + in_vlan_z = self._get_value_from_uni_tag(self.uni_z) + #in_vlan_z = self.uni_z.user_tag.value if self.uni_z.user_tag else None out_vlan_z = path[-1].get_metadata("s_vlan").value # Flows for the first UNI @@ -1040,7 +1054,7 @@ def _prepare_nni_flow(self, *args, queue_id=None): in_interface, out_interface, queue_id ) flow_mod["match"]["dl_vlan"] = in_vlan - if out_vlan != "any": + if out_vlan != "4096/4096": new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} flow_mod["actions"].insert(0, new_action) @@ -1069,7 +1083,7 @@ def _prepare_push_flow(self, *args, queue_id=None): ) # the service tag must be always pushed - if out_vlan != "any": + if out_vlan != "4096/4096": new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} flow_mod["actions"].insert(0, new_action) @@ -1081,7 +1095,7 @@ def _prepare_push_flow(self, *args, queue_id=None): flow_mod["match"]["dl_vlan"] = in_vlan if new_c_vlan: # new_in_vlan is set, so an action to set it is necessary - if new_c_vlan != "any": + if new_c_vlan != "4096/4096": new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} flow_mod["actions"].insert(0, new_action) if not in_vlan: From 60626a37c588f1fa9308ef9898ac9bc34f78bb7d Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 8 Feb 2023 12:47:34 -0500 Subject: [PATCH 16/33] Fixed linter --- db/models.py | 2 +- models/evc.py | 14 +++++--------- tox.ini | 2 +- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/db/models.py b/db/models.py index c1ea451e..560d6e26 100644 --- a/db/models.py +++ b/db/models.py @@ -43,7 +43,7 @@ class TAGDoc(BaseModel): @validator('value') def validate_value(cls, value): """Validate value when is a string""" - if type(value) == (str) and (value not in ("any", "untagged")): + if isinstance(value, str) and (value not in ("any", "untagged")): raise ValueError("value only allows 'any' and 'untagged' strings") return value diff --git a/models/evc.py b/models/evc.py index b80fdcbe..bb81fe75 100644 --- a/models/evc.py +++ b/models/evc.py @@ -905,15 +905,12 @@ def _install_nni_flows(self, path=None): @staticmethod def _get_value_from_uni_tag(uni): if uni.user_tag: - if type(uni.user_tag.value) == str: + if isinstance(uni.user_tag.value, str): if uni.user_tag.value == "any": return "4096/4096" - else: - return 0 - else: - return uni.user_tag.value - else: - return None + return 0 + return uni.user_tag.value + return None def _prepare_uni_flows(self, path=None, skip_in=False, skip_out=False): """Prepare flows to install UNIs.""" @@ -924,10 +921,9 @@ def _prepare_uni_flows(self, path=None, skip_in=False, skip_out=False): # Determine VLANs in_vlan_a = self._get_value_from_uni_tag(self.uni_a) - #in_vlan_a = self.uni_a.user_tag.value if self.uni_a.user_tag else None out_vlan_a = path[0].get_metadata("s_vlan").value + in_vlan_z = self._get_value_from_uni_tag(self.uni_z) - #in_vlan_z = self.uni_z.user_tag.value if self.uni_z.user_tag else None out_vlan_z = path[-1].get_metadata("s_vlan").value # Flows for the first UNI diff --git a/tox.ini b/tox.ini index 7024ecb7..aa850452 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = coverage,lint +envlist = lint [gh-actions] python = From 82a986910109b6219c5cff38f80bfbc873579185 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 8 Feb 2023 12:55:32 -0500 Subject: [PATCH 17/33] Reversed tox.ini --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index aa850452..7024ecb7 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = lint +envlist = coverage,lint [gh-actions] python = From ebd6dcc3ace04ee65e226b22f32dd302115b1c04 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Thu, 9 Feb 2023 11:09:26 -0500 Subject: [PATCH 18/33] Added support for intra-switch EVC --- models/evc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/evc.py b/models/evc.py index bb81fe75..42245e4b 100644 --- a/models/evc.py +++ b/models/evc.py @@ -813,8 +813,8 @@ def get_failover_flows(self): def _prepare_direct_uni_flows(self): """Prepare flows connecting two UNIs for intra-switch EVC.""" - vlan_a = self.uni_a.user_tag.value if self.uni_a.user_tag else None - vlan_z = self.uni_z.user_tag.value if self.uni_z.user_tag else None + vlan_a = self._get_value_from_uni_tag(self.uni_a) + vlan_z = self._get_value_from_uni_tag(self.uni_z) is_EVPL = (vlan_a is not None) flow_mod_az = self._prepare_flow_mod( From b4bbe2082119ddb0150e5b30383c57887df65b45 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 15 Feb 2023 11:07:52 -0500 Subject: [PATCH 19/33] Added more possible strings to `value` from TagDoc --- db/models.py | 15 ++++++++++++--- models/evc.py | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/db/models.py b/db/models.py index 560d6e26..85b9ad3d 100644 --- a/db/models.py +++ b/db/models.py @@ -2,6 +2,7 @@ # pylint: disable=unused-argument,invalid-name,unused-argument # pylint: disable=no-self-argument,no-name-in-module +import re from datetime import datetime from typing import Dict, List, Literal, Optional, Union @@ -43,9 +44,17 @@ class TAGDoc(BaseModel): @validator('value') def validate_value(cls, value): """Validate value when is a string""" - if isinstance(value, str) and (value not in ("any", "untagged")): - raise ValueError("value only allows 'any' and 'untagged' strings") - return value + if isinstance(value, int): + return value + if isinstance(value, str): + if value in ("any", "untagged"): + return value + regex = r"^(?=.{1,9}$)\d{1,4}\/\d{1,4}$" + left, right = map(int, value.split('/')) + if re.match(regex, value) and left < 4097 and right < 4097: + return value + raise ValueError("value as string allows 'any', 'untagged' and the ", + "format 'n/n' where n > 4097") class UNIDoc(BaseModel): diff --git a/models/evc.py b/models/evc.py index 42245e4b..aff25476 100644 --- a/models/evc.py +++ b/models/evc.py @@ -1079,7 +1079,7 @@ def _prepare_push_flow(self, *args, queue_id=None): ) # the service tag must be always pushed - if out_vlan != "4096/4096": + if in_vlan != "4096/4096": new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} flow_mod["actions"].insert(0, new_action) From 9bace29794b4a22c119390453e484093b5bb12ac Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Tue, 21 Feb 2023 18:11:32 -0500 Subject: [PATCH 20/33] Added support for `untagged` - Added tests. --- db/models.py | 12 +- models/evc.py | 43 ++++---- tests/unit/models/test_evc_deploy.py | 157 ++++++++++++++++++++++++++- tests/unit/test_db_models.py | 20 +++- 4 files changed, 199 insertions(+), 33 deletions(-) diff --git a/db/models.py b/db/models.py index 85b9ad3d..56f6d553 100644 --- a/db/models.py +++ b/db/models.py @@ -2,7 +2,6 @@ # pylint: disable=unused-argument,invalid-name,unused-argument # pylint: disable=no-self-argument,no-name-in-module -import re from datetime import datetime from typing import Dict, List, Literal, Optional, Union @@ -46,15 +45,10 @@ def validate_value(cls, value): """Validate value when is a string""" if isinstance(value, int): return value - if isinstance(value, str): - if value in ("any", "untagged"): - return value - regex = r"^(?=.{1,9}$)\d{1,4}\/\d{1,4}$" - left, right = map(int, value.split('/')) - if re.match(regex, value) and left < 4097 and right < 4097: - return value + if isinstance(value, str) and value in ("any", "untagged"): + return value raise ValueError("value as string allows 'any', 'untagged' and the ", - "format 'n/n' where n > 4097") + "format 'n/n'") class UNIDoc(BaseModel): diff --git a/models/evc.py b/models/evc.py index aff25476..56d873d3 100644 --- a/models/evc.py +++ b/models/evc.py @@ -827,28 +827,28 @@ def _prepare_direct_uni_flows(self): self.queue_id, is_EVPL ) - if vlan_a and vlan_z: + if vlan_a is not None and vlan_z is not None: flow_mod_az["match"]["dl_vlan"] = vlan_a flow_mod_za["match"]["dl_vlan"] = vlan_z - if vlan_z != "4096/4096": + if vlan_z not in ("4096/4096", 0): flow_mod_az["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) - if vlan_a != "4096/4096": + if vlan_a not in ("4096/4096", 0): flow_mod_za["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_a} ) - elif vlan_a: + elif vlan_a is not None: flow_mod_az["match"]["dl_vlan"] = vlan_a flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) - if vlan_a != "4096/4096": + if vlan_a not in ("4096/4096", 0): flow_mod_za["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_a} ) - elif vlan_z: + elif vlan_z is not None: flow_mod_za["match"]["dl_vlan"] = vlan_z flow_mod_za["actions"].insert(0, {"action_type": "pop_vlan"}) - if vlan_z != "4096/4096": + if vlan_z not in ("4096/4096", 0): flow_mod_az["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) @@ -904,12 +904,15 @@ def _install_nni_flows(self, path=None): @staticmethod def _get_value_from_uni_tag(uni): + """Returns the value from tag. In case of any and untagged + it should return 4096/4096 and 0 respectively""" + special = {"any": "4096/4096", "untagged": 0} + if uni.user_tag: - if isinstance(uni.user_tag.value, str): - if uni.user_tag.value == "any": - return "4096/4096" - return 0 - return uni.user_tag.value + value = uni.user_tag.value + if isinstance(value, str): + return special.get(value, value) + return value return None def _prepare_uni_flows(self, path=None, skip_in=False, skip_out=False): @@ -1050,8 +1053,7 @@ def _prepare_nni_flow(self, *args, queue_id=None): in_interface, out_interface, queue_id ) flow_mod["match"]["dl_vlan"] = in_vlan - if out_vlan != "4096/4096": - new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} + new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} flow_mod["actions"].insert(0, new_action) return flow_mod @@ -1077,29 +1079,28 @@ def _prepare_push_flow(self, *args, queue_id=None): flow_mod = self._prepare_flow_mod( in_interface, out_interface, queue_id, is_EVPL ) - # the service tag must be always pushed - if in_vlan != "4096/4096": + if in_vlan not in ("4096/4096", 0): new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} flow_mod["actions"].insert(0, new_action) new_action = {"action_type": "push_vlan", "tag_type": "s"} flow_mod["actions"].insert(0, new_action) - if in_vlan: + if in_vlan is not None: # if in_vlan is set, it must be included in the match flow_mod["match"]["dl_vlan"] = in_vlan - if new_c_vlan: + if new_c_vlan is not None: # new_in_vlan is set, so an action to set it is necessary - if new_c_vlan != "4096/4096": + if new_c_vlan not in ("4096/4096", 0): new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} flow_mod["actions"].insert(0, new_action) - if not in_vlan: + if in_vlan is None: # new_in_vlan is set, but in_vlan is not, so there was no # vlan set; then it is set now new_action = {"action_type": "push_vlan", "tag_type": "c"} flow_mod["actions"].insert(0, new_action) - elif in_vlan: + elif in_vlan is not None: # in_vlan is set, but new_in_vlan is not, so the existing vlan # must be removed new_action = {"action_type": "pop_vlan"} diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index 18f0c106..60d9fd2a 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -275,6 +275,56 @@ def test_prepare_push_flow(self): expected_flow_mod["priority"] = EPL_SB_PRIORITY self.assertEqual(expected_flow_mod, flow_mod) + def test_prepare_push_flow_any_untagged(self): + """Test _prepare_push_flow""" + attributes = { + "controller": get_controller_mock(), + "name": "custom_name", + "uni_a": get_uni_mocked(interface_port=1, is_valid=True), + "uni_z": get_uni_mocked(interface_port=2, is_valid=True), + } + evc = EVC(**attributes) + interface_a = evc.uni_a.interface + interface_z = evc.uni_z.interface + out_vlan_a = 20 + for in_vlan_a in ("4096/4096", None): + for in_vlan_z in (0, None): + with self.subTest(in_vlan_a=in_vlan_a, in_vlan_z=in_vlan_z): + # pylint: disable=protected-access + flow_mod = evc._prepare_push_flow(interface_a, interface_z, + in_vlan_a, out_vlan_a, + in_vlan_z) + expected_flow_mod = { + 'match': {'in_port': interface_a.port_number}, + 'cookie': evc.get_cookie(), + 'actions': [ + {'action_type': 'push_vlan', 'tag_type': 's'}, + {'action_type': 'set_vlan', 'vlan_id': out_vlan_a}, + { + 'action_type': 'output', + 'port': interface_z.port_number + } + ], + "priority": EPL_SB_PRIORITY, + } + if in_vlan_a and in_vlan_z is not None: + del expected_flow_mod["actions"][1] + expected_flow_mod['match']['dl_vlan'] = in_vlan_a + expected_flow_mod['priority'] = EVPL_SB_PRIORITY + elif in_vlan_a: + del expected_flow_mod["actions"][1] + expected_flow_mod['match']['dl_vlan'] = in_vlan_a + expected_flow_mod['actions'].insert(0, { + 'action_type': 'pop_vlan' + }) + expected_flow_mod["priority"] = EVPL_SB_PRIORITY + elif in_vlan_z is not None: + expected_flow_mod['actions'].insert(0, { + 'action_type': 'push_vlan', 'tag_type': 'c' + }) + expected_flow_mod["priority"] = EPL_SB_PRIORITY + self.assertEqual(expected_flow_mod, flow_mod) + @staticmethod @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") def test_install_uni_flows(send_flow_mods_mock): @@ -467,7 +517,7 @@ def test_install_nni_flows(send_flow_mods_mock): {"action_type": "set_vlan", "vlan_id": in_vlan}, {"action_type": "output", "port": in_port}, ], - "priority": EVPL_SB_PRIORITY + "priority": EVPL_SB_PRIORITY, }, ] @@ -1195,7 +1245,7 @@ def test_deploy_direct_uni_flows(send_flow_mods_mock): ) evc.uni_z.user_tag = uni_z_tag - # Test3: no TAG in both UNI_Z and UNI_Z + # Test4: no TAG in both UNI_Z and UNI_Z evc.uni_a.user_tag = None evc.uni_z.user_tag = None expected_flows_no_tag = [ @@ -1228,6 +1278,91 @@ def test_deploy_direct_uni_flows(send_flow_mods_mock): # print(evc._prepare_direct_uni_flows()) # self.assertTrue(False) + @staticmethod + @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") + def test_deploy_direct_uni_flows_untagged_any(send_flow_mods_mock): + """Test _install_direct_uni_flows with untagged and any.""" + evc = TestEVC.create_evc_intra_switch() + + evc.uni_a = get_uni_mocked(tag_value="untagged", is_valid=True) + evc.uni_z = get_uni_mocked(tag_value="untagged", is_valid=True) + expected_dpid = evc.uni_a.interface.switch.id + expected_flows = [ + { + "match": {"in_port": 1, "dl_vlan": 0}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + }, + { + "match": {"in_port": 1, "dl_vlan": 0}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + } + ] + evc._install_direct_uni_flows() + send_flow_mods_mock.assert_called_with( + expected_dpid, expected_flows + ) + + evc.uni_a = get_uni_mocked(tag_value="any", is_valid=True) + evc.uni_z = get_uni_mocked(tag_value="any", is_valid=True) + expected_dpid = evc.uni_a.interface.switch.id + expected_flows = [ + { + "match": {"in_port": 1, "dl_vlan": "4096/4096"}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + }, + { + "match": {"in_port": 1, "dl_vlan": "4096/4096"}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + } + ] + evc._install_direct_uni_flows() + send_flow_mods_mock.assert_called_with( + expected_dpid, expected_flows + ) + + evc.uni_a = get_uni_mocked(tag_value="any", is_valid=True) + evc.uni_z = get_uni_mocked(tag_value=100, is_valid=True) + expected_dpid = evc.uni_a.interface.switch.id + expected_flows = [ + { + "match": {"in_port": 1, "dl_vlan": "4096/4096"}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "set_vlan", "vlan_id": 100}, + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + }, + { + "match": {"in_port": 1, "dl_vlan": 100}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + } + ] + evc._install_direct_uni_flows() + send_flow_mods_mock.assert_called_with( + expected_dpid, expected_flows + ) + def test_is_affected_by_link(self): """Test is_affected_by_link method""" self.evc_deploy.current_path = Path(['a', 'b', 'c']) @@ -1532,3 +1667,21 @@ def test_is_eligible_for_failover_path(self): self.evc_deploy.primary_path = Path([]) self.evc_deploy.backup_path = Path([]) self.assertTrue(self.evc_deploy.is_eligible_for_failover_path()) + + def test_get_value_from_uni_tag(self): + """Test _get_value_from_uni_tag""" + uni = get_uni_mocked(tag_value=None) + value = EVC._get_value_from_uni_tag(uni) + self.assertEqual(value, None) + + uni = get_uni_mocked(tag_value="any") + value = EVC._get_value_from_uni_tag(uni) + self.assertEqual(value, "4096/4096") + + uni = get_uni_mocked(tag_value="untagged") + value = EVC._get_value_from_uni_tag(uni) + self.assertEqual(value, 0) + + uni = get_uni_mocked(tag_value=100) + value = EVC._get_value_from_uni_tag(uni) + self.assertEqual(value, 100) diff --git a/tests/unit/test_db_models.py b/tests/unit/test_db_models.py index 2a2dedfa..087b26f6 100644 --- a/tests/unit/test_db_models.py +++ b/tests/unit/test_db_models.py @@ -2,7 +2,7 @@ from unittest import TestCase from pydantic import ValidationError -from db.models import EVCBaseDoc, DocumentBaseModel +from db.models import EVCBaseDoc, DocumentBaseModel, TAGDoc class TestDBModels(TestCase): @@ -61,3 +61,21 @@ def test_document_base_model_dict(self): self.evc_dict["_id"] = "some_id" model = DocumentBaseModel(**self.evc_dict) assert "_id" not in model.dict(exclude={"_id"}) + + def test_tagdoc_value(self): + """Test TAGDoc value restrictions""" + tag_mask = {"tag_type": 1, "value": "untagged"} + tag = TAGDoc(**tag_mask) + assert tag.tag_type == 1 + assert tag.value == "untagged" + + tag_mask = {"tag_type": 1, "value": "any"} + tag = TAGDoc(**tag_mask) + assert tag.tag_type == 1 + assert tag.value == "any" + + def test_tagdoc_fail(self): + """Test TAGDoc value fail case""" + tag_fail = {"tag_type": 1, "value": "test_fail"} + with self.assertRaises(ValueError): + TAGDoc(**tag_fail) From df23b06fc9a03e36924b2c6f99be647659cfa78b Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 1 Mar 2023 09:33:36 -0500 Subject: [PATCH 21/33] Corrected EVC combinations --- models/evc.py | 86 ++++--- tests/unit/models/test_evc_deploy.py | 370 +++++++-------------------- 2 files changed, 151 insertions(+), 305 deletions(-) diff --git a/models/evc.py b/models/evc.py index 56d873d3..19132ce5 100644 --- a/models/evc.py +++ b/models/evc.py @@ -827,31 +827,55 @@ def _prepare_direct_uni_flows(self): self.queue_id, is_EVPL ) - if vlan_a is not None and vlan_z is not None: + if vlan_a is not None: flow_mod_az["match"]["dl_vlan"] = vlan_a + + if vlan_z is not None: flow_mod_za["match"]["dl_vlan"] = vlan_z - if vlan_z not in ("4096/4096", 0): + + if vlan_a not in {None, "4096/4096", 0}: + flow_mod_za["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_a} + ) + if vlan_z not in {None, "4096/4096", 0}: flow_mod_az["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) - if vlan_a not in ("4096/4096", 0): + if not vlan_z: flow_mod_za["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_a} + 0, {"action_type": "push_vlan", "tag_type": "c"} ) - elif vlan_a is not None: - flow_mod_az["match"]["dl_vlan"] = vlan_a - flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) - if vlan_a not in ("4096/4096", 0): - flow_mod_za["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_a} + if vlan_z == 0: + flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) + + elif vlan_a == "4096/4096": + if vlan_z not in {None, "4096/4096", 0}: + flow_mod_az["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) - elif vlan_z is not None: - flow_mod_za["match"]["dl_vlan"] = vlan_z - flow_mod_za["actions"].insert(0, {"action_type": "pop_vlan"}) - if vlan_z not in ("4096/4096", 0): + if vlan_z == 0: + flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) + + elif vlan_a == 0: + if vlan_z not in {None, "4096/4096", 0}: flow_mod_az["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) + flow_mod_az["actions"].insert( + 0, {"action_type": "push_vlan", "tag_type": "c"} + ) + if vlan_z: + flow_mod_za["actions"].insert(0, {"action_type": "pop_vlan"}) + + elif vlan_a is None: + if vlan_z not in {None, "4096/4096", 0}: + flow_mod_az["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_z} + ) + flow_mod_az["actions"].insert( + 0, {"action_type": "push_vlan", "tag_type": "c"} + ) + return ( self.uni_a.interface.switch.id, [flow_mod_az, flow_mod_za] ) @@ -1080,9 +1104,8 @@ def _prepare_push_flow(self, *args, queue_id=None): in_interface, out_interface, queue_id, is_EVPL ) # the service tag must be always pushed - if in_vlan not in ("4096/4096", 0): - new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} - flow_mod["actions"].insert(0, new_action) + new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} + flow_mod["actions"].insert(0, new_action) new_action = {"action_type": "push_vlan", "tag_type": "s"} flow_mod["actions"].insert(0, new_action) @@ -1090,21 +1113,26 @@ def _prepare_push_flow(self, *args, queue_id=None): if in_vlan is not None: # if in_vlan is set, it must be included in the match flow_mod["match"]["dl_vlan"] = in_vlan - if new_c_vlan is not None: - # new_in_vlan is set, so an action to set it is necessary - if new_c_vlan not in ("4096/4096", 0): - new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} + + if new_c_vlan not in {"4096/4096", 0, None}: + new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} + flow_mod["actions"].insert(0, new_action) + + if in_vlan not in {"4096/4096", 0, None}: + if new_c_vlan == 0: + new_action = {"action_type": "pop_vlan"} + flow_mod["actions"].insert(0, new_action) + + elif in_vlan == "4096/4096": + if new_c_vlan == 0: + new_action = {"action_type": "pop_vlan"} flow_mod["actions"].insert(0, new_action) - if in_vlan is None: - # new_in_vlan is set, but in_vlan is not, so there was no - # vlan set; then it is set now + + elif not in_vlan: + if new_c_vlan not in {"4096/4096", 0, None}: new_action = {"action_type": "push_vlan", "tag_type": "c"} flow_mod["actions"].insert(0, new_action) - elif in_vlan is not None: - # in_vlan is set, but new_in_vlan is not, so the existing vlan - # must be removed - new_action = {"action_type": "pop_vlan"} - flow_mod["actions"].insert(0, new_action) + return flow_mod def _prepare_pop_flow( diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index 60d9fd2a..cd791305 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -233,8 +233,8 @@ def test_prepare_push_flow(self): interface_z = evc.uni_z.interface out_vlan_a = 20 - for in_vlan_a in (10, None): - for in_vlan_z in (3, None): + for in_vlan_a in [100, "4096/4096", 0, None]: + for in_vlan_z in [50, "4096/4096", 0, None]: with self.subTest(in_vlan_a=in_vlan_a, in_vlan_z=in_vlan_z): # pylint: disable=protected-access flow_mod = evc._prepare_push_flow(interface_a, interface_z, @@ -251,78 +251,31 @@ def test_prepare_push_flow(self): 'port': interface_z.port_number } ], - "priority": EPL_SB_PRIORITY, + "priority": EVPL_SB_PRIORITY, } - if in_vlan_a and in_vlan_z: + if in_vlan_a is not None: expected_flow_mod['match']['dl_vlan'] = in_vlan_a - expected_flow_mod['actions'].insert(0, { - 'action_type': 'set_vlan', 'vlan_id': in_vlan_z - }) - expected_flow_mod['priority'] = EVPL_SB_PRIORITY - elif in_vlan_a: - expected_flow_mod['match']['dl_vlan'] = in_vlan_a - expected_flow_mod['actions'].insert(0, { - 'action_type': 'pop_vlan' - }) - expected_flow_mod["priority"] = EVPL_SB_PRIORITY - elif in_vlan_z: - expected_flow_mod['actions'].insert(0, { - 'action_type': 'set_vlan', 'vlan_id': in_vlan_z - }) - expected_flow_mod['actions'].insert(0, { - 'action_type': 'push_vlan', 'tag_type': 'c' - }) - expected_flow_mod["priority"] = EPL_SB_PRIORITY - self.assertEqual(expected_flow_mod, flow_mod) - - def test_prepare_push_flow_any_untagged(self): - """Test _prepare_push_flow""" - attributes = { - "controller": get_controller_mock(), - "name": "custom_name", - "uni_a": get_uni_mocked(interface_port=1, is_valid=True), - "uni_z": get_uni_mocked(interface_port=2, is_valid=True), - } - evc = EVC(**attributes) - interface_a = evc.uni_a.interface - interface_z = evc.uni_z.interface - out_vlan_a = 20 - for in_vlan_a in ("4096/4096", None): - for in_vlan_z in (0, None): - with self.subTest(in_vlan_a=in_vlan_a, in_vlan_z=in_vlan_z): - # pylint: disable=protected-access - flow_mod = evc._prepare_push_flow(interface_a, interface_z, - in_vlan_a, out_vlan_a, - in_vlan_z) - expected_flow_mod = { - 'match': {'in_port': interface_a.port_number}, - 'cookie': evc.get_cookie(), - 'actions': [ - {'action_type': 'push_vlan', 'tag_type': 's'}, - {'action_type': 'set_vlan', 'vlan_id': out_vlan_a}, - { - 'action_type': 'output', - 'port': interface_z.port_number - } - ], - "priority": EPL_SB_PRIORITY, - } - if in_vlan_a and in_vlan_z is not None: - del expected_flow_mod["actions"][1] - expected_flow_mod['match']['dl_vlan'] = in_vlan_a - expected_flow_mod['priority'] = EVPL_SB_PRIORITY - elif in_vlan_a: - del expected_flow_mod["actions"][1] - expected_flow_mod['match']['dl_vlan'] = in_vlan_a - expected_flow_mod['actions'].insert(0, { - 'action_type': 'pop_vlan' - }) - expected_flow_mod["priority"] = EVPL_SB_PRIORITY - elif in_vlan_z is not None: - expected_flow_mod['actions'].insert(0, { - 'action_type': 'push_vlan', 'tag_type': 'c' - }) - expected_flow_mod["priority"] = EPL_SB_PRIORITY + else: + expected_flow_mod['priority'] = EPL_SB_PRIORITY + + if in_vlan_z not in {"4096/4096", 0, None}: + new_action = {"action_type": "set_vlan", + "vlan_id": in_vlan_z} + expected_flow_mod["actions"].insert(0, new_action) + + if in_vlan_a not in {"4096/4096", 0, None}: + if in_vlan_z == 0: + new_action = {"action_type": "pop_vlan"} + expected_flow_mod["actions"].insert(0, new_action) + elif in_vlan_a == "4096/4096": + if in_vlan_z == 0: + new_action = {"action_type": "pop_vlan"} + expected_flow_mod["actions"].insert(0, new_action) + elif not in_vlan_a: + if in_vlan_z not in {"4096/4096", 0, None}: + new_action = {"action_type": "push_vlan", + "tag_type": "c"} + expected_flow_mod["actions"].insert(0, new_action) self.assertEqual(expected_flow_mod, flow_mod) @staticmethod @@ -1155,213 +1108,78 @@ def create_evc_intra_switch(): @staticmethod @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") def test_deploy_direct_uni_flows(send_flow_mods_mock): - """Test _install_direct_uni_flows.""" + """Test _install_direct_uni_flows""" evc = TestEVC.create_evc_intra_switch() - - # Test 1: both UNIs with TAG expected_dpid = evc.uni_a.interface.switch.id - expected_flows = [ - { - "match": {"in_port": 1, "dl_vlan": 82}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "set_vlan", "vlan_id": 84}, - {"action_type": "output", "port": 3}, - ], - "priority": EVPL_SB_PRIORITY - }, - { - "match": {"in_port": 3, "dl_vlan": 84}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "set_vlan", "vlan_id": 82}, - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - } - ] - - # pylint: disable=protected-access - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_once_with( - expected_dpid, expected_flows - ) - - # Test2: no TAG in UNI_A - uni_a_tag = evc.uni_a.user_tag - evc.uni_a.user_tag = None - expected_flows_no_tag_a = [ - { - "match": {"in_port": 1}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "set_vlan", "vlan_id": 84}, - {"action_type": "output", "port": 3}, - ], - "priority": EPL_SB_PRIORITY - }, - { - "match": {"in_port": 3, "dl_vlan": 84}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "pop_vlan"}, - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows_no_tag_a - ) - evc.uni_a.user_tag = uni_a_tag - # Test3: no TAG in UNI_Z - uni_z_tag = evc.uni_z.user_tag - evc.uni_z.user_tag = None - expected_flows_no_tag_z = [ - { - "match": {"in_port": 1, "dl_vlan": 82}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "pop_vlan"}, - {"action_type": "output", "port": 3}, - ], - "priority": EVPL_SB_PRIORITY - }, - { - "match": {"in_port": 3}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "set_vlan", "vlan_id": 82}, - {"action_type": "output", "port": 1}, - ], - "priority": EPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows_no_tag_z - ) - evc.uni_z.user_tag = uni_z_tag - - # Test4: no TAG in both UNI_Z and UNI_Z - evc.uni_a.user_tag = None - evc.uni_z.user_tag = None - expected_flows_no_tag = [ - { - "match": {"in_port": 1}, - "cookie": evc.get_cookie(), - "actions": [{"action_type": "output", "port": 3}], - "priority": EPL_SB_PRIORITY - }, - { - "match": {"in_port": 3}, - "cookie": evc.get_cookie(), - "actions": [{"action_type": "output", "port": 1}], - "priority": EPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows_no_tag - ) -# -# print(evc._prepare_direct_uni_flows()) -# evc.uni_a.user_tag = uni_a_tag -# uni_z_tag = evc.uni_z.user_tag -# evc.uni_z.user_tag = None -# print(evc._prepare_direct_uni_flows()) -# evc.uni_z.user_tag = uni_z_tag -# evc.uni_a.user_tag = None -# evc.uni_z.user_tag = None -# print(evc._prepare_direct_uni_flows()) -# self.assertTrue(False) - - @staticmethod - @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") - def test_deploy_direct_uni_flows_untagged_any(send_flow_mods_mock): - """Test _install_direct_uni_flows with untagged and any.""" - evc = TestEVC.create_evc_intra_switch() - - evc.uni_a = get_uni_mocked(tag_value="untagged", is_valid=True) - evc.uni_z = get_uni_mocked(tag_value="untagged", is_valid=True) - expected_dpid = evc.uni_a.interface.switch.id - expected_flows = [ - { - "match": {"in_port": 1, "dl_vlan": 0}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - }, - { - "match": {"in_port": 1, "dl_vlan": 0}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows - ) - - evc.uni_a = get_uni_mocked(tag_value="any", is_valid=True) - evc.uni_z = get_uni_mocked(tag_value="any", is_valid=True) - expected_dpid = evc.uni_a.interface.switch.id - expected_flows = [ - { - "match": {"in_port": 1, "dl_vlan": "4096/4096"}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - }, - { - "match": {"in_port": 1, "dl_vlan": "4096/4096"}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows - ) - - evc.uni_a = get_uni_mocked(tag_value="any", is_valid=True) - evc.uni_z = get_uni_mocked(tag_value=100, is_valid=True) - expected_dpid = evc.uni_a.interface.switch.id - expected_flows = [ - { - "match": {"in_port": 1, "dl_vlan": "4096/4096"}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "set_vlan", "vlan_id": 100}, - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - }, - { - "match": {"in_port": 1, "dl_vlan": 100}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows - ) + for uni_a in [100, "4096/4096", 0, None]: + for uni_z in [100, "4096/4096", 0, None]: + expected_flows = [ + { + "match": {"in_port": 1}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 3}, + ], + "priority": EPL_SB_PRIORITY + }, + { + "match": {"in_port": 3}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EPL_SB_PRIORITY + } + ] + evc.uni_a = get_uni_mocked(tag_value=uni_a, is_valid=True) + evc.uni_a.interface.port_number = 1 + evc.uni_z = get_uni_mocked(tag_value=uni_z, is_valid=True) + evc.uni_z.interface.port_number = 3 + expected_dpid = evc.uni_a.interface.switch.id + evc._install_direct_uni_flows() + if uni_a is not None: + expected_flows[0]["match"]["dl_vlan"] = uni_a + expected_flows[0]["priority"] = EVPL_SB_PRIORITY + if uni_z is not None: + expected_flows[1]["match"]["dl_vlan"] = uni_z + expected_flows[1]["priority"] = EVPL_SB_PRIORITY + + if uni_z not in {None, "4096/4096", 0}: + expected_flows[0]["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": uni_z} + ) + if uni_a not in {None, "4096/4096", 0}: + expected_flows[1]["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": uni_a} + ) + if not uni_z: + expected_flows[1]["actions"].insert( + 0, {"action_type": "push_vlan", "tag_type": "c"} + ) + if uni_z == 0: + new_action = {"action_type": "pop_vlan"} + expected_flows[0]["actions"].insert(0, new_action) + elif uni_a == "4096/4096": + if uni_z == 0: + new_action = {"action_type": "pop_vlan"} + expected_flows[0]["actions"].insert(0, new_action) + elif uni_a == 0: + if uni_z not in {None, "4096/4096", 0}: + expected_flows[0]["actions"].insert( + 0, {"action_type": "push_vlan", "tag_type": "c"} + ) + if uni_z: + new_action = {"action_type": "pop_vlan"} + expected_flows[1]["actions"].insert(0, new_action) + elif uni_a is None: + if uni_z not in {None, "4096/4096", 0}: + expected_flows[0]["actions"].insert( + 0, {"action_type": "push_vlan", "tag_type": "c"} + ) + send_flow_mods_mock.assert_called_with( + expected_dpid, expected_flows + ) def test_is_affected_by_link(self): """Test is_affected_by_link method""" From ab731a41cca572a7f15ac77782a2ff9e0cc01882 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 1 Mar 2023 09:43:28 -0500 Subject: [PATCH 22/33] Fixed linter --- models/evc.py | 1 + tests/unit/models/test_evc_deploy.py | 1 + 2 files changed, 2 insertions(+) diff --git a/models/evc.py b/models/evc.py index 19132ce5..3db0f76f 100644 --- a/models/evc.py +++ b/models/evc.py @@ -811,6 +811,7 @@ def get_failover_flows(self): return {} return self._prepare_uni_flows(self.failover_path, skip_out=True) + # pylint: disable=too-many-branches def _prepare_direct_uni_flows(self): """Prepare flows connecting two UNIs for intra-switch EVC.""" vlan_a = self._get_value_from_uni_tag(self.uni_a) diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index cd791305..22df8f81 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -1105,6 +1105,7 @@ def create_evc_intra_switch(): } return EVC(**attributes) + # pylint: disable=too-many-branches @staticmethod @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") def test_deploy_direct_uni_flows(send_flow_mods_mock): From 72c25d9c6028ade9a1c0484d12d0a855264732c3 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Tue, 7 Feb 2023 16:28:19 -0500 Subject: [PATCH 23/33] Added support to str in tag.value --- models/evc.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/models/evc.py b/models/evc.py index 3db0f76f..e1f9adbd 100644 --- a/models/evc.py +++ b/models/evc.py @@ -1105,8 +1105,9 @@ def _prepare_push_flow(self, *args, queue_id=None): in_interface, out_interface, queue_id, is_EVPL ) # the service tag must be always pushed - new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} - flow_mod["actions"].insert(0, new_action) + if out_vlan != "any": + new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} + flow_mod["actions"].insert(0, new_action) new_action = {"action_type": "push_vlan", "tag_type": "s"} flow_mod["actions"].insert(0, new_action) From d87746922c6580e31045a55472ae055d41a10809 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 8 Feb 2023 12:26:03 -0500 Subject: [PATCH 24/33] Added new static method _get_value_from_uni_tag() --- models/evc.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/models/evc.py b/models/evc.py index e1f9adbd..3db0f76f 100644 --- a/models/evc.py +++ b/models/evc.py @@ -1105,9 +1105,8 @@ def _prepare_push_flow(self, *args, queue_id=None): in_interface, out_interface, queue_id, is_EVPL ) # the service tag must be always pushed - if out_vlan != "any": - new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} - flow_mod["actions"].insert(0, new_action) + new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} + flow_mod["actions"].insert(0, new_action) new_action = {"action_type": "push_vlan", "tag_type": "s"} flow_mod["actions"].insert(0, new_action) From 17acb031f5d61066867cf3c7e0aa4329fbc422c4 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 15 Feb 2023 11:07:52 -0500 Subject: [PATCH 25/33] Added more possible strings to `value` from TagDoc --- db/models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/db/models.py b/db/models.py index 56f6d553..53314a6d 100644 --- a/db/models.py +++ b/db/models.py @@ -2,6 +2,7 @@ # pylint: disable=unused-argument,invalid-name,unused-argument # pylint: disable=no-self-argument,no-name-in-module +import re from datetime import datetime from typing import Dict, List, Literal, Optional, Union From 4c9ffc962a809d5473d4ad13e84887d280a36590 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Tue, 21 Feb 2023 18:11:32 -0500 Subject: [PATCH 26/33] Added support for `untagged` - Added tests. --- db/models.py | 1 - tests/unit/models/test_evc_deploy.py | 135 +++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 1 deletion(-) diff --git a/db/models.py b/db/models.py index 53314a6d..56f6d553 100644 --- a/db/models.py +++ b/db/models.py @@ -2,7 +2,6 @@ # pylint: disable=unused-argument,invalid-name,unused-argument # pylint: disable=no-self-argument,no-name-in-module -import re from datetime import datetime from typing import Dict, List, Literal, Optional, Union diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index 22df8f81..e3610aee 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -278,6 +278,56 @@ def test_prepare_push_flow(self): expected_flow_mod["actions"].insert(0, new_action) self.assertEqual(expected_flow_mod, flow_mod) + def test_prepare_push_flow_any_untagged(self): + """Test _prepare_push_flow""" + attributes = { + "controller": get_controller_mock(), + "name": "custom_name", + "uni_a": get_uni_mocked(interface_port=1, is_valid=True), + "uni_z": get_uni_mocked(interface_port=2, is_valid=True), + } + evc = EVC(**attributes) + interface_a = evc.uni_a.interface + interface_z = evc.uni_z.interface + out_vlan_a = 20 + for in_vlan_a in ("4096/4096", None): + for in_vlan_z in (0, None): + with self.subTest(in_vlan_a=in_vlan_a, in_vlan_z=in_vlan_z): + # pylint: disable=protected-access + flow_mod = evc._prepare_push_flow(interface_a, interface_z, + in_vlan_a, out_vlan_a, + in_vlan_z) + expected_flow_mod = { + 'match': {'in_port': interface_a.port_number}, + 'cookie': evc.get_cookie(), + 'actions': [ + {'action_type': 'push_vlan', 'tag_type': 's'}, + {'action_type': 'set_vlan', 'vlan_id': out_vlan_a}, + { + 'action_type': 'output', + 'port': interface_z.port_number + } + ], + "priority": EPL_SB_PRIORITY, + } + if in_vlan_a and in_vlan_z is not None: + del expected_flow_mod["actions"][1] + expected_flow_mod['match']['dl_vlan'] = in_vlan_a + expected_flow_mod['priority'] = EVPL_SB_PRIORITY + elif in_vlan_a: + del expected_flow_mod["actions"][1] + expected_flow_mod['match']['dl_vlan'] = in_vlan_a + expected_flow_mod['actions'].insert(0, { + 'action_type': 'pop_vlan' + }) + expected_flow_mod["priority"] = EVPL_SB_PRIORITY + elif in_vlan_z is not None: + expected_flow_mod['actions'].insert(0, { + 'action_type': 'push_vlan', 'tag_type': 'c' + }) + expected_flow_mod["priority"] = EPL_SB_PRIORITY + self.assertEqual(expected_flow_mod, flow_mod) + @staticmethod @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") def test_install_uni_flows(send_flow_mods_mock): @@ -1182,6 +1232,91 @@ def test_deploy_direct_uni_flows(send_flow_mods_mock): expected_dpid, expected_flows ) + @staticmethod + @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") + def test_deploy_direct_uni_flows_untagged_any(send_flow_mods_mock): + """Test _install_direct_uni_flows with untagged and any.""" + evc = TestEVC.create_evc_intra_switch() + + evc.uni_a = get_uni_mocked(tag_value="untagged", is_valid=True) + evc.uni_z = get_uni_mocked(tag_value="untagged", is_valid=True) + expected_dpid = evc.uni_a.interface.switch.id + expected_flows = [ + { + "match": {"in_port": 1, "dl_vlan": 0}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + }, + { + "match": {"in_port": 1, "dl_vlan": 0}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + } + ] + evc._install_direct_uni_flows() + send_flow_mods_mock.assert_called_with( + expected_dpid, expected_flows + ) + + evc.uni_a = get_uni_mocked(tag_value="any", is_valid=True) + evc.uni_z = get_uni_mocked(tag_value="any", is_valid=True) + expected_dpid = evc.uni_a.interface.switch.id + expected_flows = [ + { + "match": {"in_port": 1, "dl_vlan": "4096/4096"}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + }, + { + "match": {"in_port": 1, "dl_vlan": "4096/4096"}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + } + ] + evc._install_direct_uni_flows() + send_flow_mods_mock.assert_called_with( + expected_dpid, expected_flows + ) + + evc.uni_a = get_uni_mocked(tag_value="any", is_valid=True) + evc.uni_z = get_uni_mocked(tag_value=100, is_valid=True) + expected_dpid = evc.uni_a.interface.switch.id + expected_flows = [ + { + "match": {"in_port": 1, "dl_vlan": "4096/4096"}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "set_vlan", "vlan_id": 100}, + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + }, + { + "match": {"in_port": 1, "dl_vlan": 100}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EVPL_SB_PRIORITY + } + ] + evc._install_direct_uni_flows() + send_flow_mods_mock.assert_called_with( + expected_dpid, expected_flows + ) + def test_is_affected_by_link(self): """Test is_affected_by_link method""" self.evc_deploy.current_path = Path(['a', 'b', 'c']) From 4a407e39f463ee7eac45b0f57552c873d71fef0c Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Wed, 1 Mar 2023 09:33:36 -0500 Subject: [PATCH 27/33] Corrected EVC combinations --- tests/unit/models/test_evc_deploy.py | 66 ++++++++-------------------- 1 file changed, 18 insertions(+), 48 deletions(-) diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index e3610aee..f7d20175 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -278,54 +278,24 @@ def test_prepare_push_flow(self): expected_flow_mod["actions"].insert(0, new_action) self.assertEqual(expected_flow_mod, flow_mod) - def test_prepare_push_flow_any_untagged(self): - """Test _prepare_push_flow""" - attributes = { - "controller": get_controller_mock(), - "name": "custom_name", - "uni_a": get_uni_mocked(interface_port=1, is_valid=True), - "uni_z": get_uni_mocked(interface_port=2, is_valid=True), - } - evc = EVC(**attributes) - interface_a = evc.uni_a.interface - interface_z = evc.uni_z.interface - out_vlan_a = 20 - for in_vlan_a in ("4096/4096", None): - for in_vlan_z in (0, None): - with self.subTest(in_vlan_a=in_vlan_a, in_vlan_z=in_vlan_z): - # pylint: disable=protected-access - flow_mod = evc._prepare_push_flow(interface_a, interface_z, - in_vlan_a, out_vlan_a, - in_vlan_z) - expected_flow_mod = { - 'match': {'in_port': interface_a.port_number}, - 'cookie': evc.get_cookie(), - 'actions': [ - {'action_type': 'push_vlan', 'tag_type': 's'}, - {'action_type': 'set_vlan', 'vlan_id': out_vlan_a}, - { - 'action_type': 'output', - 'port': interface_z.port_number - } - ], - "priority": EPL_SB_PRIORITY, - } - if in_vlan_a and in_vlan_z is not None: - del expected_flow_mod["actions"][1] - expected_flow_mod['match']['dl_vlan'] = in_vlan_a - expected_flow_mod['priority'] = EVPL_SB_PRIORITY - elif in_vlan_a: - del expected_flow_mod["actions"][1] - expected_flow_mod['match']['dl_vlan'] = in_vlan_a - expected_flow_mod['actions'].insert(0, { - 'action_type': 'pop_vlan' - }) - expected_flow_mod["priority"] = EVPL_SB_PRIORITY - elif in_vlan_z is not None: - expected_flow_mod['actions'].insert(0, { - 'action_type': 'push_vlan', 'tag_type': 'c' - }) - expected_flow_mod["priority"] = EPL_SB_PRIORITY + if in_vlan_z not in {"4096/4096", 0, None}: + new_action = {"action_type": "set_vlan", + "vlan_id": in_vlan_z} + expected_flow_mod["actions"].insert(0, new_action) + + if in_vlan_a not in {"4096/4096", 0, None}: + if in_vlan_z == 0: + new_action = {"action_type": "pop_vlan"} + expected_flow_mod["actions"].insert(0, new_action) + elif in_vlan_a == "4096/4096": + if in_vlan_z == 0: + new_action = {"action_type": "pop_vlan"} + expected_flow_mod["actions"].insert(0, new_action) + elif not in_vlan_a: + if in_vlan_z not in {"4096/4096", 0, None}: + new_action = {"action_type": "push_vlan", + "tag_type": "c"} + expected_flow_mod["actions"].insert(0, new_action) self.assertEqual(expected_flow_mod, flow_mod) @staticmethod From 59258b87de71372f0481a9e6fb8196ff83a3cbc1 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Mon, 6 Mar 2023 13:14:57 -0500 Subject: [PATCH 28/33] Reduced if statements - Added comments --- models/evc.py | 73 +++++++++++----------------- tests/unit/models/test_evc_deploy.py | 21 +------- 2 files changed, 30 insertions(+), 64 deletions(-) diff --git a/models/evc.py b/models/evc.py index 3db0f76f..736403e3 100644 --- a/models/evc.py +++ b/models/evc.py @@ -811,7 +811,6 @@ def get_failover_flows(self): return {} return self._prepare_uni_flows(self.failover_path, skip_out=True) - # pylint: disable=too-many-branches def _prepare_direct_uni_flows(self): """Prepare flows connecting two UNIs for intra-switch EVC.""" vlan_a = self._get_value_from_uni_tag(self.uni_a) @@ -834,14 +833,19 @@ def _prepare_direct_uni_flows(self): if vlan_z is not None: flow_mod_za["match"]["dl_vlan"] = vlan_z + if vlan_z not in {None, "4096/4096", 0}: + flow_mod_az["actions"].insert( + 0, {"action_type": "set_vlan", "vlan_id": vlan_z} + ) + if not vlan_a: + flow_mod_az["actions"].insert( + 0, {"action_type": "push_vlan", "tag_type": "c"} + ) + if vlan_a not in {None, "4096/4096", 0}: flow_mod_za["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_a} ) - if vlan_z not in {None, "4096/4096", 0}: - flow_mod_az["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_z} - ) if not vlan_z: flow_mod_za["actions"].insert( 0, {"action_type": "push_vlan", "tag_type": "c"} @@ -849,33 +853,11 @@ def _prepare_direct_uni_flows(self): if vlan_z == 0: flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) - elif vlan_a == "4096/4096": - if vlan_z not in {None, "4096/4096", 0}: - flow_mod_az["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_z} - ) - if vlan_z == 0: - flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) + elif vlan_a == "4096/4096" and vlan_z == 0: + flow_mod_az["actions"].insert(0, {"action_type": "pop_vlan"}) - elif vlan_a == 0: - if vlan_z not in {None, "4096/4096", 0}: - flow_mod_az["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_z} - ) - flow_mod_az["actions"].insert( - 0, {"action_type": "push_vlan", "tag_type": "c"} - ) - if vlan_z: - flow_mod_za["actions"].insert(0, {"action_type": "pop_vlan"}) - - elif vlan_a is None: - if vlan_z not in {None, "4096/4096", 0}: - flow_mod_az["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": vlan_z} - ) - flow_mod_az["actions"].insert( - 0, {"action_type": "push_vlan", "tag_type": "c"} - ) + elif vlan_a == 0 and vlan_z: + flow_mod_za["actions"].insert(0, {"action_type": "pop_vlan"}) return ( self.uni_a.interface.switch.id, [flow_mod_az, flow_mod_za] @@ -1083,7 +1065,6 @@ def _prepare_nni_flow(self, *args, queue_id=None): return flow_mod - # pylint: disable=too-many-arguments def _prepare_push_flow(self, *args, queue_id=None): """Prepare push flow. @@ -1116,23 +1097,27 @@ def _prepare_push_flow(self, *args, queue_id=None): flow_mod["match"]["dl_vlan"] = in_vlan if new_c_vlan not in {"4096/4096", 0, None}: + # new_in_vlan is an integer but zero, action to set is required new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} flow_mod["actions"].insert(0, new_action) - if in_vlan not in {"4096/4096", 0, None}: - if new_c_vlan == 0: - new_action = {"action_type": "pop_vlan"} - flow_mod["actions"].insert(0, new_action) + if in_vlan not in {"4096/4096", 0, None} and new_c_vlan == 0: + # # new_in_vlan is an integer but zero and new_c_vlan does not + # a pop action is required + new_action = {"action_type": "pop_vlan"} + flow_mod["actions"].insert(0, new_action) - elif in_vlan == "4096/4096": - if new_c_vlan == 0: - new_action = {"action_type": "pop_vlan"} - flow_mod["actions"].insert(0, new_action) + elif in_vlan == "4096/4096" and new_c_vlan == 0: + # if in_vlan match with any tags and new_c_vlan does not + # a pop action is required + new_action = {"action_type": "pop_vlan"} + flow_mod["actions"].insert(0, new_action) - elif not in_vlan: - if new_c_vlan not in {"4096/4096", 0, None}: - new_action = {"action_type": "push_vlan", "tag_type": "c"} - flow_mod["actions"].insert(0, new_action) + elif not in_vlan and new_c_vlan not in {"4096/4096", 0, None}: + # new_in_vlan is an integer but zero and in_vlan is not set + # then it is set now + new_action = {"action_type": "push_vlan", "tag_type": "c"} + flow_mod["actions"].insert(0, new_action) return flow_mod diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index f7d20175..abc0f4cc 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -220,6 +220,7 @@ def test_prepare_pop_flow(self): } self.assertEqual(expected_flow_mod, flow_mod) + # pylint: disable=too-many-branches def test_prepare_push_flow(self): """Test prepare push flow method.""" attributes = { @@ -278,26 +279,6 @@ def test_prepare_push_flow(self): expected_flow_mod["actions"].insert(0, new_action) self.assertEqual(expected_flow_mod, flow_mod) - if in_vlan_z not in {"4096/4096", 0, None}: - new_action = {"action_type": "set_vlan", - "vlan_id": in_vlan_z} - expected_flow_mod["actions"].insert(0, new_action) - - if in_vlan_a not in {"4096/4096", 0, None}: - if in_vlan_z == 0: - new_action = {"action_type": "pop_vlan"} - expected_flow_mod["actions"].insert(0, new_action) - elif in_vlan_a == "4096/4096": - if in_vlan_z == 0: - new_action = {"action_type": "pop_vlan"} - expected_flow_mod["actions"].insert(0, new_action) - elif not in_vlan_a: - if in_vlan_z not in {"4096/4096", 0, None}: - new_action = {"action_type": "push_vlan", - "tag_type": "c"} - expected_flow_mod["actions"].insert(0, new_action) - self.assertEqual(expected_flow_mod, flow_mod) - @staticmethod @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") def test_install_uni_flows(send_flow_mods_mock): From ad2e5584699f85b572ff5ab8f0cd0630d80289a0 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Tue, 7 Mar 2023 11:52:23 -0500 Subject: [PATCH 29/33] Changed priorities depending on new cases - Ensured string for `dl_vlan` flow data --- models/evc.py | 27 +++--- settings.py | 2 + tests/unit/models/test_evc_deploy.py | 138 +++++++-------------------- 3 files changed, 48 insertions(+), 119 deletions(-) diff --git a/models/evc.py b/models/evc.py index 736403e3..271fea22 100644 --- a/models/evc.py +++ b/models/evc.py @@ -816,22 +816,20 @@ def _prepare_direct_uni_flows(self): vlan_a = self._get_value_from_uni_tag(self.uni_a) vlan_z = self._get_value_from_uni_tag(self.uni_z) - is_EVPL = (vlan_a is not None) flow_mod_az = self._prepare_flow_mod( self.uni_a.interface, self.uni_z.interface, - self.queue_id, is_EVPL + self.queue_id, vlan_a ) - is_EVPL = (vlan_z is not None) flow_mod_za = self._prepare_flow_mod( self.uni_z.interface, self.uni_a.interface, - self.queue_id, is_EVPL + self.queue_id, vlan_z ) if vlan_a is not None: - flow_mod_az["match"]["dl_vlan"] = vlan_a + flow_mod_az["match"]["dl_vlan"] = str(vlan_a) if vlan_z is not None: - flow_mod_za["match"]["dl_vlan"] = vlan_z + flow_mod_za["match"]["dl_vlan"] = str(vlan_z) if vlan_z not in {None, "4096/4096", 0}: flow_mod_az["actions"].insert( @@ -1029,7 +1027,7 @@ def get_id_from_cookie(cookie): return f"{evc_id:x}".zfill(14) def _prepare_flow_mod(self, in_interface, out_interface, - queue_id=None, is_EVPL=True): + queue_id=None, vlan=True): """Prepare a common flow mod.""" default_actions = [ {"action_type": "output", "port": out_interface.port_number} @@ -1047,8 +1045,12 @@ def _prepare_flow_mod(self, in_interface, out_interface, if self.sb_priority: flow_mod["priority"] = self.sb_priority else: - if is_EVPL: + if vlan not in {"4096/4096", None}: + # EVPL, untagged and the rest flow_mod["priority"] = settings.EVPL_SB_PRIORITY + elif vlan == "4096/4096": + # Any + flow_mod["priority"] = settings.ANY_SB_PRIORITY else: flow_mod["priority"] = settings.EPL_SB_PRIORITY return flow_mod @@ -1059,7 +1061,7 @@ def _prepare_nni_flow(self, *args, queue_id=None): flow_mod = self._prepare_flow_mod( in_interface, out_interface, queue_id ) - flow_mod["match"]["dl_vlan"] = in_vlan + flow_mod["match"]["dl_vlan"] = str(in_vlan) new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} flow_mod["actions"].insert(0, new_action) @@ -1081,9 +1083,8 @@ def _prepare_push_flow(self, *args, queue_id=None): """ # assign all arguments in_interface, out_interface, in_vlan, out_vlan, new_c_vlan = args - is_EVPL = (in_vlan is not None) flow_mod = self._prepare_flow_mod( - in_interface, out_interface, queue_id, is_EVPL + in_interface, out_interface, queue_id, in_vlan ) # the service tag must be always pushed new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} @@ -1094,7 +1095,7 @@ def _prepare_push_flow(self, *args, queue_id=None): if in_vlan is not None: # if in_vlan is set, it must be included in the match - flow_mod["match"]["dl_vlan"] = in_vlan + flow_mod["match"]["dl_vlan"] = str(in_vlan) if new_c_vlan not in {"4096/4096", 0, None}: # new_in_vlan is an integer but zero, action to set is required @@ -1129,7 +1130,7 @@ def _prepare_pop_flow( flow_mod = self._prepare_flow_mod( in_interface, out_interface, queue_id ) - flow_mod["match"]["dl_vlan"] = out_vlan + flow_mod["match"]["dl_vlan"] = str(out_vlan) new_action = {"action_type": "pop_vlan"} flow_mod["actions"].insert(0, new_action) return flow_mod diff --git a/settings.py b/settings.py index b3bd7be3..8fc177f6 100644 --- a/settings.py +++ b/settings.py @@ -41,6 +41,8 @@ # is not set in a request EVPL_SB_PRIORITY = 20000 EPL_SB_PRIORITY = 10000 +ANY_SB_PRIORITY = 15000 +UNTAGGED_SB_PRIORITY = 20000 # Time (seconds) to check if an evc has been updated # or flows have been deleted. diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index abc0f4cc..356c5acf 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -18,7 +18,9 @@ MANAGER_URL, SDN_TRACE_CP_URL, EVPL_SB_PRIORITY, - EPL_SB_PRIORITY + EPL_SB_PRIORITY, + ANY_SB_PRIORITY, + UNTAGGED_SB_PRIORITY ) # NOQA from napps.kytos.mef_eline.exceptions import FlowModException # NOQA from napps.kytos.mef_eline.tests.helpers import ( @@ -210,7 +212,8 @@ def test_prepare_pop_flow(self): ) expected_flow_mod = { - "match": {"in_port": interface_a.port_number, "dl_vlan": in_vlan}, + "match": {"in_port": interface_a.port_number, + "dl_vlan": str(in_vlan)}, "cookie": evc.get_cookie(), "actions": [ {"action_type": "pop_vlan"}, @@ -255,7 +258,7 @@ def test_prepare_push_flow(self): "priority": EVPL_SB_PRIORITY, } if in_vlan_a is not None: - expected_flow_mod['match']['dl_vlan'] = in_vlan_a + expected_flow_mod['match']['dl_vlan'] = str(in_vlan_a) else: expected_flow_mod['priority'] = EPL_SB_PRIORITY @@ -269,6 +272,7 @@ def test_prepare_push_flow(self): new_action = {"action_type": "pop_vlan"} expected_flow_mod["actions"].insert(0, new_action) elif in_vlan_a == "4096/4096": + expected_flow_mod['priority'] = ANY_SB_PRIORITY if in_vlan_z == 0: new_action = {"action_type": "pop_vlan"} expected_flow_mod["actions"].insert(0, new_action) @@ -299,7 +303,7 @@ def test_install_uni_flows(send_flow_mods_mock): { "match": { "in_port": evc.uni_a.interface.port_number, - "dl_vlan": evc.uni_a.user_tag.value, + "dl_vlan": str(evc.uni_a.user_tag.value), }, "cookie": evc.get_cookie(), "actions": [ @@ -324,9 +328,9 @@ def test_install_uni_flows(send_flow_mods_mock): { "match": { "in_port": evc.primary_links[0].endpoint_a.port_number, - "dl_vlan": evc.primary_links[0] - .get_metadata("s_vlan") - .value, + "dl_vlan": str(evc.primary_links[0] + .get_metadata("s_vlan") + .value), }, "cookie": evc.get_cookie(), "actions": [ @@ -348,7 +352,7 @@ def test_install_uni_flows(send_flow_mods_mock): { "match": { "in_port": evc.uni_z.interface.port_number, - "dl_vlan": evc.uni_z.user_tag.value, + "dl_vlan": str(evc.uni_z.user_tag.value), }, "cookie": evc.get_cookie(), "actions": [ @@ -373,9 +377,9 @@ def test_install_uni_flows(send_flow_mods_mock): { "match": { "in_port": evc.primary_links[-1].endpoint_b.port_number, - "dl_vlan": evc.primary_links[-1] - .get_metadata("s_vlan") - .value, + "dl_vlan": str(evc.primary_links[-1] + .get_metadata("s_vlan") + .value), }, "cookie": evc.get_cookie(), "actions": [ @@ -456,7 +460,7 @@ def test_install_nni_flows(send_flow_mods_mock): expected_flow_mods = [ { - "match": {"in_port": in_port, "dl_vlan": in_vlan}, + "match": {"in_port": in_port, "dl_vlan": str(in_vlan)}, "cookie": evc.get_cookie(), "actions": [ {"action_type": "set_vlan", "vlan_id": out_vlan}, @@ -465,7 +469,7 @@ def test_install_nni_flows(send_flow_mods_mock): "priority": EVPL_SB_PRIORITY }, { - "match": {"in_port": out_port, "dl_vlan": out_vlan}, + "match": {"in_port": out_port, "dl_vlan": str(out_vlan)}, "cookie": evc.get_cookie(), "actions": [ {"action_type": "set_vlan", "vlan_id": in_vlan}, @@ -1141,17 +1145,21 @@ def test_deploy_direct_uni_flows(send_flow_mods_mock): expected_dpid = evc.uni_a.interface.switch.id evc._install_direct_uni_flows() if uni_a is not None: - expected_flows[0]["match"]["dl_vlan"] = uni_a - expected_flows[0]["priority"] = EVPL_SB_PRIORITY + expected_flows[0]["match"]["dl_vlan"] = str(uni_a) if uni_z is not None: - expected_flows[1]["match"]["dl_vlan"] = uni_z - expected_flows[1]["priority"] = EVPL_SB_PRIORITY + expected_flows[1]["match"]["dl_vlan"] = str(uni_z) if uni_z not in {None, "4096/4096", 0}: + expected_flows[1]["priority"] = EVPL_SB_PRIORITY expected_flows[0]["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": uni_z} ) + elif uni_z == "4096/4096": + expected_flows[1]["priority"] = ANY_SB_PRIORITY + elif uni_z == 0: + expected_flows[1]["priority"] = UNTAGGED_SB_PRIORITY if uni_a not in {None, "4096/4096", 0}: + expected_flows[0]["priority"] = EVPL_SB_PRIORITY expected_flows[1]["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": uni_a} ) @@ -1160,13 +1168,16 @@ def test_deploy_direct_uni_flows(send_flow_mods_mock): 0, {"action_type": "push_vlan", "tag_type": "c"} ) if uni_z == 0: + expected_flows[1]["priority"] = UNTAGGED_SB_PRIORITY new_action = {"action_type": "pop_vlan"} expected_flows[0]["actions"].insert(0, new_action) elif uni_a == "4096/4096": + expected_flows[0]["priority"] = ANY_SB_PRIORITY if uni_z == 0: new_action = {"action_type": "pop_vlan"} expected_flows[0]["actions"].insert(0, new_action) elif uni_a == 0: + expected_flows[0]["priority"] = UNTAGGED_SB_PRIORITY if uni_z not in {None, "4096/4096", 0}: expected_flows[0]["actions"].insert( 0, {"action_type": "push_vlan", "tag_type": "c"} @@ -1183,91 +1194,6 @@ def test_deploy_direct_uni_flows(send_flow_mods_mock): expected_dpid, expected_flows ) - @staticmethod - @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") - def test_deploy_direct_uni_flows_untagged_any(send_flow_mods_mock): - """Test _install_direct_uni_flows with untagged and any.""" - evc = TestEVC.create_evc_intra_switch() - - evc.uni_a = get_uni_mocked(tag_value="untagged", is_valid=True) - evc.uni_z = get_uni_mocked(tag_value="untagged", is_valid=True) - expected_dpid = evc.uni_a.interface.switch.id - expected_flows = [ - { - "match": {"in_port": 1, "dl_vlan": 0}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - }, - { - "match": {"in_port": 1, "dl_vlan": 0}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows - ) - - evc.uni_a = get_uni_mocked(tag_value="any", is_valid=True) - evc.uni_z = get_uni_mocked(tag_value="any", is_valid=True) - expected_dpid = evc.uni_a.interface.switch.id - expected_flows = [ - { - "match": {"in_port": 1, "dl_vlan": "4096/4096"}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - }, - { - "match": {"in_port": 1, "dl_vlan": "4096/4096"}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows - ) - - evc.uni_a = get_uni_mocked(tag_value="any", is_valid=True) - evc.uni_z = get_uni_mocked(tag_value=100, is_valid=True) - expected_dpid = evc.uni_a.interface.switch.id - expected_flows = [ - { - "match": {"in_port": 1, "dl_vlan": "4096/4096"}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "set_vlan", "vlan_id": 100}, - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - }, - { - "match": {"in_port": 1, "dl_vlan": 100}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EVPL_SB_PRIORITY - } - ] - evc._install_direct_uni_flows() - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows - ) - def test_is_affected_by_link(self): """Test is_affected_by_link method""" self.evc_deploy.current_path = Path(['a', 'b', 'c']) @@ -1359,26 +1285,26 @@ def test_remove_path_flows(self, *args): { 'cookie': 12249790986447749121, 'cookie_mask': 18446744073709551615, - 'match': {'in_port': 9, 'dl_vlan': 5} + 'match': {'in_port': 9, 'dl_vlan': '5'} }, ] expected_flows_2 = [ { 'cookie': 12249790986447749121, 'cookie_mask': 18446744073709551615, - 'match': {'in_port': 10, 'dl_vlan': 5} + 'match': {'in_port': 10, 'dl_vlan': '5'} }, { 'cookie': 12249790986447749121, 'cookie_mask': 18446744073709551615, - 'match': {'in_port': 11, 'dl_vlan': 6} + 'match': {'in_port': 11, 'dl_vlan': '6'} }, ] expected_flows_3 = [ { 'cookie': 12249790986447749121, 'cookie_mask': 18446744073709551615, - 'match': {'in_port': 12, 'dl_vlan': 6} + 'match': {'in_port': 12, 'dl_vlan': '6'} }, ] From b5e4fbea6c67c11ecd6d59576c5e4804b610b36a Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Tue, 7 Mar 2023 12:57:17 -0500 Subject: [PATCH 30/33] Reversed string vlan, temporal --- models/evc.py | 10 ++++---- tests/unit/models/test_evc_deploy.py | 36 ++++++++++++++-------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/models/evc.py b/models/evc.py index 271fea22..5799ce20 100644 --- a/models/evc.py +++ b/models/evc.py @@ -826,10 +826,10 @@ def _prepare_direct_uni_flows(self): ) if vlan_a is not None: - flow_mod_az["match"]["dl_vlan"] = str(vlan_a) + flow_mod_az["match"]["dl_vlan"] = vlan_a if vlan_z is not None: - flow_mod_za["match"]["dl_vlan"] = str(vlan_z) + flow_mod_za["match"]["dl_vlan"] = vlan_z if vlan_z not in {None, "4096/4096", 0}: flow_mod_az["actions"].insert( @@ -1061,7 +1061,7 @@ def _prepare_nni_flow(self, *args, queue_id=None): flow_mod = self._prepare_flow_mod( in_interface, out_interface, queue_id ) - flow_mod["match"]["dl_vlan"] = str(in_vlan) + flow_mod["match"]["dl_vlan"] = in_vlan new_action = {"action_type": "set_vlan", "vlan_id": out_vlan} flow_mod["actions"].insert(0, new_action) @@ -1095,7 +1095,7 @@ def _prepare_push_flow(self, *args, queue_id=None): if in_vlan is not None: # if in_vlan is set, it must be included in the match - flow_mod["match"]["dl_vlan"] = str(in_vlan) + flow_mod["match"]["dl_vlan"] = in_vlan if new_c_vlan not in {"4096/4096", 0, None}: # new_in_vlan is an integer but zero, action to set is required @@ -1130,7 +1130,7 @@ def _prepare_pop_flow( flow_mod = self._prepare_flow_mod( in_interface, out_interface, queue_id ) - flow_mod["match"]["dl_vlan"] = str(out_vlan) + flow_mod["match"]["dl_vlan"] = out_vlan new_action = {"action_type": "pop_vlan"} flow_mod["actions"].insert(0, new_action) return flow_mod diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index 356c5acf..4c5f59b4 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -213,7 +213,7 @@ def test_prepare_pop_flow(self): expected_flow_mod = { "match": {"in_port": interface_a.port_number, - "dl_vlan": str(in_vlan)}, + "dl_vlan": in_vlan}, "cookie": evc.get_cookie(), "actions": [ {"action_type": "pop_vlan"}, @@ -258,7 +258,7 @@ def test_prepare_push_flow(self): "priority": EVPL_SB_PRIORITY, } if in_vlan_a is not None: - expected_flow_mod['match']['dl_vlan'] = str(in_vlan_a) + expected_flow_mod['match']['dl_vlan'] = in_vlan_a else: expected_flow_mod['priority'] = EPL_SB_PRIORITY @@ -303,7 +303,7 @@ def test_install_uni_flows(send_flow_mods_mock): { "match": { "in_port": evc.uni_a.interface.port_number, - "dl_vlan": str(evc.uni_a.user_tag.value), + "dl_vlan": evc.uni_a.user_tag.value, }, "cookie": evc.get_cookie(), "actions": [ @@ -328,9 +328,9 @@ def test_install_uni_flows(send_flow_mods_mock): { "match": { "in_port": evc.primary_links[0].endpoint_a.port_number, - "dl_vlan": str(evc.primary_links[0] - .get_metadata("s_vlan") - .value), + "dl_vlan": evc.primary_links[0] + .get_metadata("s_vlan") + .value, }, "cookie": evc.get_cookie(), "actions": [ @@ -352,7 +352,7 @@ def test_install_uni_flows(send_flow_mods_mock): { "match": { "in_port": evc.uni_z.interface.port_number, - "dl_vlan": str(evc.uni_z.user_tag.value), + "dl_vlan": evc.uni_z.user_tag.value, }, "cookie": evc.get_cookie(), "actions": [ @@ -377,9 +377,9 @@ def test_install_uni_flows(send_flow_mods_mock): { "match": { "in_port": evc.primary_links[-1].endpoint_b.port_number, - "dl_vlan": str(evc.primary_links[-1] - .get_metadata("s_vlan") - .value), + "dl_vlan": evc.primary_links[-1] + .get_metadata("s_vlan") + .value, }, "cookie": evc.get_cookie(), "actions": [ @@ -460,7 +460,7 @@ def test_install_nni_flows(send_flow_mods_mock): expected_flow_mods = [ { - "match": {"in_port": in_port, "dl_vlan": str(in_vlan)}, + "match": {"in_port": in_port, "dl_vlan": in_vlan}, "cookie": evc.get_cookie(), "actions": [ {"action_type": "set_vlan", "vlan_id": out_vlan}, @@ -469,7 +469,7 @@ def test_install_nni_flows(send_flow_mods_mock): "priority": EVPL_SB_PRIORITY }, { - "match": {"in_port": out_port, "dl_vlan": str(out_vlan)}, + "match": {"in_port": out_port, "dl_vlan": out_vlan}, "cookie": evc.get_cookie(), "actions": [ {"action_type": "set_vlan", "vlan_id": in_vlan}, @@ -1145,9 +1145,9 @@ def test_deploy_direct_uni_flows(send_flow_mods_mock): expected_dpid = evc.uni_a.interface.switch.id evc._install_direct_uni_flows() if uni_a is not None: - expected_flows[0]["match"]["dl_vlan"] = str(uni_a) + expected_flows[0]["match"]["dl_vlan"] = uni_a if uni_z is not None: - expected_flows[1]["match"]["dl_vlan"] = str(uni_z) + expected_flows[1]["match"]["dl_vlan"] = uni_z if uni_z not in {None, "4096/4096", 0}: expected_flows[1]["priority"] = EVPL_SB_PRIORITY @@ -1285,26 +1285,26 @@ def test_remove_path_flows(self, *args): { 'cookie': 12249790986447749121, 'cookie_mask': 18446744073709551615, - 'match': {'in_port': 9, 'dl_vlan': '5'} + 'match': {'in_port': 9, 'dl_vlan': 5} }, ] expected_flows_2 = [ { 'cookie': 12249790986447749121, 'cookie_mask': 18446744073709551615, - 'match': {'in_port': 10, 'dl_vlan': '5'} + 'match': {'in_port': 10, 'dl_vlan': 5} }, { 'cookie': 12249790986447749121, 'cookie_mask': 18446744073709551615, - 'match': {'in_port': 11, 'dl_vlan': '6'} + 'match': {'in_port': 11, 'dl_vlan': 6} }, ] expected_flows_3 = [ { 'cookie': 12249790986447749121, 'cookie_mask': 18446744073709551615, - 'match': {'in_port': 12, 'dl_vlan': '6'} + 'match': {'in_port': 12, 'dl_vlan': 6} }, ] From aa791e634db984b4bb9d58b501de297b1a3781bf Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Fri, 10 Mar 2023 10:37:20 -0500 Subject: [PATCH 31/33] Polished code -Added new function `get_priority` -Added enum to yml `Tag.value` -Added `self.subTest` to for loop -Deleted leftover string --- controllers/__init__.py | 15 +-- db/models.py | 3 +- models/evc.py | 20 ++-- openapi.yml | 3 + tests/unit/models/test_evc_deploy.py | 160 ++++++++++++++------------- 5 files changed, 107 insertions(+), 94 deletions(-) diff --git a/controllers/__init__.py b/controllers/__init__.py index 65b26143..7314cd4b 100644 --- a/controllers/__init__.py +++ b/controllers/__init__.py @@ -72,15 +72,12 @@ def get_circuit(self, circuit_id: str) -> Optional[Dict]: def upsert_evc(self, evc: Dict) -> Optional[Dict]: """Update or insert an EVC""" utc_now = datetime.utcnow() - try: - model = EVCBaseDoc( - **{ - **evc, - **{"_id": evc["id"]} - } - ) - except ValidationError as err: - raise err + model = EVCBaseDoc( + **{ + **evc, + **{"_id": evc["id"]} + } + ) updated = self.db.evcs.find_one_and_update( {"_id": evc["id"]}, diff --git a/db/models.py b/db/models.py index 56f6d553..f468ec98 100644 --- a/db/models.py +++ b/db/models.py @@ -47,8 +47,7 @@ def validate_value(cls, value): return value if isinstance(value, str) and value in ("any", "untagged"): return value - raise ValueError("value as string allows 'any', 'untagged' and the ", - "format 'n/n'") + raise ValueError("value as string allows 'any' and 'untagged'") class UNIDoc(BaseModel): diff --git a/models/evc.py b/models/evc.py index 5799ce20..fb34e46a 100644 --- a/models/evc.py +++ b/models/evc.py @@ -1026,6 +1026,17 @@ def get_id_from_cookie(cookie): evc_id = cookie - (settings.COOKIE_PREFIX << 56) return f"{evc_id:x}".zfill(14) + @staticmethod + def get_priority(vlan): + """Return priority value depending on vlan value""" + if vlan not in {"4096/4096", None, 0}: + return settings.EVPL_SB_PRIORITY + if vlan == 0: + return settings.UNTAGGED_SB_PRIORITY + if vlan == "4096/4096": + return settings.ANY_SB_PRIORITY + return settings.EPL_SB_PRIORITY + def _prepare_flow_mod(self, in_interface, out_interface, queue_id=None, vlan=True): """Prepare a common flow mod.""" @@ -1045,14 +1056,7 @@ def _prepare_flow_mod(self, in_interface, out_interface, if self.sb_priority: flow_mod["priority"] = self.sb_priority else: - if vlan not in {"4096/4096", None}: - # EVPL, untagged and the rest - flow_mod["priority"] = settings.EVPL_SB_PRIORITY - elif vlan == "4096/4096": - # Any - flow_mod["priority"] = settings.ANY_SB_PRIORITY - else: - flow_mod["priority"] = settings.EPL_SB_PRIORITY + flow_mod["priority"] = self.get_priority(vlan) return flow_mod def _prepare_nni_flow(self, *args, queue_id=None): diff --git a/openapi.yml b/openapi.yml index 6568ccc0..962fa78b 100644 --- a/openapi.yml +++ b/openapi.yml @@ -607,6 +607,9 @@ components: - type: integer format: int32 - type: string + enum: + - any + - untagged CircuitSchedule: # Can be referenced via '#/components/schemas/CircuitSchedule' type: object diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index 4c5f59b4..3d852210 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -257,10 +257,9 @@ def test_prepare_push_flow(self): ], "priority": EVPL_SB_PRIORITY, } + expected_flow_mod["priority"] = evc.get_priority(in_vlan_a) if in_vlan_a is not None: expected_flow_mod['match']['dl_vlan'] = in_vlan_a - else: - expected_flow_mod['priority'] = EPL_SB_PRIORITY if in_vlan_z not in {"4096/4096", 0, None}: new_action = {"action_type": "set_vlan", @@ -272,7 +271,6 @@ def test_prepare_push_flow(self): new_action = {"action_type": "pop_vlan"} expected_flow_mod["actions"].insert(0, new_action) elif in_vlan_a == "4096/4096": - expected_flow_mod['priority'] = ANY_SB_PRIORITY if in_vlan_z == 0: new_action = {"action_type": "pop_vlan"} expected_flow_mod["actions"].insert(0, new_action) @@ -1111,88 +1109,86 @@ def create_evc_intra_switch(): return EVC(**attributes) # pylint: disable=too-many-branches - @staticmethod @patch("napps.kytos.mef_eline.models.evc.EVC._send_flow_mods") - def test_deploy_direct_uni_flows(send_flow_mods_mock): + def test_deploy_direct_uni_flows(self, send_flow_mods_mock): """Test _install_direct_uni_flows""" evc = TestEVC.create_evc_intra_switch() expected_dpid = evc.uni_a.interface.switch.id for uni_a in [100, "4096/4096", 0, None]: for uni_z in [100, "4096/4096", 0, None]: - expected_flows = [ - { - "match": {"in_port": 1}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 3}, - ], - "priority": EPL_SB_PRIORITY - }, - { - "match": {"in_port": 3}, - "cookie": evc.get_cookie(), - "actions": [ - {"action_type": "output", "port": 1}, - ], - "priority": EPL_SB_PRIORITY - } - ] - evc.uni_a = get_uni_mocked(tag_value=uni_a, is_valid=True) - evc.uni_a.interface.port_number = 1 - evc.uni_z = get_uni_mocked(tag_value=uni_z, is_valid=True) - evc.uni_z.interface.port_number = 3 - expected_dpid = evc.uni_a.interface.switch.id - evc._install_direct_uni_flows() - if uni_a is not None: - expected_flows[0]["match"]["dl_vlan"] = uni_a - if uni_z is not None: - expected_flows[1]["match"]["dl_vlan"] = uni_z - - if uni_z not in {None, "4096/4096", 0}: - expected_flows[1]["priority"] = EVPL_SB_PRIORITY - expected_flows[0]["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": uni_z} - ) - elif uni_z == "4096/4096": - expected_flows[1]["priority"] = ANY_SB_PRIORITY - elif uni_z == 0: - expected_flows[1]["priority"] = UNTAGGED_SB_PRIORITY - if uni_a not in {None, "4096/4096", 0}: - expected_flows[0]["priority"] = EVPL_SB_PRIORITY - expected_flows[1]["actions"].insert( - 0, {"action_type": "set_vlan", "vlan_id": uni_a} - ) - if not uni_z: - expected_flows[1]["actions"].insert( - 0, {"action_type": "push_vlan", "tag_type": "c"} - ) - if uni_z == 0: - expected_flows[1]["priority"] = UNTAGGED_SB_PRIORITY - new_action = {"action_type": "pop_vlan"} - expected_flows[0]["actions"].insert(0, new_action) - elif uni_a == "4096/4096": - expected_flows[0]["priority"] = ANY_SB_PRIORITY - if uni_z == 0: - new_action = {"action_type": "pop_vlan"} - expected_flows[0]["actions"].insert(0, new_action) - elif uni_a == 0: - expected_flows[0]["priority"] = UNTAGGED_SB_PRIORITY - if uni_z not in {None, "4096/4096", 0}: - expected_flows[0]["actions"].insert( - 0, {"action_type": "push_vlan", "tag_type": "c"} - ) - if uni_z: - new_action = {"action_type": "pop_vlan"} - expected_flows[1]["actions"].insert(0, new_action) - elif uni_a is None: + with self.subTest(uni_a=uni_a, uni_z=uni_z): + expected_flows = [ + { + "match": {"in_port": 1}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 3}, + ], + "priority": EPL_SB_PRIORITY + }, + { + "match": {"in_port": 3}, + "cookie": evc.get_cookie(), + "actions": [ + {"action_type": "output", "port": 1}, + ], + "priority": EPL_SB_PRIORITY + } + ] + evc.uni_a = get_uni_mocked(tag_value=uni_a, is_valid=True) + evc.uni_a.interface.port_number = 1 + evc.uni_z = get_uni_mocked(tag_value=uni_z, is_valid=True) + evc.uni_z.interface.port_number = 3 + expected_dpid = evc.uni_a.interface.switch.id + evc._install_direct_uni_flows() + if uni_a is not None: + expected_flows[0]["match"]["dl_vlan"] = uni_a + if uni_z is not None: + expected_flows[1]["match"]["dl_vlan"] = uni_z + expected_flows[0]["priority"] = EVC.get_priority(uni_a) + expected_flows[1]["priority"] = EVC.get_priority(uni_z) + if uni_z not in {None, "4096/4096", 0}: expected_flows[0]["actions"].insert( - 0, {"action_type": "push_vlan", "tag_type": "c"} + 0, {"action_type": "set_vlan", "vlan_id": uni_z} ) - send_flow_mods_mock.assert_called_with( - expected_dpid, expected_flows - ) + + if uni_a not in {None, "4096/4096", 0}: + expected_flows[1]["actions"].insert( + 0, {"action_type": "set_vlan", + "vlan_id": uni_a} + ) + if not uni_z: + expected_flows[1]["actions"].insert( + 0, {"action_type": "push_vlan", + "tag_type": "c"} + ) + if uni_z == 0: + new_action = {"action_type": "pop_vlan"} + expected_flows[0]["actions"].insert(0, new_action) + elif uni_a == "4096/4096": + if uni_z == 0: + new_action = {"action_type": "pop_vlan"} + expected_flows[0]["actions"].insert(0, new_action) + elif uni_a == 0: + if uni_z not in {None, "4096/4096", 0}: + expected_flows[0]["actions"].insert( + 0, {"action_type": "push_vlan", + "tag_type": "c"} + ) + if uni_z: + new_action = {"action_type": "pop_vlan"} + expected_flows[1]["actions"].insert(0, new_action) + elif uni_a is None: + if uni_z not in {None, "4096/4096", 0}: + expected_flows[0]["actions"].insert( + 0, {"action_type": "push_vlan", + "tag_type": "c"} + ) + send_flow_mods_mock.assert_called_with( + expected_dpid, expected_flows + ) def test_is_affected_by_link(self): """Test is_affected_by_link method""" @@ -1516,3 +1512,17 @@ def test_get_value_from_uni_tag(self): uni = get_uni_mocked(tag_value=100) value = EVC._get_value_from_uni_tag(uni) self.assertEqual(value, 100) + + def test_get_priority(self): + """Test get_priority_from_vlan""" + evpl_value = EVC.get_priority(100) + self.assertEqual(evpl_value, EVPL_SB_PRIORITY) + + untagged_value = EVC.get_priority(0) + self.assertEqual(untagged_value, UNTAGGED_SB_PRIORITY) + + any_value = EVC.get_priority("4096/4096") + self.assertEqual(any_value, ANY_SB_PRIORITY) + + epl_value = EVC.get_priority(None) + self.assertEqual(epl_value, EPL_SB_PRIORITY) From 128f80c18531bc62aee2156945d7eb51ec1e9047 Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Fri, 10 Mar 2023 14:39:31 -0500 Subject: [PATCH 32/33] Added new dictionary to EVC, `special_cases` --- CHANGELOG.rst | 1 + controllers/__init__.py | 2 -- models/evc.py | 7 +++++-- tests/unit/models/test_evc_deploy.py | 8 ++++---- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index aba8e1d6..78429e19 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -11,6 +11,7 @@ Added - Added more content keys ``evc_id, name, metadata, active, enabled, uni_a, uni_z`` to events from ``mef_eline`` - Added ``uni_a`` and ``uni_z`` to ``attributes_requiring_redeploy`` - Added ``metadata`` to EVC schema +- Allow the creation of ``any`` and ``untagged`` EVC. Changed ======= diff --git a/controllers/__init__.py b/controllers/__init__.py index 7314cd4b..e2fd3115 100644 --- a/controllers/__init__.py +++ b/controllers/__init__.py @@ -5,7 +5,6 @@ from typing import Dict, Optional import pymongo -from pydantic import ValidationError from pymongo.collection import ReturnDocument from pymongo.errors import AutoReconnect from tenacity import retry_if_exception_type, stop_after_attempt, wait_random @@ -78,7 +77,6 @@ def upsert_evc(self, evc: Dict) -> Optional[Dict]: **{"_id": evc["id"]} } ) - updated = self.db.evcs.find_one_and_update( {"_id": evc["id"]}, { diff --git a/models/evc.py b/models/evc.py index fb34e46a..6610e2c3 100644 --- a/models/evc.py +++ b/models/evc.py @@ -155,6 +155,9 @@ def __init__(self, controller, **kwargs): # dict with the user original request (input) self._requested = kwargs + # Special cases: No tag, any, untagged + self.special_cases = {None, "4096/4096", 0} + def sync(self): """Sync this EVC in the MongoDB.""" self.updated_at = now() @@ -831,7 +834,7 @@ def _prepare_direct_uni_flows(self): if vlan_z is not None: flow_mod_za["match"]["dl_vlan"] = vlan_z - if vlan_z not in {None, "4096/4096", 0}: + if vlan_z not in self.special_cases: flow_mod_az["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_z} ) @@ -840,7 +843,7 @@ def _prepare_direct_uni_flows(self): 0, {"action_type": "push_vlan", "tag_type": "c"} ) - if vlan_a not in {None, "4096/4096", 0}: + if vlan_a not in self.special_cases: flow_mod_za["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": vlan_a} ) diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index 3d852210..f1a213ea 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -1149,12 +1149,12 @@ def test_deploy_direct_uni_flows(self, send_flow_mods_mock): expected_flows[0]["priority"] = EVC.get_priority(uni_a) expected_flows[1]["priority"] = EVC.get_priority(uni_z) - if uni_z not in {None, "4096/4096", 0}: + if uni_z not in evc.special_cases: expected_flows[0]["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": uni_z} ) - if uni_a not in {None, "4096/4096", 0}: + if uni_a not in evc.special_cases: expected_flows[1]["actions"].insert( 0, {"action_type": "set_vlan", "vlan_id": uni_a} @@ -1172,7 +1172,7 @@ def test_deploy_direct_uni_flows(self, send_flow_mods_mock): new_action = {"action_type": "pop_vlan"} expected_flows[0]["actions"].insert(0, new_action) elif uni_a == 0: - if uni_z not in {None, "4096/4096", 0}: + if uni_z not in evc.special_cases: expected_flows[0]["actions"].insert( 0, {"action_type": "push_vlan", "tag_type": "c"} @@ -1181,7 +1181,7 @@ def test_deploy_direct_uni_flows(self, send_flow_mods_mock): new_action = {"action_type": "pop_vlan"} expected_flows[1]["actions"].insert(0, new_action) elif uni_a is None: - if uni_z not in {None, "4096/4096", 0}: + if uni_z not in evc.special_cases: expected_flows[0]["actions"].insert( 0, {"action_type": "push_vlan", "tag_type": "c"} From 5c98c93c79bffd79edb81fb824d46250e4db683e Mon Sep 17 00:00:00 2001 From: Aldo Ortega Date: Mon, 13 Mar 2023 10:45:27 -0400 Subject: [PATCH 33/33] Added exceptio handling in update() - Replace leftover dictionaries with `special_cases` --- main.py | 2 ++ models/evc.py | 8 ++++---- tests/unit/models/test_evc_deploy.py | 6 +++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/main.py b/main.py index 8c89be12..917b2d2c 100644 --- a/main.py +++ b/main.py @@ -302,6 +302,8 @@ def update(self, circuit_id): enable, redeploy = evc.update( **self._evc_dict_with_instances(data) ) + except ValidationError as exception: + raise BadRequest(str(exception)) from exception except ValueError as exception: log.error(exception) log.debug("update result %s %s", exception, 400) diff --git a/models/evc.py b/models/evc.py index 6610e2c3..f7ddcd18 100644 --- a/models/evc.py +++ b/models/evc.py @@ -1032,7 +1032,7 @@ def get_id_from_cookie(cookie): @staticmethod def get_priority(vlan): """Return priority value depending on vlan value""" - if vlan not in {"4096/4096", None, 0}: + if vlan not in {None, "4096/4096", 0}: return settings.EVPL_SB_PRIORITY if vlan == 0: return settings.UNTAGGED_SB_PRIORITY @@ -1104,12 +1104,12 @@ def _prepare_push_flow(self, *args, queue_id=None): # if in_vlan is set, it must be included in the match flow_mod["match"]["dl_vlan"] = in_vlan - if new_c_vlan not in {"4096/4096", 0, None}: + if new_c_vlan not in self.special_cases: # new_in_vlan is an integer but zero, action to set is required new_action = {"action_type": "set_vlan", "vlan_id": new_c_vlan} flow_mod["actions"].insert(0, new_action) - if in_vlan not in {"4096/4096", 0, None} and new_c_vlan == 0: + if in_vlan not in self.special_cases and new_c_vlan == 0: # # new_in_vlan is an integer but zero and new_c_vlan does not # a pop action is required new_action = {"action_type": "pop_vlan"} @@ -1121,7 +1121,7 @@ def _prepare_push_flow(self, *args, queue_id=None): new_action = {"action_type": "pop_vlan"} flow_mod["actions"].insert(0, new_action) - elif not in_vlan and new_c_vlan not in {"4096/4096", 0, None}: + elif not in_vlan and new_c_vlan not in self.special_cases: # new_in_vlan is an integer but zero and in_vlan is not set # then it is set now new_action = {"action_type": "push_vlan", "tag_type": "c"} diff --git a/tests/unit/models/test_evc_deploy.py b/tests/unit/models/test_evc_deploy.py index f1a213ea..7f0370a2 100644 --- a/tests/unit/models/test_evc_deploy.py +++ b/tests/unit/models/test_evc_deploy.py @@ -261,12 +261,12 @@ def test_prepare_push_flow(self): if in_vlan_a is not None: expected_flow_mod['match']['dl_vlan'] = in_vlan_a - if in_vlan_z not in {"4096/4096", 0, None}: + if in_vlan_z not in evc.special_cases: new_action = {"action_type": "set_vlan", "vlan_id": in_vlan_z} expected_flow_mod["actions"].insert(0, new_action) - if in_vlan_a not in {"4096/4096", 0, None}: + if in_vlan_a not in evc.special_cases: if in_vlan_z == 0: new_action = {"action_type": "pop_vlan"} expected_flow_mod["actions"].insert(0, new_action) @@ -275,7 +275,7 @@ def test_prepare_push_flow(self): new_action = {"action_type": "pop_vlan"} expected_flow_mod["actions"].insert(0, new_action) elif not in_vlan_a: - if in_vlan_z not in {"4096/4096", 0, None}: + if in_vlan_z not in evc.special_cases: new_action = {"action_type": "push_vlan", "tag_type": "c"} expected_flow_mod["actions"].insert(0, new_action)