Merge "[OVN] Fix updating network segmentation ID" into stable/yoga

Author: Zuul

neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py

@@ -179,6 +179,16 @@ def sb_ovn(self):
     def sb_ovn(self, val):
         self._sb_ovn = val
 
+    def get_supported_vif_types(self):
+        vif_types = set()
+        for ch in self.sb_ovn.chassis_list().execute(check_error=True):
+            dp_type = ch.external_ids.get('datapath-type', '')
+            if dp_type == ovn_const.CHASSIS_DATAPATH_NETDEV:
+                vif_types.add(portbindings.VIF_TYPE_VHOST_USER)
+            else:
+                vif_types.add(portbindings.VIF_TYPE_OVS)
+        return list(vif_types)
+
     def check_vlan_transparency(self, context):
         """OVN driver vlan transparency support."""
         vlan_transparency_network_types = [
@@ -213,6 +223,10 @@ def connectivity(self):
     def supported_extensions(self, extensions):
         return set(ovn_extensions.ML2_SUPPORTED_API_EXTENSIONS) & extensions
 
+    @staticmethod
+    def provider_network_attribute_updates_supported():
+        return [provider_net.SEGMENTATION_ID]
+
     def subscribe(self):
         registry.subscribe(self.pre_fork_initialize,
                            resources.PROCESS,

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -1954,6 +1954,15 @@ def update_network(self, context, network, original_network=None):
                     if ovn_ls_azs != neutron_net_azs:
                         self.sync_ha_chassis_group(context, network['id'], txn)
 
+            # Update the segment tags, if any
+            segments = segments_db.get_network_segments(context, network['id'])
+            for segment in segments:
+                tag = segment.get(segment_def.SEGMENTATION_ID)
+                tag = [] if tag is None else tag
+                lport_name = utils.ovn_provnet_port_name(segment['id'])
+                txn.add(self._nb_idl.set_lswitch_port(lport_name=lport_name,
+                                                      tag=tag, if_exists=True))
+
             self._qos_driver.update_network(txn, network, original_network)
 
         if check_rev_cmd.result == ovn_const.TXN_COMMITTED:

neutron/plugins/ml2/plugin.py

@@ -916,16 +916,26 @@ def _update_segmentation_id(self, context, network, net_data):
         vif_types = [portbindings.VIF_TYPE_UNBOUND,
                      portbindings.VIF_TYPE_BINDING_FAILED]
         for mech_driver in self.mechanism_manager.ordered_mech_drivers:
-            if (isinstance(mech_driver.obj,
-                           mech_agent.AgentMechanismDriverBase) and
-                    provider_net.SEGMENTATION_ID in mech_driver.obj.
+            if (provider_net.SEGMENTATION_ID in mech_driver.obj.
                     provider_network_attribute_updates_supported()):
-                agent_type = mech_driver.obj.agent_type
-                agents = self.get_agents(
-                    context, filters={'agent_type': [agent_type]})
-                for agent in agents:
-                    vif_types.append(
-                        mech_driver.obj.get_supported_vif_type(agent))
+                if isinstance(mech_driver.obj,
+                        mech_agent.AgentMechanismDriverBase):
+                    agent_type = mech_driver.obj.agent_type
+                    agents = self.get_agents(
+                        context, filters={'agent_type': [agent_type]})
+                    for agent in agents:
+                        vif_types.append(
+                            mech_driver.obj.get_supported_vif_type(agent))
+                else:
+                    # For non-AgentMechanismDriverBase drivers such as OVN
+                    # which inherits from MechanismDriver
+                    # TODO(lucasagomes): Perhaps we should
+                    # include get_supported_vif_types() as part of the
+                    # MechanismDriver's api in neutron-lib and remove this
+                    # check in the future ?
+                    if getattr(mech_driver.obj, 'get_supported_vif_types'):
+                        vif_types.extend(
+                            mech_driver.obj.get_supported_vif_types())
 
         if ports_obj.Port.check_network_ports_by_binding_types(
                 context, network['id'], vif_types, negative_search=True):

neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py

@@ -2547,6 +2547,40 @@ def test_sync_ha_chassis_group_no_az_hints(self):
         self.nb_ovn.ha_chassis_group_add_chassis.assert_has_calls(
             expected_calls, any_order=True)
 
+    def test_update_network_segmentation_id(self):
+        new_vlan_tag = 123
+        net_arg = {pnet.NETWORK_TYPE: 'vlan',
+                   pnet.PHYSICAL_NETWORK: 'physnet1',
+                   pnet.SEGMENTATION_ID: '1'}
+        net = self._make_network(self.fmt, 'net1', True,
+                                 arg_list=(pnet.NETWORK_TYPE,
+                                           pnet.PHYSICAL_NETWORK,
+                                           pnet.SEGMENTATION_ID,),
+                                 **net_arg)['network']
+        # Make sure the network was created with 1 VLAN segment
+        segments = segments_db.get_network_segments(self.context, net['id'])
+        segment = segments[0]
+        self.assertEqual(len(segments), 1)
+        self.assertEqual(segment['segmentation_id'], 1)
+
+        # Issue an update to the network changing the segmentation_id
+        data = {'network': {pnet.SEGMENTATION_ID: new_vlan_tag}}
+        req = self.new_update_request('networks', data, net['id'])
+        res = self.deserialize(self.fmt, req.get_response(self.api))
+        self.assertEqual(new_vlan_tag, res['network'][pnet.SEGMENTATION_ID])
+
+        # Assert the tag was changed in the Neutron database
+        segments = segments_db.get_network_segments(self.context, net['id'])
+        segment = segments[0]
+        self.assertEqual(len(segments), 1)
+        self.assertEqual(segment['segmentation_id'], new_vlan_tag)
+
+        # Assert the tag was changed in the OVN database
+        expected_call = mock.call(
+            lport_name=ovn_utils.ovn_provnet_port_name(segment['id']),
+            tag=new_vlan_tag, if_exists=True)
+        self.nb_ovn.set_lswitch_port.assert_has_calls([expected_call])
+
 
 class OVNMechanismDriverTestCase(MechDriverSetupBase,
                                  test_plugin.Ml2PluginV2TestCase):
@@ -2891,6 +2925,10 @@ def _test_segments_helper(self):
                          segment_id=self.seg_2['id']) as subnet:
             self.sub_2 = subnet
 
+    # TODO(lucasagomes): This test should use <mock>.assert_has_calls()
+    # to validate if the method was called with the correct values instead
+    # of peeking at call_args_list otherwise every time there's a new
+    # call to the mocked method the indexes will change
     def test_create_segments_subnet_metadata_ip_allocation(self):
         self._test_segments_helper()
         ovn_nb_api = self.mech_driver.nb_ovn
@@ -2899,34 +2937,34 @@ def test_create_segments_subnet_metadata_ip_allocation(self):
         # created subnet.
         self.assertIn(
             '10.0.1.2',
-            ovn_nb_api.set_lswitch_port.call_args_list[0][1]['addresses'][0])
+            ovn_nb_api.set_lswitch_port.call_args_list[2][1]['addresses'][0])
 
         # Assert that the second subnet address also has been allocated for
         # metadata port.
         self.assertIn(
             '10.0.2.2',
-            ovn_nb_api.set_lswitch_port.call_args_list[1][1]['addresses'][0])
+            ovn_nb_api.set_lswitch_port.call_args_list[6][1]['addresses'][0])
         # Assert also that the first subnet address is in this update
         self.assertIn(
             '10.0.1.2',
-            ovn_nb_api.set_lswitch_port.call_args_list[1][1]['addresses'][0])
+            ovn_nb_api.set_lswitch_port.call_args_list[6][1]['addresses'][0])
         self.assertEqual(
-            ovn_nb_api.set_lswitch_port.call_count, 2)
+            ovn_nb_api.set_lswitch_port.call_count, 7)
 
         # Make sure both updates where on same metadata port
         args_list = ovn_nb_api.set_lswitch_port.call_args_list
         self.assertEqual(
             'ovnmeta-%s' % self.net['network']['id'],
-            args_list[1][1]['external_ids']['neutron:device_id'])
+            args_list[6][1]['external_ids']['neutron:device_id'])
         self.assertEqual(
-            args_list[1][1]['external_ids']['neutron:device_id'],
-            args_list[0][1]['external_ids']['neutron:device_id'])
+            args_list[6][1]['external_ids']['neutron:device_id'],
+            args_list[2][1]['external_ids']['neutron:device_id'])
         self.assertEqual(
-            args_list[1][1]['external_ids']['neutron:device_owner'],
-            args_list[0][1]['external_ids']['neutron:device_owner'])
+            args_list[6][1]['external_ids']['neutron:device_owner'],
+            args_list[2][1]['external_ids']['neutron:device_owner'])
         self.assertEqual(
             const.DEVICE_OWNER_DISTRIBUTED,
-            args_list[1][1]['external_ids']['neutron:device_owner'])
+            args_list[6][1]['external_ids']['neutron:device_owner'])
 
     def test_create_segments_mixed_allocation_prohibited(self):
         self._test_segments_helper()

