openstack/ovn-octavia-provider
Code Issues Proposed changes

Ensure OVN-LB is properly configured upon LS removal from LR

Browse Source 
If an ovn-lb is created (VIP and members) in a LS (neutron network)
that has 2 subnets (IPv4 + IPv6), and this LS is connected to a LR,
removing the LS from the LR leads to the removal of the ovn-lb from
the LS and consequently to remove it from the OVN SB DB as it is not
associated to any datapath

This is a problem on the _find_ls_for_lr function that looks for
all the LR ports, and get the network name from them, therefore,
even though the port for the LS got deleted, there is still another
port from the other subnet pointing to the same network (LS), which
is the culprit to delete the ovn-lb from that LS.

With this patch, the VIP IP version is consider so that the router
ports that belongs to the other subnet are not considered and the
ovn-lb is not therefore removed from the LS.

Closes-Bug: #1992363
Change-Id: I7b6dd9a31020d942d391726662e9b5ed9d76dc1f
This commit is contained in:
Luis Tomas Bolivar
2022-10-10 18:58:28 +02:00
parent 07a41614c0
commit 512b2c83b5
2 changed files with 44 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
ovn_octavia_provider/helper.py
View File

@@ -645,7 +645,9 @@ class OvnProviderHelper():
commands.append(
self.ovn_nbdb_api.lr_lb_del(ovn_lr.uuid, ovn_lb.uuid,
if_exists=True))
for net in self._find_ls_for_lr(ovn_lr):
lb_vip = netaddr.IPNetwork(
ovn_lb.external_ids.get(ovn_const.LB_EXT_IDS_VIP_KEY))
for net in self._find_ls_for_lr(ovn_lr, ip_version=lb_vip.version):
commands.append(self.ovn_nbdb_api.ls_lb_del(
net, ovn_lb.uuid, if_exists=True))
return commands
@@ -655,7 +657,9 @@ class OvnProviderHelper():
commands.append(
self.ovn_nbdb_api.lr_lb_add(ovn_lr.uuid, ovn_lb.uuid,
may_exist=True))
for net in self._find_ls_for_lr(ovn_lr):
lb_vip = netaddr.IPNetwork(
ovn_lb.external_ids.get(ovn_const.LB_EXT_IDS_VIP_KEY))
for net in self._find_ls_for_lr(ovn_lr, ip_version=lb_vip.version):
commands.append(self.ovn_nbdb_api.ls_lb_add(
net, ovn_lb.uuid, may_exist=True))
@@ -710,11 +714,13 @@ class OvnProviderHelper():
return self._del_lb_to_lr_association(ovn_lb, ovn_lr, lr_ref)
return self._add_lb_to_lr_association(ovn_lb, ovn_lr, lr_ref)
def _find_ls_for_lr(self, router):
def _find_ls_for_lr(self, router, ip_version):
ls = []
for port in router.ports:
if port.gateway_chassis:
continue
if netaddr.IPNetwork(port.networks[0]).version != ip_version:
continue
port_network_name = port.external_ids.get(
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY)
if port_network_name:

45
ovn_octavia_provider/tests/unit/test_helper.py
View File

@@ -188,7 +188,8 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
ext_ids={
ovn_const.LB_EXT_IDS_LR_REF_KEY: 'neutron-%s' % net_id,
ovn_const.LB_EXT_IDS_LS_REFS_KEY:
'{\"neutron-%s\": 1}' % net_id})
'{\"neutron-%s\": 1}' % net_id,
ovn_const.LB_EXT_IDS_VIP_KEY: self.vip_address})
self.ref_lb2 = fakes.FakeLB(
uuid=uuidutils.generate_uuid(),
admin_state_up=True,
@@ -201,7 +202,8 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
ext_ids={
ovn_const.LB_EXT_IDS_LR_REF_KEY: 'neutron-%s' % net_id,
ovn_const.LB_EXT_IDS_LS_REFS_KEY:
'{\"neutron-%s\": 1}' % net_id})
'{\"neutron-%s\": 1}' % net_id,
ovn_const.LB_EXT_IDS_VIP_KEY: self.vip_address})
# TODO(mjozefcz): Consider using FakeOVNRouter.
self.router = fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={'load_balancer': [self.ref_lb1],
@@ -2193,36 +2195,59 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
p1 = fakes.FakeOVNPort.create_one_port(attrs={
'gateway_chassis': [],
'external_ids': {
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'foo1'}})
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'foo1'},
'networks': ["10.0.0.1/24"]})
p2 = fakes.FakeOVNPort.create_one_port(attrs={
'gateway_chassis': [],
'external_ids': {
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'foo2'}})
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'foo2'},
'networks': ["10.0.10.1/24"]})
self.router.ports.append(p1)
self.router.ports.append(p2)
res = self.helper._find_ls_for_lr(self.router)
res = self.helper._find_ls_for_lr(self.router, n_const.IP_VERSION_4)
self.assertListEqual(['neutron-foo1', 'neutron-foo2'], res)
def test__find_ls_for_lr_net_not_found(self):
p1 = fakes.FakeOVNPort.create_one_port(attrs={
'gateway_chassis': [],
'external_ids': {
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'foo1'}})
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'foo1'},
'networks': ["10.0.0.1/24"]})
p2 = fakes.FakeOVNPort.create_one_port(attrs={
'gateway_chassis': [],
'external_ids': {}})
'external_ids': {},
'networks': ["10.0.10.1/24"]})
self.router.ports.append(p2)
self.router.ports.append(p1)
res = self.helper._find_ls_for_lr(self.router)
res = self.helper._find_ls_for_lr(self.router, n_const.IP_VERSION_4)
self.assertListEqual(['neutron-foo1'], res)
def test__find_ls_for_lr_different_ip_version(self):
p1 = fakes.FakeOVNPort.create_one_port(attrs={
'gateway_chassis': [],
'external_ids': {
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'foo1'},
'networks': ["10.0.0.1/24"]})
p2 = fakes.FakeOVNPort.create_one_port(attrs={
'gateway_chassis': [],
'external_ids': {
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'foo2'},
'networks': ["fdaa:4ad8:e8fb::/64"]})
self.router.ports.append(p2)
self.router.ports.append(p1)
res = self.helper._find_ls_for_lr(self.router, n_const.IP_VERSION_4)
self.assertListEqual(['neutron-foo1'], res)
res = self.helper._find_ls_for_lr(self.router, n_const.IP_VERSION_6)
self.assertListEqual(['neutron-foo2'], res)
def test__find_ls_for_lr_gw_port(self):
p1 = fakes.FakeOVNPort.create_one_port(attrs={
'gateway_chassis': ['foo-gw-chassis'],
'external_ids': {
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'foo1'}})
ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: 'foo1'},
'networks': ["10.0.0.1/24"]})
self.router.ports.append(p1)
result = self.helper._find_ls_for_lr(self.router)
result = self.helper._find_ls_for_lr(self.router, n_const.IP_VERSION_4)
self.assertListEqual([], result)
@mock.patch.object(