Merge pull request #922 from rackerlabs/PUC-925

fix: check static network segments on bind

Author: mfencik

python/neutron-understack/neutron_understack/neutron_understack_mech.py

@@ -20,6 +20,7 @@
 from neutron_understack.undersync import Undersync
 
 from .ml2_type_annotations import NetworkContext
+from .ml2_type_annotations import NetworkSegmentDict
 from .ml2_type_annotations import PortContext
 
 LOG = logging.getLogger(__name__)
@@ -267,7 +268,9 @@ def _tenant_network_port_cleanup(self, context: PortContext):
             original_binding, self.nb
         )
 
-        if not utils.ports_bound_to_segment(segment_id):
+        if not utils.ports_bound_to_segment(
+            segment_id
+        ) and utils.is_dynamic_network_segment(segment_id):
             context.release_dynamic_segment(segment_id)
             self.nb.delete_vlan(segment_id)
 
@@ -369,7 +372,8 @@ def _bind_port_segment(self, context: PortContext, segment):
             vlan_group_name,
         )
 
-        dynamic_segment = context.allocate_dynamic_segment(
+        current_vlan_segment = self._vlan_segment_for_physnet(context, vlan_group_name)
+        dynamic_segment = current_vlan_segment or context.allocate_dynamic_segment(
             segment={
                 "network_type": p_const.TYPE_VLAN,
                 "physical_network": vlan_group_name,
@@ -399,6 +403,21 @@ def _bind_port_segment(self, context: PortContext, segment):
             status=p_const.PORT_STATUS_ACTIVE,
         )
 
+    def _vlan_segment_for_physnet(
+        self, context: PortContext, physnet: str
+    ) -> NetworkSegmentDict | None:
+        for segment in context.network.network_segments:
+            if (
+                segment[api.NETWORK_TYPE] == p_const.TYPE_VLAN
+                and segment[api.PHYSICAL_NETWORK] == physnet
+            ):
+                LOG.info(
+                    "vlan segment: %(segment)s already preset for physnet: "
+                    "%(physnet)s",
+                    {"segment": segment, "physnet": physnet},
+                )
+                return segment
+
     def invoke_undersync(self, vlan_group_name: str):
         self.undersync.sync_devices(
             vlan_group=vlan_group_name,

python/neutron-understack/neutron_understack/tests/test_trunk.py

@@ -108,6 +108,9 @@ def test_when_parent_port_is_bound(
             "neutron_understack.utils.allocate_dynamic_segment",
             return_value=vlan_network_segment,
         )
+        mocker.patch(
+            "neutron_understack.utils.network_segment_by_physnet", return_value=None
+        )
         mocker.patch("neutron_understack.utils.create_binding_profile_level")
         mocker.patch.object(understack_trunk_driver.nb, "add_port_vlan_associations")
         understack_trunk_driver._handle_tenant_vlan_id_and_switchport_config(
@@ -220,7 +223,7 @@ def test_when_parent_port_is_bound(
         understack_trunk_driver._clean_parent_port_switchport_config(trunk, [subport])
 
         understack_trunk_driver.nb.remove_port_network_associations.assert_called_once_with(
-            interface_uuid=str(port_id), network_ids_to_remove={network_id}
+            interface_uuid=str(port_id), vlan_ids_to_remove={network_id}
         )
         understack_trunk_driver.undersync.sync_devices.assert_called_once_with(
             vlan_group="physnet",
@@ -264,6 +267,9 @@ def test_when_segment_is_unused_by_other_ports(
         mocker.patch(
             "neutron_understack.utils.ports_bound_to_segment", return_value=False
         )
+        mocker.patch(
+            "neutron_understack.utils.is_dynamic_network_segment", return_value=True
+        )
         mocker.patch.object(understack_trunk_driver.nb, "delete_vlan")
         mocker.patch("neutron_understack.utils.release_dynamic_segment")
 

python/neutron-understack/neutron_understack/trunk.py

@@ -171,7 +171,11 @@ def _handle_segment_allocation(
             subport_network_id = utils.fetch_subport_network_id(
                 subport_id=subport["port_id"]
             )
-            network_segment = utils.allocate_dynamic_segment(
+            current_segment = utils.network_segment_by_physnet(
+                network_id=subport_network_id,
+                physnet=vlan_group_name,
+            )
+            network_segment = current_segment or utils.allocate_dynamic_segment(
                 network_id=subport_network_id,
                 physnet=vlan_group_name,
             )
@@ -246,7 +250,9 @@ def _delete_binding_level(self, port_id: str, host: str) -> PortBindingLevel:
 
     def _delete_unused_segment(self, segment_id: str) -> NetworkSegment:
         network_segment = utils.network_segment_by_id(segment_id)
-        if not utils.ports_bound_to_segment(segment_id):
+        if not utils.ports_bound_to_segment(
+            segment_id
+        ) and utils.is_dynamic_network_segment(segment_id):
             utils.release_dynamic_segment(segment_id)
             self.nb.delete_vlan(vlan_id=segment_id)
         return network_segment
@@ -282,7 +288,7 @@ def _handle_subports_removal(
         vlan_ids_to_remove = self._handle_segment_deallocation(subports, binding_host)
         self.nb.remove_port_network_associations(
             interface_uuid=connected_interface_id,
-            network_ids_to_remove=vlan_ids_to_remove,
+            vlan_ids_to_remove=vlan_ids_to_remove,
         )
 
         if invoke_undersync and vlan_group_name:

python/neutron-understack/neutron_understack/utils.py

@@ -77,6 +77,26 @@ def network_segment_by_id(id: str) -> NetworkSegment:
     return NetworkSegment.get_object(context, id=id)
 
 
+def network_segment_by_physnet(network_id: str, physnet: str) -> NetworkSegment | None:
+    """Fetches vlan network segments for network in particular physnet.
+
+    We return first segment on purpose, there shouldn't be more, but if
+    that is the case, it may be intended for some reason and we don't want
+    to halt the code.
+    """
+    context = n_context.get_admin_context()
+
+    segments = NetworkSegment.get_objects(
+        context,
+        network_id=network_id,
+        physical_network=physnet,
+        network_type=constants.TYPE_VLAN,
+    )
+    if not segments:
+        return
+    return segments[0]
+
+
 def release_dynamic_segment(segment_id: str) -> None:
     context = n_context.get_admin_context()
     core_plugin = directory.get_plugin()  # Get the core plugin
@@ -85,6 +105,11 @@ def release_dynamic_segment(segment_id: str) -> None:
         core_plugin.type_manager.release_dynamic_segment(context, segment_id)
 
 
+def is_dynamic_network_segment(segment_id: str) -> bool:
+    segment = network_segment_by_id(segment_id)
+    return segment.is_dynamic
+
+
 def fetch_connected_interface_uuid(binding_profile: dict, nautobot: Nautobot) -> str:
     """Fetches the connected interface UUID from the port's binding profile.