neutron/tests/unit/plugins/ml2/test_plugin.py

@@ -533,6 +533,37 @@ def test__update_segmentation_id_agentless_mech_drivers(self):
                  portbindings.VIF_TYPE_BINDING_FAILED],
                 negative_search=True)
 
+    @mock.patch.object(ml2_plugin, 'isinstance')
+    @mock.patch.object(port_obj.Port, 'check_network_ports_by_binding_types')
+    def test__update_segmentation_id_non_AgentMechanismDriverBase(
+            self, mock_check_network, mock_isinstance):
+        plugin = directory.get_plugin()
+
+        mock_check_network.return_value = False
+        mock_isinstance.return_value = False
+        mock_update_net_segment = mock.patch.object(
+            plugin.type_manager, 'update_network_segment').start()
+
+        fake_vif_types = ['fake-vif-type0', 'fake-vif-type1']
+        fake_driver = mock.Mock()
+        fake_driver.obj.get_supported_vif_types.return_value = (
+            fake_vif_types)
+        fake_driver.obj.provider_network_attribute_updates_supported.\
+            return_value = {pnet.SEGMENTATION_ID: 1000}
+        plugin.mechanism_manager.ordered_mech_drivers = [fake_driver]
+
+        with self.network() as net:
+            net_data = {pnet.SEGMENTATION_ID: 1000}
+            plugin._update_segmentation_id(self.context, net['network'],
+                                           net_data)
+
+            # Assert the new method get_supported_vif_types() has been called
+            fake_driver.obj.get_supported_vif_types.assert_called_once_with()
+
+            # Assert the update method has been called
+            mock_update_net_segment.assert_called_once_with(
+                self.context, net['network'], net_data, mock.ANY)
+
     def test_update_network_with_empty_body(self):
         with self.network() as network:
             network_id = network["network"]["id"]

releasenotes/notes/ovn-update-vlan-id-749d8f17999243f5.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixes an issue in the ML2/OVN driver where the network
+    segment tag was not being updated in the OVN Northbound
+    database. For more information, see bug `1944708
+    <https://bugs.launchpad.net/neutron/+bug/1944708>`_.