[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

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

@@ -509,6 +509,11 @@ class OVNClient(object):
                     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

neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_resources.py

@@ -14,6 +14,7 @@
 
 from unittest 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
@@ -1048,6 +1049,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'