diff --git a/python/neutron-understack/neutron_understack/ironic.py b/python/neutron-understack/neutron_understack/ironic.py new file mode 100644 index 000000000..14b67547c --- /dev/null +++ b/python/neutron-understack/neutron_understack/ironic.py @@ -0,0 +1,32 @@ +from keystoneauth1 import loading as ks_loading +from openstack import connection +from openstack.baremetal.baremetal_service import BaremetalService +from openstack.baremetal.v1.port import Port as BaremetalPort +from oslo_config import cfg + + +class IronicClient: + def __init__(self): + self.irclient = self._get_ironic_client() + + def _get_session(self, group: str) -> ks_loading.session.Session: + auth = ks_loading.load_auth_from_conf_options(cfg.CONF, group) + session = ks_loading.load_session_from_conf_options(cfg.CONF, group, auth=auth) + return session + + def _get_ironic_client(self) -> BaremetalService: + session = self._get_session("ironic") + + return connection.Connection( + session=session, oslo_conf=cfg.CONF, connect_retries=cfg.CONF.http_retries + ).baremetal + + def baremetal_port_by_mac(self, mac_addr: str) -> BaremetalPort | None: + try: + return next(self.irclient.ports(details=True, address=mac_addr)) + except StopIteration: + return None + + def baremetal_port_physical_network(self, mac_addr: str) -> str | None: + port = self.baremetal_port_by_mac(mac_addr) + return port.physical_network if port else None diff --git a/python/neutron-understack/neutron_understack/neutron_understack_mech.py b/python/neutron-understack/neutron_understack/neutron_understack_mech.py index 04639a656..287b16776 100644 --- a/python/neutron-understack/neutron_understack/neutron_understack_mech.py +++ b/python/neutron-understack/neutron_understack/neutron_understack_mech.py @@ -13,7 +13,7 @@ from neutron_understack import config from neutron_understack import utils -from neutron_understack import vlan_group_name_convention +from neutron_understack.ironic import IronicClient from neutron_understack.nautobot import Nautobot from neutron_understack.nautobot import VlanPayload from neutron_understack.trunk import UnderStackTrunkDriver @@ -40,6 +40,7 @@ def initialize(self): conf = cfg.CONF.ml2_understack self.nb = Nautobot(conf.nb_url, conf.nb_token) self.undersync = Undersync(conf.undersync_token, conf.undersync_url) + self.ironic_client = IronicClient() self.trunk_driver = UnderStackTrunkDriver.create(self) self.subscribe() @@ -228,16 +229,6 @@ def create_port_postcommit(self, context): def update_port_precommit(self, context): pass - 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 update_port_postcommit(self, context): """Tenant network port cleanup in the UnderCloud infrastructure. @@ -248,20 +239,20 @@ def update_port_postcommit(self, context): Only in the update_port_postcommit do we have access to the original context, from which we can access the binding information. - - # 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) - 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: + if not baremetal_vnic: + return + + vlan_group_name = self.ironic_client.baremetal_port_physical_network( + context.current["mac_address"] + ) + + if current_vif_unbound and original_vif_other: self._tenant_network_port_cleanup(context) if vlan_group_name: self.invoke_undersync(vlan_group_name) @@ -305,7 +296,15 @@ def delete_port_precommit(self, context): 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) + + baremetal_vnic = context.current["binding:vnic_type"] == "baremetal" + if not baremetal_vnic: + return + + vlan_group_name = self.ironic_client.baremetal_port_physical_network( + context.current["mac_address"] + ) + 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: @@ -349,9 +348,19 @@ def _bind_port_segment(self, context: PortContext, segment): connected_interface_uuid = utils.fetch_connected_interface_uuid( context.current["binding:profile"], self.nb ) - vlan_group_name = self._vlan_group_name(context) - if vlan_group_name is None: - raise Exception("bind_port_segment: no switch info in local_link_info") + mac_address = context.current["mac_address"] + + vlan_group_name = self.ironic_client.baremetal_port_physical_network( + mac_address + ) + + if not vlan_group_name: + LOG.error( + "bind_port_segment: no physical_network found for baremetal " + "port with mac address: %(mac)s", + {"mac": mac_address}, + ) + return LOG.debug( "bind_port_segment: interface %s network %s vlan group %s", @@ -396,15 +405,6 @@ def invoke_undersync(self, vlan_group_name: str): 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 diff --git a/python/neutron-understack/neutron_understack/tests/conftest.py b/python/neutron-understack/neutron_understack/tests/conftest.py index 6462d175f..79811d4c1 100644 --- a/python/neutron-understack/neutron_understack/tests/conftest.py +++ b/python/neutron-understack/neutron_understack/tests/conftest.py @@ -22,6 +22,7 @@ from neutron_lib.api.definitions import portbindings from neutron_lib.callbacks.events import DBEventPayload +from neutron_understack.ironic import IronicClient from neutron_understack.nautobot import Nautobot from neutron_understack.neutron_understack_mech import UnderstackDriver from neutron_understack.tests.helpers import Ml2PluginNoInit @@ -76,6 +77,19 @@ def host_id() -> uuid.UUID: return uuid.uuid4() +@pytest.fixture +def mac_address() -> str: + mac = [ + 0x00, + 0x16, + 0x3E, + random.randint(0x00, 0x7F), + random.randint(0x00, 0xFF), + random.randint(0x00, 0xFF), + ] + return ":".join([f"{i:02x}" for i in mac]) + + @pytest.fixture def network(project_id, network_id) -> Network: return Network(id=str(network_id), project_id=project_id, external=None) @@ -199,8 +213,8 @@ def subport_model(vlan_num) -> SubPortModel: @pytest.fixture -def port_model() -> PortModel: - return PortModel() +def port_model(mac_address) -> PortModel: + return PortModel(mac_address=mac_address) @pytest.fixture @@ -257,10 +271,16 @@ def nautobot_client(mocker) -> Nautobot: @pytest.fixture -def understack_driver(nautobot_client) -> UnderstackDriver: +def ironic_client(mocker) -> IronicClient: + return mocker.MagicMock(spec_set=IronicClient) + + +@pytest.fixture +def understack_driver(nautobot_client, ironic_client) -> UnderstackDriver: driver = UnderstackDriver() driver.nb = nautobot_client driver.undersync = Undersync("auth_token", "api_url") + driver.ironic_client = ironic_client return driver @@ -269,6 +289,15 @@ def understack_trunk_driver(understack_driver) -> UnderStackTrunkDriver: return UnderStackTrunkDriver.create(understack_driver) +@pytest.fixture +def ironic_baremetal_port_physical_network(mocker, understack_driver) -> None: + mocker.patch.object( + understack_driver.ironic_client, + "baremetal_port_physical_network", + return_value="physnet", + ) + + @pytest.fixture(autouse=True) def undersync_sync_devices_patch(mocker, understack_driver) -> None: mocker.patch.object(understack_driver.undersync, "sync_devices") 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 5ef38d872..2d4e04f6c 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 @@ -14,6 +14,7 @@ def test_with_simple_port(self, understack_driver, port_context): understack_driver.undersync.sync_devices.assert_called_once() +@pytest.mark.usefixtures("ironic_baremetal_port_physical_network") class TestBindPort: def test_with_no_trunk( self, @@ -42,7 +43,7 @@ def test_with_no_trunk( interface_uuid="FAKE ID", native_vlan_id=1800, allowed_vlans_ids={1800}, - vlan_group_name="a1-1-network", + vlan_group_name="physnet", ) @pytest.mark.parametrize("port_dict", [{"trunk": True}], indirect=True) diff --git a/python/neutron-understack/neutron_understack/tests/test_trunk.py b/python/neutron-understack/neutron_understack/tests/test_trunk.py index 617584145..353e10456 100644 --- a/python/neutron-understack/neutron_understack/tests/test_trunk.py +++ b/python/neutron-understack/neutron_understack/tests/test_trunk.py @@ -52,6 +52,7 @@ def test_when_subports_are_not_present( ) +@pytest.mark.usefixtures("ironic_baremetal_port_physical_network") @pytest.mark.usefixtures("utils_fetch_subport_network_id_patch") class Test_HandleTenantVlanIDAndSwitchportConfig: def test_when_ucvni_tenant_vlan_id_is_not_set_yet( @@ -115,11 +116,11 @@ def test_when_parent_port_is_bound( understack_trunk_driver.nb.add_port_vlan_associations.assert_called_once_with( interface_uuid=str(port_id), - vlan_group_name="a1-1-network", + vlan_group_name="physnet", allowed_vlans_ids={1800}, ) understack_trunk_driver.undersync.sync_devices.assert_called_once_with( - vlan_group="a1-1-network", + vlan_group="physnet", dry_run=cfg.CONF.ml2_understack.undersync_dry_run, ) @@ -191,6 +192,7 @@ def test_when_subports_are_not_present( ) +@pytest.mark.usefixtures("ironic_baremetal_port_physical_network") @pytest.mark.usefixtures("utils_fetch_subport_network_id_patch") class Test_CleanParentPortSwitchportConfig: def test_when_parent_port_is_bound( @@ -221,7 +223,7 @@ def test_when_parent_port_is_bound( interface_uuid=str(port_id), network_ids_to_remove={network_id} ) understack_trunk_driver.undersync.sync_devices.assert_called_once_with( - vlan_group="a1-1-network", + vlan_group="physnet", dry_run=cfg.CONF.ml2_understack.undersync_dry_run, ) diff --git a/python/neutron-understack/neutron_understack/tests/test_utils.py b/python/neutron-understack/neutron_understack/tests/test_utils.py index a39a6056f..d171a72fd 100644 --- a/python/neutron-understack/neutron_understack/tests/test_utils.py +++ b/python/neutron-understack/neutron_understack/tests/test_utils.py @@ -54,22 +54,3 @@ def test_no_binding_profile(self, port_object): port_object.bindings[0].profile = {} result = utils.parent_port_is_bound(port_object) assert result is False - - -class TestVlanGroupNameFromBindingProfile: - def test_when_switch_name_is_present(self, port_object): - binding_profile = port_object.bindings[0].profile - result = utils.vlan_group_name_from_binding_profile(binding_profile) - assert result == "a1-1-network" - - def test_when_switch_name_is_not_present(self, port_object): - binding_profile = port_object.bindings[0].profile - binding_profile["local_link_information"][0].pop("switch_info") - result = utils.vlan_group_name_from_binding_profile(binding_profile) - assert result is None - - def test_when_switch_name_is_non_standard(self, port_object): - binding_profile = port_object.bindings[0].profile - binding_profile["local_link_information"][0]["switch_info"] = "blah" - with pytest.raises(ValueError): - utils.vlan_group_name_from_binding_profile(binding_profile) diff --git a/python/neutron-understack/neutron_understack/trunk.py b/python/neutron-understack/neutron_understack/trunk.py index 03e157d0b..8923491ae 100644 --- a/python/neutron-understack/neutron_understack/trunk.py +++ b/python/neutron-understack/neutron_understack/trunk.py @@ -50,6 +50,7 @@ def __init__( ) self.nb = self.plugin_driver.nb self.undersync = self.plugin_driver.undersync + self.ironic_client = self.plugin_driver.ironic_client @property def is_loaded(self): @@ -193,7 +194,9 @@ def _add_subports_networks_to_parent_port_switchport( binding_profile, self.nb ) - vlan_group_name = utils.vlan_group_name_from_binding_profile(binding_profile) + vlan_group_name = self.ironic_client.baremetal_port_physical_network( + parent_port.mac_address + ) allowed_vlan_ids = self._handle_segment_allocation( subports, vlan_group_name, binding_host ) @@ -226,10 +229,14 @@ def _clean_parent_port_switchport_config( return binding_profile = parent_port_obj.bindings[0].profile binding_host = parent_port_obj.bindings[0].host + vlan_group_name = self.ironic_client.baremetal_port_physical_network( + parent_port_obj.mac_address + ) self._handle_subports_removal( binding_profile=binding_profile, binding_host=binding_host, subports=subports, + vlan_group_name=vlan_group_name, ) def _delete_binding_level(self, port_id: str, host: str) -> PortBindingLevel: @@ -266,8 +273,8 @@ def _handle_subports_removal( binding_host: str, subports: list[SubPort], invoke_undersync: bool = True, + vlan_group_name: str | None = None, ) -> None: - vlan_group_name = utils.vlan_group_name_from_binding_profile(binding_profile) connected_interface_id = utils.fetch_connected_interface_uuid( binding_profile, self.nb ) @@ -278,7 +285,7 @@ def _handle_subports_removal( network_ids_to_remove=vlan_ids_to_remove, ) - if invoke_undersync: + if invoke_undersync and vlan_group_name: self._trigger_undersync(vlan_group_name) def subports_added(self, resource, event, trunk_plugin, payload): diff --git a/python/neutron-understack/neutron_understack/utils.py b/python/neutron-understack/neutron_understack/utils.py index 0fb1f9dde..b0e2a9338 100644 --- a/python/neutron-understack/neutron_understack/utils.py +++ b/python/neutron-understack/neutron_understack/utils.py @@ -8,7 +8,6 @@ from neutron_lib.api.definitions import segment as segment_def from neutron_lib.plugins import directory -from neutron_understack import vlan_group_name_convention from neutron_understack.nautobot import Nautobot @@ -85,15 +84,6 @@ def release_dynamic_segment(segment_id: str) -> None: core_plugin.type_manager.release_dynamic_segment(context, segment_id) -def vlan_group_name_from_binding_profile(binding_profile: dict) -> str | None: - 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 fetch_connected_interface_uuid(binding_profile: dict, nautobot: Nautobot) -> str: """Fetches the connected interface UUID from the port's binding profile. diff --git a/python/neutron-understack/neutron_understack/vlan_group_name_convention.py b/python/neutron-understack/neutron_understack/vlan_group_name_convention.py deleted file mode 100644 index 1da1afd39..000000000 --- a/python/neutron-understack/neutron_understack/vlan_group_name_convention.py +++ /dev/null @@ -1,25 +0,0 @@ -# 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())}" - )