OVNNB backend: Fix LbDelCommand when using vip param

Fix issue in ovn_northbound LbDelCommand where logic
for removing vip while using if_exists=True was not
doing the right thing.

In order to be consistent with as ovn-nbctl [1], this
change also introduces a modified behavior. When the
last vip is removed, it will also remove the load
balancer instance.

[1]: 1e2f4aaab6/utilities/ovn-nbctl.c (L2870)

Closes-Bug: #1877673
Change-Id: Ie8ae8f5efae2afb958ed2010053ff04372cc1c51
(cherry picked from commit 8cb3b56fa9)
This commit is contained in:
Flavio Fernandes 2020-05-08 17:40:45 -04:00 committed by Flavio Fernandes
parent 4bfac95b2f
commit c5b598dd8d
3 changed files with 42 additions and 9 deletions

View File

@ -686,11 +686,15 @@ class API(api.API):
def lb_del(self, lb, vip=None, if_exists=False):
"""Remove a load balancer or just the VIP from a load balancer
If all vips of the load balancer are removed, the load balancer
instance will be removed as well. If vip parameter is 'None'
this will cause all vips (and load balancer) to be removed.
:param lb: The name or uuid of a load balancer
:type lb: string or uuid.UUID
:param vip: The VIP on the load balancer to match
:param vip: The VIP to match. If None, match all vips
:type: string
:param if_exists: If True, don't fail if the port doesn't exist
:param if_exists: If True, don't fail if the lb/vip doesn't exist
:type if_exists: boolean
"""

View File

@ -1048,16 +1048,19 @@ class LbDelCommand(cmd.BaseCommand):
def run_idl(self, txn):
try:
lb = self.api.lookup('Load_Balancer', self.lb)
if self.vip:
if self.vip in lb.vips:
if self.if_exists:
return
lb.delkey('vips', self.vip)
else:
lb.delete()
except idlutils.RowNotFound:
if not self.if_exists:
raise
return
if self.vip:
if self.vip in lb.vips:
lb.delkey('vips', self.vip)
elif not self.if_exists:
raise idlutils.RowNotFound(table='Load_Balancer', col=self.vip,
match=self.lb)
# Remove load balancer if vips were not provided or no vips are left.
if not self.vip or not lb.vips:
lb.delete()
class LbListCommand(cmd.ReadOnlyCommand):

View File

@ -1126,6 +1126,32 @@ class TestLoadBalancerOps(OvnNorthboundTest):
self.api.lb_del(utils.get_rand_device_name(), if_exists=True).execute(
check_error=True)
def test_lb_del_vip_if_exists(self):
name = utils.get_rand_device_name()
lb = self._lb_add(name, '192.0.2.1', ['10.0.0.1'])
lb2 = self._lb_add(name, '192.0.2.2', ['10.1.0.1'])
self.assertEqual(lb, lb2)
# Remove vip that does not exist.
self.api.lb_del(lb.name, '9.9.9.9', if_exists=True
).execute(check_error=True)
self.assertIn('192.0.2.1', lb.vips)
self.assertIn('192.0.2.2', lb.vips)
# Remove one vip.
self.api.lb_del(lb.name, '192.0.2.1', if_exists=True
).execute(check_error=True)
self.assertNotIn('192.0.2.1', lb.vips)
# Attempt to remove same vip a second time with if_exists=True.
self.api.lb_del(lb.name, '192.0.2.1', if_exists=True).execute(
check_error=True)
# Attempt to remove same vip a third time with if_exists=False.
cmd = self.api.lb_del(lb.name, '192.0.2.1', if_exists=False)
self.assertRaises(idlutils.RowNotFound, cmd.execute, check_error=True)
# Remove last vip and make sure that lb is also removed.
self.assertIn('192.0.2.2', lb.vips)
self.api.lb_del(lb.name, '192.0.2.2').execute(check_error=True)
cmd = self.api.lb_del(lb.name)
self.assertRaises(idlutils.RowNotFound, cmd.execute, check_error=True)
def test_lb_list(self):
lbs = {self._lb_add(utils.get_rand_device_name(), '192.0.2.1',
['10.0.0.1', '10.0.0.2']) for _ in range(3)}