Fix "_sync_metadata_ports" with no DHCP subnets

When a subnet does not have DHCP configured, the metadata port does
not have an IP address on this CIDR. The method
"OvnNbSynchronizer.sync_networks_ports_and_dhcp_opts", was always
setting an IP address for the metadata ports, regardless of the subnet
configuration (with or without DHCP).

The method "_sync_metadata_ports", in charge of synchronizing the
metadata ports, now filters the subnets by the parameter "enable_dhcp".

In case of having a subnet with DHCP enabled, if the metadata port is
missing the subnet IP addresses, the method adds them.

In case of having a subnet without DHCP enabled, if the metadata port
has an IP address on the subnet, the method removes it.

Closes-Bug: #1939726
Change-Id: I09cc14dff6933aae63cbd43a29f9221f405ecede

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

@@ -2186,12 +2186,15 @@ class OVNClient(object):
         This function will allocate an IP address for the metadata port of
         the given network in all its IPv4 subnets or the given subnet.
         """
-        def update_metadata_port_fixed_ips(metadata_port, subnet_ids):
+        def update_metadata_port_fixed_ips(metadata_port, add_subnet_ids,
+                                           del_subnet_ids):
             wanted_fixed_ips = [
                 {'subnet_id': fixed_ip['subnet_id'],
                  'ip_address': fixed_ip['ip_address']} for fixed_ip in
-                metadata_port['fixed_ips']]
+                metadata_port['fixed_ips'] if
-            wanted_fixed_ips.extend({'subnet_id': s_id} for s_id in subnet_ids)
+                fixed_ip['subnet_id'] not in del_subnet_ids]
+            wanted_fixed_ips.extend({'subnet_id': s_id} for s_id in
+                                    add_subnet_ids)
             port = {'id': metadata_port['id'],
                     'port': {'network_id': network_id,
                              'fixed_ips': wanted_fixed_ips}}
@@ -2216,12 +2219,13 @@ class OVNClient(object):
         # metadata port.
         if subnet_id:
             if subnet_id not in port_subnet_ids:
-                update_metadata_port_fixed_ips(metadata_port, [subnet_id])
+                update_metadata_port_fixed_ips(metadata_port, [subnet_id], [])
             return
 
         # Retrieve all subnets in this network
         subnets = self._plugin.get_subnets(context, filters=dict(
-            network_id=[network_id], ip_version=[const.IP_VERSION_4]))
+            network_id=[network_id], ip_version=[const.IP_VERSION_4],
+            enable_dhcp=[True]))
 
         subnet_ids = set(s['id'] for s in subnets)
 
@@ -2229,7 +2233,8 @@ class OVNClient(object):
         # allocate one.
         if subnet_ids != port_subnet_ids:
             update_metadata_port_fixed_ips(metadata_port,
-                                           subnet_ids - port_subnet_ids)
+                                           subnet_ids - port_subnet_ids,
+                                           port_subnet_ids - subnet_ids)
 
     def get_parent_port(self, port_id):
         return self._nb_idl.get_parent_port(port_id)

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

@@ -1700,22 +1700,49 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
     def test_update_metadata_port_no_subnet(self):
         ovn_conf.cfg.CONF.set_override('ovn_metadata_enabled', True,
                                        group='ovn')
-        fixed_ips = [{'subnet_id': 'subnet1', 'ip_address': 'ip_add1'}]
         with mock.patch.object(
-                self.mech_driver._ovn_client, '_find_metadata_port',
+                self.mech_driver._ovn_client, '_find_metadata_port') as \
-                return_value={'fixed_ips': fixed_ips, 'id': 'metadata_id'}), \
+                mock_metaport, \
-                mock.patch.object(self.mech_driver._plugin, 'get_subnets',
+                mock.patch.object(self.mech_driver._plugin, 'get_subnets') as \
-                                  return_value=[{'id': 'subnet1'},
+                mock_get_subnets, \
-                                                {'id': 'subnet2'}]), \
                 mock.patch.object(self.mech_driver._plugin, 'update_port') as \
                 mock_update_port:
+            # Port with IP in subnet1; subnet1 and subnet2 with DHCP.
+            mock_get_subnets.return_value = [{'id': 'subnet1'},
+                                             {'id': 'subnet2'}]
+            fixed_ips = [{'subnet_id': 'subnet1', 'ip_address': 'ip_add1'}]
+            mock_metaport.return_value = {'fixed_ips': fixed_ips,
+                                          'id': 'metadata_id'}
             self.mech_driver._ovn_client.update_metadata_port(self.context,
                                                               'net_id')
+            port = {'id': 'metadata_id',
+                    'port': {'network_id': 'net_id', 'fixed_ips': fixed_ips}}
             fixed_ips.append({'subnet_id': 'subnet2'})
-            port = {'id': 'metadata_id', 'port': {
-                'network_id': 'net_id', 'fixed_ips': fixed_ips}}
             mock_update_port.assert_called_once_with(
                 mock.ANY, 'metadata_id', port)
+            mock_update_port.reset_mock()
+
+            # Port with IP in subnet1; subnet1 with DHCP, subnet2 without DHCP.
+            mock_get_subnets.return_value = [{'id': 'subnet1'}]
+            fixed_ips = [{'subnet_id': 'subnet1', 'ip_address': 'ip_add1'}]
+            mock_metaport.return_value = {'fixed_ips': fixed_ips,
+                                          'id': 'metadata_id'}
+            self.mech_driver._ovn_client.update_metadata_port(self.context,
+                                                              'net_id')
+            mock_update_port.assert_not_called()
+
+            # Port with IP in subnet1; subnet1 without DHCP.
+            mock_get_subnets.return_value = []
+            fixed_ips = [{'subnet_id': 'subnet1', 'ip_address': 'ip_add1'}]
+            mock_metaport.return_value = {'fixed_ips': fixed_ips,
+                                          'id': 'metadata_id'}
+            self.mech_driver._ovn_client.update_metadata_port(self.context,
+                                                              'net_id')
+            port = {'id': 'metadata_id',
+                    'port': {'network_id': 'net_id', 'fixed_ips': []}}
+            mock_update_port.assert_called_once_with(
+                mock.ANY, 'metadata_id', port)
+            mock_update_port.reset_mock()
 
     @mock.patch.object(provisioning_blocks, 'is_object_blocked')
     @mock.patch.object(provisioning_blocks, 'provisioning_complete')