From af2b26afe68645e33e2fb7e14e8afe6c7bc2063c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20JJ=C3=B3zefczyk?= Date: Wed, 4 Mar 2020 15:03:15 +0000 Subject: [PATCH] Do not try to refresh vips on OVN LB that will be deleted When there are multiple listeners with different protocols configured on OVN LB, and one of listeners needs to be deleted, there is no need to refresh vips on given OVN LB. If there are no listners withing same protocol left the given OVN LB could be deleted and there is no need to refresh vips then. Change-Id: If15f817617facade10005878c8dfc7f467ce75a9 Closes-Bug: 1866087 --- ovn_octavia_provider/driver.py | 21 ++++++++++++------- .../tests/unit/test_driver.py | 21 +++++++++++++++---- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/ovn_octavia_provider/driver.py b/ovn_octavia_provider/driver.py index 82b286c8..fc2c2ed2 100644 --- a/ovn_octavia_provider/driver.py +++ b/ovn_octavia_provider/driver.py @@ -316,6 +316,7 @@ class OvnProviderHelper(object): def _clean_lb_if_empty(self, ovn_lb, lb_id, external_ids): commands = [] + lb_to_delete = False if OvnProviderHelper._is_lb_empty(external_ids): # Verify if its only OVN LB defined. If so - leave with # undefined protocol. If there is different for other protocol @@ -328,10 +329,11 @@ class OvnProviderHelper(object): commands.append( self.ovn_nbdb_api.db_set( 'Load_Balancer', ovn_lb.uuid, ('protocol', []))) - else: + elif len(defined_ovn_lbs) > 1: # Delete the lb. commands.append(self.ovn_nbdb_api.lb_del(ovn_lb.uuid)) - return commands + lb_to_delete = True + return (commands, lb_to_delete) def lb_delete_lrp_assoc_handler(self, row): try: @@ -1233,11 +1235,14 @@ class OvnProviderHelper(object): # Set LB protocol to undefined only if there are no more # listeners and pools defined in the LB. - commands.extend( - self._clean_lb_if_empty( - ovn_lb, listener['loadbalancer_id'], external_ids)) - commands.extend( - self._refresh_lb_vips(ovn_lb.uuid, external_ids)) + cmds, lb_to_delete = self._clean_lb_if_empty( + ovn_lb, listener['loadbalancer_id'], external_ids) + commands.extend(cmds) + # Do not refresh vips if OVN LB for given protocol + # has pending delete operation. + if not lb_to_delete: + commands.extend( + self._refresh_lb_vips(ovn_lb.uuid, external_ids)) self._execute_commands(commands) except Exception: LOG.exception(EXCEPTION_MSG, "deletion of listener") @@ -1444,7 +1449,7 @@ class OvnProviderHelper(object): commands.extend( self._clean_lb_if_empty( - ovn_lb, pool['loadbalancer_id'], external_ids)) + ovn_lb, pool['loadbalancer_id'], external_ids)[0]) self._execute_commands(commands) if listener_id: diff --git a/ovn_octavia_provider/tests/unit/test_driver.py b/ovn_octavia_provider/tests/unit/test_driver.py index e9fb6b69..950e1bc6 100644 --- a/ovn_octavia_provider/tests/unit/test_driver.py +++ b/ovn_octavia_provider/tests/unit/test_driver.py @@ -1397,7 +1397,12 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): self.helper.ovn_nbdb_api.lb_del.assert_not_called() @mock.patch.object(ovn_driver.OvnProviderHelper, '_is_lb_empty') - def test_listener_delete_ovn_lb_empty_lb_empty(self, lb_empty): + def test_listener_delete_ovn_lb_empty_octavia_lb_empty(self, lb_empty): + """That test situation when the OVN and Octavia LBs are empty. + + That test situation when both OVN and Octavia LBs are empty, + but we cannot remove OVN LB row. + """ lb_empty.return_value = True self.helper.listener_delete(self.listener) self.helper.ovn_nbdb_api.db_remove.assert_called_once_with( @@ -1410,10 +1415,13 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): ('protocol', []))]) @mock.patch.object(ovn_driver.OvnProviderHelper, '_is_lb_empty') - def test_listener_delete_ovn_lb_empty_lb_not_empty(self, lb_empty): + def test_listener_delete_ovn_lb_empty_octavia_lb_not_empty(self, lb_empty): + """We test if we can remove one LB with not used protocol""" + ovn_lb_udp = copy.copy(self.ovn_lb) + ovn_lb_udp.protocol = ['udp'] self.mock_find_ovn_lbs.stop() self.helper.ovn_nbdb_api.db_find_rows.return_value.\ - execute.side_effect = [[self.ovn_lb], []] + execute.side_effect = [[self.ovn_lb], [self.ovn_lb, ovn_lb_udp]] lb_empty.return_value = True self.helper.listener_delete(self.listener) self.helper.ovn_nbdb_api.db_remove.assert_called_once_with( @@ -1421,6 +1429,10 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): 'external_ids', 'listener_%s' % self.listener_id) self.helper.ovn_nbdb_api.lb_del.assert_called_once_with( self.ovn_lb.uuid) + # Validate that the vips column hasn't been touched, because + # in previous command we remove the LB, so there is no need + # to update it. + self.helper.ovn_nbdb_api.db_set.assert_not_called() def test_pool_create(self): status = self.helper.pool_create(self.pool) @@ -1627,9 +1639,10 @@ class TestOvnProviderHelper(TestOvnOctaviaBase): @mock.patch.object(ovn_driver.OvnProviderHelper, '_is_lb_empty') def test_pool_delete_ovn_lb_empty_lb_not_empty(self, lb_empty): + ovn_lb_udp = copy.copy(self.ovn_lb) self.mock_find_ovn_lbs.stop() self.helper.ovn_nbdb_api.db_find_rows.return_value.\ - execute.side_effect = [[self.ovn_lb], []] + execute.side_effect = [[self.ovn_lb], [self.ovn_lb, ovn_lb_udp]] lb_empty.return_value = True self.helper.pool_delete(self.pool) self.helper.ovn_nbdb_api.db_remove.assert_called_once_with(