[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
This commit is contained in:
Flavio Fernandes 2020-09-23 15:28:55 -04:00 committed by Flavio Fernandes
parent e77f631257
commit be3669258c
2 changed files with 69 additions and 0 deletions

View File

@ -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

View File

@ -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'