diff --git a/components/images-openstack.yaml b/components/images-openstack.yaml index 04c023280..6aedfb51d 100644 --- a/components/images-openstack.yaml +++ b/components/images-openstack.yaml @@ -43,7 +43,7 @@ images: neutron_metadata: "ghcr.io/rackerlabs/understack/neutron:2026.1" neutron_ovn_metadata: "ghcr.io/rackerlabs/understack/neutron:2026.1" neutron_openvswitch_agent: "ghcr.io/rackerlabs/understack/neutron:2026.1" - neutron_server: "ghcr.io/rackerlabs/understack/neutron:2026.1" + neutron_server: "ghcr.io/rackerlabs/understack/neutron:pr-2456" neutron_rpc_server: "ghcr.io/rackerlabs/understack/neutron:2026.1" neutron_ovn_maintenance_worker: "ghcr.io/rackerlabs/understack/neutron:2026.1" neutron_ironic_agent: "ghcr.io/rackerlabs/understack/neutron:2026.1" diff --git a/python/neutron-understack/neutron_understack/config.py b/python/neutron-understack/neutron_understack/config.py index ea8f80cd9..851a71de6 100644 --- a/python/neutron-understack/neutron_understack/config.py +++ b/python/neutron-understack/neutron_understack/config.py @@ -25,11 +25,11 @@ ), cfg.ListOpt( "default_tenant_vlan_id_range", - default=[1, 3799], + default=[2, 3871], item_type=cfg.types.Integer(min=1, max=4094), help=( - "List of 2 comma separated integers, that represents a VLAN range, that" - "will be used for mapped VLANs on the switches." + "Two comma-separated integers defining the inclusive VLAN range used " + "for mapped VLANs on switches and accepted for tenant trunk subports." ), ), ] diff --git a/python/neutron-understack/neutron_understack/tests/conftest.py b/python/neutron-understack/neutron_understack/tests/conftest.py index a49db540b..00ebcad3b 100644 --- a/python/neutron-understack/neutron_understack/tests/conftest.py +++ b/python/neutron-understack/neutron_understack/tests/conftest.py @@ -279,13 +279,17 @@ def port_context(network_context, port_dict, port_binding, ml2_plugin) -> PortCo @pytest.fixture -def understack_driver(oslo_config) -> UnderstackDriver: +def understack_driver(mocker, oslo_config) -> UnderstackDriver: """The understack driver, which must never reach out to Undersync. No ``undersync`` attribute is injected on purpose, so a reintroduced ``self.undersync.sync(...)`` fails loudly instead of passing against a mock. + The trunk collaborator is mocked because these unit tests do not call the + driver's production ``initialize()`` hook. """ - return UnderstackDriver() + driver = UnderstackDriver() + driver.trunk_driver = mocker.create_autospec(UnderstackTrunkDriver, instance=True) + return driver @pytest.fixture diff --git a/python/neutron-understack/neutron_understack/tests/scenarios/SCENARIOS.md b/python/neutron-understack/neutron_understack/tests/scenarios/SCENARIOS.md index e1a6fc633..43eb1311b 100644 --- a/python/neutron-understack/neutron_understack/tests/scenarios/SCENARIOS.md +++ b/python/neutron-understack/neutron_understack/tests/scenarios/SCENARIOS.md @@ -293,13 +293,30 @@ adds/removes must reconcile the parent's switch (VLAN group). parent binding there is no physnet to carry the subport on and nothing to reconcile -### TRUNK-SEGID-RANGE-01 — subport seg_id outside the allowed range is rejected +### TRUNK-PARENT-BIND-NATIVE-01 — binding rejects an existing native VLAN collision +- given: an unbound baremetal parent with a trunk whose existing subport tag + will equal the parent's native VLAN on its eventual physical network +- when: the parent is vif-attached and that native VLAN is selected +- then: the parent binding fails with no parent or subport binding levels and + no `undersync.sync`; the collision could not be checked when the trunk or + subport was created because the unbound parent had no physical network or + native VLAN yet + +### TRUNK-SEGID-NATIVE-01 — subport seg_id matching the native VLAN is rejected - given: a bound baremetal parent with a trunk -- when: a subport is added with a segmentation_id outside `[1, 3799]` +- when: a subport is added with a segmentation_id equal to the VLAN segment of + the parent's network on the parent's physical network - then: `SubportSegmentationIDError` (raised in the SUBPORTS PRECOMMIT_CREATE callback, surfaced as `CallbackFailure`), with no binding level or dynamic segment allocated +### TRUNK-SEGID-RANGE-01 — subport seg_id outside the tenant range is rejected +- given: a bound baremetal parent with a trunk +- when: a subport is added with a VLAN segmentation_id outside the configured + `default_tenant_vlan_id_range` +- then: the request is rejected before a binding level or dynamic segment is + allocated + ### TRUNK-ORDER-01 — subport removal deallocates before it notifies Undersync - given: a bound baremetal parent with a trunk and an attached subport - when: the subport is removed diff --git a/python/neutron-understack/neutron_understack/tests/scenarios/test_trunk_operations.py b/python/neutron-understack/neutron_understack/tests/scenarios/test_trunk_operations.py index 0de428899..63081391b 100644 --- a/python/neutron-understack/neutron_understack/tests/scenarios/test_trunk_operations.py +++ b/python/neutron-understack/neutron_understack/tests/scenarios/test_trunk_operations.py @@ -12,6 +12,7 @@ from neutron.plugins.ml2 import db as ml2_db from neutron.services.trunk import exceptions as trunk_exc from neutron_lib import constants as p_const +from neutron_lib.api.definitions import portbindings from neutron_lib.callbacks import exceptions as cb_exc from neutron_understack.tests.scenarios.base import DEFAULT_PHYSNET @@ -32,6 +33,33 @@ def _plain_port(self, net_id): assert res.status_int == 201, res.body return self.deserialize(self.fmt, res)["port"]["id"] + def _unbound_baremetal_port(self, net_id): + res = self._create_port( + self.fmt, + net_id, + arg_list=(portbindings.VNIC_TYPE,), + is_admin=True, + **{portbindings.VNIC_TYPE: portbindings.VNIC_BAREMETAL}, + ) + assert res.status_int == 201, res.body + return self.deserialize(self.fmt, res)["port"] + + def _vif_attach(self, port_id, physnet=DEFAULT_PHYSNET, host="host-a"): + data = { + "port": { + portbindings.HOST_ID: host, + portbindings.PROFILE: self.baremetal_binding_profile(physnet=physnet), + } + } + req = self.new_update_request("ports", data, port_id, as_service=True) + with mock.patch( + "neutron_understack.utils.fetch_network_node_trunk_id", + return_value=_FAKE_NN_TRUNK, + ): + res = req.get_response(self.api) + assert res.status_int == 200, res.body + return self.deserialize(self.fmt, res)["port"] + def _make_trunk(self, parent_id): trunk = self.trunk_plugin.create_trunk( self.context, @@ -230,6 +258,36 @@ def test_subport_add_unbound_parent_no_sync(self): ) self.undersync_mock.sync.assert_not_called() + @pytest.mark.scenario("TRUNK-PARENT-BIND-NATIVE-01") + def test_unbound_trunk_rejects_native_vlan_collision_on_parent_bind(self): + """Binding must revalidate subports once the native VLAN is known.""" + parent_net = self._make_network(self.fmt, "parent-net", True)["network"]["id"] + parent = self._unbound_baremetal_port(parent_net) + subport_net = self._make_network(self.fmt, "subport-net", True)["network"]["id"] + subport_id = self._plain_port(subport_net) + trunk_id = self._make_trunk(parent["id"]) + + # This is valid while the parent is unbound because its native VLAN is + # not known yet. Pin the segment that vif-attach will select so the + # eventual collision is deterministic. + self._add_subport(trunk_id, subport_id, seg_id=SUBPORT_VLAN) + native_segment = { + segments_db.NETWORK_TYPE: p_const.TYPE_VLAN, + segments_db.PHYSICAL_NETWORK: DEFAULT_PHYSNET, + segments_db.SEGMENTATION_ID: SUBPORT_VLAN, + } + segments_db.add_network_segment( + self.context, parent_net, native_segment, is_dynamic=True + ) + + self.undersync_mock.reset_mock() + updated = self._vif_attach(parent["id"]) + + assert updated[portbindings.VIF_TYPE] == portbindings.VIF_TYPE_BINDING_FAILED + assert not ml2_db.get_binding_level_objs(self.context, parent["id"], "host-a") + assert not ml2_db.get_binding_level_objs(self.context, subport_id, "host-a") + self.undersync_mock.sync.assert_not_called() + @pytest.mark.scenario("TRUNK-MULTI-01") def test_multiple_subports_add_syncs(self): parent_net = self._make_network(self.fmt, "parent-net", True)["network"]["id"] @@ -274,20 +332,46 @@ def test_multiple_subports_add_syncs(self): assert segment_a != segment_b self.undersync_mock.sync.assert_any_call(DEFAULT_PHYSNET) - @pytest.mark.scenario("TRUNK-SEGID-RANGE-01") - def test_subport_segid_out_of_range_rejected(self): + @pytest.mark.scenario("TRUNK-SEGID-NATIVE-01") + def test_subport_segid_matching_native_vlan_rejected(self): parent_net = self._make_network(self.fmt, "parent-net", True)["network"]["id"] parent_id = self._bind_baremetal_port(parent_net, DEFAULT_PHYSNET, "host-a") subport_net = self._make_network(self.fmt, "subport-net", True)["network"]["id"] subport_id = self._plain_port(subport_net) trunk_id = self._make_trunk(parent_id) + native_segment = segments_db.get_dynamic_segment( + self.context, parent_net, physical_network=DEFAULT_PHYSNET + ) - # 4000 is a valid VLAN but outside the default tenant range [1, 3799]. - # subports_added raises SubportSegmentationIDError, which the SUBPORTS - # PRECOMMIT_CREATE callback machinery re-raises as CallbackFailure. + # subports_added raises SubportSegmentationIDError when the requested + # tag is the native VLAN, and the callback machinery wraps it. + with pytest.raises(cb_exc.CallbackFailure) as exc_info: + self._add_subport( + trunk_id, + subport_id, + seg_id=native_segment[segments_db.SEGMENTATION_ID], + ) + assert "matches the native VLAN" in str(exc_info.value) + assert not ml2_db.get_binding_level_objs(self.context, subport_id, "host-a") + assert ( + segments_db.get_dynamic_segment( + self.context, subport_net, physical_network=DEFAULT_PHYSNET + ) + is None + ) + + @pytest.mark.scenario("TRUNK-SEGID-RANGE-01") + def test_subport_segid_outside_configured_range_is_rejected(self): + parent_net = self._make_network(self.fmt, "parent-net", True)["network"]["id"] + parent_id = self._bind_baremetal_port(parent_net, DEFAULT_PHYSNET, "host-a") + subport_net = self._make_network(self.fmt, "subport-net", True)["network"]["id"] + subport_id = self._plain_port(subport_net) + trunk_id = self._make_trunk(parent_id) with pytest.raises(cb_exc.CallbackFailure) as exc_info: self._add_subport(trunk_id, subport_id, seg_id=4000) - assert "Segmentation ID" in str(exc_info.value) + assert "outside the configured tenant trunk VLAN range 2-3871" in str( + exc_info.value + ) assert not ml2_db.get_binding_level_objs(self.context, subport_id, "host-a") assert ( segments_db.get_dynamic_segment( diff --git a/python/neutron-understack/neutron_understack/tests/test_trunk.py b/python/neutron-understack/neutron_understack/tests/test_trunk.py index d7555e7a3..a9458f2a9 100644 --- a/python/neutron-understack/neutron_understack/tests/test_trunk.py +++ b/python/neutron-understack/neutron_understack/tests/test_trunk.py @@ -4,6 +4,7 @@ from neutron_understack import utils from neutron_understack.trunk import SubportSegmentationIDError +from neutron_understack.trunk import SubportSegmentationIDRangeError class TestSubportsAdded: @@ -55,9 +56,9 @@ def test_when_subports_are_not_present( @pytest.mark.usefixtures("_utils_fetch_subport_network_id_patch") class Test_HandleTenantVlanIDAndSwitchportConfig: def test_that_check_subports_segmentation_id_is_called( - self, mocker, understack_trunk_driver, trunk, subport, network_id, vlan_num + self, mocker, understack_trunk_driver, trunk, subport ): - mocker.patch("neutron_understack.utils.fetch_port_object") + parent_port = mocker.patch("neutron_understack.utils.fetch_port_object") mocker.patch( "neutron_understack.utils.parent_port_is_bound", return_value=False ) @@ -68,7 +69,9 @@ def test_that_check_subports_segmentation_id_is_called( [subport], trunk ) - subport_seg_id_check.assert_called_once() + subport_seg_id_check.assert_called_once_with( + [subport], trunk.id, parent_port.return_value + ) def test_when_parent_port_is_bound( self, @@ -267,24 +270,118 @@ def test_when_segment_is_used_by_other_ports( class TestConfigureTrunk: + def test_returns_without_configuring_when_port_is_not_a_trunk_parent( + self, + mocker, + understack_trunk_driver, + port_id, + vlan_network_segment, + ): + mocker.patch( + "neutron_understack.utils.fetch_trunk_by_parent_port", return_value=None + ) + add_subports = mocker.patch.object( + understack_trunk_driver, "_add_subports_networks_to_parent_port_switchport" + ) + + understack_trunk_driver.configure_trunk(port_id, vlan_network_segment) + + add_subports.assert_not_called() + def test_that_add_subports_networks_is_called( self, mocker, understack_trunk_driver, + trunk, port_object, port_id, + vlan_network_segment, ): + mocker.patch( + "neutron_understack.utils.fetch_trunk_by_parent_port", return_value=trunk + ) mocker.patch( "neutron_understack.utils.fetch_port_object", return_value=port_object ) - mocker.patch.object( + mocker.patch( + "neutron_understack.utils.fetch_network_node_trunk_id", + return_value=str(trunk.id), + ) + add_subports = mocker.patch.object( + understack_trunk_driver, "_add_subports_networks_to_parent_port_switchport" + ) + + understack_trunk_driver.configure_trunk(port_id, vlan_network_segment) + + add_subports.assert_called_once_with( + parent_port=port_object, + subports=trunk.sub_ports, + ) + + def test_rejects_subport_matching_newly_selected_native_vlan( + self, + mocker, + understack_trunk_driver, + port_object, + port_id, + trunk_id, + subport, + vlan_network_segment, + trunk, + ): + trunk.id = trunk_id + mocker.patch( + "neutron_understack.utils.fetch_port_object", return_value=port_object + ) + mocker.patch( + "neutron_understack.utils.fetch_trunk_by_parent_port", return_value=trunk + ) + mocker.patch( + "neutron_understack.utils.fetch_network_node_trunk_id", + return_value="different-trunk-id", + ) + add_subports = mocker.patch.object( + understack_trunk_driver, "_add_subports_networks_to_parent_port_switchport" + ) + subport.segmentation_id = vlan_network_segment.segmentation_id + + with pytest.raises(SubportSegmentationIDError, match="matches the native VLAN"): + understack_trunk_driver.configure_trunk(port_id, vlan_network_segment) + + add_subports.assert_not_called() + + def test_network_node_trunk_allows_native_vlan_as_subport_tag( + self, + mocker, + understack_trunk_driver, + port_object, + port_id, + trunk_id, + subport, + vlan_network_segment, + trunk, + ): + trunk.id = trunk_id + mocker.patch( + "neutron_understack.utils.fetch_port_object", return_value=port_object + ) + mocker.patch( + "neutron_understack.utils.fetch_trunk_by_parent_port", return_value=trunk + ) + mocker.patch( + "neutron_understack.utils.fetch_network_node_trunk_id", + return_value=str(trunk_id), + ) + add_subports = mocker.patch.object( understack_trunk_driver, "_add_subports_networks_to_parent_port_switchport" ) - understack_trunk_driver.configure_trunk({}, port_id) + subport.segmentation_id = vlan_network_segment.segmentation_id - understack_trunk_driver._add_subports_networks_to_parent_port_switchport.assert_called_once_with( + understack_trunk_driver.configure_trunk(port_id, vlan_network_segment) + + add_subports.assert_called_once_with( parent_port=port_object, - subports=[], + subports=[subport], ) @@ -310,71 +407,181 @@ def test_without_subports(self, mocker, understack_trunk_driver): deallocate.assert_called_once_with([], "host-a") +@pytest.mark.usefixtures("oslo_config") class TestCheckSubportsSegmentationId: def test_when_trunk_id_is_network_node_trunk_id( self, mocker, understack_trunk_driver, trunk_id, + subport, + port_object, ): - # Mock fetch_network_node_trunk_id to return the trunk_id mocker.patch( "neutron_understack.utils.fetch_network_node_trunk_id", return_value=str(trunk_id), ) - # Mock to ensure the function returns early and doesn't call this - allowed_ranges_mock = mocker.patch( - "neutron_understack.utils.allowed_tenant_vlan_id_ranges" + find_segment = mocker.patch( + "neutron_understack.utils.network_segment_by_physnet" ) + subport.segmentation_id = 4000 + result = understack_trunk_driver._check_subports_segmentation_id( - [], str(trunk_id) + [subport], str(trunk_id), port_object ) - # Should not call allowed_tenant_vlan_id_ranges because it returns early - allowed_ranges_mock.assert_not_called() + + find_segment.assert_not_called() assert result is None - def test_when_segmentation_id_is_in_allowed_range( + def test_when_segmentation_id_differs_from_native_vlan( self, mocker, understack_trunk_driver, trunk_id, subport, + port_object, + network_id, + vlan_network_segment, ): - # Mock fetch_network_node_trunk_id to return a different trunk ID mocker.patch( "neutron_understack.utils.fetch_network_node_trunk_id", return_value="different-trunk-id", ) - allowed_ranges = mocker.patch( - "neutron_understack.utils.allowed_tenant_vlan_id_ranges", - return_value=[(1, 1500)], + find_segment = mocker.patch( + "neutron_understack.utils.network_segment_by_physnet", + return_value=vlan_network_segment, ) - subport.segmentation_id = 500 + port_object.network_id = str(network_id) + subport.segmentation_id = 2000 + result = understack_trunk_driver._check_subports_segmentation_id( - [subport], trunk_id + [subport], trunk_id, port_object + ) + + find_segment.assert_called_once_with( + network_id=str(network_id), physnet="physnet" ) - allowed_ranges.assert_called_once() assert result is None - def test_when_segmentation_id_is_not_in_allowed_range( + @pytest.mark.parametrize("segmentation_id", [2, 3871]) + def test_when_segmentation_id_is_on_configured_range_boundary( self, mocker, understack_trunk_driver, trunk_id, subport, + port_object, + segmentation_id, ): - # Mock fetch_network_node_trunk_id to return a different trunk ID mocker.patch( "neutron_understack.utils.fetch_network_node_trunk_id", return_value="different-trunk-id", ) + port_object.bindings[0].vif_type = portbindings.VIF_TYPE_UNBOUND + subport.segmentation_id = segmentation_id + + result = understack_trunk_driver._check_subports_segmentation_id( + [subport], trunk_id, port_object + ) + + assert result is None + + @pytest.mark.parametrize("segmentation_id", [1, 3872, 4094]) + def test_when_segmentation_id_is_outside_configured_range( + self, + mocker, + understack_trunk_driver, + trunk_id, + subport, + port_object, + segmentation_id, + ): mocker.patch( - "neutron_understack.utils.allowed_tenant_vlan_id_ranges", - return_value=[(1, 1500)], + "neutron_understack.utils.fetch_network_node_trunk_id", + return_value="different-trunk-id", ) - subport.segmentation_id = 1600 - with pytest.raises(SubportSegmentationIDError): - understack_trunk_driver._check_subports_segmentation_id([subport], trunk_id) + find_segment = mocker.patch( + "neutron_understack.utils.network_segment_by_physnet" + ) + subport.segmentation_id = segmentation_id + + with pytest.raises( + SubportSegmentationIDRangeError, match="outside the configured" + ): + understack_trunk_driver._check_subports_segmentation_id( + [subport], trunk_id, port_object + ) + + find_segment.assert_not_called() + + def test_uses_configured_tenant_vlan_range( + self, + mocker, + oslo_config, + understack_trunk_driver, + trunk_id, + subport, + port_object, + ): + oslo_config.config( + default_tenant_vlan_id_range=[100, 200], group="ml2_understack" + ) + mocker.patch( + "neutron_understack.utils.fetch_network_node_trunk_id", + return_value="different-trunk-id", + ) + subport.segmentation_id = 99 + + with pytest.raises(SubportSegmentationIDRangeError, match="100-200"): + understack_trunk_driver._check_subports_segmentation_id( + [subport], trunk_id, port_object + ) + + def test_when_segmentation_id_matches_native_vlan( + self, + mocker, + understack_trunk_driver, + trunk_id, + subport, + port_object, + network_id, + vlan_network_segment, + ): + mocker.patch( + "neutron_understack.utils.fetch_network_node_trunk_id", + return_value="different-trunk-id", + ) + mocker.patch( + "neutron_understack.utils.network_segment_by_physnet", + return_value=vlan_network_segment, + ) + port_object.network_id = str(network_id) + subport.segmentation_id = vlan_network_segment.segmentation_id + + with pytest.raises(SubportSegmentationIDError, match="matches the native VLAN"): + understack_trunk_driver._check_subports_segmentation_id( + [subport], trunk_id, port_object + ) + + def test_when_parent_port_is_unbound( + self, mocker, understack_trunk_driver, trunk_id, subport, port_object + ): + mocker.patch( + "neutron_understack.utils.fetch_network_node_trunk_id", + return_value="different-trunk-id", + ) + port_object.bindings[0].vif_type = portbindings.VIF_TYPE_UNBOUND + subport.segmentation_id = 2000 + find_segment = mocker.patch( + "neutron_understack.utils.network_segment_by_physnet" + ) + + result = understack_trunk_driver._check_subports_segmentation_id( + [subport], trunk_id, port_object + ) + + find_segment.assert_not_called() + assert result is None @pytest.mark.parametrize("binding_profile", [{"physical_network": None}], indirect=True) diff --git a/python/neutron-understack/neutron_understack/tests/test_understack_mech.py b/python/neutron-understack/neutron_understack/tests/test_understack_mech.py index 5eedca7f0..687c4c03d 100644 --- a/python/neutron-understack/neutron_understack/tests/test_understack_mech.py +++ b/python/neutron-understack/neutron_understack/tests/test_understack_mech.py @@ -310,12 +310,11 @@ def test_uses_existing_vlan_segment( next_segments_to_bind=[vlan_network_segment], ) - def test_with_no_trunk( + def test_without_trunk_details_still_checks_for_trunk( self, mocker, port_context, understack_driver, - understack_trunk_driver, vlan_network_segment, ): mocker.patch.object( @@ -325,9 +324,12 @@ def test_with_no_trunk( port_context._prepare_to_bind(port_context.network.network_segments) understack_driver.bind_port(port_context) - understack_driver.trunk_driver = understack_trunk_driver port_context.allocate_dynamic_segment.assert_called_once() + understack_driver.trunk_driver.configure_trunk.assert_called_once_with( + port_context.current["id"], + vlan_network_segment, + ) vxlan_segment = next( s for s in port_context.network.network_segments @@ -371,19 +373,26 @@ def test_does_not_bind_when_physical_network_missing( assert port_context.current["id"] in caplog.text @pytest.mark.parametrize("port_dict", [{"trunk": True}], indirect=True) - def test_with_trunk_details( - self, mocker, understack_driver, port_context, understack_trunk_driver + def test_checks_for_trunk_with_port_dict_extension( + self, + mocker, + understack_driver, + port_context, + vlan_network_segment, ): - mocker.patch( - "neutron_understack.utils.fetch_subport_network_id", return_value="112233" + mocker.patch.object( + port_context, + "allocate_dynamic_segment", + return_value=vlan_network_segment, ) mocker.patch.object(port_context, "continue_binding") port_context._prepare_to_bind(port_context.network.network_segments) - understack_driver.trunk_driver = understack_trunk_driver - mocker.patch.object(understack_driver.trunk_driver, "configure_trunk") understack_driver.bind_port(port_context) - understack_driver.trunk_driver.configure_trunk.assert_called_once() + understack_driver.trunk_driver.configure_trunk.assert_called_once_with( + port_context.current["id"], + vlan_network_segment, + ) port_context.continue_binding.assert_called_once() diff --git a/python/neutron-understack/neutron_understack/tests/test_utils.py b/python/neutron-understack/neutron_understack/tests/test_utils.py index d47daef56..d324e494d 100644 --- a/python/neutron-understack/neutron_understack/tests/test_utils.py +++ b/python/neutron-understack/neutron_understack/tests/test_utils.py @@ -14,6 +14,19 @@ from neutron_understack import utils +class TestFetchTrunkByParentPort: + def test_fetches_by_parent_port_id(self, mocker): + context = mocker.patch( + "neutron_understack.utils.n_context.get_admin_context" + ).return_value + get_object = mocker.patch("neutron_understack.utils.trunk_obj.Trunk.get_object") + + result = utils.fetch_trunk_by_parent_port("parent-port-id") + + assert result is get_object.return_value + get_object.assert_called_once_with(context, port_id="parent-port-id") + + class TestParentPortIsBound: def test_truthy_conditions(self, port_object): """Truthy conditions. diff --git a/python/neutron-understack/neutron_understack/trunk.py b/python/neutron-understack/neutron_understack/trunk.py index 6b7e7fd40..fdb536c41 100644 --- a/python/neutron-understack/neutron_understack/trunk.py +++ b/python/neutron-understack/neutron_understack/trunk.py @@ -24,8 +24,15 @@ class SubportSegmentationIDError(exc.NeutronException): message = ( "Segmentation ID: %(seg_id)s cannot be set to the Subport: " - "%(subport_id)s as it falls outside of allowed ranges: " - "%(network_segment_ranges)s. Please use different Segmentation ID." + "%(subport_id)s because it matches the native VLAN on physical " + "network: %(physical_network)s. Please use a different Segmentation ID." + ) + + +class SubportSegmentationIDRangeError(exc.NeutronException): + message = ( + "VLAN %(seg_id)s for subport %(subport_id)s is outside the configured " + "tenant trunk VLAN range %(minimum)s-%(maximum)s." ) @@ -87,8 +94,8 @@ def register(self, resource, event, trigger, payload=None): def _handle_tenant_vlan_id_and_switchport_config( self, subports: list[SubPort], trunk: Trunk ) -> None: - self._check_subports_segmentation_id(subports, trunk.id) parent_port_obj = utils.fetch_port_object(trunk.port_id) + self._check_subports_segmentation_id(subports, trunk.id, parent_port_obj) if utils.parent_port_is_bound(parent_port_obj): self._add_subports_networks_to_parent_port_switchport( @@ -96,37 +103,89 @@ def _handle_tenant_vlan_id_and_switchport_config( ) def _check_subports_segmentation_id( - self, subports: list[SubPort], trunk_id: str + self, subports: list[SubPort], trunk_id: str, parent_port: Port ) -> None: - """Checks if a subport's segmentation_id is within the allowed range. + """Validate tenant tags and reject a parent native VLAN collision. A switchport cannot have a mapped VLAN ID equal to the native VLAN ID. - Since the user specifies the VLAN ID (segmentation_id) when adding a - subport, an error is raised if it falls within any VLAN network segment - range, as these ranges are used to allocate VLAN tags for all VLAN - segments, including native VLANs. - - The only case where this check is not required is for a network node - trunk, since its subport segmentation_ids are the same as the network - segment VLAN tags allocated to the subports. Therefore, there is no - possibility of conflict with the native VLAN. + Resolve the native VLAN from the parent port's network and physical + network so that multi-segment networks are checked against the segment + used on this particular switch. + + The network-node trunk is exempt because its segmentation IDs are + internally allocated fabric VLANs, not tenant-selected mapped VLANs. + They are the same tags as the subports' network segments, so they also + cannot conflict with the parent through VLAN mapping. """ if trunk_id == utils.fetch_network_node_trunk_id(): return - ns_ranges = utils.allowed_tenant_vlan_id_ranges() + minimum, maximum = cfg.CONF.ml2_understack.default_tenant_vlan_id_range for subport in subports: - seg_id = subport.segmentation_id - if not utils.segmentation_id_in_ranges(seg_id, ns_ranges): + seg_id = int(subport["segmentation_id"]) + if not minimum <= seg_id <= maximum: + raise SubportSegmentationIDRangeError( + seg_id=seg_id, + subport_id=subport["port_id"], + minimum=minimum, + maximum=maximum, + ) + + if not utils.parent_port_is_bound(parent_port): + return + + physical_network = parent_port.bindings[0].profile.get("physical_network") + if not physical_network: + # The create path reports the more specific missing-physnet error + # when it attempts to configure the parent switchport. + return + + native_segment = utils.network_segment_by_physnet( + network_id=parent_port.network_id, + physnet=physical_network, + ) + if native_segment is None: + return + + native_vlan_id = int(native_segment.segmentation_id) + self._check_subports_native_vlan(subports, physical_network, native_vlan_id) + + def _check_subports_native_vlan( + self, + subports: list[SubPort] | list[dict], + physical_network: str, + native_vlan_id: int, + ) -> None: + for subport in subports: + seg_id = int(subport["segmentation_id"]) + if seg_id == native_vlan_id: raise SubportSegmentationIDError( seg_id=seg_id, - subport_id=subport.port_id, - network_segment_ranges=utils.printable_ranges(ns_ranges), + subport_id=subport["port_id"], + physical_network=physical_network, ) - def configure_trunk(self, trunk_details: dict, port_id: str) -> None: + def configure_trunk(self, port_id: str, native_segment: dict) -> None: + """Configure a trunk when its parent acquires a native VLAN. + + Resolve the trunk from the database rather than relying on the + ``trunk_details`` API extension. That extension is not guaranteed to + be present in the port dictionary used by ML2 during binding. + """ + trunk = utils.fetch_trunk_by_parent_port(port_id) + if trunk is None: + return + parent_port_obj = utils.fetch_port_object(port_id) - subports = trunk_details.get("sub_ports", []) + subports = trunk.sub_ports + + if subports and str(trunk.id) != utils.fetch_network_node_trunk_id(): + self._check_subports_native_vlan( + subports, + native_segment["physical_network"], + int(native_segment["segmentation_id"]), + ) + self._add_subports_networks_to_parent_port_switchport( parent_port=parent_port_obj, subports=subports ) diff --git a/python/neutron-understack/neutron_understack/understack_mech.py b/python/neutron-understack/neutron_understack/understack_mech.py index 7f0c59194..8b37e1e31 100644 --- a/python/neutron-understack/neutron_understack/understack_mech.py +++ b/python/neutron-understack/neutron_understack/understack_mech.py @@ -380,10 +380,8 @@ def _bind_port_segment(self, context: PortContext, segment): LOG.debug("bind_port_segment: Native VLAN segment %s", dynamic_segment) - trunk_details = context.current.get("trunk_details") or {} port_id = context.current["id"] - if trunk_details: - self.trunk_driver.configure_trunk(trunk_details, port_id) + self.trunk_driver.configure_trunk(port_id, dynamic_segment) LOG.debug("continue_binding for segment: %s", segment) context.continue_binding( diff --git a/python/neutron-understack/neutron_understack/utils.py b/python/neutron-understack/neutron_understack/utils.py index e3017158e..160fa12dd 100644 --- a/python/neutron-understack/neutron_understack/utils.py +++ b/python/neutron-understack/neutron_understack/utils.py @@ -34,6 +34,11 @@ def fetch_port_object(port_id: str) -> port_obj.Port: return port +def fetch_trunk_by_parent_port(port_id: str) -> trunk_obj.Trunk | None: + context = n_context.get_admin_context() + return trunk_obj.Trunk.get_object(context, port_id=port_id) + + def create_neutron_port_for_segment( segment: NetworkSegmentDict, context: PortContext ) -> PortDict: