Merge pull request #103 from stackhpc/upstream/master-2025-02-10

Synchronise master with upstream

Author: priteau

Diff for: doc/source/dev/dev-quickstart.rst

@@ -81,8 +81,8 @@ verifies that ovs port has been assigned to correct VLAN::
    bash ~/networking-generic-switch/devstack/exercise.sh
 
 
-Test with real hardware:
-------------------------
+Test with real hardware
+-----------------------
 
 Add information about hardware switch to Networking-generic-switch
 config ``/etc/neutron/plugins/ml2/ml2_conf_genericswitch.ini`` and

Diff for: networking_generic_switch/generic_switch_mech.py

@@ -376,7 +376,7 @@ def update_port_postcommit(self, context):
                           {'switch_port': port_id, 'switch_info': switch_info,
                            'segmentation_id': segmentation_id})
                 # Move port to network
-                if is_802_3ad and hasattr(switch, 'plug_bond_to_network'):
+                if is_802_3ad:
                     switch.plug_bond_to_network(port_id, segmentation_id)
                 else:
                     switch.plug_port_to_network(port_id, segmentation_id)
@@ -466,6 +466,10 @@ def bind_port(self, context):
         can use with ports.
         """
 
+        # NOTE(vsaienko): Important that this method remains lightweight.
+        # The actual port handling is done in update_port_postcommit.
+        # For more info please read docstring.
+
         port = context.current
         network = context.network.current
         binding_profile = port['binding:profile']
@@ -477,44 +481,7 @@ def bind_port(self, context):
             if not self._is_link_valid(port, network):
                 return
 
-            is_802_3ad = self._is_802_3ad(port)
-            for link in local_link_information:
-                port_id = link.get('port_id')
-                switch_info = link.get('switch_info')
-                switch_id = link.get('switch_id')
-                switch = device_utils.get_switch_device(
-                    self.switches, switch_info=switch_info,
-                    ngs_mac_address=switch_id)
-
-                segments = context.segments_to_bind
-                # If segmentation ID is None, set vlan 1
-                segmentation_id = segments[0].get('segmentation_id') or 1
-
-                # Fail if port or vlan not in allow list
-                if not switch.is_allowed(port_id, segmentation_id):
-                    LOG.warn("Skipped binding port %(port_id)s, "
-                             "port %(port)s in segment "
-                             "%(segment_id)s on device %(device)s, as either "
-                             "the port or vlan is not on the allow list",
-                             {'port_id': port['id'], 'port': port_id,
-                              'device': switch_info,
-                              'segment_id': segmentation_id})
-                    return
-
-                LOG.debug("Putting port %(port_id)s on %(switch_info)s "
-                          "to vlan: %(segmentation_id)s",
-                          {'port_id': port_id, 'switch_info': switch_info,
-                           'segmentation_id': segmentation_id})
-                # Move port to network
-                if is_802_3ad and hasattr(switch, 'plug_bond_to_network'):
-                    switch.plug_bond_to_network(port_id, segmentation_id)
-                else:
-                    switch.plug_port_to_network(port_id, segmentation_id)
-                LOG.info("Successfully bound port %(port_id)s in segment "
-                         "%(segment_id)s on device %(device)s",
-                         {'port_id': port['id'], 'device': switch_info,
-                          'segment_id': segmentation_id})
-
+            segments = context.segments_to_bind
             context.set_binding(segments[0][api.ID],
                                 portbindings.VIF_TYPE_OTHER, {})
 
@@ -540,6 +507,7 @@ def _is_link_valid(self, port, network):
         local_link_information = binding_profile.get('local_link_information')
 
         for link in local_link_information:
+            port_id = link.get('port_id')
             switch_info = link.get('switch_info')
             switch_id = link.get('switch_id')
             switch = device_utils.get_switch_device(
@@ -555,6 +523,7 @@ def _is_link_valid(self, port, network):
 
             physnet = network['provider:physical_network']
             switch_physnets = switch._get_physical_networks()
+            segmentation_id = network.get('provider:segmentation_id') or 1
 
             if switch_physnets and physnet not in switch_physnets:
                 LOG.error("Cannot bind port %(port)s as device %(device)s "
@@ -563,6 +532,17 @@ def _is_link_valid(self, port, network):
                           {'port': port['id'], 'device': switch_info,
                            'physnet': physnet})
                 return False
+
+            # Fail if port or vlan not in allow list
+            if not switch.is_allowed(port_id, segmentation_id):
+                LOG.warn("Skipped binding port %(port_id)s, "
+                         "port %(port)s in segment "
+                         "%(segment_id)s on device %(device)s, as either "
+                         "the port or vlan is not on the allow list",
+                         {'port_id': port['id'], 'port': port_id,
+                          'device': switch_info,
+                          'segment_id': segmentation_id})
+                return False
         return True
 
     @staticmethod
@@ -639,7 +619,7 @@ def _unplug_port_from_network(self, port, network):
                       {'port': port_id, 'switch_info': switch_info,
                        'segmentation_id': segmentation_id})
             try:
-                if is_802_3ad and hasattr(switch, 'unplug_bond_from_network'):
+                if is_802_3ad:
                     switch.unplug_bond_from_network(port_id, segmentation_id)
                 else:
                     switch.delete_port(port_id, segmentation_id)

Diff for: networking_generic_switch/tests/unit/test_generic_switch_mech.py

@@ -904,7 +904,7 @@ def test_bind_port(self, m_apc, m_list):
                                       mock_context.current['id'],
                                       resources.PORT,
                                       'GENERICSWITCH')
-        self.switch_mock.plug_port_to_network.assert_called()
+        self.switch_mock.plug_port_to_network.assert_not_called()
 
     @mock.patch.object(provisioning_blocks, 'add_provisioning_component',
                        autospec=True)
@@ -946,7 +946,7 @@ def test_bind_portgroup(self, m_apc, m_list):
                                           mock_context.current['id'],
                                           resources.PORT,
                                           'GENERICSWITCH')])
-        self.switch_mock.plug_port_to_network.assert_called()
+        self.switch_mock.plug_port_to_network.assert_not_called()
 
     @mock.patch.object(provisioning_blocks, 'add_provisioning_component',
                        autospec=True)
@@ -993,7 +993,7 @@ def test_bind_portgroup_802_3ad(self, m_apc, m_list):
                                           resources.PORT,
                                           'GENERICSWITCH')])
         self.switch_mock.plug_port_to_network.assert_not_called()
-        self.switch_mock.plug_bond_to_network.assert_called()
+        self.switch_mock.plug_bond_to_network.assert_not_called()
 
     @mock.patch.object(provisioning_blocks, 'add_provisioning_component',
                        autospec=True)
@@ -1030,7 +1030,8 @@ def test_bind_port_with_physnet(self, m_apc, m_list):
                                       mock_context.current['id'],
                                       resources.PORT,
                                       'GENERICSWITCH')
-        self.switch_mock.plug_port_to_network.assert_called()
+        # NOTE(vsaienko): make sure we do not call heavy methods in bind_port
+        self.switch_mock.plug_port_to_network.assert_not_called()
 
     @mock.patch.object(provisioning_blocks, 'add_provisioning_component',
                        autospec=True)