From fa685aff117626ac7ba37fe916a1da49ef4751b8 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Mon, 14 Apr 2025 09:12:48 +0100 Subject: [PATCH 1/4] Remove VlanManager - we are no longer restricting vlan numbers This was added to keep vlan numbers globally unique to experiment with OVN integration but we don't need it any more. --- .../neutron_understack_mech.py | 7 +- .../neutron_understack/vlan_manager.py | 112 ------------------ 2 files changed, 2 insertions(+), 117 deletions(-) delete mode 100644 python/neutron-understack/neutron_understack/vlan_manager.py diff --git a/python/neutron-understack/neutron_understack/neutron_understack_mech.py b/python/neutron-understack/neutron_understack/neutron_understack_mech.py index 2f2ad4c43..ebabecf12 100644 --- a/python/neutron-understack/neutron_understack/neutron_understack_mech.py +++ b/python/neutron-understack/neutron_understack/neutron_understack_mech.py @@ -19,7 +19,6 @@ from neutron_understack.nautobot import VlanPayload from neutron_understack.trunk import UnderStackTrunkDriver from neutron_understack.undersync import Undersync -from neutron_understack.vlan_manager import VlanManager LOG = logging.getLogger(__name__) @@ -40,7 +39,6 @@ def initialize(self): self.nb = Nautobot(conf.nb_url, conf.nb_token) self.undersync = Undersync(conf.undersync_token, conf.undersync_url) self.trunk_driver = UnderStackTrunkDriver.create(self) - self.vlan_manager = VlanManager(self.nb, conf) self.subscribe() def subscribe(self): @@ -57,9 +55,8 @@ def subscribe(self): cancellable=True, ) - def create_network_precommit(self, context: NetworkContext): - if cfg.CONF.ml2_understack.enforce_unique_vlans_in_fabric: - self.vlan_manager.create_vlan_for_network(context) + def create_network_precommit(self, context): + pass def create_network_postcommit(self, context): network = context.current diff --git a/python/neutron-understack/neutron_understack/vlan_manager.py b/python/neutron-understack/neutron_understack/vlan_manager.py deleted file mode 100644 index aa4deb338..000000000 --- a/python/neutron-understack/neutron_understack/vlan_manager.py +++ /dev/null @@ -1,112 +0,0 @@ -import logging - -from neutron.db.models.plugins.ml2.vlanallocation import VlanAllocation -from neutron.db.models.segment import NetworkSegment -from neutron_lib import constants as p_const -from neutron_lib import context as neutron_context -from neutron_lib.plugins.ml2.api import NetworkContext -from sqlalchemy import update - -LOG = logging.getLogger(__name__) -MAX_VLAN_ATTEMPTS = 4096 - - -class VlanManager: - def __init__(self, nb, conf): - self.nb = nb - self.conf = conf - - def create_vlan_for_network(self, context: NetworkContext): - """Ensures that Vlan ID for a newly created network is available. - - This method checks if the VLAN is available across the whole fabric, - and in case where it isn't, it will attempt to allocate new one - and repeat the checks until successful or we run out of vlans.. - """ - if not context.current: - raise RuntimeError("no current context provided.") - - vlan_tag = int(context.current["provider:segmentation_id"]) - allocated = self._allocate_vlan(context, vlan_tag) - - if allocated: - self._update_segmentation_id(context, vlan_tag) - self._new_vlan_mark_allocated(context, allocated) - - def _allocate_vlan( - self, context: NetworkContext, vlan_tag: int - ) -> VlanAllocation | None: - """Attempts to allocate a VLAN ID, trying multiple times if necessary. - - Returns: - allocation when new segment was assigned - None when the original segment was free - """ - alloc = None - attempts = 0 - while attempts < MAX_VLAN_ATTEMPTS: - if self._is_vlan_available( - self.conf.network_node_switchport_uuid, - vlan_tag, - ): - LOG.debug("Vlan %s is available for all VLANGroups.", vlan_tag) - return alloc - - LOG.info( - "Vlan %s is reported to be used in fabric associated with " - "this VlanGroup. Trying next one...", - vlan_tag, - ) - alloc = self._find_next_available_vlan(context) - vlan_tag = alloc.vlan_id - attempts += 1 - raise RuntimeError("No available VLANs found after multiple attempts.") - - def _is_vlan_available(self, interface_id: str, vlan_tag: int) -> bool: - """Checks if VLAN ID is available in Nautobot.""" - return self.nb.check_vlan_availability( - interface_id=interface_id, vlan_tag=vlan_tag - ) - - def _find_next_available_vlan(self, context): - """Figures out what the next available VLAN ID for a given network is.""" - if len(context.network_segments) != 1: - raise ValueError("Multi-segment networks are not supported.") - - vlan_type_driver = context._plugin.type_manager.drivers.get("vlan", {}).obj - if vlan_type_driver is None: - raise RuntimeError("no VlanTypeDriver available.") - - admin_context = neutron_context.get_admin_context() - physical_network = context.current["provider:physical_network"] - segment_record = vlan_type_driver.allocate_partially_specified_segment( - context=admin_context, physical_network=physical_network - ) - return segment_record - - def _update_segmentation_id(self, context, vlan_tag): - """Updates segmentation ID for the provider and all network segments.""" - context.current["provider:segmentation_id"] = vlan_tag - - # Update all segments' segmentation_id - session = context.plugin_context.session - results = [] - for segment in context.network_segments: - if segment["network_type"] == p_const.TYPE_VLAN: - results.append(self._update_id_on_segment(session, segment, vlan_tag)) - return results - - def _update_id_on_segment(self, session, segment, vlan_tag): - """Updates segmentation_id on a single network segment.""" - update_segment_statement = ( - update(NetworkSegment) - .where(NetworkSegment.id == segment["id"]) - .values(segmentation_id=vlan_tag) - ) - - return session.execute(update_segment_statement) - - def _new_vlan_mark_allocated(self, context, alloc): - session = context.plugin_context.session - alloc.allocated = True - return alloc.save(session) From 58ec5762cd3005aa0cb0980230d63cf9ee40f70e Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Mon, 14 Apr 2025 09:20:55 +0100 Subject: [PATCH 2/4] Add error checking (just to keep the declared type honest) --- python/neutron-understack/neutron_understack/utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/python/neutron-understack/neutron_understack/utils.py b/python/neutron-understack/neutron_understack/utils.py index 6d8eb020d..8d6532fa4 100644 --- a/python/neutron-understack/neutron_understack/utils.py +++ b/python/neutron-understack/neutron_understack/utils.py @@ -10,7 +10,10 @@ def fetch_port_object(port_id: str) -> port_obj.Port: context = n_context.get_admin_context() - return port_obj.Port.get_object(context, id=port_id) + port = port_obj.Port.get_object(context, id=port_id) + if port is None: + raise ValueError(f"Failed to fetch Port with ID {port_id}") + return port def fetch_connected_interface_uuid( From acbf30db0e37252739adb63e819eaecb6ccb0f81 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Mon, 14 Apr 2025 09:22:55 +0100 Subject: [PATCH 3/4] Suppress type warning for mismatched base class definition --- .../neutron_understack/neutron_understack_mech.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/neutron-understack/neutron_understack/neutron_understack_mech.py b/python/neutron-understack/neutron_understack/neutron_understack_mech.py index ebabecf12..b374064bc 100644 --- a/python/neutron-understack/neutron_understack/neutron_understack_mech.py +++ b/python/neutron-understack/neutron_understack/neutron_understack_mech.py @@ -31,7 +31,7 @@ class UnderstackDriver(MechanismDriver): resource_provider_uuid5_namespace = UUID("6eae3046-4072-11ef-9bcf-d6be6370a162") @property - def connectivity(self): + def connectivity(self): # type: ignore return portbindings.CONNECTIVITY_L2 def initialize(self): From 3cc4a53d3d918562eac6bef0965b0cd3af28a499 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Mon, 14 Apr 2025 09:29:59 +0100 Subject: [PATCH 4/4] Assign Dynamic Network Segments in each cabinet where a Network is bound The old scheme used a Nautobot plugin to assign a VLAN number outside of Openstack, and this is what was configured on the switch. This caused issues because Neutron had no knowledge of these VLANs, which make it difficult to have Neutron drive any layer 3 services or even trunk ports. Now we have Neutron allocate "Dynamic" network segments instead, and these are pushed to Nautobot, to keep Nautobot's VLANs in sync with Neutron's Network Segments. To facilitate we are using the "physical network" of each Ironic baremetal port to identify the vlan group where that switch port lives. For every vlan group we have created a Network Segment Range with the same name. Dynamic network segments of type "VLAN" are assigned from those ranges. We implement some new API calls to replace the "prep switch interface" and "detach port" Nautobot jobs which were previously making interface updates in addition to assigning VLAN numbers. Specifically, we now update the vlans on the switch port and we toggle it's "provisioning" state. Undersync is invoked with a VLAN Group parameter. We now identify the VLAN Group by name instead of UUID. The name is directly discoverable from Neutron whereas the UUID would have to be looked up in Nautobot. (Undersync was already updated to accept either type of parameter.) In Neutron we now exclusively use type "VXLAN" and so all support for type "VLAN" Networks has been removed. This also includes the "provisioning" network which has been recreated as type VXLAN in our environment. There is an opportunity to remove our special "vlan 4010" case handling of the provisioning network and have this created by Neutron using the same mechanisms as for as any other tenant network, but that has not happened yet. Trunk ports have first-class support in Neutron but they are handled by a separate model, disjoint from "normal" ports, and using trunk ports exercises totally different ml2 driver callbacks. Trunk port support is coming in a separate future PR. --- .../neutron_understack/nautobot.py | 108 ++++-- .../neutron_understack_mech.py | 336 ++++++++++-------- .../neutron_understack/tests/conftest.py | 6 +- .../neutron_understack/tests/helpers.py | 12 +- .../tests/test_neutron_understack_mech.py | 134 +++---- .../neutron_understack/tests/test_trunk.py | 241 ------------- .../neutron_understack/undersync.py | 27 +- .../vlan_group_name_convention.py | 25 ++ 8 files changed, 364 insertions(+), 525 deletions(-) delete mode 100644 python/neutron-understack/neutron_understack/tests/test_trunk.py create mode 100644 python/neutron-understack/neutron_understack/vlan_group_name_convention.py diff --git a/python/neutron-understack/neutron_understack/nautobot.py b/python/neutron-understack/neutron_understack/nautobot.py index fffd69eda..a9815a124 100644 --- a/python/neutron-understack/neutron_understack/nautobot.py +++ b/python/neutron-understack/neutron_understack/nautobot.py @@ -212,49 +212,88 @@ def add_tenant_vlan_tag_to_ucvni(self, network_uuid: str, vlan_tag: int) -> dict def subnet_delete(self, uuid: str) -> dict: return self.make_api_request("DELETE", f"/api/ipam/prefixes/{uuid}/") - def prep_switch_interface( + def configure_port_status(self, interface_uuid: str, status: str) -> dict: + url = f"/api/dcim/interfaces/{interface_uuid}/" + payload = {"status": {"name": status}} + return self.make_api_request("PATCH", url, payload) + + def set_port_vlan_associations( self, - connected_interface_id: str, - ucvni_uuid: str, - vlan_tag: int | None, - modify_native_vlan: bool | None = True, + interface_uuid: str, + native_vlan_id: int | None, + allowed_vlans_ids: set[int], + vlan_group_name: str, ) -> dict: - """Runs a Nautobot Job to update a switch interface for tenant mode. - - The nautobot job will assign vlans as required and set the interface - into the correct mode for "normal" tenant operation. + """Set the tagged and untagged vlan(s) on an interface.""" + url = f"/api/dcim/interfaces/{interface_uuid}/" - The dictionary with vlan group ID and vlan tag is returned. - """ - url = "/api/plugins/undercloud-vni/prep_switch_interface" - payload = { - "ucvni_id": str(ucvni_uuid), - "connected_interface_id": str(connected_interface_id), - "modify_native_vlan": modify_native_vlan, - "vlan_tag": vlan_tag, + payload: dict = { + "tagged_vlans": [ + _vlan_payload(vlan_group_name, vlan_id) for vlan_id in allowed_vlans_ids + ], } - return self.make_api_request("POST", url, payload) - def detach_port(self, connected_interface_id: str, ucvni_uuid: str) -> str: - """Runs a Nautobot Job to cleanup a switch interface. + if native_vlan_id is not None: + payload["untagged_vlan"] = _vlan_payload(vlan_group_name, native_vlan_id) - The nautobot job will find a VLAN that is bound to the UCVNI, remove it - from the Interface and if the VLAN is unused it will delete it. + return self.make_api_request("PATCH", url, payload) + + def add_port_vlan_associations( + self, + interface_uuid: str, + allowed_vlans_ids: set[int], + vlan_group_name: str, + ) -> dict: + """Adds the specified vlan(s) to interface untagged/tagged vlans.""" + url = f"/api/dcim/interfaces/{interface_uuid}/" + + current_state = self.make_api_request("GET", f"{url}?depth=1") + + current_tagged_vlans = { + tagged_vlan["vid"] for tagged_vlan in current_state.get("tagged_vlans", []) + } + + tagged_vlans = current_tagged_vlans.union(allowed_vlans_ids) - The vlan group ID is returned. - """ - url = "/api/plugins/undercloud-vni/detach_port" payload = { - "ucvni_uuid": str(ucvni_uuid), - "connected_interface_id": str(connected_interface_id), + "tagged_vlans": [ + _vlan_payload(vlan_group_name, vlan_id) for vlan_id in tagged_vlans + ], } - resp_data = self.make_api_request("POST", url, payload) + return self.make_api_request("PATCH", url, payload) - return resp_data["vlan_group_id"] + def remove_port_network_associations( + self, interface_uuid: str, network_ids_to_remove: set[str] + ): + query = """ + query($interface_id: ID!){ + interface(id: $interface_id){ + name + untagged_vlan {id network: rel_ucvni_vlans { id }} + tagged_vlans {id network: rel_ucvni_vlans { id }} + } + } + """ + variables = {"interface_id": interface_uuid} + current = self.api.graphql.query(query, variables).json["data"]["interface"] + LOG.debug("Nautobot %s query result: %s", variables, current) - def configure_port_status(self, interface_uuid: str, status: str) -> dict: url = f"/api/dcim/interfaces/{interface_uuid}/" - payload = {"status": {"name": status}} + payload = {} + + current_untagged_network = current["untagged_vlan"]["network"]["id"] + if current_untagged_network in network_ids_to_remove: + payload["untagged_vlan"] = None + + payload["tagged_vlans"] = [ + tagged_vlan["id"] + for tagged_vlan in current["tagged_vlans"] + if ( + tagged_vlan.get("network") + and tagged_vlan.get("network")["id"] not in network_ids_to_remove + ) + ] + return self.make_api_request("PATCH", url, payload) def fetch_vlan_group_uuid(self, device_uuid: str) -> str: @@ -303,3 +342,10 @@ def create_vlan_and_associate_vlan_to_ucvni(self, vlan: VlanPayload): ) from error else: return result + + +def _vlan_payload(vlan_group_name: str, vlan_id: int) -> dict: + return { + "vlan_group": {"name": vlan_group_name}, + "vid": vlan_id, + } diff --git a/python/neutron-understack/neutron_understack/neutron_understack_mech.py b/python/neutron-understack/neutron_understack/neutron_understack_mech.py index b374064bc..3b16c97c6 100644 --- a/python/neutron-understack/neutron_understack/neutron_understack_mech.py +++ b/python/neutron-understack/neutron_understack/neutron_understack_mech.py @@ -10,11 +10,11 @@ from neutron_lib.callbacks import resources from neutron_lib.plugins.ml2 import api from neutron_lib.plugins.ml2.api import MechanismDriver -from neutron_lib.plugins.ml2.api import NetworkContext from oslo_config import cfg from neutron_understack import config from neutron_understack import utils +from neutron_understack import vlan_group_name_convention from neutron_understack.nautobot import Nautobot from neutron_understack.nautobot import VlanPayload from neutron_understack.trunk import UnderStackTrunkDriver @@ -71,6 +71,7 @@ def create_network_postcommit(self, context): if provider_type not in [p_const.TYPE_VLAN, p_const.TYPE_VXLAN]: return + ucvni_group = conf.ucvni_group ucvni_response = self.nb.ucvni_create( network_id=network_id, @@ -115,6 +116,7 @@ def delete_network_postcommit(self, context): return self.nb.ucvni_delete(network_id) + LOG.info( "network %(net_id)s has been deleted from ucvni_group %(ucvni_group)s, " "physnet %(physnet)s", @@ -221,186 +223,206 @@ def create_port_postcommit(self, context): def update_port_precommit(self, context): pass - def _fetch_subports_network_ids(self, trunk_details: dict) -> list: + def _fetch_subports_network_ids(self, trunk_details: dict | None) -> list: + if trunk_details is None: + return [] + network_uuids = [ utils.fetch_subport_network_id(subport.get("port_id")) for subport in trunk_details.get("sub_ports", []) ] return network_uuids - def _configure_trunk( - self, trunk_details: dict, connected_interface_uuid: str - ) -> None: - network_uuids = self._fetch_subports_network_ids(trunk_details) - for network_uuid in network_uuids: - self.nb.prep_switch_interface( - connected_interface_id=connected_interface_uuid, - ucvni_uuid=network_uuid, - modify_native_vlan=False, - vlan_tag=None, - ) - def update_port_postcommit(self, context): - self._delete_tenant_port_on_unbound(context) + """Tenant network port cleanup in the UnderCloud infrastructure. - def delete_port_precommit(self, context): - pass + This is triggered in the update_port_postcommit call as in the + delete_port_postcommit call there is no binding profile information + anymore, hence there is no way for us to identify which baremetal port + needs cleanup. - def delete_port_postcommit(self, context): - provisioning_network = ( - cfg.CONF.ml2_understack.provisioning_network - or cfg.CONF.ml2_type_understack.provisioning_network - ) + Only in the update_port_postcommit do we have access to the original + context, from which we can access the binding information. - network_id = context.current["network_id"] - if network_id == provisioning_network: - connected_interface_uuid = utils.fetch_connected_interface_uuid( - context.current["binding:profile"], LOG - ) - port_status = "Active" - configure_port_status_data = self.nb.configure_port_status( - connected_interface_uuid, port_status - ) - switch_uuid = configure_port_status_data.get("device", {}).get("id") - nb_vlan_group_id = UUID(self.nb.fetch_vlan_group_uuid(switch_uuid)) - self.undersync.sync_devices( - vlan_group_uuids=str(nb_vlan_group_id), - dry_run=cfg.CONF.ml2_understack.undersync_dry_run, - ) + # TODO: garbage collection of unused VLAN-type network segments. We + # create these dynamic segments on the fly so they might get left behind + # as the ports disappear. If a VLAN is left in a cabinet with nobody + # using it, it can be deleted. + """ + vlan_group_name = self._vlan_group_name(context) - def _configure_switchport_on_bind(self, context: PortContext) -> None: - trunk_details = context.current.get("trunk_details", {}) + baremetal_vnic = context.current["binding:vnic_type"] == "baremetal" + current_vif_unbound = context.vif_type == portbindings.VIF_TYPE_UNBOUND + original_vif_other = context.original_vif_type == portbindings.VIF_TYPE_OTHER + current_vif_other = context.vif_type == portbindings.VIF_TYPE_OTHER + + if baremetal_vnic and current_vif_unbound and original_vif_other: + self._tenant_network_port_cleanup(context) + if vlan_group_name: + self.invoke_undersync(vlan_group_name) + elif current_vif_other and vlan_group_name: + self.invoke_undersync(vlan_group_name) + + def _tenant_network_port_cleanup(self, context: PortContext): network_id = context.current["network_id"] + trunk_details = context.current.get("trunk_details", {}) connected_interface_uuid = utils.fetch_connected_interface_uuid( - context.current["binding:profile"], LOG + context.original["binding:profile"], LOG ) - if trunk_details: - self._configure_trunk(trunk_details, connected_interface_uuid) + networks_to_remove = set(self._fetch_subports_network_ids(trunk_details)) + networks_to_remove.add(network_id) + + # TODO: does this needs to clean up subports too? - nb_vlan_group_id = self.update_nautobot( - network_id, connected_interface_uuid, None + LOG.debug( + "update_port_postcommit removing vlans %s from interface %s ", + networks_to_remove, + connected_interface_uuid, ) - self.undersync.sync_devices( - vlan_group_uuids=str(nb_vlan_group_id), - dry_run=cfg.CONF.ml2_understack.undersync_dry_run, + self.nb.remove_port_network_associations( + connected_interface_uuid, networks_to_remove + ) + + def delete_port_precommit(self, context): + pass + + def delete_port_postcommit(self, context): + # Only clean up provisioning ports. Everything else is left to get + # cleaned up upon the next change in that cabinet. + vlan_group_name = self._vlan_group_name(context) + if vlan_group_name and is_provisioning_network(context.current["network_id"]): + # Signals end of the provisioning / cleaning cycle, so we + # put the port back to its normal tenant mode: + self._set_nautobot_port_status(context, "Active") + self.invoke_undersync(vlan_group_name) + + def _allocate_dynamic_vlan_segment( + self, context: PortContext, physical_network: str, network_id: str + ) -> dict: + """Allocate a dynamic VLAN-type network segment, if none already exist. + + This will result in exactly one Segment per physical_network per + Network. If a Segment already exists for this physical_network then it + will not create another one. + + Does the same as ml2 driver context.allocate_dynamic_segment method, + except that this method allows the caller to specify the network_id. + """ + LOG.info( + "Obtaining Dynamic Segment of type VLAN, physical_network=%s network=%s", + physical_network, + network_id, + ) + return context._plugin.type_manager.allocate_dynamic_segment( + context._plugin_context, + network_id, + { + "network_type": p_const.TYPE_VLAN, + "physical_network": physical_network, + }, ) def bind_port(self, context: PortContext) -> None: + """Bind the VXLAN network segment and allocate dynamic VLAN segments. + + Our "context" knows a Port, a Network and a list of Segments. + + We find the first (hopefully only) segment of type vxlan. This is the + one we bind. There may be other segments, but we only bind the vxlan + one. + + We obtain the dynamic segment for this (network, vlan_group). + + We configure the nautobot switch interface with the new VLAN(s). + + Then make the required call in to the black box: context.set_binding + which tells the framework that we have dealt with this port and they + don't need to retry or handle this some other way. + + We expect to receive a call to update_port_postcommit soon after this, + which means that changes made here will get pushed to the switch at that + time. + """ + if is_provisioning_network(context.current["network_id"]): + self._set_nautobot_port_status(context, "Provisioning-Interface") + for segment in context.network.network_segments: - if self.check_segment(segment): - context.set_binding( - segment[api.ID], - portbindings.VIF_TYPE_OTHER, - {}, - status=p_const.PORT_STATUS_ACTIVE, - ) - LOG.debug("Bound segment: %s", segment) - self._configure_switchport_on_bind(context) + if segment[api.NETWORK_TYPE] == p_const.TYPE_VXLAN: + self._bind_port_segment(context, segment) return - else: - LOG.debug( - "Refusing to bind port for segment ID %(id)s, " - "segment %(seg)s, phys net %(physnet)s, and " - "network type %(nettype)s", - { - "id": segment[api.ID], - "seg": segment[api.SEGMENTATION_ID], - "physnet": segment[api.PHYSICAL_NETWORK], - "nettype": segment[api.NETWORK_TYPE], - }, - ) - - def check_segment(self, segment): - """Verify a segment is valid for the Understack MechanismDriver. - - Verify the requested segment is supported by Understack and return True or - False to indicate this to callers. - """ - network_type = segment[api.NETWORK_TYPE] - return network_type in [ - p_const.TYPE_VXLAN, - p_const.TYPE_VLAN, - ] - def check_vlan_transparency(self, context): - pass + def _bind_port_segment(self, context: PortContext, segment): + network_id = context.current["network_id"] + connected_interface_uuid = utils.fetch_connected_interface_uuid( + context.current["binding:profile"], LOG + ) + vlan_group_name = self._vlan_group_name(context) + if vlan_group_name is None: + LOG.debug("bind ignoring %s, no vlan group yet", segment) + return - def update_nautobot( - self, - network_id: str, - connected_interface_uuid: str, - vlan_tag: int | None, - ) -> UUID: - """Updates Nautobot with the new network ID and connected interface UUID. - - If the network ID is a provisioning network, sets the interface status to - "Provisioning-Interface" and configures Nautobot for provisioning mode. - If the network ID is a tenant network, sets the interface status to a tenant - status and triggers a Nautobot Job to update the switch interface for tenant - mode. In either case, retrieves and returns the VLAN Group UUID for the - specified network and interface. - :param network_id: The ID of the network. - :param connected_interface_uuid: The UUID of the connected interface. - :return: The VLAN group UUID. - """ - provisioning_network = ( - cfg.CONF.ml2_understack.provisioning_network - or cfg.CONF.ml2_type_understack.provisioning_network + LOG.debug( + "bind_port_segment interface %s network %s vlan group %s", + connected_interface_uuid, + network_id, + vlan_group_name, ) - if network_id == provisioning_network: - port_status = "Provisioning-Interface" - configure_port_status_data = self.nb.configure_port_status( - connected_interface_uuid, port_status - ) - switch_uuid = configure_port_status_data.get("device", {}).get("id") - return UUID(self.nb.fetch_vlan_group_uuid(switch_uuid)) - else: - vlan_group_id = self.nb.prep_switch_interface( - connected_interface_id=connected_interface_uuid, - ucvni_uuid=network_id, - vlan_tag=vlan_tag, - )["vlan_group_id"] - return UUID(vlan_group_id) - - def _clean_trunks(self, trunk_details: dict, connected_interface_uuid: str) -> None: - network_uuids = self._fetch_subports_network_ids(trunk_details) - for network_uuid in network_uuids: - self.nb.detach_port(connected_interface_uuid, network_uuid) - - def _delete_tenant_port_on_unbound(self, context): - """Tenant network port cleanup in the UnderCloud infrastructure. + new_segment = self._allocate_dynamic_vlan_segment( + context, + physical_network=vlan_group_name, + network_id=network_id, + ) + LOG.debug("Native VLAN segment %s", new_segment) + native_vlan_id = new_segment["segmentation_id"] - This is triggered in the update_port_postcommit call as in the - delete_port_postcommit call there is no binding profile information - anymore, hence there is no way for us to identify which baremetal port - needs cleanup. + # Traffic to the native vlan is always allowed: + allowed_vlan_ids = set([native_vlan_id]) - Only in the update_port_postcommit we have access to the original context, - from which we can access the binding information. - """ - if ( - context.current["binding:vnic_type"] == "baremetal" - and context.vif_type == portbindings.VIF_TYPE_UNBOUND - and context.original_vif_type == portbindings.VIF_TYPE_OTHER - ): - connected_interface_uuid = utils.fetch_connected_interface_uuid( - context.original["binding:profile"], LOG - ) - trunk_details = context.current.get("trunk_details", {}) - if trunk_details: - self._clean_trunks(trunk_details, connected_interface_uuid) + self.nb.set_port_vlan_associations( + connected_interface_uuid, + native_vlan_id, + allowed_vlan_ids, + vlan_group_name, + ) - network_id = context.current["network_id"] - nb_vlan_group_id = UUID( - self.nb.detach_port(connected_interface_uuid, network_id) - ) - self.undersync.sync_devices( - vlan_group_uuids=str(nb_vlan_group_id), - dry_run=cfg.CONF.ml2_understack.undersync_dry_run, + trunk_details = context.current.get("trunk_details") or {} + port_id = context.current["id"] + if trunk_details: + LOG.debug( + "bind_port: configure_trunk for port_id %s trunk_details %s", + port_id, + trunk_details, ) + # self.trunk_driver.configure_trunk(trunk_details, port_id) + + LOG.debug("set_binding for segment: %s", segment) + context.set_binding( + new_segment[api.ID], + portbindings.VIF_TYPE_OTHER, + {}, + status=p_const.PORT_STATUS_ACTIVE, + ) + + def invoke_undersync(self, vlan_group_name: str): + self.undersync.sync_devices( + vlan_group=vlan_group_name, + dry_run=cfg.CONF.ml2_understack.undersync_dry_run, + ) + + def _vlan_group_name(self, context: PortContext) -> str | None: + binding_profile = context.current.get("binding:profile", {}) + local_link_info = binding_profile.get("local_link_information", []) + switch_names = [ + link["switch_info"] for link in local_link_info if "switch_info" in link + ] + if switch_names: + return vlan_group_name_convention.for_switch(switch_names[0]) + + def check_vlan_transparency(self, context): + pass def _fetch_and_delete_nautobot_namespace(self, name: str) -> None: namespace_uuid = self.nb.fetch_namespace_by_name(name) @@ -469,3 +491,17 @@ def _delete_vlan(self, segment): self.nb.delete_vlan( vlan_id=segment.get("id"), ) + + def _set_nautobot_port_status(self, context: PortContext, status: str): + profile = context.current["binding:profile"] + interface_uuid = utils.fetch_connected_interface_uuid(profile, LOG) + LOG.debug("Set interface %s to %s status", interface_uuid, status) + self.nb.configure_port_status(interface_uuid, status=status) + + +def is_provisioning_network(network_id: str) -> bool: + provisioning_network = ( + cfg.CONF.ml2_understack.provisioning_network + or cfg.CONF.ml2_type_understack.provisioning_network + ) + return network_id == provisioning_network diff --git a/python/neutron-understack/neutron_understack/tests/conftest.py b/python/neutron-understack/neutron_understack/tests/conftest.py index 37168b3f7..2e2e431b2 100644 --- a/python/neutron-understack/neutron_understack/tests/conftest.py +++ b/python/neutron-understack/neutron_understack/tests/conftest.py @@ -175,6 +175,7 @@ def binding_profile(request, port_id) -> str: { "port_id": req.get("port_id", str(port_id)), "switch_id": "11:22:33:44:55:66", + "switch_info": "a1-1-1.iad3.rackspace.net", } ] } @@ -266,11 +267,6 @@ def undersync_sync_devices_patch(mocker, understack_driver) -> None: mocker.patch.object(understack_driver.undersync, "sync_devices") -@pytest.fixture -def update_nautobot_patch(mocker, understack_driver) -> None: - mocker.patch.object(understack_driver, "update_nautobot") - - @pytest.fixture def utils_fetch_subport_network_id_patch(mocker, network_id) -> None: mocker.patch( diff --git a/python/neutron-understack/neutron_understack/tests/helpers.py b/python/neutron-understack/neutron_understack/tests/helpers.py index 746d9215b..fb1e40b5f 100644 --- a/python/neutron-understack/neutron_understack/tests/helpers.py +++ b/python/neutron-understack/neutron_understack/tests/helpers.py @@ -4,7 +4,6 @@ from neutron.db.models_v2 import Network from neutron.db.models_v2 import Port from neutron.db.models_v2 import Subnet -from neutron.plugins.ml2.managers import TypeManager from neutron.plugins.ml2.plugin import Ml2Plugin from neutron.services.trunk.plugin import TrunkPlugin @@ -17,7 +16,7 @@ class Ml2PluginNoInit(Ml2Plugin): """ def __init__(self): - self.type_manager = TypeManager() + self.type_manager = FakeTypeManager() def construct_port_dict(self, port: Port) -> dict: port_dict = self._make_port_dict(port, process_extensions=False) @@ -43,6 +42,15 @@ def extend_network_dict_with_segment( return network_dict +class FakeTypeManager: + def allocate_dynamic_segment(self, *_): + print(f"FakeTypeManager trace: allocate_dynamic_segment{_}") + return {"id": "22222222-3333-4444-5555-666666666666", "segmentation_id": 666} + + def extend_network_with_provider_segments(self, *_): + print(f"FakeTypeManager trace: extend_network_with_provider_segments{_}") + + def extend_port_dict_with_trunk(port_dict: dict, port: Port) -> dict: port_dict["bulk"] = True TrunkPlugin._extend_port_trunk_details(port_dict, port) diff --git a/python/neutron-understack/neutron_understack/tests/test_neutron_understack_mech.py b/python/neutron-understack/neutron_understack/tests/test_neutron_understack_mech.py index c90acaa03..9fc37192f 100644 --- a/python/neutron-understack/neutron_understack/tests/test_neutron_understack_mech.py +++ b/python/neutron-understack/neutron_understack/tests/test_neutron_understack_mech.py @@ -1,70 +1,26 @@ +from dataclasses import dataclass from unittest.mock import ANY import pytest -from neutron_lib.api.definitions import portbindings from neutron_understack import utils from neutron_understack.nautobot import VlanPayload -class TestUpdateNautobot: - def test_for_tenant_network(self, mocker, understack_driver, vlan_group_id): - attrs = {"vlan_group_id": str(vlan_group_id)} - mocker.patch.object( - understack_driver.nb, "prep_switch_interface", return_value=attrs - ) - understack_driver.update_nautobot("111", "222", 333) - - understack_driver.nb.prep_switch_interface.assert_called_once_with( - connected_interface_id="222", ucvni_uuid="111", vlan_tag=333 - ) - - def test_for_provisioning_network(self, mocker, understack_driver, vlan_group_id): - mocker.patch.object( - understack_driver.nb, - "configure_port_status", - return_value={"device": {"id": "444"}}, - ) - mocker.patch.object( - understack_driver.nb, - "fetch_vlan_group_uuid", - return_value=str(vlan_group_id), - ) +class TestUpdatePortPostCommit: + def test_with_simple_port(self, understack_driver, port_context): + understack_driver.update_port_postcommit(port_context) - understack_driver.update_nautobot("change_me", "333", 123) - - understack_driver.nb.configure_port_status.assert_called_once_with( - "333", "Provisioning-Interface" - ) - understack_driver.nb.fetch_vlan_group_uuid.assert_called_once_with("444") - - -class TestUpdatePortPostcommit: - @pytest.mark.parametrize( - "port_dict", [{"vif_type": portbindings.VIF_TYPE_UNBOUND}], indirect=True - ) - def test_vif_type_unbound(self, mocker, understack_driver, port_context): - spy_delete_tenant_port = mocker.spy( - understack_driver, "_delete_tenant_port_on_unbound" - ) - result = understack_driver.update_port_postcommit(port_context) - - spy_delete_tenant_port.assert_called_once() - assert result is None + understack_driver.undersync.sync_devices.assert_called_once() -@pytest.mark.usefixtures("update_nautobot_patch") class TestBindPort: def test_with_no_trunk(self, mocker, port_context, understack_driver): - mocker.patch.object(understack_driver, "_configure_trunk") mocker.patch("neutron_understack.utils.fetch_connected_interface_uuid") understack_driver.bind_port(port_context) - understack_driver.update_nautobot.assert_called_once() - understack_driver._configure_trunk.assert_not_called() utils.fetch_connected_interface_uuid.assert_called_once() - understack_driver.undersync.sync_devices.assert_called_once() @pytest.mark.parametrize("port_dict", [{"trunk": True}], indirect=True) def test_with_trunk_details(self, mocker, understack_driver, port_context, port_id): @@ -73,32 +29,6 @@ def test_with_trunk_details(self, mocker, understack_driver, port_context, port_ ) understack_driver.bind_port(port_context) - understack_driver.nb.prep_switch_interface.assert_called_once_with( - connected_interface_id=str(port_id), - ucvni_uuid="112233", - modify_native_vlan=False, - vlan_tag=None, - ) - - -class Test_DeleteTenantPortOnUnbound: - @pytest.mark.parametrize("port_dict", [{"trunk": True}], indirect=True) - def test_when_trunk_details_are_present( - self, mocker, understack_driver, port_context, vlan_group_id, port_id - ): - mocker.patch( - "neutron_understack.utils.fetch_subport_network_id", return_value="112233" - ) - - mocker.patch.object( - understack_driver.nb, - "detach_port", - return_value=str(vlan_group_id), - ) - port_context._binding.vif_type = portbindings.VIF_TYPE_UNBOUND - - understack_driver._delete_tenant_port_on_unbound(port_context) - understack_driver.nb.detach_port.assert_any_call(str(port_id), "112233") class TestCreateSubnetPostCommit: @@ -200,15 +130,57 @@ def test_vxlan_network( project_id, ): mocker.patch.object(understack_driver, "_create_nautobot_namespace") - understack_driver.create_network_postcommit(network_context) + + @dataclass + class FakeContext: + current = { + "id": "3b5f0bb1-cd53-4c71-b129-1fe7550dfdf4", + "name": "humpback", + "tenant_id": "f9b40d4a39c4403ab5567da17e71906a", + "admin_state_up": True, + "mtu": 9000, + "status": "ACTIVE", + "subnets": [], + "standard_attr_id": 3926, + "shared": False, + "project_id": "f9b40d4a39c4403ab5567da17e71906a", + "router:external": False, + "provider:network_type": "vxlan", + "provider:physical_network": None, + "provider:segmentation_id": 200025, + "is_default": False, + "availability_zone_hints": [], + "availability_zones": [], + "ipv4_address_scope": None, + "ipv6_address_scope": None, + "vlan_transparent": None, + "description": "", + "l2_adjacency": True, + "tags": [], + "created_at": "2025-03-14T07:06:52Z", + "updated_at": "2025-03-14T07:06:52Z", + "revision_number": 1, + } + network_segments = [ + { + "id": "9e56eb8d-f9ec-47d2-ac80-3fde76087c38", + "network_type": "vxlan", + "physical_network": None, + "segmentation_id": 200025, + "network_id": "3b5f0bb1-cd53-4c71-b129-1fe7550dfdf4", + } + ] + + understack_driver.create_network_postcommit(FakeContext()) + understack_driver.nb.ucvni_create.assert_called_once_with( - network_id=str(network_id), - project_id=str(project_id), + network_id="3b5f0bb1-cd53-4c71-b129-1fe7550dfdf4", + project_id="f9b40d4a39c4403ab5567da17e71906a", ucvni_group=str(ucvni_group_id), - segmentation_id=network_context.current["provider:segmentation_id"], - network_name=network_context.current["name"], + segmentation_id=200025, + network_name="humpback", ) understack_driver._create_nautobot_namespace.assert_called_once_with( - str(network_id), + "3b5f0bb1-cd53-4c71-b129-1fe7550dfdf4", network_context.current["router:external"], ) diff --git a/python/neutron-understack/neutron_understack/tests/test_trunk.py b/python/neutron-understack/neutron_understack/tests/test_trunk.py deleted file mode 100644 index d4e749e01..000000000 --- a/python/neutron-understack/neutron_understack/tests/test_trunk.py +++ /dev/null @@ -1,241 +0,0 @@ -import pytest -from neutron.plugins.ml2.driver_context import portbindings -from oslo_config import cfg - -from neutron_understack.trunk import SubportSegmentationIDError - - -class TestSubportsAdded: - def test_that_handler_is_called( - self, mocker, understack_trunk_driver, trunk_payload, subport, trunk - ): - mocker.patch.object( - understack_trunk_driver, "_handle_tenant_vlan_id_and_switchport_config" - ) - - understack_trunk_driver.subports_added("", "", "", trunk_payload) - - ( - understack_trunk_driver._handle_tenant_vlan_id_and_switchport_config.assert_called_once_with( - [subport], trunk - ) - ) - - -class TestTrunkCreated: - def test_when_subports_are_present( - self, mocker, understack_trunk_driver, trunk_payload, subport, trunk - ): - mocker.patch.object( - understack_trunk_driver, "_handle_tenant_vlan_id_and_switchport_config" - ) - understack_trunk_driver.trunk_created("", "", "", trunk_payload) - - ( - understack_trunk_driver._handle_tenant_vlan_id_and_switchport_config.assert_called_once_with( - [subport], trunk - ) - ) - - def test_when_subports_are_not_present( - self, mocker, understack_trunk_driver, trunk_payload, subport, trunk - ): - mocker.patch.object( - understack_trunk_driver, "_handle_tenant_vlan_id_and_switchport_config" - ) - trunk.sub_ports = [] - understack_trunk_driver.trunk_created("", "", "", trunk_payload) - - ( - understack_trunk_driver._handle_tenant_vlan_id_and_switchport_config.assert_not_called() - ) - - -@pytest.mark.usefixtures("utils_fetch_subport_network_id_patch") -class Test_HandleTenantVlanIDAndSwitchportConfig: - def test_when_ucvni_tenant_vlan_id_is_not_set_yet( - self, mocker, understack_trunk_driver, trunk, subport, network_id, vlan_num - ): - mocker.patch.object( - understack_trunk_driver.nb, "fetch_ucvni_tenant_vlan_id", return_value=None - ) - mocker.patch.object( - understack_trunk_driver, "_handle_parent_port_switchport_config" - ) - understack_trunk_driver._handle_tenant_vlan_id_and_switchport_config( - [subport], trunk - ) - - understack_trunk_driver.nb.add_tenant_vlan_tag_to_ucvni.assert_called_once_with( - network_uuid=str(network_id), vlan_tag=vlan_num - ) - understack_trunk_driver._handle_parent_port_switchport_config.assert_called_once() - - def test_when_segmentation_id_is_different_to_tenant_vlan_id( - self, mocker, understack_trunk_driver, vlan_num, subport, trunk - ): - mocker.patch.object( - understack_trunk_driver.nb, - "fetch_ucvni_tenant_vlan_id", - return_value=(vlan_num + 1), - ) - mocker.patch.object( - understack_trunk_driver, "_handle_parent_port_switchport_config" - ) - mocker.patch.object(subport, "delete") - - with pytest.raises(SubportSegmentationIDError): - understack_trunk_driver._handle_tenant_vlan_id_and_switchport_config( - [subport], trunk - ) - - def test_when_parent_port_is_bound( - self, - mocker, - understack_trunk_driver, - trunk, - subport, - port_object, - vlan_group_id, - port_id, - network_id, - ): - mocker.patch.object(understack_trunk_driver, "_handle_tenant_vlan_id_config") - mocker.patch( - "neutron_understack.utils.fetch_port_object", return_value=port_object - ) - mocker.patch.object( - understack_trunk_driver.nb, - "prep_switch_interface", - return_value={"vlan_group_id": vlan_group_id}, - ) - understack_trunk_driver._handle_tenant_vlan_id_and_switchport_config( - [subport], trunk - ) - - understack_trunk_driver.nb.prep_switch_interface.assert_called_once_with( - connected_interface_id=str(port_id), - ucvni_uuid=str(network_id), - vlan_tag=None, - modify_native_vlan=False, - ) - understack_trunk_driver.undersync.sync_devices.assert_called_once_with( - vlan_group_uuids=str(vlan_group_id), - dry_run=cfg.CONF.ml2_understack.undersync_dry_run, - ) - - def test_when_parent_port_is_unbound( - self, mocker, understack_trunk_driver, trunk, subport, port_object - ): - mocker.patch.object(understack_trunk_driver, "_handle_tenant_vlan_id_config") - port_object.bindings[0].vif_type = portbindings.VIF_TYPE_UNBOUND - mocker.patch( - "neutron_understack.utils.fetch_port_object", return_value=port_object - ) - mocker.patch.object( - understack_trunk_driver, "_add_subport_network_to_parent_port_switchport" - ) - understack_trunk_driver._handle_tenant_vlan_id_and_switchport_config( - [subport], trunk - ) - - ( - understack_trunk_driver._add_subport_network_to_parent_port_switchport.assert_not_called() - ) - - -class TestSubportsDeleted: - def test_that_clean_parent_port_is_triggered( - self, mocker, understack_trunk_driver, trunk_payload, trunk, subport - ): - mocker.patch.object( - understack_trunk_driver, "_clean_parent_port_switchport_config" - ) - - understack_trunk_driver.subports_deleted("", "", "", trunk_payload) - - ( - understack_trunk_driver._clean_parent_port_switchport_config.assert_called_once_with( - trunk, [subport] - ) - ) - - -class TestTrunkDeleted: - def test_when_subports_are_present( - self, mocker, understack_trunk_driver, trunk_payload, trunk, subport - ): - mocker.patch.object( - understack_trunk_driver, "_clean_parent_port_switchport_config" - ) - - understack_trunk_driver.trunk_deleted("", "", "", trunk_payload) - - ( - understack_trunk_driver._clean_parent_port_switchport_config.assert_called_once_with( - trunk, [subport] - ) - ) - - def test_when_subports_are_not_present( - self, mocker, understack_trunk_driver, trunk_payload, trunk, subport - ): - mocker.patch.object( - understack_trunk_driver, "_clean_parent_port_switchport_config" - ) - - trunk.sub_ports = [] - understack_trunk_driver.trunk_deleted("", "", "", trunk_payload) - - ( - understack_trunk_driver._clean_parent_port_switchport_config.assert_not_called() - ) - - -@pytest.mark.usefixtures("utils_fetch_subport_network_id_patch") -class Test_CleanParentPortSwitchportConfig: - def test_when_parent_port_is_bound( - self, - mocker, - understack_trunk_driver, - trunk, - subport, - port_object, - vlan_group_id, - port_id, - network_id, - ): - mocker.patch( - "neutron_understack.utils.fetch_port_object", return_value=port_object - ) - mocker.patch.object( - understack_trunk_driver.nb, "detach_port", return_value=str(vlan_group_id) - ) - - understack_trunk_driver._clean_parent_port_switchport_config(trunk, [subport]) - - understack_trunk_driver.nb.detach_port.assert_called_once_with( - connected_interface_id=str(port_id), ucvni_uuid=str(network_id) - ) - understack_trunk_driver.undersync.sync_devices.assert_called_once_with( - vlan_group_uuids=str(vlan_group_id), - dry_run=cfg.CONF.ml2_understack.undersync_dry_run, - ) - - def test_when_parent_port_is_unbound( - self, mocker, understack_trunk_driver, port_object, trunk, subport - ): - port_object.bindings[0].vif_type = portbindings.VIF_TYPE_UNBOUND - mocker.patch( - "neutron_understack.utils.fetch_port_object", return_value=port_object - ) - mocker.patch.object( - understack_trunk_driver, - "_remove_subport_network_from_parent_port_switchport", - ) - - understack_trunk_driver._clean_parent_port_switchport_config(trunk, [subport]) - - ( - understack_trunk_driver._remove_subport_network_from_parent_port_switchport.assert_not_called() - ) diff --git a/python/neutron-understack/neutron_understack/undersync.py b/python/neutron-understack/neutron_understack/undersync.py index c00d5aab1..b2c0a36d0 100644 --- a/python/neutron-understack/neutron_understack/undersync.py +++ b/python/neutron-understack/neutron_understack/undersync.py @@ -38,17 +38,14 @@ def _log_and_raise_for_status(self, response: requests.Response): raise UndersyncError() from error def sync_devices( - self, vlan_group_uuids: str | list[str], force=False, dry_run=False + self, vlan_group: str, force=False, dry_run=False ) -> requests.Response: - if isinstance(vlan_group_uuids, list): - vlan_group_uuids = ",".join(vlan_group_uuids) - if dry_run: - return self.dry_run(vlan_group_uuids) + return self.dry_run(vlan_group) elif force: - return self.force(vlan_group_uuids) + return self.force(vlan_group) else: - return self.sync(vlan_group_uuids) + return self.sync(vlan_group) @cached_property def client(self): @@ -59,9 +56,9 @@ def client(self): } return session - def _undersync_post(self, action: str, uuids: str) -> requests.Response: + def _undersync_post(self, action: str, vlan_group: str) -> requests.Response: response = self.client.post( - f"{self.api_url}/v1/vlan-group/{uuids}/{action}", timeout=self.timeout + f"{self.api_url}/v1/vlan-group/{vlan_group}/{action}", timeout=self.timeout ) LOG.debug( "undersync %(action)s resp: %(resp)s", @@ -70,11 +67,11 @@ def _undersync_post(self, action: str, uuids: str) -> requests.Response: self._log_and_raise_for_status(response) return response - def sync(self, uuids: str) -> requests.Response: - return self._undersync_post("sync", uuids) + def sync(self, vlan_group: str) -> requests.Response: + return self._undersync_post("sync", vlan_group) - def dry_run(self, uuids: str) -> requests.Response: - return self._undersync_post("dry-run", uuids) + def dry_run(self, vlan_group: str) -> requests.Response: + return self._undersync_post("dry-run", vlan_group) - def force(self, uuids: str) -> requests.Response: - return self._undersync_post("force", uuids) + def force(self, vlan_group: str) -> requests.Response: + return self._undersync_post("force", vlan_group) diff --git a/python/neutron-understack/neutron_understack/vlan_group_name_convention.py b/python/neutron-understack/neutron_understack/vlan_group_name_convention.py new file mode 100644 index 000000000..1da1afd39 --- /dev/null +++ b/python/neutron-understack/neutron_understack/vlan_group_name_convention.py @@ -0,0 +1,25 @@ +# Please keep this in sync with data_center.py in this repo: +VLAN_GROUP_SUFFIXES = { + "-1": "network", + "-2": "network", + "-1f": "storage", + "-2f": "storage", + "-3f": "storage-appliance", + "-4f": "storage-appliance", + "-1d": "bmc", +} + + +def for_switch(switch_name: str) -> str: + switch_name = switch_name.split(".")[0] + + for switch_name_suffix, vlan_group_suffix in VLAN_GROUP_SUFFIXES.items(): + if switch_name.endswith(switch_name_suffix): + cabinet_name = switch_name.removesuffix(switch_name_suffix) + return f"{cabinet_name}-{vlan_group_suffix}" + + raise ValueError( + f"Could not determine the VLAN GROUP name for Switch {switch_name}. We " + f"only have a convention to do this for switch names ending " + f"in one of the suffixes {list(VLAN_GROUP_SUFFIXES.keys())}" + )