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 2f2ad4c43..3b16c97c6 100644 --- a/python/neutron-understack/neutron_understack/neutron_understack_mech.py +++ b/python/neutron-understack/neutron_understack/neutron_understack_mech.py @@ -10,16 +10,15 @@ 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 from neutron_understack.undersync import Undersync -from neutron_understack.vlan_manager import VlanManager LOG = logging.getLogger(__name__) @@ -32,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): @@ -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 @@ -74,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, @@ -118,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", @@ -224,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) @@ -472,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/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( 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())}" + ) 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)