commit 952f6d0a1de14b38884b3cd43230c24e3b4e77e6 Author: Flavio Fernandes Date: Tue Sep 29 12:25:51 2020 -0400 [OVN] update_port should not remove values from external_ids Prior to this patch, the OVNClient implementation for neutron's update_port was setting the external_ids of the affected logical switch port to a hard-coded dictionary. This meant that any key value pairs that were not listed there would simply get removed. This would make it impossible for any other users of the external_id to have a reliable way of storing its data. One of such users could be the ovn-octavia-provider. Closes-Bug: #1896827 Change-Id: Ie580534e4d91f1ca2e1dc8331632d49d4720e7ba (cherry picked from Neutron commit be3669258cb6f8a6c43b2f016a344185075fa544) diff --git a/networking_ovn/common/ovn_client.py b/networking_ovn/common/ovn_client.py index 8615f8b..fc3631d 100644 --- a/networking_ovn/common/ovn_client.py +++ b/networking_ovn/common/ovn_client.py @@ -520,6 +520,11 @@ class OVNClient(object): admin_context, txn, port, addr_pairs_diff.removed, unset=True) + # Keep key value pairs that were in the original external ids + # of the ovn port and we did not touch. + for k, v in ovn_port.external_ids.items(): + external_ids.setdefault(k, v) + # NOTE(lizk): Fail port updating if port doesn't exist. This # prevents any new inserted resources to be orphan, such as port # dhcp options or ACL rules for port, e.g. a port was created diff --git a/networking_ovn/tests/functional/test_ovn_db_resources.py b/networking_ovn/tests/functional/test_ovn_db_resources.py index fec0f9f..e4dc475 100644 --- a/networking_ovn/tests/functional/test_ovn_db_resources.py +++ b/networking_ovn/tests/functional/test_ovn_db_resources.py @@ -15,6 +15,7 @@ import mock +import copy import netaddr from neutron_lib.api.definitions import dns as dns_apidef from neutron_lib.utils import net as n_net @@ -1052,6 +1053,69 @@ class TestDNSRecords(base.TestOVNFunctionalBase): self._validate_dns_records([]) +class TestPortExternalIds(base.TestOVNFunctionalBase): + + def _get_lsp_external_id(self, port_id): + ovn_port = self.nb_api.lookup('Logical_Switch_Port', port_id) + return copy.deepcopy(ovn_port.external_ids) + + def _set_lsp_external_id(self, port_id, **pairs): + external_ids = self._get_lsp_external_id(port_id) + for key, val in pairs.items(): + external_ids[key] = val + self.nb_api.set_lswitch_port(lport_name=port_id, + external_ids=external_ids).execute() + + def _create_lsp(self): + n1 = self._make_network(self.fmt, 'n1', True) + res = self._create_subnet(self.fmt, n1['network']['id'], '10.0.0.0/24') + subnet = self.deserialize(self.fmt, res)['subnet'] + p = self._make_port(self.fmt, n1['network']['id'], + fixed_ips=[{'subnet_id': subnet['id']}]) + port_id = p['port']['id'] + return port_id, self._get_lsp_external_id(port_id) + + def test_port_update_has_ext_ids(self): + port_id, ext_ids = self._create_lsp() + self.assertIsNotNone(ext_ids) + + def test_port_update_add_ext_id(self): + port_id, ext_ids = self._create_lsp() + ext_ids['another'] = 'value' + self._set_lsp_external_id(port_id, another='value') + self.assertEqual(ext_ids, self._get_lsp_external_id(port_id)) + + def test_port_update_change_ext_id_value(self): + port_id, ext_ids = self._create_lsp() + ext_ids['another'] = 'value' + self._set_lsp_external_id(port_id, another='value') + self.assertEqual(ext_ids, self._get_lsp_external_id(port_id)) + ext_ids['another'] = 'value2' + self._set_lsp_external_id(port_id, another='value2') + self.assertEqual(ext_ids, self._get_lsp_external_id(port_id)) + + def test_port_update_with_foreign_ext_ids(self): + port_id, ext_ids = self._create_lsp() + new_ext_ids = {ovn_const.OVN_PORT_FIP_EXT_ID_KEY: '1.11.11.1', + 'foreign_key2': 'value1234'} + self._set_lsp_external_id(port_id, **new_ext_ids) + ext_ids.update(new_ext_ids) + self.assertEqual(ext_ids, self._get_lsp_external_id(port_id)) + # invoke port update and make sure the the values we added to the + # external_ids remain undisturbed. + data = {'port': {'extra_dhcp_opts': [{'ip_version': 4, + 'opt_name': 'ip-forward-enable', + 'opt_value': '0'}]}} + port_req = self.new_update_request('ports', data, port_id) + port_req.get_response(self.api) + actual_ext_ids = self._get_lsp_external_id(port_id) + # update port should have not removed keys it does not use from the + # external ids of the lsp. + self.assertEqual('1.11.11.1', + actual_ext_ids.get(ovn_const.OVN_PORT_FIP_EXT_ID_KEY)) + self.assertEqual('value1234', actual_ext_ids.get('foreign_key2')) + + class TestNBDbResourcesOverTcp(TestNBDbResources): def get_ovsdb_server_protocol(self): return 'tcp'