From fe9e596d3e8c28801093adfd6fb50d6f8786142d Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Thu, 30 Sep 2021 16:28:19 +0000 Subject: [PATCH] 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 Conflicts: neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py Change-Id: I09cc14dff6933aae63cbd43a29f9221f405ecede (cherry picked from commit 5e32dddc11c6ec73afc83d2bfc7c4beaa252de0c) --- .../ovn/mech_driver/ovsdb/ovn_client.py | 17 +++++--- .../ovn/mech_driver/test_mech_driver.py | 43 +++++++++++++++---- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 6da7054cf24..3141c40a78e 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -2092,12 +2092,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']] - wanted_fixed_ips.extend({'subnet_id': s_id} for s_id in subnet_ids) + metadata_port['fixed_ips'] if + 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}} @@ -2122,12 +2125,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=[4])) + network_id=[network_id], ip_version=[const.IP_VERSION_4], + enable_dhcp=[True])) subnet_ids = set(s['id'] for s in subnets) @@ -2135,7 +2139,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) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index cda6e3a6770..ed9347cdc06 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -1682,22 +1682,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', - return_value={'fixed_ips': fixed_ips, 'id': 'metadata_id'}), \ - mock.patch.object(self.mech_driver._plugin, 'get_subnets', - return_value=[{'id': 'subnet1'}, - {'id': 'subnet2'}]), \ + self.mech_driver._ovn_client, '_find_metadata_port') as \ + mock_metaport, \ + mock.patch.object(self.mech_driver._plugin, 'get_subnets') as \ + mock_get_subnets, \ 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')